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

[WIP] sock_secure module #9450

Closed
wants to merge 10 commits into from

Conversation

rfuentess
Copy link
Contributor

@rfuentess rfuentess commented Jun 28, 2018

Contribution description

This PoC PR is trying to achieve the second step for the modules proposed in #7649 (TLSMAN) and #7397 (sock_secure). Sock_secure has as objective of adding an extension to sock which makes transparent the addition of a secure channel to current applications.

NOTE: This is making use of sock_udp for sending the DTLS records.

Working in progress

This is still in early steps and should be considered a proof of concept. The API is not finished. Comments about the API, or a better integration with sock are more than welcome.

Currently sock_secure is able to create the resources, establish the secure channel, send and receive upper data by means of the secure channel, and release resources.

Important steps to do:

  • Add the ability of renegotiating the current channel or recovered it if it was lost.
  • Be able to determine if resources are already allocated.
  • Be able to work more transparently for server or client (for gcoap at least)
    • (4th/july) Add timeout when listening for incoming upper data
  • Create a test case
  • Upgrade the example to something more useful (right now is duplicating dtls-echo)
    • Client should be able to preserve the secure channel and renegotiating it at will.

Current support for CoAP

nanocoap

I'm adding two commits with the integration of sock_secure over nanocoap.
This is working fine with the default example.

gcoap

In a very fast attempt to implement sock_secure for gcoap I screwed gcoap's machine state. The CoAP Secure message was sent and received, but the memo mechanism was unable to register the message and thus was dropping the received answer. Before attempting something similar to #7177 (e.g. adding more mbox messages) I would like to discuss with @kb2ma, maybe in the next riot summit?

Other CoAP apps

It's possible to use gcoap to generate the CoAP message and then sending it manually instead of using gcoap_req_send2(). Applications like this one should be easier to adapt for sock_secure.

Issues/PRs references

Discussed on:

Depends on:

@kb2ma
Copy link
Member

kb2ma commented Jun 28, 2018

@rfuentess, I definitely will experiment with this and get back to you on integration with gcoap!

@kYc0o
Copy link
Contributor

kYc0o commented Jun 29, 2018

Could you squash it properly and solve the conflicts?

Raul Fuentes added 8 commits July 4, 2018 14:32
- Support for tinyDTLS
Test application for TLSMAN using tinyDTLS and sock_udp.
- TinyDTLS socket and gnrc are separated
WARNING: This is a proof of concept

TLSMAN is used by this module for creating a secure channel.

- There is a server and client behavior for this first approach
WARNING: This is a POC (as the final API would not use
         sock_secure_write or sock_secure_read)

An introduction to the steps required for adding  (D)TLS support to
current applications.
@kb2ma
Copy link
Member

kb2ma commented Jul 4, 2018

I have reviewed sock_secure and plan to try integrating it with gcoap. The example app and nanocoap_server integration are helpful. I have a couple of requests:

I will want to test against a DTLS client or server on my laptop. What works well for you? libcoap, aiocoap? Please let me know any tips for setup. DTLS is new to me, so I'll probably have some questions.

Can you share the experimental code for gcoap that you mention in the PR description? I'm sure I'll be more productive if I extend your work to get past the difficulty rather than start from scratch. :-) It would at least help me validate how I think the integration should work.

For full integration, gcoap will need the ability to set a timeout on secure_sock listening, but I expect we can figure this out.

@rfuentess rfuentess force-pushed the module/sock_secure/beta branch from 8f0f455 to a126c88 Compare July 4, 2018 13:07
@rfuentess
Copy link
Contributor Author

Now that #7615 is merged I rebased with master and fixed the conflict with sys/net/application_layer/nanocoap/sock.c

For now the server side of nano_coap is working with sock_secure. I'll be working on adapting nanocoap_request() also.

@rfuentess
Copy link
Contributor Author

I will want to test against a DTLS client or server on my laptop. What works well for you? libcoap, aiocoap? Please let me know any tips for setup. DTLS is new to me, so I'll probably have some questions.

After the last RIOT summit, aicooap added formal support for the client and tinydtls. The python dtls-socket module required there was using the default keys from tinydtls example last year, which are the same one I'm using for RIOT. So probably this will be the easy path for a CoAPS client over Linux.

Can you share the experimental code for gcoap that you mention in the PR description? I'm sure I'll be more productive if I extend your work to get past the difficulty rather than start from scratch. :-) It would at least help me validate how I think the integration should work.

Is this one, but is far from be optimal.

@rfuentess
Copy link
Contributor Author

I just added an experimental support for a nanocoap client with support for sock_secure().

For full integration, gcoap will need the ability to set a timeout on secure_sock listening, but I expect we can figure this out.

Yes, I'll update tlsman for accepting a custom timeout if provided by sock_secure.

@kb2ma
Copy link
Member

kb2ma commented Jul 6, 2018

Thanks for the tips, @rfuentess! With @obgm's recent message on the riot-users list, I decided to try libcoap rather than aiocoap. I did build libcoap from the master branch using @chrysn's notes though.

On the server side, I used the latest code for this PR and included the debug flag (CFLAGS += -DTINYDTLS_DEBUG) in the nanocoap_server Makefile. I then setup a tap interface to run on native, and started nanocoap server.

