Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

tinydtls_sock_dtls: save session info after data successfully decrypted #14307

Merged
merged 2 commits into from
Jun 18, 2020
Merged

tinydtls_sock_dtls: save session info after data successfully decrypted #14307

merged 2 commits into from
Jun 18, 2020

Conversation

pokgak
Copy link
Contributor

@pokgak pokgak commented Jun 18, 2020

Contribution description

This PR fixes a bug (#12907 (comment)) in the tinydtls implementation of sock dtls where the session information is not saved after the data is successfully decrypted.

Testing procedure

Test using examples/dtls-sock using two native nodes. Try send normal message, empty, etc. The result should be the same with the version in master.

@pokgak pokgak requested a review from leandrolanzieri as a code owner June 18, 2020 10:04
@pokgak pokgak requested a review from miri64 June 18, 2020 10:04
@miri64 miri64 added Area: pkg Area: External package ports Area: security Area: Security-related libraries and subsystems CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Jun 18, 2020
@miri64 miri64 added this to the Release 2020.07 milestone Jun 18, 2020
@miri64
Copy link
Member

miri64 commented Jun 18, 2020

LGTM, however, what about the other fix (#12907 (comment))

@pokgak
Copy link
Contributor Author

pokgak commented Jun 18, 2020

LGTM, however, what about the other fix (#12907 (comment))

Added in 44f2f50

@pokgak
Copy link
Contributor Author

pokgak commented Jun 18, 2020

All green!

@miri64 miri64 added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Jun 18, 2020
miri64
miri64 approved these changes Jun 18, 2020
View reviewed changes
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK. Tested this by sending a couple of UDP packets from a client to a server using examples/dtls-sock.

@miri64
Copy link
Member

miri64 commented Jun 18, 2020

BTW. Starting a server on a node and sending on the same node to localhost ::1 does not work :-/.

@miri64 miri64 merged commit f402de2 into RIOT-OS:master Jun 18, 2020
@pokgak
Copy link
Contributor Author

pokgak commented Jun 18, 2020

That has never worked on master either, main reason I would say is because how the application blocks on while waiting for handshake to finish.

@miri64
Copy link
Member

miri64 commented Jun 18, 2020

AFAIK the application does not do a busy wait, so that does not make sense.

@pokgak
Copy link
Contributor Author

pokgak commented Jun 19, 2020

I think this is low-priority for now but I opened #14315 to note this in the issue tracker.

pokgak pushed a commit to pokgak/RIOT that referenced this pull request Jun 19, 2020
Squashed commit of the following:
1d852d8 
commit 877790599bcf582681bc1663290ccf3cb241d9eb
Merge: a4a5483 ba8d178
Author: M Aiman Ismail <muhammadaimanbin.ismail@haw-hamburg.de>
Date:   Fri Jun 19 16:33:59 2020 +0200

    Merge branch 'tinydtls/enh/async-sock' of https://github.com/miri64/RIOT into tinydtls/enh/async-sock

commit ba8d178
Author: Martine S. Lenders <mail@martine-lenders.eu>
Date:   Fri May 22 16:25:05 2020 +0200

    tests: provide test for tinydtls with sock_async

commit 7dc4f78
Author: Martine S. Lenders <mail@martine-lenders.eu>
Date:   Tue Jun 16 14:04:48 2020 +0200

    sock_async: add missing license header to `net/sock/async/types.h`

commit 4013c61
Author: Martine Lenders <m.lenders@fu-berlin.de>
Date:   Mon Dec 9 13:51:29 2019 +0100

    tinydtls: add `sock_async` support for `sock_dtls`

commit 1b23037
Author: Martine S. Lenders <mail@martine-lenders.eu>
Date:   Fri May 22 16:24:38 2020 +0200

    sock_async: change flags for new sock_dtls behavior

commit f402de2
Merge: 2bdf9b1 44f2f50
Author: Martine Lenders <m.lenders@fu-berlin.de>
Date:   Thu Jun 18 15:20:58 2020 +0200

    Merge pull request RIOT-OS#14307 from pokgak/tinydtls/save-session-info

    tinydtls_sock_dtls: save session info after data successfully decrypted

commit 44f2f50
Author: M Aiman Ismail <muhammadaimanbin.ismail@haw-hamburg.de>
Date:   Thu Jun 18 14:14:12 2020 +0200

    tinydtls_sock_dtls: don't put alert & EVENT_CONNECT in mbox

commit 2e16b90
Author: M Aiman Ismail <muhammadaimanbin.ismail@haw-hamburg.de>
Date:   Thu Jun 18 11:44:02 2020 +0000

    tinydtls_sock_dtls: save session information after data decrypted

commit a4a5483
Author: Martine S. Lenders <mail@martine-lenders.eu>
Date:   Thu Jun 18 07:51:40 2020 +0200

    fixup! tests: provide test for tinydtls with sock_async

commit 0b729ff
Author: Martine S. Lenders <mail@martine-lenders.eu>
Date:   Thu Jun 18 07:50:22 2020 +0200

    fixup! tinydtls: add `sock_async` support for `sock_dtls`

commit d6f0af6
Author: Martine S. Lenders <mail@martine-lenders.eu>
Date:   Fri May 22 16:25:05 2020 +0200

    tests: provide test for tinydtls with sock_async

commit 497fed7
Author: Martine S. Lenders <mail@martine-lenders.eu>
Date:   Tue Jun 16 14:45:29 2020 +0200

    fixup! tinydtls: add `sock_async` support for `sock_dtls`

commit 44e8be1
Author: Martine S. Lenders <mail@martine-lenders.eu>
Date:   Tue Jun 16 14:13:42 2020 +0200

    fixup! tinydtls: add `sock_async` support for `sock_dtls`

commit 5bff9bf
Author: Martine S. Lenders <mail@martine-lenders.eu>
Date:   Tue Jun 16 14:09:23 2020 +0200

    fixup! tinydtls: add `sock_async` support for `sock_dtls`

commit 609b0af
Author: Martine S. Lenders <mail@martine-lenders.eu>
Date:   Tue Jun 16 14:06:07 2020 +0200

    fixup! tinydtls: add `sock_async` support for `sock_dtls`

commit 0f17576
Author: Martine S. Lenders <mail@martine-lenders.eu>
Date:   Tue Jun 16 14:04:48 2020 +0200

    sock_async: add missing license header to `net/sock/async/types.h`

commit 6b55527
Author: Martine Lenders <m.lenders@fu-berlin.de>
Date:   Mon Dec 9 13:51:29 2019 +0100

    tinydtls: add `sock_async` support for `sock_dtls`

commit 8a8b912
Author: Martine S. Lenders <mail@martine-lenders.eu>
Date:   Fri May 22 16:24:38 2020 +0200

    sock_async: change flags for new sock_dtls behavior
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miri64 miri64 miri64 approved these changes

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

Assignees
No one assigned
Labels
Area: pkg Area: External package ports Area: security Area: Security-related libraries and subsystems CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
Release 2020.07
Development

Successfully merging this pull request may close these issues.
2 participants
@pokgak @miri64