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.

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

pkg/tinydtls: enforce the default dtls user params to be configurable #20478

Merged
merged 2 commits into from
Mar 27, 2024

Conversation

mariemC
Copy link
Contributor

@mariemC mariemC commented Mar 18, 2024

Contribution description

Incorporating the tinydtls build package, this code addresses a failure encountered during the DTLS handshake between the client and the server. The issue arose from the client imposing default user parameters, which mandate setting extended master secret and renegotiation info to 1. However, not all servers support these extensions. To ensure greater flexibility, it's more appropriate to make these parameters user-configurable

Testing procedure

I utilized the 'examples/gcoap_dtls' on a native environment for the client. However, for the server, I deployed it on my Ubuntu machine, which lacks support for these extensions
results when the extensions are set to 0:

2024-03-18 16:45:30,983 # RIOT native interrupts/signals initialized.
2024-03-18 16:45:30,984 # RIOT native board initialized.
2024-03-18 16:45:30,985 # RIOT native hardware initialization complete.
2024-03-18 16:45:30,985 # 
2024-03-18 16:45:30,986 # main(): This is RIOT! (Version: 2024.04-devel-390-ge5f033-Mariem/dtls_default_user_params_fix)
2024-03-18 16:45:30,987 # default credentials will be used
2024-03-18 16:45:30,987 # gcoap example app
2024-03-18 16:45:30,988 # All up, running the shell now
nib route add 7 :: fe80::58da:6eff:fe50:5b94re
2024-03-18 16:45:33,294 # nib route add 7 :: fe80::58da:6eff:fe50:5b94
coap get "[fe80::58da:6eff:fe50:5b94%7]" /.well-known/core
2024-03-18 16:45:34,715 # coap get "[fe80::58da:6eff:fe50:5b94%7]" /.well-known/core
2024-03-18 16:45:34,716 # gcoap_cli: sending msg ID 42940, 23 bytes
> 2024-03-18 16:45:34,774 # gcoap: response Success, code 2.05, 455 bytes
2024-03-18 16:45:34,776 # </fw>,</fw/1>;title="Retrieve a firmware update",</fw/1/d>,</fw/1/d/42>,</fw/1/m>,</fw/1/m/AAAAAA>,</fw/2>;title="Retrieve a firmware update, API version 2",</fw/2/d>,</fw/2/d/42>,</fw/2/m>,</fw/2/m/AAAAAA>,</t>,</t/1>;title="Retrieve system time in milliseconds since unix epoch",</din>,</din/1>;title="Data Record Input, version 1",</din/1/AAAAAA>,</cmd>,</cmd/1>;title="Retrieve commands for execution on the device",</cmd/1/AAAAAA>,</.well-known/core>

results when the extensions are set to 1:

2024-03-18 16:45:56,159 # main(): This is RIOT! (Version: 2024.04-devel-390-ge5f033-Mariem/dtls_default_user_params_fix)
2024-03-18 16:45:56,159 # default credentials will be used
2024-03-18 16:45:56,160 # gcoap example app
2024-03-18 16:45:56,160 # All up, running the shell now
nib route add 7 :: fe80::58da:6eff:fe50:5b94
2024-03-18 16:45:58,584 # nib route add 7 :: fe80::58da:6eff:fe50:5b94
coap get "[fe80::58da:6eff:fe50:5b94%7]" /.well-known/core
2024-03-18 16:46:00,738 # coap get "[fe80::58da:6eff:fe50:5b94%7]" /.well-known/core
2024-03-18 16:46:00,738 # gcoap_cli: sending msg ID 19650, 23 bytes
2024-03-18 16:46:00,745 # error in check_server_hello err: -552
2024-03-18 16:46:00,746 # error 0x0228 handling handshake packet of type: server_hello (2), state 8

@github-actions github-actions bot added the Area: pkg Area: External package ports label Mar 18, 2024
Copy link
Contributor

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

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

Thanks for this. I think this should be better off as a tinydtls configuration. For this I'd suggest to contribute the introduction of macros upstream. If, for now, you still want to contribute the patch to RIOT, please check my other comment about making the macros generic.

@benpicco
Copy link
Contributor

benpicco commented Mar 19, 2024

tinyDTLS is usually pretty good with incorporating our changes upstream, so you might want to directly open a PR in their repo to keep the patches to the pkg to a minimum.

@mariemC
Copy link
Contributor Author

mariemC commented Mar 19, 2024

I'll open a PR in tinydtls repo

@mariemC mariemC force-pushed the Mariem/dtls_default_user_params_fix branch from 76fa9db to a734c23 Compare March 21, 2024 16:56
@mariemC mariemC requested a review from maribu as a code owner March 21, 2024 16:56
@github-actions github-actions bot added Area: network Area: Networking Area: sys Area: System labels Mar 21, 2024
@mariemC
Copy link
Contributor Author

mariemC commented Mar 21, 2024

I have opened a PR in tinydtls, and they have suggested to adjust the default params via get_user_params() cb.

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 22, 2024
@benpicco
Copy link
Contributor

That's also a good solution.

@riot-ci
Copy link

riot-ci commented Mar 22, 2024

Murdock results

✔️ PASSED

208e757 pkg/tinydtls/contrib: add get user params

Success Failures Total Runtime
10009 0 10009 10m:14s

Artifacts

@mariemC mariemC force-pushed the Mariem/dtls_default_user_params_fix branch from a734c23 to 208e757 Compare March 22, 2024 11:20
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Looks good to me

@leandrolanzieri leandrolanzieri dismissed their stale review March 22, 2024 14:07

A better solution was found

@kfessel
Copy link
Contributor

kfessel commented Mar 26, 2024

@leandrolanzieri please link

@benpicco benpicco added this pull request to the merge queue Mar 26, 2024
Merged via the queue into RIOT-OS:master with commit 222a2e1 Mar 27, 2024
27 checks passed
@MrKevinWeiss MrKevinWeiss added this to the Release 2024.04 milestone Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking Area: pkg Area: External package ports Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants