Tls mutual test

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 <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);
@@ -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

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

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
 
 <!-- Macros -->
 

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)
 {
@@ -55,7 +76,6 @@ test_tls_port_zero_bind(void)
 	nng_sockaddr    sa;
 	nng_listener    l;
 	nng_dialer      d;
-	char            addr[NNG_MAXADDRSTRLEN];
 	const nng_url  *url;
 
 	c1 = tls_server_config();
@@ -66,16 +86,97 @@ test_tls_port_zero_bind(void)
 	NUTS_PASS(nng_listener_set_tls(l, c1));
 	NUTS_PASS(nng_listener_start(l, 0));
 	NUTS_PASS(nng_listener_get_url(l, &url));
-	nng_url_sprintf(addr, sizeof(addr), url);
 	NUTS_MATCH(nng_url_scheme(url), "tls+tcp");
-	NUTS_TRUE(memcmp(addr, "tls+tcp://", 6) == 0);
 	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(&d, s2, addr));
+	NUTS_PASS(nng_dialer_create_url(&d, s2, url));
+	NUTS_PASS(nng_dialer_set_tls(d, c2));
+	NUTS_PASS(nng_dialer_start(d, 0));
+	NUTS_CLOSE(s2);
+	NUTS_CLOSE(s1);
+	nng_tls_config_free(c1);
+	nng_tls_config_free(c2);
+}
+
+void
+test_tls_bad_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();
+	c2 = tls_client_config();
+
+	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));
+	// a valid cert, but not the one that signed the config!
+	NUTS_PASS(nng_tls_config_ca_chain(c1, 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));
+#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);
@@ -285,5 +386,7 @@ NUTS_TESTS = {
 	{ "tls keep alive option", test_tls_keep_alive_option },
 	{ "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 },
 };

src/supplemental/tls/mbedtls/tls.c

@@ -152,7 +152,7 @@ tls_log_warn(const char *msgid, const char *context, int errnum)
 {
 	char errbuf[256];
 	mbedtls_strerror(errnum, errbuf, sizeof(errbuf));
-	nng_log_warn(msgid, "%s: %s", context, errbuf);
+	nng_log_warn(msgid, "%s: %d - %s", context, errnum, errbuf);
 }
 
 // tls_mk_err converts an mbed error to an NNG error.

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);