gcoap: add DTLS integration

[pokgak]

Contribution description

This PR add DTLS support in gcoap.

Among the changes done in this PR are:

• Add a new function gcoap_set_credential_tag() to tell credman which credential to use. For other application that is similar to gcoap where the user does not have direct access to the sock used, this is how DTLS integration using DTLS sock API can be done.
• Provide top level transport layer (96e3a68) and use common name for the sock (6c8f9cd) to abstracts away the transport mechanism from gcoap logic. This is contributed by @kb2ma. More on that in net/gcoap: Improve DTLS integration pokgak/RIOT#1.
• Example of using DTLS in a gcoap application.

Section 9.1 of the CoAP RFC mentions checks that must be done for CoAP implementations that integrates DTLS. I have not done any of said checks in this PR yet.

Testing procedure

NOTE: This PR will currently fail compile due to dependencies on other PR. For testing, you will need to manually pull in changes from #11943 and #12529.

Testing have been done between two samr21-xpro and also between a nucleo-l476rg and a RPI with 802.15.4 radio. On the RPI, I've tested this PR with the following stack on the Pi:

• libcoap + tinydtls as server and client using the example applications coap-server and coap-client
• aiocoap + tinydtls as client using aiocoap-client
• coap-shell as client which uses californium + scandium

For testing between two RIOT nodes, simply flash example/gcoap onto the boards and send a CoAP get from the client to the server using:

coap get <server ip> 5684 /.well-known/core

When testing with a Pi, I encountered some problem when using link-local IPv6 so it is better to use the global address. For that some changes are needed to the Makefile:

-USEMODULE += gnrc_ipv6_default
+USEMODULE += gnrc_ipv6_router_default

...

+CFLAGS += -DGNRC_IPV6_NIB_CONF_SLAAC=1

Then setup RADVD on the Pi and you can proceed with the instructions above.

Issues/PRs references

DTLS sock tracking issue: #10897

[pokgak]

Pinging people that I think might be interested in this @kb2ma @leandrolanzieri @smlng @tcschmidt @chrysn

[benpicco]

What's missing for this to not be WIP anymore?

[kb2ma]

Thanks for working to move this forward, @benpicco. For my part I can only say it is high on my list to review after 2019.10 is released.

[tcschmidt]

@benpicco I guess you're right, it's not WIP anymore. It just waits for the review ... and we all understand why :-)

[benpicco]

Oh I don't know why - I thought I'd wait until this is not WIP anymore before beginning a review.

[benpicco]

@pokgak If this is not WIP anymore, please squash!
Review hasn't started yet and it's much easier to review individual commits.

[smlng]

@pokgak please clarify: is #11943 needed for this or not? Bc if it is, we should add the dependency and "waiting for PR" label, too. Also it makes more sense to merge #11943 first.

[pokgak]

Thanks for the interest!

What's missing for this to not be WIP anymore?

Some checks as required by the RFC is still not done yet.

Section 9.1 of the CoAP RFC mentions checks that must be done for CoAP implementations that integrates DTLS. I have not done any of said checks in this PR yet.

These needs to be move to another PR:

• Move gcoap_init() out of auto_init to first load the credentials before start listening for CoAP requests. It is called manually from user application (here: gcoap_cli.c)

• Add a new function gcoap_set_credential_tag() to tell credman which credential to use. For other application that is similar to gcoap where the user does not have direct access to the sock used, this is how DTLS integration using DTLS sock API can be done.

Other than that, I have an incomplete version of DTLS sock with WolfSSL (currently this PR is only using tinydtls with DTLS sock), , that I'm using to make sure that the changes I've done here is not specific to tinydtls. For example setting the sock_dtls_session_t currently is specific to tinydtls:

RIOT/sys/net/application_layer/gcoap/gcoap.c

Lines 118 to 123 in 8a2f70a

	sock_dtls_session_t session;
	/* convert sock_udp_ep_t to sock_dtls_session_t */
	session.dtls_session.port = remote->port;
	session.dtls_session.ifindex = remote->netif;
	session.dtls_session.size = sizeof(ipv6_addr_t) + /* addr */
	sizeof(unsigned short); /* port */