Using the libcoap build described above, I was able to run this command:

$ ./coap-client -u Client_identity -k secretPSK -m get coaps://[fe80::200:bbff:febb:2%tap0]/riot/board

native

So, step 0 works! I plan to update the nanocoap_server README.md and push to your branch. I think this will help others get started.

I'm also getting a sense for how to adapt gcoap's event loop to sock_secure. I'll try something minimal first with the commit you provided above.

@kb2ma
Copy link
Member

kb2ma commented Jul 9, 2018

Added PR #3 to rfuentess source branch for sock_secure README for nanocoap_server example.

@kb2ma
Copy link
Member

kb2ma commented Aug 17, 2018

Sorry to take so long to follow-up. I reviewed tlsman and sock_secure. In their present form it seems difficult to integrate with gcoap. They expect a sock to take either the client or server role, but gcoap's sock handles both. gcoap also runs its own event loop and handles timeouts, but tlsman/sock_secure want to manage the complete message exchange.

So, I decided to experiment with the basic tinydtls flow, based on the DTLS Usage page in the tinydtls source documentation, as well as tlsman. I ended up creating a RIOT module, tdsec, as a kind of 'simplest possible' sock wrapper for tinydtls to work with gcoap. Later on, I looked at the dtls-echo example, and tdsec is similar as a generic abstraction for a sock interface to make that work.

Be sure to read the module comment at the top of tdsec.h. You'll notice that the functions tdsec_read() and tdsec_send() aren't exactly symmetric. tdsec_read() only focuses on decrypting and providing the received data via callback. This segmented/asynchronous approach works well for gcoap, but not well or an application like nanocoap. I could create a more comprehensive function, tdsec_recv(), that works synchronously to wait and return the received message.

Status:

  • Got a simple client and server to sort of communicate over tap using the gcoap example. Use 'tapsetup --create 2', like dtls-echo. Just try to send a message from the CLI, like 'coap get <addr> 5684 /riot/board'. Only works with the first message; server can't decrypt following messages. Hopefully, this problem isn't fatal. :-)

  • gcoap example works as server over tap from libcoap client, using develop branch (commit 9299403). When used as client to libcoap, it sends Client Hello, but libcoap server does not respond.

I may continue to work on tdsec as a simple, bottom-up tool for the job. At any rate, I hope it gives you some ideas as a way to integrate tinydtls with gcoap. Any feedback appreciated!

@rfuentess
Copy link
Contributor Author

Hi @kb2ma, sorry for my silence, I was hoping to be able to review your module and progress a little more on these days but sadly was impossible for me. I’ll be able to start to work on this again in two or three weeks.

Considering the work for TLSMAN, I want to discuss a little more with @obgm the feasibility of removing the client/server approach for TLSMAN/tinyDTLS.

Also, probably @kb2ma’s tdsec module would benefit from PR #8149, but I think this one I think is on the medium or far future.

@kb2ma
Copy link
Member

kb2ma commented Aug 31, 2018

No worries on the delay. Looking forward to discussion when you get the chance.

@miri64 miri64 added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: arduino API Area: Arduino wrapper API Area: security Area: Security-related libraries and subsystems Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. and removed Area: arduino API Area: Arduino wrapper API labels Oct 17, 2018
@miri64 miri64 added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Oct 17, 2018
@pokgak pokgak mentioned this pull request Feb 1, 2019
8 tasks
@miri64
Copy link
Member

miri64 commented Feb 1, 2019

As he mentioned in #10897 @pokgak, @MichelRottleuthner, @haukepetersen, myself and several others revisited the proposed (D)TLS APIs for a project related work and we came to the conclusion that this API ‑ as it is currently proposed ‑ is too involved for the embedded context. Furthermore, it directly contradicts the requirement that a DTLS API should be transparent to the underlying transport layer API [ref missing (someone mentioned that this is discussed in somewhere)], which also makes sense from a usage point of view when implementing a library such as gcoap which is supposed to support both secure (coaps://) and unsecure (coap://) communication (as discussed by @kb2ma).

We would rather propose to go for the more "thin wrapper" approach and have a sock_dtls and sock_tls first on which basis a unified API (like this one) can be build on top. However, to be perfectly honest, we don't see the requirement for that (yet).

@pokgak proposed a solution for sock_dtls based on the proposals by @kb2ma (tdsec.h) and @danielinux (sock_tls.h) in #10897. We decided to go for yet another approach, since those two both are to dependent on the underlying library (for tdsec.h it's mostly naming and parameters and sock_tls.h didn't even provide a wrapper around wolfSSL's send/recv functions last time we looked at it in detail in late December). The solution in #10897 is more "in the spirit of sock" as it plugs directly into the existing sock_udp solution and the decision of what DTLS implementation is used is made at compile-time (as for sock_udp e.g. with the UDP implementation respectively).

While this initial work was valuable to kickstart the discussion around a unified API for DTLS we want converge on a viable solution most people can agree with, so I hope that it is okay, that we close this PR for now and revisit your proposal once a requirement for a unified transport layer security API arises.

Regarding the async_sock discussion: That also needs some work, which I promise continue very soon. This in mind, the asynchronous behavior for that should be introduced independent from @pokgak's proposal, either in #8149 itself or a follow-up to both.

Feel free to reopen if you disagree.

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 Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants