Skip to content

Commit

Permalink
tls: add a mutual authentication test
Browse files Browse the repository at this point in the history
Also, make it clearer that TLS keys and certificates can only
be set once on a configuration.  (mbedTLS makes this confusing!)

This mutual test is only fully validated on mbed, because wolfSSL
seems to not properly validate this in many configurations.
  • Loading branch information
gdamore committed Nov 23, 2024
1 parent b4ef0f3 commit 9bbb134
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 6 deletions.
8 changes: 3 additions & 5 deletions docs/man/nng_tls_config_own_cert.3tls.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ nng_tls_config_own_cert - configure own certificate and key
[source, c]
----
#include <nng/nng.h>
#include <nng/supplemental/tls/tls.h>
int nng_tls_config_own_cert(nng_tls_config *cfg, const char *cert,
const char *key, const char *pass);
Expand All @@ -38,10 +37,9 @@ have it, and will have to in order to validate this certificate anyway).
The _key_ may be encrypted with a password, in which can be supplied in _pass_.
The value `NULL` should be supplied for _pass_ if the key is not encrypted.

On servers, it is possible to call this function multiple times for the
same configuration.
This can be useful for specifying different parameters
to be used for different cryptographic algorithms.
This cannot be called more than once for a given TLS configuration.
(Earlier versions of NNG allowed this, but it was never used, brittle,
and the source of confusion.)

The certificate located in _cert_ and _key_ must be NUL (`\0`) terminated C
strings containing
Expand Down
7 changes: 7 additions & 0 deletions docs/ref/migrate/nng1.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@ Support for very old TLS versions 1.0 and 1.1 is removed.
Further, the `NNG_TLS_1_0` and `NNG_TLS_1_1` constants are also removed.
Applications should use `NNG_TLS_1_2` or even `NNG_TLS_1_3` instead.

## Only One TLS Key/Cert Per Configuration

The ability to configure multiple keys and certificates for a given TLS configuration object is removed.
(The [`nng_tls_config_own_cert`] will return [`NNG_EBUSY`] if it has already been called for the configuration.)
The intended purpose was to support alternative cryptographic algorithms, but this is not necessary, was never
used, and was error prone.

## Support for Local Addresses in Dial URLs Removed

NNG 1.x had an undocumented ability to specify the local address to bind
Expand Down
2 changes: 2 additions & 0 deletions docs/ref/xref.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@
[`nng_recv`]: /TODO.md
[`nng_listener_get_url`]: /TODO.md
[`nng_dialer_get_url`]: /TODO.md
[`nng_tls_config`]: /TODO.md
[`nng_tls_config_own_cert`]: /TODO.md

<!-- Macros -->

Expand Down
67 changes: 67 additions & 0 deletions src/sp/transport/tls/tls_tran_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@ tls_server_config(void)
return (c);
}

static nng_tls_config *
tls_server_config_ecdsa(void)
{
nng_tls_config *c;
NUTS_PASS(nng_tls_config_alloc(&c, NNG_TLS_MODE_SERVER));
NUTS_PASS(nng_tls_config_own_cert(
c, nuts_ecdsa_server_crt, nuts_ecdsa_server_key, NULL));
return (c);
}

static nng_tls_config *
tls_config_psk(nng_tls_mode mode, const char *name, uint8_t *key, size_t len)
{
Expand All @@ -46,6 +56,17 @@ tls_client_config(void)
return (c);
}

static nng_tls_config *
tls_client_config_ecdsa(void)
{
nng_tls_config *c;
NUTS_PASS(nng_tls_config_alloc(&c, NNG_TLS_MODE_CLIENT));
NUTS_PASS(nng_tls_config_own_cert(
c, nuts_ecdsa_client_crt, nuts_ecdsa_client_key, NULL));
NUTS_PASS(nng_tls_config_ca_chain(c, nuts_ecdsa_server_crt, NULL));
return (c);
}

