pkg/tinydtls/contrib/sock_dtls: fix ep_to_session#17849
Merged
miri64 merged 1 commit intoRIOT-OS:masterfrom Mar 25, 2022
Merged
pkg/tinydtls/contrib/sock_dtls: fix ep_to_session#17849miri64 merged 1 commit intoRIOT-OS:masterfrom
miri64 merged 1 commit intoRIOT-OS:masterfrom
Conversation
leandrolanzieri
added
the
Type: bug
Member
|
Niiice, will test if this takes care of the random handshake problems I had during my DoC evaluation once I am back from IETF (or if I find a quiet minute during IETF)! |
kfessel
reviewed
Mar 23, 2022
kfessel
added
CI: ready for build
github-actions
bot
added
Area: examples
Member
Do you have some references for that? The only doc on that I found was for the POSIX-version where the port is included in I still have problems establishing a session over CoAPS from a RIOT client to an Maybe the proper fix could be, to change tinydtls so that the In any case, I think |
Member
|
Here is an excerpt from the log output of the Detailsand here is the |
Contributor
Author
The reason the handshake was failing in my case was because of an inconsistency in the cookie creation, because in these lines the hmac is updated with the address, and the size used was larger than the address. The two extra bytes messed the cookie creation randomly. https://github.com/eclipse/tinydtls/blob/706888256c3e03d9fcf1ec37bb1dd6499213be3c/dtls.c#L338-L339
Will change to this |
Member
To be clear, since the #include <stdint.h>
#include "net/ipv6/addr.h"
typedef struct {
unsigned char size;
struct {
ipv6_addr_t addr;
uint16_t port;
} addr;
int ifindex;
} session_t;in the |
Member
Maybe @obgm can help here 😅 |
Member
I think the cookie creation is not the problem I face, the server reports the cookie as valid, if I enable DEBUG there: Details |
Contributor
Author
|
It makes sense to include the port for the cookie creation, this should be done as @miri64 proposes. I'll update this PR to add a patch to the package, and also make the PR with the same change upstream. |
Member
See also doc on |
Contributor
|
Thank you for clearing that up @miri64 - i missed thinking of socket-addr as addr + port (or some other complete location information of the other end of the connection) |
Contributor
Author
|
Turns out there already was a function to initialize the session, and sets the |
leandrolanzieri
force-pushed
the
pr/pkg/tinydtls/fix_ep_to_session
branch
from
cd980d4 to
bca4f81
Compare
Contributor
Author
|
Upstream PR at eclipse-tinydtls/tinydtls#127 |
miri64
reviewed
Mar 24, 2022
Member
|
I'm fine with the changes, how they are at the moment. My faults indicate, that there is most likely yet another overflow somewhere, but that's a challenge for another day. Please squash. |
leandrolanzieri
force-pushed
the
pr/pkg/tinydtls/fix_ep_to_session
branch
from
1590e50 to
981e4aa
Compare
leandrolanzieri
force-pushed
the
pr/pkg/tinydtls/fix_ep_to_session
branch
from
981e4aa to
53b6aba
Compare
Contributor
Author
|
CI failed because I did not update the example to the new structure. It should be fixed now |
leandrolanzieri
added
CI: ready for build
kaspar030
added
CI: ready for build
Contributor
Author
|
Murdock is green, not sure what's up with the other checks |
Member
|
I guess another Github outage? Could you force-push? |
miri64
added
the
CI: skip compile test
leandrolanzieri
force-pushed
the
pr/pkg/tinydtls/fix_ep_to_session
branch
from
53b6aba to
05db36a
Compare
Yes, that's the way I left some comments in the tinydtls PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contribution description
The size field in the session_t structure should reflect the size of the addr field for the particular OS. This was producing random errors when calculating the client_hello cookie, because the address is part of the hash, and the length used to update the
sha256_updatefunction was set to 18.Testing procedure
dtls_sockapplication should work reliably.Issues/PRs references
None