sys: new sock submodule for DTLS

[pokgak]

Contribution description

This PR is part of the PR series to introduce a new DTLS sock module in RIOT.

The module introduces a new sock type sock_dtls. Similar to the TCP/UDP sock, DTLS sock needs to be implemented separately for each underlying DTLS implementation.

This PR also introduces a new net_dtls group in Doxygen to group and document all related DTLS components together for easier discovery.

The example implementation of this PR is provided by #11943.

Testing procedure

make static-test and make doc produces no errors or warnings.

Issues/PRs references

Depends on #11564
Previous discussion #10897

[miri64]

DTLS_PSK and DTLS_ECC should be exposed to the doc somewhere. And maybe renamed SOCK_DTLS_PSK/SOCK_DTLS_ECC. How about adding

#ifdef DOXYGEN
#define SOCK_DTLS_PSK (0) /**< PSK mode */
#define SOCK_DTLS_ECC (0) /**< ECC mode */
#endif

or something similar to net/sock/dtls.h? Also, it's not clear to me, if the two are mutually exclusive, if yes it might be better if having a SOCK_DTLS_MODE instead that is set to an enumerated list of possible values wouldn't be better.

[miri64]

Using ECC credentials is a bit iffy currently. It works, but takes too long (>10s) to send and receive the data. Probably the same problem as #11859.

Not sure this helps with the problem at hand, but I wanted to point out that there is an implementation guideline for representation of elliptic curves by the light-weigh implementation guidance working group of the IETF: https://tools.ietf.org/html/draft-ietf-lwig-curve-representations-08

[pokgak]

DTLS_PSK and DTLS_ECC should be exposed to the doc somewhere. And maybe renamed SOCK_DTLS_PSK/SOCK_DTLS_ECC. How about adding

#ifdef DOXYGEN
#define SOCK_DTLS_PSK (0) /**< PSK mode */
#define SOCK_DTLS_ECC (0) /**< ECC mode */
#endif

or something similar to net/sock/dtls.h?

I think it is better to expose it in tinydtls pkg instead of DTLS sock as DTLS_PSK and DTLS_ECC is tinydtls specific and other DTLS implementation may not have/use defines to enable support for specific cipher suites (e.g. wolfSSL uses wolfSSL_CTX_set_cipher_list()). There are also other defines that are not documented anywhere other than examples/dtls-echo README such as DTLS_CONTEXT_MAX and more.

Or would documenting it all in a new doxygen group (e.g. net_encryption) be better? The group could also describe caveats and configuration options of each DTLS implementation and explain how to use credman with DTLS sock. In general, information on how to get started with using (D)TLS in RIOT.

Also, it's not clear to me, if the two are mutually exclusive

No, it is not mutually exclusive. A DTLS sock should be able to use multiple cipher suites.

[miri64]

I think it is better to expose it in tinydtls pkg instead of DTLS sock as DTLS_PSK and DTLS_ECC is tinydtls specific and other DTLS implementation may not have/use defines to enable support for specific cipher suites (e.g. wolfSSL uses wolfSSL_CTX_set_cipher_list()). There are also other defines that are not documented anywhere other than examples/dtls-echo README such as DTLS_CONTEXT_MAX and more.

Also, it's not clear to me, if the two are mutually exclusive

No, it is not mutually exclusive. A DTLS sock should be able to use multiple cipher suites.

This should not be part of this PR. But how about making these (pseudo) submodules of tinydtls then: tinydtls_psk and tinydtls_ecc. DTLS_CONTEXT_MAX seems to be more of a configuration variable.

Or would documenting it all in a new doxygen group (e.g. net_encryption) be better? The group could also describe caveats and configuration options of each DTLS implementation and explain how to use credman with DTLS sock. In general, information on how to get started with using (D)TLS in RIOT.

I don't see why a new group would be necessary. If those variables are tinycrypt specific, they should go into pkg_tinycrypt (maybe also exposed to config). The doc for the caveats and how to get started would be better suited in a new group net_dtls maybe (which may refer to sock_dtls and the various implementation, but not group them specifically, see e.g. net_ipv6 or net_udp), the usage of credman with dtls_sock should go into net_dtls_sock as it is important for its usage.

[pokgak]

I'm not sure about having tinydtls_psk and tinydtls_ecc as submodules but I'll try it first and open a PR for that first along with the configuration variable documentation and we can continue to discuss it there.

