golioth / golioth-zephyr-sdk

Golioth SDK For Zephyr
https://www.golioth.io
Apache License 2.0
66 stars 19 forks source link

golioth: introduce GOLIOTH_CIPHERSUITES Kconfig option #319

Closed mniestroj closed 1 year ago

mniestroj commented 1 year ago

(D)TLS negotiation happens by using TLS sockets, which can be either offloaded to external TLS stack (e.g. nRF9160 modem TLS layer) or handled by mbedTLS library using Zephyr TLS "native" sockets implementation. So far no ciphersuites were selected explicitly by this SDK, so selection of negotiated ciphersuites was up to underlying TLS stack (e.g. mbedTLS library for "native" TLS sockets). This is fine as long as there is limited mbedTLS configuration and limited supported ciphersuites by Golioth server.

As of today with default configuration (e.g. when building samples from samples/ directory), mbedTLS proposes to use bunch of RSA cert-based ciphersuites and bunch of PSK-based ciphersuites. Golioth server only supports PSK-based ciphersuites, so PSK ciphersuite is being negotiated. Well done.

With the upcoming cert-based authentication support in Golioth server there is a potential problem. Even when SDK is provisioned with PSK (and PSK-ID), still all the enabled cert-based authentication algorithms are proposed in (D)TLS ClientHello message. This allows Golioth server to choose cert-based authentication over PSK-based, as the former is considered more secure. If this happens, then mbedTLS library tries to lookup certificates for negotiated connection, which fails as only PSK + PSK-ID were provisioned. (D)TLS connection attempt is aborted and restarted in a while, with the same result. This creates an endless loop. This isn't a problem yet with samples included in samples/ directory, as Golioth server will support ECDSA-based ciphersuites (at least in the next step) instead of RSA-based ciphersuites. However, potential problem is still there in case Golioth server gets support for RSA-based ciphersuites. ECDSA-based ciphersuite support in Golioth server still creates a problem for devices with extended (compared to configuration used in samples/) mbedTLS configuration, with support for ECDSA being enabled (e.g. when application needs ECDSA algorithms for other purposes than Golioth cloud connection). In such case mbedTLS will include ECDSA-based ciphersuites in (D)TLS ClientHello message, so Golioth server will select ECDSA-based ciphersuite (as they are prioritized over PSK-based ciphersuites) for which device might not have credentials configured (when device firmware is configured for PSK-based authentication). Such case also results in endless loop.

Introduce a Kconfig option that allows to select which specific ciphersuites to use during (D)TLS handshake. This makes sure that server selected ciphersuite (from the list of client proposed ciphersuites) is both supported and provisioned with credentials. Default value for CONFIG_MBEDTLS=y builds (for which we have most control) depends on enabled cryptographic algorithms and prioritizes ciphersuites following mbedTLS internal priorities. When CONFIG_MBEDTLS_BUILTIN=n all server supported ciphersuites are selected, which allows TLS layer to choose any of proposed algorithms.

The same as for Zephyr CONFIG_MBEDTLS_BUILTIN was done for NCS flavour of mbedTLS (ZEPHYR_NRF_MODULE && MBEDTLS_TLS_LIBRARY).

github-actions[bot] commented 1 year ago

Visit the preview URL for this PR (updated for commit 54a7b77):

https://golioth-zephyr-sdk-doxygen-dev--pr319-ciphersuites-lim-12bor9df.web.app

(expires Thu, 17 Nov 2022 16:39:37 GMT)

šŸ”„ via Firebase Hosting GitHub Action šŸŒŽ

Sign: a389eefadf4b4b68a539327b3459dd66c142cf49

mniestroj commented 1 year ago

Unless I'm missing something, this checkpatch warning is a false-positive:

a7d8ebc2984f95581c6b47256a7c784f677bacdb:162: WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (8, 0)
#1[6](https://github.com/golioth/golioth-zephyr-sdk/actions/runs/3438665742/jobs/5735014754#step:5:7)2: FILE: net/golioth/golioth.c:29:
+   FOR_EACH_NONEMPTY_TERM(GOLIOTH_CIPHERSUITE_ENTRY, (,), GOLIOTH_CIPHERSUITES)
+};