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

sock_dtls: a DTLS sock API #10897

Open
6 of 8 tasks
pokgak opened this issue Jan 29, 2019 · 18 comments
Open
6 of 8 tasks

sock_dtls: a DTLS sock API #10897

pokgak opened this issue Jan 29, 2019 · 18 comments
Assignees
Labels
Area: network Area: Networking Area: security Area: Security-related libraries and subsystems Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. Type: new feature The issue requests / The PR implemements a new feature for RIOT Type: tracking The issue tracks and organizes the sub-tasks of a larger effort

Comments

@pokgak
Copy link
Contributor

pokgak commented Jan 29, 2019

This issue will be used to track the current status of DTLS sock implementation and integration with gcoap. I'll try to keep it updated.

DTLS Sock TODO List


Description

sock_dtls aims to provide a wrapper around DTLS stack in RIOT so that users can choose the underlying stack by a USEMODULE in the Makefile.

I am aware of @rfuentess's sock_secure and @danielinux's wolfSSL effort and have reviewed it together with @tcschmidt, @waehlisch, @miri64 (who also helps me with the API), @MichelRottleuthner and others (feel free to chime in if there is anything I missed/had it wrong here). This API hopes to solve the comments that we have, such as:

  1. Targeting DTLS and TLS is (mostly) not needed in RIOT use case. The users typically know from the start which underlying stack to use
  2. connect, listen in traditional socket has a different meaning when used in DTLS compared to TLS.
  3. Sending and receiving of packets using sock_udp internally. sock_dtls is responsible for encrypting, decrypting of the packets and handshake. This should help by minimizing changes when integrating with existing applications such as gcoap.

To implement this API, the implementors of stack must provide the definition the structs (like in sock_udp, sock_tcp) through the modules/pkgs.

Edit: The DTLS sock API is now merged in master. See sys/net/sock/dtls.h. Removed the API from this post for readability.

@miri64 miri64 mentioned this issue Feb 1, 2019
5 tasks
@miri64 miri64 added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. Area: security Area: Security-related libraries and subsystems Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels Feb 1, 2019
@miri64 miri64 added this to the Release 2019.04 milestone Feb 1, 2019
@rfuentess
Copy link
Contributor

Sounds perfect for me 👍

Sadly, I was unable to retake my work with my original approach on the last months. I'll try to help with this new approach.

@kb2ma
Copy link
Member

kb2ma commented Feb 5, 2019

