diff --git a/docs/man/nng_tls_config_own_cert.3tls.adoc b/docs/man/nng_tls_config_own_cert.3tls.adoc index 660273055..814bc94c6 100644 --- a/docs/man/nng_tls_config_own_cert.3tls.adoc +++ b/docs/man/nng_tls_config_own_cert.3tls.adoc @@ -18,7 +18,6 @@ nng_tls_config_own_cert - configure own certificate and key [source, c] ---- #include -#include int nng_tls_config_own_cert(nng_tls_config *cfg, const char *cert, const char *key, const char *pass); @@ -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 diff --git a/docs/ref/migrate/nng1.md b/docs/ref/migrate/nng1.md index 2b305b756..cb7bc5393 100644 --- a/docs/ref/migrate/nng1.md +++ b/docs/ref/migrate/nng1.md @@ -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 diff --git a/docs/ref/xref.md b/docs/ref/xref.md index d36f96e8c..5e030bede 100644 --- a/docs/ref/xref.md +++ b/docs/ref/xref.md @@ -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 diff --git a/src/sp/transport/tls/tls_tran_test.c b/src/sp/transport/tls/tls_tran_test.c index c6889b23f..6e1835a66 100644 --- a/src/sp/transport/tls/tls_tran_test.c +++ b/src/sp/transport/tls/tls_tran_test.c @@ -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) { @@ -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) { @@ -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) { @@ -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 }, }; diff --git a/src/supplemental/tls/tls_common.c b/src/supplemental/tls/tls_common.c index aa34b533d..c3c4d3c35 100644 --- a/src/supplemental/tls/tls_common.c +++ b/src/supplemental/tls/tls_common.c @@ -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 @@ -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);