A function to wrap that will need to be added to the DTLS sock API. Added this to TODO in tracking issue.

@pokgak please clarify: is #11943 needed for this or not? Bc if it is, we should add the dependency and "waiting for PR" label, too. Also it makes more sense to merge #11943 first.

I prefer #11943 gets merged first before review on this PR, so you could set the "waiting for PR" label for now.

[pokgak]

I updated this PR and it is now not a WIP anymore. Compiling will still fail because of dependencies on other PRs (#11943, #12529).

Section 9.1 of the CoAP RFC mentions checks that must be done for CoAP implementations that integrate DTLS. I have not done any of the said checks in this PR yet.

I skipped this check based on the response I got in obgm/libcoap#420 (comment).

Other than that, I have an incomplete version of DTLS sock with WolfSSL (currently this PR is only using tinydtls with DTLS sock), , that I'm using to make sure that the changes I've done here are not specific to tinydtls. For example setting the sock_dtls_session_t currently is specific to tinydtls:

RIOT/sys/net/application_layer/gcoap/gcoap.c

Lines 118 to 123 in 8a2f70a

	sock_dtls_session_t session;
	/* convert sock_udp_ep_t to sock_dtls_session_t */
	session.dtls_session.port = remote->port;
	session.dtls_session.ifindex = remote->netif;
	session.dtls_session.size = sizeof(ipv6_addr_t) + /* addr */
	sizeof(unsigned short); /* port */

This can be done in follow up PR for DTLS sock. I prefer not to change that PR anymore until it is merged.

TLDR: this PR is ready for review and testing it as is is not possible but I tested it locally by pulling in the dependencies from #11943 and #12529.

[benpicco]

So this needs a rebase now 😃

[kb2ma]

I expected to see a comment similar to what was in Makefile.dep. The additional 128 bytes deserves some explanation so the user can change it if that is reasonable.

Payload size might be bigger when using DTLS with ECC, causing errors not present
when using plain CoAP

[pokgak]

Rephrased and added the comment in dfdbb86

[kb2ma]

Why did you choose 10 for this value? Why not 1 or 5?

[pokgak]

This can be any unsigned values. No reason specifically why 10 is chosen here.

[kb2ma]

More importantly, why define this value here? If it was defined in gcoap.c or gcoap.h, you would not need gcoap_set_credential_tag(), right? Is there any value to defining it in the application? Would gcoap ever need more than one credential? Do we need a distinction between a gcoap server credential and a client credential?

[kb2ma]

Following this line of thinking, why not move all of the code from this PR in gcoap_cli.c into gcoap itself? gcoap provides CoAP as a service, so why should every application need to add this code? With this approach the application/user only needs to provide credentials.c.

[pokgak]

More importantly, why define this value here? If it was defined in gcoap.c or gcoap.h, you would not need gcoap_set_credential_tag(), right? Is there any value to defining it in the application?

Yup, you're right. The credential tag is defined in the client to avoid reserving values for the tag used in gcoap. Other modules (e.g. LWM2M) might also hard-code the tag used for the DTLS sock and then there will be a need to track the available tag somewhere to avoid collisions.

Would gcoap ever need more than one credential?

Yes. gcoap might be talking to other secure coap clients/servers that use different credentials.

Do we need a distinction between a gcoap server credential and a client credential?

No.

Following this line of thinking, why not move all of the code from this PR in gcoap_cli.c into gcoap itself? gcoap provides CoAP as a service, so why should every application need to add this code? With this approach, the application/user only needs to provide credentials.c.

The credentials are added in gcoap_cli.c because we do not want to make any assumption or limit what the name of the variables used for the credentials should be.

In general, the design of coupling DTLS sock with credman is to avoid limiting what the name of the credentials should be or where it can be found. This is left to the user to define.

[kb2ma]

Functional testing is going OK. I'm updating the automated tests in riot-coap-pytest. It took some work to update my installation of libcoap and aiocoap for DTLS, as you can see in the Materials section of README.md. I think it will take another day or two to get through this testing.

[kb2ma]

I'd like to be sure this PR supports use of more than one credential. So, suppose there are two credentials, C1 and C2, with tags 1 and 2 respectively. C1 is the credential the gcoap CLI uses to initialize gcoap. Let's assume SOCK_DTLS_GCOAP_TAG is 1.

An endpoint A sends a GET request to the gcoap device using credential C1. gcoap's _tl_sock of type sock_dtls_t receives the request, the credential matches, and gcoap sends the response to endpoint A.

Now suppose I use the gcoap CLI to send a message to an endpoint B. However, endpoint B requires use of credential C2. The CLI uses gcoap_req_send() to send the message to B. However, gcoap_req_send() calls the internal _tl_send() function, which uses gcoap's _tl_sock, which uses credential C1. This request will fail then, won't it?

Let's assume we find a way for gcoap to send the message to endpoint B with a different sock. Now gcoap also needs to listen for a response on this sock for credential C2 as well as listen on the sock for credential C1. I don't see how this is possible without something like the async mechanism we have been discussing.

Without DTLS, gcoap acts as a messaging hub. All communication can pass through the single sock_udp_tsock. It seems like there needs to be some multiplexing somewhere for gcoap to associate the appropriate sock_dtls_t and credential with a given request/response.

Am I missing something here?

[pokgak]

Now suppose I use the gcoap CLI to send a message to an endpoint B. However, endpoint B requires use of credential C2. The CLI uses gcoap_req_send() to send the message to B. However, gcoap_req_send() calls the internal _tl_send() function, which uses gcoap's _tl_sock, which uses credential C1. This request will fail then, won't it?

Yes, sending a message to endpoint B, which uses credential C2 will fail.

---

The above part should answer your question. The following is an idea that I had in the earlier implementation of DTLS sock.

One approach to support use of more than one credential is to use a different sock for each credential as you've described above. Another approach to support more than one credential is to modify how a DTLS sock gets its credentials from credman (the credential storage module).

The reason why a DTLS sock only supports one credential is that each sock can only have a single credential tag. See sock_dtls::tag below:

RIOT/pkg/tinydtls/include/sock_dtls_types.h

Lines 37 to 50 in cfeb2a3

	struct sock_dtls {
	dtls_context_t *dtls_ctx; /**< TinyDTLS context for sock */
	sock_udp_t *udp_sock; /**< Underlying UDP sock to use */
	mbox_t mbox; /**< Mailbox for internal event
	handling */
	msg_t mbox_queue[SOCK_DTLS_MBOX_SIZE]; /**< Queue for struct
	sock_dtls::mbox */
	uint8_t *buf; /**< Buffer to pass decrypted data
	back to user */
	size_t buflen; /**< Size of buffer */
	credman_tag_t tag; /**< Credential tag of a registered
	(D)TLS credential */
	dtls_peer_type role; /**< DTLS role of the socket */
	};

My early implementation actually uses an array of credman_tag_t instead of just a single credman_tag_t:

/* current implementation */
struct sock_dtls {
    ...
    credman_tag_t tag;
    ...
};

/* earlier implementation */
struct sock_dtls {
    ...
    credman_tag_t tag[MAX_CREDMAN_TAG];
    ...
} 

With this approach, supporting more than one credential per sock can be done by adding a new tag to the array. When looking for which credential to use, the DTLS sock can go through the tag array and find a credential that matches.

The problem with this approach is that going through the tag array each time can be costly if there are a lot of credential tags added to the sock. There is also the problem of how to know which credential tag to use to when a packet arrived. But this approach is just an idea that I had implemented half-way before giving up due to said (possible) problems and opted for a simple and limited current implementation (which proves to be problematic now)

TLDR: The current DTLS sock implementation is limited to only one credential per sock due to the design choices of DTLS sock.

[kb2ma]

Thanks for the explanation.

My early implementation actually uses an array of credman_tag_t instead of just a single credman_tag_t

gcoap certainly would appreciate the ability to continue to use a single sock. The limitation of a single credential significantly affects the usefulness of gcoap. If you think it's reasonable to extend dtls_sock in this way in the near future, then I won't get too concerned over the present limitation, and we can move forward. Please add a warning to the DTLS section of the gcoap doc though.

The problem with this approach is that going through the tag array each time can be costly if there are a lot of credential tags added to the sock.

If an application needs a lot of credential tags, they need the resources to deal with it. I would think that most applications would have limited requirements here.

There is also the problem of how to know which credential tag to use to when a packet arrived.

It seems like there is a strong association between a remote endpoint / DTLS session and a credential. dtls_sock could use the remote to find the expected credential.

[kb2ma]

It might be worth specifically mentioning configuration parameters here. I got hung up for a while on the default value of 1 for tinydtls DTLS_PEER_MAX.

[kb2ma]

I have finished a first pass at functional testing and updating the riot-coap-pytest automated tests. You can see what works in the Setup section of the test README.md.

There are 3 issues at present:

1. The deadlock addressed in tinydtls: sock_dtls: fix deadlock in sock_dtls_recv() #12959, which causes confirmable retries (con_retry_test) to fail
2. credman limitation of a single credential per sock, described above
3. cord_ep and cord_epsim examples use a synchronous messaging model which fails for DTLS handshaking.

On (3) I was able to manually register with a DTLS based RD server using the gcoap CLI example in PSK mode, but the cord_ep example froze early in handshaking.

[kb2ma]

Another issue, as first described in 12959:

There is no mechanism in gcoap to close a session. I suppose this situation is manageable if gcoap only talks to at most DTLS_PEER_MAX (tinydtls) unique peers. On the other hand, there are scenarios that would like to return a 5.03 Service Unavailable if unable to open a new session.

[kb2ma]

@pokgak, @cgundogan -- what's the status with this PR and #12959? Are you blocked or just otherwise busy? Is the best solution to work sock_async into gcoap and #12907 for sock_dtls?

[pokgak]

sorry @kb2ma, I'm a stuck with another project right now and won't have much time to work on this for now.

Is the best solution to work sock_async into gcoap and #12907 for sock_dtls?

Main problem right now is that sock_dtls blocks while waiting for handshake messages. If we can avoid that with sock_async then it may be the solution.

[kb2ma]

Main problem right now is that sock_dtls blocks while waiting for handshake messages. If we can avoid that with sock_async then it may be the solution.

Gotcha. #13386 adds sock_async to gcoap and should be ready to experiment with when you get a chance.

[MichelRottleuthner]

Just for the record: I played around with this PR a bit and had some problems using it together with various counterparts on the linux side (ie. libcoap, aiocoap, coap-shell).
The guide @kb2ma linked above then lead me in the right direction. Thanks a lot for the writeup, it was very helpful!.
There still seems to be a problem that is somehow related to the tinydtls version. When I set up tinydtls-cython as described in the linked guide it worked with this PR, but rebasing this PR on top of master broke it again. I didn't dig deeper yet as it was hard to tell if the problems came from tinydtls itself or some other part of the integration. Could be just related to the last version bump (#12912)? Anyway, just let me know when I should give it another shot. I'd really like to see this merged :)

