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

RFC: secure_sock - Integration of (D)TLS stacks for RIOT O.S. #7397

Closed
rfuentess opened this issue Jul 21, 2017 · 14 comments
Closed

RFC: secure_sock - Integration of (D)TLS stacks for RIOT O.S. #7397

rfuentess opened this issue Jul 21, 2017 · 14 comments
Assignees
Labels
Area: network Area: Networking Area: pkg Area: External package ports 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.

Comments

@rfuentess
Copy link
Contributor

rfuentess commented Jul 21, 2017

Briefly discussed in the PR #7177, further work for integrating DTLS to RIOT OS must be adhered to RIOT philosophy. And therefore, the previous PR, although functional, should be upgraded.


Edited: This post was edited to reflect the main critiques (up to July/2017).

---------------------------------------------------
|                                                 |
|    Application / Library                        |
|                                                 |
---------------------------------------------------
                |
                |   secure_sock_api
---------------------------------------------------
|                                                 |
|    Secure_Sock                                  |
|                                                 |
|          ----------------------------------     |
|          |  TLSMAN                        |     |
|          |                                |     |
|          |  -------------   ------------- |     |
|          |  | tinydtls  |   | WolfSSL   | |     |
|          |  |           |   |           | |     |
|          |  -------------   ------------- |     |
|          ----------------------------------     |
---------------------------------------------------
        |                   |
        | sock_tcp          | sock_udp
        |                   |
        .                   .
        .                   .
        .                   .

This is the proposition of the first API for handling secure communication with RIOT O.S. This first API is intended to be the main API for the users and applications.

secure_sock tries to mimic standard socks functions (create, send, receive) for making easy the integration of a (D)TLS layer to current applications (like gcoap).

It's important to note that with this API the endpoints and socks are not defined as TCP or UDP because that is determined by the (D)TLS stack used for the application, which is handled by the second API.

The second API, TLSMAN (TLS Manager) configures, handles and transmit the handshake messages as well the encrypted upper layer data. The API's objective is to make relatively easy to adding support for other TLS stacks than tinyDTLS in the future (By example wolfSSL and mbedTLS) without impacting (too much) on already defined applications.


Any suggestions or comments?

@miri64
Copy link
Member

miri64 commented Jul 21, 2017

Why not reuse names from sock_tcp? I'm thinking especially about sock_dtls_listen[ing]() and sock_dtls_s/stop/disconnect/(). Also: might it make sense to make it the two-object-style we have there as well (create to create client, listen to create server listening queue)?

@miri64
Copy link
Member

miri64 commented Jul 21, 2017

Might be just a mistake, but why is there no recv()?

@rfuentess
Copy link
Contributor Author

Might be just a mistake, but why is there no recv()?