The doc for the caveats and how to get started would be better suited in a new group net_dtls maybe (which may refer to sock_dtls and the various implementation, but not group them specifically, see e.g. net_ipv6 or net_udp), the usage of credman with dtls_sock should go into net_dtls_sock as it is important for its usage.

Yeah this makes more sense. I added the documentation on how to use DTLS sock in net_sock_dtls (33793d7) and general info about DTLS support in RIOT in net_dtls (e5be939).

[miri64]

Could you maybe split this PR into at least two:

• Introduction of the sock_dtls API
• Implementation of sock_dtls in tinydtls

This would help greatly with the review.

[pokgak]

Addressed the comments and split the PR into two. This PR now introduces the DTLS sock API and a new Doxygen group net_dtls to group related DTLS components. The implementation is provided by #11943. Updated the OP.

[miri64]

Thanks. I'll have a look later this week.

[miri64]

Only reviewed the API and its documentation so far. Not the more general documentation on top.

[miri64]

Review of high-level doc.

[miri64]

Please don't advertise gotos in the high-level doc.

[pokgak]

I'll rewrite this without using gotos.

[pokgak]

If I understand your question correctly, for now it could be done using #ifdef. A more generic config options that are not stack dependent would be ideal but I don't have any idea for that right now.

sock_dtls_create();

#ifdef MODULE_WOLFSSL_SOCK_DTLS
sock_dtls_config_t config = {
  .cipherlist = "...";
  .version = DTLSv1_2,
};
sock_dtls_set_config(&config);
#endif

sock_dtls_establish_session();

[pokgak]

On second thought, doing it like the example from my last comment doesn't actually solve the problem. It is more straightforward to just use #ifdefs and call the DTLS stack function for setting the config option directly.

For a more full-featured DTLS implementation like wolfSSL, there are other configs that can be set. Adding a new function for each of the config options will just bloat the API and make it harder to use.

But this is still a valid problem, right?

[miri64]

For a more full-featured DTLS implementation like wolfSSL, there are other configs that can be set. Adding a new function for each of the config options will just bloat the API and make it harder to use.

But this is still a valid problem, right?

Yeah, I am thinking for a while about something similar to getsockopt/setsockopt for sock, however a clear idea did not crystalize yet (will go in a similar direction as netopt though most likely). For now I'd say it is out of the scope of this PR, as their is no general solution for sock either.

[pokgak]

Added some commits. I'll update the higher level documentation if these changes are okay.

(8eb26a9) - The current API does not specify whether the UDP sock used is also closed when sock_dtls_close() is called. To avoid calling sock_udp_close() twice, I think it should not close the UDP sock. I'm not sure how sock_udp_close() behaves if called twice with the same UDP sock but with this, we do not have to worry about this problem.

(8d40d5f) - Added sock_dtls_method_t to specify version number and role of the endpoint. Values for DTLS 1.0 and 1.2 are taken from RFC4347 and RFC6347 respectively. ProtocolVersion are as define in RFC5246. Some DTLS implementation also defines ProtocolVersion in their library. I'm not sure if it will cause a problem when it is defined twice.

[miri64]

(8eb26a9) - The current API does not specify whether the UDP sock used is also closed when sock_dtls_close() is called. To avoid calling sock_udp_close() twice, I think it should not close the UDP sock. I'm not sure how sock_udp_close() behaves if called twice with the same UDP sock but with this, we do not have to worry about this problem.

✔️

(8d40d5f) - Added sock_dtls_method_t to specify version number and role of the endpoint. Values for DTLS 1.0 and 1.2 are taken from RFC4347 and RFC6347 respectively. ProtocolVersion are as define in RFC5246. Some DTLS implementation also defines ProtocolVersion in their library. I'm not sure if it will cause a problem when it is defined twice.

There will be a conflict. See below.

[miri64]

SOCK_DTLS_1_0 to stick to the naming scheme?

[pokgak]

Renamed.

[miri64]

Great! I think we are getting to a point were we can merge this. I'll have another look soon (hopefully before my vacation next week).

[miri64]

Hopefully last round.

[pokgak]

Addressed comments.

[pokgak]

Sorry, Github UI is confusing. I thought I'm replying directly to the comments. I'll re-post my replies.

Edit: nvm. it seems okay.

[miri64]

Needs another rebase. Other than that, I believe we can finally merge this. You may squash during that rebase.

[pokgak]

Squashed and rebased.

[miri64]

ACK. This PR only introduces API and pseudo-modules (which do not compile). [So it needs no testing]

[pokgak]

Thanks @miri64 for the review!