[kb2ma]

Thanks a lot for the writeup, it was very helpful!.

Glad to hear it!

Anyway, just let me know when I should give it another shot. I'd really like to see this merged :)

If you can verify #12907 sock_async for tinydtls, I can try rebasing this work on that in @pokgak's absence. At least he should be available for questions.

[MichelRottleuthner]

If you can verify #12907 sock_async for tinydtls (...)

my first attempt failed as we don't have a test yet and I'm not sure how to use it properly, but I bet that's solvable with @miri64's help ;)

[tcschmidt]

If you can verify #12907 sock_async for tinydtls (...)

my first attempt failed as we don't have a test yet and I'm not sure how to use it properly, but I bet that's solvable with @miri64's help ;)

@pokgak can you help resolving (or fixing) this?

[pokgak]

Yup, I'm currently also looking at #12907.

[pokgak]

There have been some major changes to gcoap and sock_dtls since this PR is opened. #12907 is working now too. I'm closing this PR and will open another one with the updated version.

My plan is to first rework the credman workflow as per my discussion with @kb2ma and then use that with the new async sock_dtls.

[benpicco]

Any news on this?

[pokgak]

I'm not working on this at the moment.

[benpicco]

Should we take this over?

[pokgak]

Yes, go ahead. I think @leandrolanzieri looked into this already, so maybe you guys can sync up.

[kfessel]

#15549 did that