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

nanocoap_sock: implement DTLS socket #18724

Merged
merged 6 commits into from
Jan 13, 2023
Merged

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Oct 11, 2022

Contribution description

This implements DTLS support for NanoCoAP.

When the nanocoap_dtls module is used, the nanocoap_sock_t struct is extended with session information.

All NanoCoAP API functions will automatically handle the new transport type, nanocoap_sock_url_connect() will automatically select the DTLS socket when a coaps:// URL is used.

I'm a bit confused by credman. I now define a CONFIG_NANOCOAP_SOCK_DTLS_TAG by which NanoCoAP can get the credential tag to select the right credential for the connection without having to add new parameter to the API. This is convenient, but it only allows a single credential to be used with NanoCoAP (which is probably fine for the use case, there is only ever a single endpoint).

Testing procedure

The tests/nanocoap_cli application works without modification. I used examples/gcoap_dtls for the server:

log of a nanocoap_cli session
# CoAP request
> ncget coap://[fe80::58b0:8ff:fe67:ad82]/vfs/
/vfs/core
/vfs/fw/
/vfs/main.c

# CoAPS request
> ncget coaps://[fe80::58b0:8ff:fe67:ad82]/vfs/
/vfs/core
/vfs/fw/
/vfs/main.c

# block-wise CoAPS request
> ncget coaps://[fe80::58b0:8ff:fe67:ad82]/vfs/main.c -
/*
 * Copyright (C) 2022 ML!PA Consulting GmbH
 *
 * This file is subject to the terms and conditions of the GNU Lesser
 * General Public License v2.1. See the file LICENSE in the top level
 * directory for more details.
 */

/**
 * @ingroup     examples
 * @{
 *
 * @file
 * @brief       Example application for demonstrating the GCoAP file server
 *
 * @author      Benjamin Valentin <benjamin.valentin@ml-pa.com>
 * @}
 */

#include "kernel_defines.h"
#include "net/gcoap.h"
#include "net/gcoap/fileserver.h"
#include "shell.h"
#include "vfs_default.h"

#define MAIN_QUEUE_SIZE (4)
static msg_t _main_msg_queue[MAIN_QUEUE_SIZE];

/* CoAP resources. Must be sorted by path (ASCII order). */
static const coap_resource_t _resources[] = {
    { "/vfs",
      COAP_GET |
#if IS_USED(MODULE_GCOAP_FILESERVER_PUT)
      COAP_PUT |
#endif
#if IS_USED(MODULE_GCOAP_FILESERVER_DELETE)
      COAP_DELETE |
#endif
      COAP_MATCH_SUBTREE,
      gcoap_fileserver_handler, VFS_DEFAULT_DATA },
};

static gcoap_listener_t _listener = {
    .resources = _resources,
    .resources_len = ARRAY_SIZE(_resources),
};

int main(void)
{
    msg_init_queue(_main_msg_queue, MAIN_QUEUE_SIZE);
    gcoap_register_listener(&_listener);

    char line_buf[SHELL_DEFAULT_BUFSIZE];
    shell_run(NULL, line_buf, SHELL_DEFAULT_BUFSIZE);

    return 0;
}
add fileserver to gcoap_dtls
diff --git a/examples/gcoap/server.c b/examples/gcoap/server.c
index bf2315cd01..ff00d67d53 100644
--- a/examples/gcoap/server.c
+++ b/examples/gcoap/server.c
@@ -29,6 +29,9 @@
 #include "net/utils.h"
 #include "od.h"
 
+#include "net/gcoap/fileserver.h"
+#include "vfs_default.h"
+
 #include "gcoap_example.h"
 
 #define ENABLE_DEBUG 0