void
test_tls_port_zero_bind(void)
{
Expand Down Expand Up @@ -110,13 +131,58 @@ test_tls_bad_cert_mutual(void)
NUTS_TRUE(sa.s_in.sa_addr = nuts_be32(0x7f000001));
NUTS_PASS(nng_dialer_create_url(&d, s2, url));
NUTS_PASS(nng_dialer_set_tls(d, c2));
#ifdef NNG_TLS_ENGINE_MBEDTLS
NUTS_FAIL(nng_dialer_start(d, 0), NNG_ECRYPTO);
#else
// WolfSSL doesn't validate this here.
nng_dialer_start(d, 0);
#endif
nng_msleep(50);
NUTS_CLOSE(s2);
NUTS_CLOSE(s1);
nng_tls_config_free(c1);
nng_tls_config_free(c2);
}

void
test_tls_cert_mutual(void)
{
nng_socket s1;
nng_socket s2;
nng_tls_config *c1, *c2;
nng_sockaddr sa;
nng_listener l;
nng_dialer d;
const nng_url *url;

c1 = tls_server_config_ecdsa();
c2 = tls_client_config_ecdsa();

NUTS_ENABLE_LOG(NNG_LOG_DEBUG);
NUTS_OPEN(s1);
NUTS_OPEN(s2);
NUTS_PASS(nng_tls_config_auth_mode(c1, NNG_TLS_AUTH_MODE_REQUIRED));
NUTS_PASS(nng_tls_config_ca_chain(c1, nuts_ecdsa_server_crt, NULL));
NUTS_PASS(nng_tls_config_ca_chain(c2, nuts_ecdsa_server_crt, NULL));
NUTS_PASS(nng_listener_create(&l, s1, "tls+tcp://127.0.0.1:0"));
NUTS_PASS(nng_listener_set_tls(l, c1));
NUTS_PASS(nng_listener_start(l, 0));
NUTS_PASS(nng_listener_get_url(l, &url));
NUTS_MATCH(nng_url_scheme(url), "tls+tcp");
NUTS_PASS(nng_listener_get_addr(l, NNG_OPT_LOCADDR, &sa));
NUTS_TRUE(sa.s_in.sa_family == NNG_AF_INET);
NUTS_TRUE(sa.s_in.sa_port != 0);
NUTS_TRUE(sa.s_in.sa_addr = nuts_be32(0x7f000001));
NUTS_PASS(nng_dialer_create_url(&d, s2, url));
NUTS_PASS(nng_dialer_set_tls(d, c2));
NUTS_PASS(nng_dialer_start(d, 0));
nng_msleep(50);
NUTS_CLOSE(s2);
NUTS_CLOSE(s1);
nng_tls_config_free(c1);
nng_tls_config_free(c2);
}

void
test_tls_malformed_address(void)
{
Expand Down Expand Up @@ -321,5 +387,6 @@ NUTS_TESTS = {
{ "tls recv max", test_tls_recv_max },
{ "tls pre-shared key", test_tls_psk },
{ "tsl bad cert mutual", test_tls_bad_cert_mutual },
{ "tsl cert mutual", test_tls_cert_mutual },
{ NULL, NULL },
};
9 changes: 8 additions & 1 deletion src/supplemental/tls/tls_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ struct nng_tls_config {
nni_mtx lock;
int ref;
bool busy;
bool key_is_set;
size_t size;

// ... engine config data follows
Expand Down Expand Up @@ -1140,10 +1141,16 @@ nng_tls_config_own_cert(
{
int rv;
nni_mtx_lock(&cfg->lock);
if (cfg->busy) {
// NB: we cannot set the key if we already have done so.
// This is because some lower layers create a "stack" of keys
// and certificates, and this will almost certainly lead to confusion.
if (cfg->busy || cfg->key_is_set) {
rv = NNG_EBUSY;
} else {
rv = cfg->ops.own_cert((void *) (cfg + 1), cert, key, pass);
if (rv == 0) {
cfg->key_is_set = true;
}
}
nni_mtx_unlock(&cfg->lock);
return (rv);
Expand Down

0 comments on commit 9bbb134

Please sign in to comment.