Thanks for pursuing this! The API looks good at this level: send/recv functions as a wrapper for the underlying UDP sock, with additional setup/teardown infrastructure. The timeout parameter for sock_dtls_recv() is essential for gcoap, as is the ability to interrupt it when listening indefinitely. (The ability to interrupt probably is part of #8149.)

I suggest developing a very simple end to end implementation to get a feel for how the API works for applications.

@kb2ma
Copy link
Member

kb2ma commented Feb 5, 2019

You also might want to look at OSCORE, and the recently open sourced c_OSCORE embedded implementation [1,2]. From an application perspective it would be nice if there was some commonality with a DTLS based approach, but I don't know if that is feasible.

[1] https://datatracker.ietf.org/doc/draft-ietf-core-object-security/
[2] https://mailarchive.ietf.org/arch/msg/core/PYheXvjx0vNZ3wPhB8-NaJ2Es8M

@miri64
Copy link
Member

miri64 commented Feb 5, 2019

From an application perspective it would be nice if there was some commonality with a DTLS based approach, but I don't know if that is feasible.

OSCORE and DTLS are very different approaches to communication encryption (one is on the application layer, one is on the transport layer). For now we should focus to have a common API to one protocol and then the other and then it might make sense to have a common API for both on top of this. But I don't think that we should mix those two on the very entry level into the protocol. Otherwise, we risk getting too complicated and too overengineered.

@kb2ma
Copy link
Member

kb2ma commented Feb 5, 2019

I agree on starting simply, @miri64! Perhaps the most valuable API comparison between DTLS and OSCORE would be at the level of message encryption/decryption.

@miri64
Copy link
Member

miri64 commented Feb 5, 2019

I agree on starting simply, @miri64! Perhaps the most valuable API comparison between DTLS and OSCORE would be at the level of message encryption/decryption.

Well to begin with OSCORE is more about encrypting the payload of a CoAP message (across different CoAP domains), while DTLS is about encrypting the payload of a UDP datagram (or datagrams of other protocols like DCCP or SCTP), so how it would interact with CoAP would already be pretty different (and in OSCORE's case as far as I understand pretty CoAP-specific).

@pokgak
Copy link
Contributor Author

pokgak commented Mar 13, 2019

I opened a PR #11177 with latest draft and initial TinyDTLS implementation of the API. Changes since last initial draft:

  1. sock_dtls_init, sock_dtls_close_session and sock_dtls_destroy now have void as return value (previously int)
  2. sock_dtls_close_session have additional sock_dtls_session_t as parameter

First post will be updated.

Following is what I'm considering right now and hope to get some feedback before I proceed with it:

  1. Remove sock_dtls_init from the API and move dtls_init call that is needed by TinyDTLS to autoinit. wolfSSL doesn't have such init functions.

  2. Remove the queue from sock_dtls_init_server because TinyDTLS already has its own
    internal queue to keep track of all established session. Having an additional queue will just be
    a duplicate of it. Looking at wolfSSL PR, they also doesn't have any queue to to save established
    session. So I think it will be okay to remove it.

    The new sock_dtls_init_server will then look like this:

void sock_dtls_init_server(sock_dtls_t *sock);

Just leaving my thoughts out here. Removing the queue will also mean that sock_dtls_init_server will do nothing more than setting the role of the endpoint to a DTLS_SERVER (see code). So maybe sock_dtls_init_server could be removed altogether from the API but I'm not sure about this and leaning more towards leaving it there in case anything needs it in the future.

@miri64
Copy link
Member

miri64 commented Aug 6, 2019

@pokgak how about adding a checklist (see task list here) to the OP? This makes it easier to find it later ;-). It will then also be shown as the issue's progress in the issue overview.

@miri64 miri64 added the Type: tracking The issue tracks and organizes the sub-tasks of a larger effort label Aug 6, 2019
@pokgak
Copy link
Contributor Author

pokgak commented Aug 6, 2019

Thanks for the tip. I updated the OP with the checklist and deleted my comment with the list to avoid confusion.

@pokgak
Copy link
Contributor Author

pokgak commented Aug 28, 2019

Update: #11909 is now merged. OP updated.

@miri64
Copy link
Member

miri64 commented Jul 4, 2020

@pokgak what are your plans going forward on this?

@pokgak
Copy link
Contributor Author

pokgak commented Jul 7, 2020

I'm currently integrating the new async dtls into gcoap but it'll be a bit slow as I can't focus on this much. Currently, I can already send a simple coap message with some dirty changes here and there. I'll work on making separate PRs for this changes. Current progress can be tracked here.

@miri64
Copy link
Member

miri64 commented Jul 7, 2020

* Add implementation of the DTLS sock API for WolfSSL - WIP

Is this still on the table?

@pokgak
Copy link
Contributor Author

pokgak commented Jul 7, 2020

Yes. I do have a somewhat working implementation here before I got sidetracked by other things. That will require a refactor now that sock dtls API is a lot more different now.

@miri64
Copy link
Member

miri64 commented Nov 5, 2020

@janosbrodbeck since you basically already picked up "Add DTLS integration for gcoap using DTLS sock", might there be a way to coax you into also looking into implementing sock_dtls for WolfSSL? 😁

@miri64
Copy link
Member

miri64 commented Nov 5, 2020

Or maybe @fjmolinas, who I know, is also doing sock_dtls stuff.

@janosbrodbeck
Copy link
Member

Coaxing would not be necessary ;) But want to finish the DTLS integration first, I expect feedback and lots of change requests when the PR arrives.

Add general way to set/get sock_dtls_session_t from a sock_udp_ep_t and vice-versa is also still an open task and could be important/useful.

But depends on what is really meant by it. Restoring an existing session from an endpoint? (tinydtls seems to store some context internally - but just did a quick look into tinydtls last week). Would like to have a look at it before.

@Flole998
Copy link
Contributor

@pokgak Are you planning on continuing your work on that wolfSSL implementation? If not maybe I will find some time to pick up your work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking Area: security Area: Security-related libraries and subsystems Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. Type: new feature The issue requests / The PR implemements a new feature for RIOT Type: tracking The issue tracks and organizes the sub-tasks of a larger effort
Projects
None yet
Development

No branches or pull requests

9 participants