Skip to content

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

Merged
benpicco merged 2 commits intoRIOT-OS:masterfrom
mariemC:Mariem/dtls_default_user_params_fix
Mar 27, 2024
Merged

pkg/tinydtls: enforce the default dtls user params to be configurable#20478
benpicco merged 2 commits intoRIOT-OS:masterfrom
mariemC:Mariem/dtls_default_user_params_fix

Conversation

@mariemC
Copy link
Copy Markdown
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
leandrolanzieri
leandrolanzieri previously requested changes Mar 18, 2024
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Contributor

benpicco commented Mar 19, 2024 •
edited
Loading

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
Copy Markdown
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
Copy Markdown
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
benpicco reviewed Mar 22, 2024
View reviewed changes
@benpicco
Copy link
Copy Markdown
Contributor

benpicco commented Mar 22, 2024

That's also a good solution.

@riot-ci
Copy link
Copy Markdown

riot-ci commented Mar 22, 2024 •
edited
Loading

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
benpicco
benpicco approved these changes Mar 22, 2024
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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
@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

Reviewers

@benpicco benpicco benpicco approved these changes

@leandrolanzieri leandrolanzieri leandrolanzieri left review comments

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

Assignees

No one assigned

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

Milestone

Release 2024.04

Development

Successfully merging this pull request may close these issues.

6 participants

@mariemC @benpicco @riot-ci @kfessel @leandrolanzieri @MrKevinWeiss