This is mostly because of the behavior of tinyDTLS. The receptions of UDP packets are processed by a callback which classified them between DTLS records or other things. Once that identifies one DTLS record as a data layer record, will retrieve the upper data and then process it (by example (By example module_dtls:sys/net/gcoap.c#120 ).

But, I can use a recv to force the client to wait until the reception of the first DTLS data layer. Also, I think that create and listen will receive pointers to functions for handling the upper data received in the callbacks.

@miri64
Copy link
Member

miri64 commented Jul 23, 2017

This is mostly because of the behavior of tinyDTLS. The receptions of UDP packets are processed by a callback which classified them between DTLS records or other things. Once that identifies one DTLS record as a data layer record, will retrieve the upper data and then process it (by example (By example module_dtls:sys/net/gcoap.c#120 ).

But, I can use a recv to force the client to wait until the reception of the first DTLS data layer.

Implementation behavior should not dictate the make-up of an API (see emb6 vs lwIP vs GNRC which in their behavior and access strategies are vastly different). The reception callback can be e.g. fill an mbox (that is a member of the sock_dtls type), that the receive function then empties.

Also, I think that create and listen will receive pointers to functions for handling the upper data received in the callbacks.

That sounds awfully implementation specific.

@rfuentess
Copy link
Contributor Author

I did a makeover based on the main critiques, including using again the term "secure_sock" instead of "dtls_sock" and the use of a second API.

In this propose, the secure_sock API is for the users and applications, this API tries to mimic standard socks functions (create, send, receive) for making easy the steps for adding this module to current applications (like gcoap). It's important to note that at this point, the endpoints and socks can be TCP or UDP because they are determined by the TLS stack used for the application, which is handled by the second API.

The secondary API, TLSMAN (TLS Manager) configures, handles and transmit the handshake messages as well the encrypted upper layer data. The API's objective is to make relatively easy to adding support for other TLS stacks than tinyDTLS in the future (By example wolfSSL and mbedTLS) without impacting (too much) on already defined applications.

For this first RFC I'll focus more in the secure_sock than in tlsman. Still, this is the full diagram for both APIs:


            ---------------------------------------------------
            |                                                 |
            |    Application / Library                        |
            |                                                 |
            ---------------------------------------------------
                            |
                            |   secure_sock_api
            ---------------------------------------------------
            |                                                 |
            |    Secure_Sock                                  |
            |                                                 |
            |          ----------------------------------     |
            |          |                                |     |
            |          |  TLSMAN                        |     |
            |          |          --------------------  |     |
            |          |          | tinydtls         |  |     |
            |          |          |                  |  |     |
            |          |          --------------------  |     |
            |          ----------------------------------     |
            --------------------------------------------------|
                    |                   |
                    | sock_tcp          | sock_udp
                    |                   |
                    .                   .
                    .                   .
                    .                   .

API1: secure_sock

API to interact between the user application layer and the socks layers.
Based on openssl, gnutls and af_ktls.

SECURE_SOCK_DTLS_MAX_RETRANSMISIONS /* Defined by user app */

SECURE_SOCK_DATAGRAM /* Connection is datagram oriented (DTLS) */
SECURE_SOCK_NONBLOCK /* Connection should not block */
SECURE_SOCK_BLOCKING !SECURE_SOCK_NONBLOCK

/* Based on IETF RFC 7252 9.1.3 (and only those supported by tinyDTLS for now)*/
SECURE_SOCK_CIPHER_PSK_IDS
SECURE_SOCK_CIPHER_RPK_IDS
SECURE_SOCK_CIPHER_LIST = {SECURE_SOCK_CIPHER_PSK_IDS, SECURE_SOCK_CIPHER_RPK_IDS}

SECURE_SOCK_SERVER /* Identify this side of the (D)TLS session as the server */
SECURE_SOCK_CLIENT /* Identify this side of the (D)TLS session as the client */

typedef struct _sock_tls_ep secure_sock_ep_t;

typedef struct {
    tlsman_session_t   *session /*< Structure is defined and handled by API2 */
    secure_sock_ep_t *local
    secure_sock_ep_t *remote
} secure_sock_session_t

/*
 * The key(s) used by this node.
 * TODO: Passing the keys by TPM or URI
 */
typedef struct {
   ...          /* For now is loaded from a header file  */
} secure_sock_key_peer_t


/*
 * Checks if there is an unread  DTLS Data App  recprd. If the return value is
 * non-zero the next call to secure_sock_record_recv() is guaranteed not to block.
 */
size_t secure_sock_record_check_pending(secure_sock_session_t session)

/*
 * If a secure_sock function returns a negative error code you may feed that
 * value to this function to see if the error condition is fatal to a (D)TLS
 * session (i.e., must be terminated).
 */
size_t secure_sock_error_is_fatal (size_t error)

/*
 * TODO: Starts the renegotiation of the (D)TLS session.
 * @session The secure session to be renegotiated
 * @returns 0 in success, otherwise error code.
 */
size_t secure_sock_renew(secure_sock_session_t *session, unsigned int flags )

/*
 * Termination of the (D)TLS session.
 *  @session The secure session to terminate.
 */
void secure_sock_stop(secure_sock_session_t *session)

/*
 * Load the (D)TLS stack to use. (See API2).
 * @flags - They can be composed by:
 *     SECURE_SOCK_DATAGRAM - Load only DTLS stacks (such as tinyDTLS)
 * @ciphers Load the cipher suites list (SECURE_SOCK_CIPHER_LIST)
 * @return: 0 on success, or an error code.
 */
size_t secure_sock_init ( unsigned int flags, unsigned int ciphers )

/*
 * Tries to create a new (D)TLS session for establishing a new secure channel.
 * @session  If NULL will generate the new session. Otherwise ends without error.
 * @flags    Identify the side of this channel (client or server).
 * @local    The local peer.   
 * @remote   The remote peer (it can be NULL).
 * @return: 0 on success (or if there is already a session ready), otherwise error code.
 */
size_t secure_sock_create(secure_sock_session_t *session, unsigned int flags,
                   secure_sock_ep_t *local, secure_sock_ep_t *remote,
                   secure_sock_key_peer_t keys   )  

/*
 *  Send data by the secure channel (side is not relevant).
 *  @session The secure session to use for transmission.
 *  @flags   If SECURE_SOCK_NONBLOCK is set, the function does not wait the channel to be established.
 *  @data    Upper layer data to send by the (D)TLS channel.
 *  @data_size Size of the data to send
 * @return: 0 on success, otherwise error code.
 */
size_t secure_sock_send(secure_sock_session_t *session,  unsigned int flags,
                 const void *data, size_t data_size )


/*
 * The Client side of the session starts the DTLS channel.
 *  @session The secure session to use.
 *  @flags   If SECURE_SOCK_NONBLOCK is set. The function ends before the channel is established.
 */
size_t secure_sock_connect(secure_sock_session_t *session,  unsigned int flags )


/*
 * The server side of the session is put on listening mode.
 * NOTE: If not timeout is set, this function ends until a (D)TLS Data App record is received.
 *
 * @session The secure session to use for listening.
 * @data     Pointer where the received data should be stored.
 * @max_len  Maximum space available at  @data.
 * @timeout  Timeout for receive in microseconds.
 *           If 0 and no data is available, the function returns immediately.
 *           May be SOCK_NO_TIMEOUT for no timeout (wait until data is available).
 * @returns  0, if no received data is available, but everything is in order.
 *           ETIMEDOUT, if @timeout expired.
 */
secure_sock_listening(secure_sock_session_t *session, void *data, size_t max_len,
                      uint32_t timeout)


/*
 * The client side of the session is waiting for a DTLS Layer application record.
 * @session  The secure session to use.
 * @data     Pointer where the received data should be stored.
 * @max_len  Maximum space available at  @data.
 * @timeout  Timeout for receive in microseconds.
 *           If 0 and no data is available, the function returns immediately.
 *           May be SOCK_NO_TIMEOUT for no timeout (wait until data is available).
 * @returns  0, if no received data is available, but everything is in order.
 *           ETIMEDOUT, if @timeout expired.
 */
secure_sock_recv (secure_sock_session_t *session, void *data, size_t max_len,
                      uint32_t timeout )

@kaspar030
Copy link
Contributor

@rfuentess maybe we need to go back two steps, with this API.

I think it would look fundamently different if sock would have

  1. asynchronous operations
  2. generic read/write (sock_read((sock_t*)sock_udp) vs. sock_udp_read(sock_udp))
  3. scatter/gather operations

All of them are not trivial, but would tremendously simplify what you're trying to stack on top here.

@rfuentess
Copy link
Contributor Author

@kaspar030, I have been thinking about the feasibility of turning all the current sock types into a single one. It's not in conflict with the original designs of having different types? [1].

However, one approach that we can take for testing can be generate a simple module for Asynchronous I/O SOCK (AIO_SOCK) which can try to use a generic sock for all the types (plain IP, TCP, UDP or secure). If this approach results practical and efficient, then we can slowly swapping it by the original ones. Still, the secure sock probably will still require to run over TCP or UDP.

Also, the API proposed here would be indeed compacted as a result of the AIO_SOCK module: _send(), _connect(), _recv() and _listening() can be invoked by a simple write() or receive() (with extra arguments).

@rfuentess
Copy link
Contributor Author

  1. asynchronous operations

After further reviewing this challenge, it's seems that the best route for the asynchronous communication relies on completing it inside of the sock_udp. Therefore, the secure_sock could be progressing in parallel to it.

As told before, I'm going to modify the API for secure_sock for considering asynchronous component.

  1. generic read/write (sock_read((sock_t*)sock_udp) vs. sock_udp_read(sock_udp))

This is in particular, I'm not sure if it can help us. Because of the following reasons:

I. Although sock_udp_t and sock_ip_t are exactly the same structure, and sock_tcp_t seems to be only a proof of concept. The functions behind each type (such as sock_udp_send and sock_udp_recv) focus on checking the parsing and the components of the protocol (port address, ipv4/ipv6 address, tcp options, etc.) and eventually they call a generic gnrc_sock_send and gnrc_sock_recv. Therefore, I think that this suggested new raw functions will be more complex than they should be.

II. I find more intuitive to use sock_udp when we want to send a UDP message, and a sock_ip when we wish to send raw IPv6 messages (even if the payload is a raw UDP message). In a similar fashion, a secure_sock can be more intuitive for the user when he wants to establish a secure channel for its data.

  1. scatter/gather operations

Probably this could help by improving some aspects of tinyDTLS. I don't have too much experience with it, but we could start discussions of its implementation in the next RIOT Summit :)


If it's OK for everyone, I'll update the original post for reflecting the changes to the API.

@rfuentess
Copy link
Contributor Author

One of the main issues that I'm having with tinyDTLS and RIOT is the handling of the memory required for the data structures of the former.

Right now, I'm using the original code which uses malloc/free functions. Yet, RIOT's free() function is in a TODO state.

For communication between sensors with a single external node, this is not a big issue, as long the DTLS session is neither closed or renewed constantly. Otherwise, tinyDTLS will eventually exhaust all the RAM available of the node.

Also, the use of malloc/free force me to inflate the stack size for the thread establishing the DTLS channel.

I'm thinking of using macro functions (fixed arrays) and replacing the calls of malloc/free by them. Yet, I'm not an expert on memory issues, and this is a very delicate topic for RIOT, therefore I'm open to suggestions.

@rfuentess rfuentess changed the title RFC: Integration of (tiny)DTLS for RIOT O.S. RFC: secure_sock - Integration of (D)TLS stacks for RIOT O.S. Sep 27, 2017
@rfuentess
Copy link
Contributor Author

I edited the title and the OP for referencing the second API. The separation is intended for having the discussion of the secure_sock here, while details of tinyDTLS, wolfSSL and other stacks in the second RFC.

@miri64 miri64 added Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. Area: network Area: Networking Area: pkg Area: External package ports Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels Oct 25, 2017
@kYc0o kYc0o mentioned this issue Oct 29, 2017
9 tasks
@rfuentess
Copy link
Contributor Author

This an update on this issue, after some off-line discussions, as well as some online discussions.

In general, this module faces the following challenges:

  • The sock_secure should be integrated into sock as an optional component (similar to sock_async).
  • It's intended that a generic sock takes the place of the sock_tcp, sock_udp and sock_ip in the future.
  • Therefore, sock_secure should be integrated into the previous and not running over sock_[udp, tcp, ip] as I had originally proposed.
  • Also, modifications to current modules (e.g. gcoap) should be minimum.

The work for TLSMAN is almost agnostic for sock. But, sadly the ideas proposed by me for sock_secure so far are not well compatible.


I'll paste verbatim an eraser of the generic sock proposed by Kaspr030 (comment):

typedef struct {
read, write, close, get/set, ...
} sock_ops_t;
typedef struct {
  sock_ops_t ops;
} sock_t;

typedef struct {
    sock_t super;
...
} sock_tcp_t;

static inline sock_read(sock_t *sock, ...) {
    sock->ops->read(sock, ...);
}
...

With this in mind, I was considering the following as a candidate for the API:

/* Loads the (D)TLS stack and it(s) cipher suite(s). Must be called once time  */
sock_secure_initialized()

/* Reads the payload/data of the last packet received. Tries to extract the
 * encrypted data from a (D)TLS Data App record  */
sock_secure_read()

/* Encrypts the packet buffer received into a (D)TLS Data App record */
sock_secure_write()

/* Updates the status of the (D)TLS session (the secure channel between local
 * and remote).  */
sock_secure_update()

/* Executes a step of the (D)TLS handshake (by processing the last DTLS record
 * or by sending the next one) between the local and remote members of the sock */
sock_secure_handshake()

The latest three are to be used inside of sock.read and sock.write as shown in the figures below. I'm assuming that sock_t will have an additional field of parameters or that a sock_secure_t sock structure exists. Also, that this generic sock will eventually call gnrc_sock_[send,recv] as the current ones do it.

Diagram for sock.write()

Diagram for sock.read()

Finally, and for trying to take advantage of sock_async, and to make more simple the development of new apps, the API can also have the following:

/* Used by a client side to start the handshake with the remote member of the
 * sock */
sock_secure_connect()

/* A more generic and simple approach than sock.read (for the server side only) */
sock_secure_listen()

@rfuentess rfuentess mentioned this issue Jun 21, 2018
6 tasks
@alemusix
Copy link

alemusix commented Jun 27, 2018

Hi @rfuentess
I've downloaded your fork of RIOT
https://github.com/rfuentess/RIOT/tree/Module_DTLS_Develop
I'd like to test the gcoap example with DTLS. In the examples/gcoap folder I've commented out the this line in the Makefile

USEMODULE += gnrc_dtls

when I compile the example I get this error

make BOARD=cc2538dk

Warning: no PORT set!
Building application "gcoap" for "cc2538dk" with MCU "cc2538".

"make" -C /home/alex/Documents/RIOT-Module_DTLS_Develop/pkg/nanocoap
"make" -C /home/alex/Documents/RIOT-Module_DTLS_Develop/examples/gcoap/bin/pkg/cc2538dk/nanocoap/nanocoap
"make" -C /home/alex/Documents/RIOT-Module_DTLS_Develop/pkg/tinydtls
make -C /home/alex/Documents/RIOT-Module_DTLS_Develop/examples/gcoap/bin/pkg/cc2538dk/tinydtls
/home/alex/Documents/RIOT-Module_DTLS_Develop/examples/gcoap/bin/pkg/cc2538dk/tinydtls/dtls_time.c: In function 'dtls_ticks':
/home/alex/Documents/RIOT-Module_DTLS_Develop/examples/gcoap/bin/pkg/cc2538dk/tinydtls/dtls_time.c:60:3: error: implicit declaration of function 'gettimeofday' [-Werror=implicit-function-declaration]
gettimeofday(&tv, NULL);
^~~~~~~~~~~~
cc1: all warnings being treated as errors
/home/alex/Documents/RIOT-Module_DTLS_Develop/Makefile.base:81: recipe for target '/home/alex/Documents/RIOT-Module_DTLS_Develop/examples/gcoap/bin/cc2538dk/tinydtls/dtls_time.o' failed
make[2]: *** [/home/alex/Documents/RIOT-Module_DTLS_Develop/examples/gcoap/bin/cc2538dk/tinydtls/dtls_time.o] Error 1
Makefile:13: recipe for target 'all' failed
make[1]: *** [all] Error 2
/home/alex/Documents/RIOT-Module_DTLS_Develop/examples/gcoap/../../Makefile.include:343: recipe for target '/home/alex/Documents/RIOT-Module_DTLS_Develop/examples/gcoap/bin/cc2538dk/tinydtls.a' failed
make: *** [/home/alex/Documents/RIOT-Module_DTLS_Develop/examples/gcoap/bin/cc2538dk/tinydtls.a] Error 2

what am I doing wrong? If this not the right place to contact you please tell me where I can. I need assistance. Thanks

@rfuentess
Copy link
Contributor Author

Hi @alemusix

when I compile the example I get this error

/dtls_time.c:60:3: error: implicit declaration of function 'gettimeofday' [-Werror=implicit-function-declaration]
gettimeofday(&tv, NULL);
^~~~~~~~~~~~
cc1: all warnings being treated as errors

This seems to be more a problem with GCC. Maybe this can be of help.

Also, the branch that you were using (module_gdtls_develop) was very old, and I just removed it. The one that had the "last version" of that work is module_gdtls and was used for #7177. It's working, but the support for tinyDTLS is not good enough due the use of malloc/free and is missing a lot of upgrades for gcoap.

I think you would have better time looking at #9450 that has my last efforts on this issue.

@miri64
Copy link
Member

miri64 commented Feb 1, 2019

See #9450 (comment)

@miri64 miri64 closed this as completed Feb 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking Area: pkg Area: External package ports 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.
Projects
None yet
Development

No branches or pull requests

4 participants