@@ -65,6 +68,16 @@ static ssize_t _riot_board_handler(coap_pkt_t* pdu, uint8_t *buf, size_t len, co
 static const coap_resource_t _resources[] = {
     { "/cli/stats", COAP_GET | COAP_PUT, _stats_handler, NULL },
     { "/riot/board", COAP_GET, _riot_board_handler, NULL },
+    { "/vfs",
+      COAP_GET |
+#if IS_USED(MODULE_GCOAP_FILESERVER_PUT)
+      COAP_PUT |
+#endif
+#if IS_USED(MODULE_GCOAP_FILESERVER_DELETE)
+      COAP_DELETE |
+#endif
+      COAP_MATCH_SUBTREE,
+      gcoap_fileserver_handler, VFS_DEFAULT_DATA },
 };
 
 static const char *_link_params[] = {
diff --git a/examples/gcoap_dtls/Makefile b/examples/gcoap_dtls/Makefile
index 078c224838..9241c0f268 100644
--- a/examples/gcoap_dtls/Makefile
+++ b/examples/gcoap_dtls/Makefile
@@ -16,6 +16,9 @@ USEMODULE += netdev_default
 # use GNRC by default
 LWIP ?= 0
 
+USEMODULE += gcoap_fileserver
+USEMODULE += vfs_default
+
 ifeq (0,$(LWIP))
   USEMODULE += auto_init_gnrc_netif
   # Specify the mandatory networking modules

Issues/PRs references

@github-actions github-actions bot added Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: pkg Area: External package ports Area: sys Area: System Area: tests Area: tests and testing framework labels Oct 11, 2022
@miri64
Copy link
Member

miri64 commented Oct 11, 2022

Does it really make sense to add the complexity of DTLS into nanocoap? IMHO, for firmware updates, using OSCORE makes much more sense.

@benpicco
Copy link
Contributor Author

300 lines is hardly that much complexity.
The largest part, nanocoap_sock_dtls_connect() was mostly copy & paste from sock_dodtls' _connect_server() and should be moved to a common place as a follow-up.

NanoCoAP can be used for much more than firmware updates and if you don't enable nanocoap_dtls, there is no added cost.

Also AFAIK we do not have an OSCORE implementation yet.

@benpicco benpicco force-pushed the nanocoap_dtls branch 2 times, most recently from 4874932 to 3d0b9e2 Compare October 11, 2022 18:36
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 11, 2022
@riot-ci
Copy link

riot-ci commented Oct 11, 2022

Murdock results

✔️ PASSED

4f57628 sys/suit: accept coaps:// URLs

Success Failures Total Runtime
6768 0 6768 14m:02s

Artifacts

@miri64
Copy link
Member

miri64 commented Oct 11, 2022

300 lines is hardly that much complexity.

Last time I checked, TinyDTLS had much more than 300 lines of code...

Also AFAIK we do not have an OSCORE implementation yet.

Here you go: https://gitlab.com/oscore/liboscore/-/tree/master/tests/riot-tests :-)

@benpicco benpicco requested a review from fabian18 October 11, 2022 19:38
@maribu
Copy link
Member

maribu commented Oct 11, 2022

I agree with @benpicco. After all, we need to maintain the TinyDTLS package anyway. So we can just as well make use of it where sensible.

The added complexity to nanocoap is quite manageable.

@benpicco
Copy link
Contributor Author

Thank you for the review!

bors merge

@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

4 similar comments
@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@benpicco benpicco removed the Area: timers Area: timer subsystems label Jan 13, 2023
@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

Stopped waiting for PR status (GitHub check) without running due to duplicate requests to run. You may check Bors to see that this PR is included in a batch by one of the other requests.

@benpicco
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

Already running a review

bors bot added a commit that referenced this pull request Jan 13, 2023
18459: makefiles/suit: make it possible to accept multiple SUIT keys r=benpicco a=benpicco



18724: nanocoap_sock: implement DTLS socket r=benpicco a=benpicco



18763: sys/tiny_strerror: add missing error codes r=maribu a=maribu

### Contribution description

When double-checking the error codes provided by newlib by default (without magic defines, such as `__LINUX_ERRNO_EXTENSIONS__` or `__CYGWIN__`), some where still missing in `tiny_strerror()`. This adds the missing ones.

This in turn showed that three errno codes were missing in the avr-libc compat `errno.h`, which are added as well.

### Testing procedure

Murdock should double check that the added errno codes indeed are defined by default.

### Issues/PRs references

None

Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de>
Co-authored-by: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

This PR was included in a batch that was canceled, it will be automatically retried

bors bot added a commit that referenced this pull request Jan 13, 2023
18459: makefiles/suit: make it possible to accept multiple SUIT keys r=kaspar030 a=benpicco



18724: nanocoap_sock: implement DTLS socket r=benpicco a=benpicco



18763: sys/tiny_strerror: add missing error codes r=maribu a=maribu

### Contribution description

When double-checking the error codes provided by newlib by default (without magic defines, such as `__LINUX_ERRNO_EXTENSIONS__` or `__CYGWIN__`), some where still missing in `tiny_strerror()`. This adds the missing ones.

This in turn showed that three errno codes were missing in the avr-libc compat `errno.h`, which are added as well.

### Testing procedure

Murdock should double check that the added errno codes indeed are defined by default.

### Issues/PRs references

None

19136: CI: re-add "synchronize" event to check-labels r=kaspar030 a=kaspar030



Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de>
Co-authored-by: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
Co-authored-by: Kaspar Schleiser <kaspar@schleiser.de>
@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

Build failed (retrying...):

@miri64
Copy link
Member

miri64 commented Jan 13, 2023

bors retry

@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

Already running a review

@miri64
Copy link
Member

miri64 commented Jan 13, 2023

bors cancel
bors merge

@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

Canceled.

@benpicco
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

Already running a review

@bors bors bot merged commit fb603f2 into RIOT-OS:master Jan 13, 2023
@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

Build succeeded:

@benpicco benpicco deleted the nanocoap_dtls branch January 13, 2023 17:12
@jia200x jia200x added this to the Release 2023.04 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: OTA Area: Over-the-air updates Area: pkg Area: External package ports Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants