fixes #186 Suggested API changes for nng TLS certs

4 files changed, 73 insertions, 400 deletions

diff --git a/src/nng.h b/src/nng.h
index 05e99771..5cb5f5ec 100644
--- a/src/nng.h
+++ b/src/nng.h
@@ -617,19 +617,21 @@ NNG_DECL int nng_tls_config_server_name(nng_tls_config *, const char *);
 // nng_tls_config_ca_cert configures one or more CAs used for validation
 // of peer certificates.  Multiple CAs (and their chains) may be configured
 // by either calling this multiple times, or by specifying a list of
-// certificates as concatenated data.  The certs may be in PEM or DER
-// format.
-NNG_DECL int nng_tls_config_ca_cert(nng_tls_config *, const uint8_t *, size_t);
-
-// nng_tls_config_crl loads a certificate revocation list.  Again, these
-// are in X.509 format (either PEM or DER).
-NNG_DECL int nng_tls_config_crl(nng_tls_config *, const uint8_t *, size_t);
-
-// nng_tls_config_cert is used to load our own certificate.  For servers,
-// this may be called more than once to configure multiple different keys,
-// for example with different algorithms depending on what the peer supports.
-// On the client, only a single option is available.
-NNG_DECL int nng_tls_config_cert(nng_tls_config *, const uint8_t *, size_t);
+// certificates as concatenated data.  The final argument is an optional CRL
+// (revokation list) for the CA, also in PEM.  Both PEM strings are ASCIIZ
+// format (except that the CRL may be NULL).
+NNG_DECL int nng_tls_config_ca_chain(
+    nng_tls_config *, const char *, const char *);
+
+// nng_tls_config_own_cert is used to load our own certificate and public
+// key.  For servers, this may be called more than once to configure multiple
+// different keys, for example with different algorithms depending on what
+// the peer supports. On the client, only a single option is available.
+// The first two arguments are the cert (or validation chain) and the
+// key as PEM format ASCIIZ strings.  The final argument is an optional
+// password and may be NULL.
+NNG_DECL int nng_tls_config_own_cert(
+    nng_tls_config *, const char *, const char *, const char *);
 
 // nng_tls_config_key is used to pass our own private key.
 NNG_DECL int nng_tls_config_key(nng_tls_config *, const uint8_t *, size_t);
diff --git a/src/supplemental/tls/mbedtls/tls.c b/src/supplemental/tls/mbedtls/tls.c
index 742265f1..c37d3d13 100644
--- a/src/supplemental/tls/mbedtls/tls.c
+++ b/src/supplemental/tls/mbedtls/tls.c
@@ -1,6 +1,6 @@
 //
-// Copyright 2017 Staysail Systems, Inc. <info@staysail.tech>
-// Copyright 2017 Capitar IT Group BV <info@capitar.com>
+// Copyright 2018 Staysail Systems, Inc. <info@staysail.tech>
+// Copyright 2018 Capitar IT Group BV <info@capitar.com>
 //
 // This software is supplied under the terms of the MIT License, a
 // copy of which should be located in the distribution where this
@@ -52,13 +52,8 @@
 #endif
 
 typedef struct nni_tls_certkey {
-	char *             pass;
-	uint8_t *          crt;
-	uint8_t *          key;
-	size_t             crtlen;
-	size_t             keylen;
-	mbedtls_x509_crt   mcrt;
-	mbedtls_pk_context mpk;
+	mbedtls_x509_crt   crt;
+	mbedtls_pk_context key;
 	nni_list_node      node;
 } nni_tls_certkey;
 
@@ -180,17 +175,8 @@ nni_tls_config_fini(nng_tls_config *cfg)
 	}
 	while ((ck = nni_list_first(&cfg->certkeys))) {
 		nni_list_remove(&cfg->certkeys, ck);
-		if (ck->pass) {
-			nni_strfree(ck->pass);
-		}
-		if (ck->crt) {
-			nni_free(ck->crt, ck->crtlen);
-		}
-		if (ck->key) {
-			nni_free(ck->key, ck->keylen);
-		}
-		mbedtls_x509_crt_free(&ck->mcrt);
-		mbedtls_pk_free(&ck->mpk);
+		mbedtls_x509_crt_free(&ck->crt);
+		mbedtls_pk_free(&ck->key);
 
 		NNI_FREE_STRUCT(ck);
 	}
@@ -333,44 +319,7 @@ nni_tls_init(nni_tls **tpp, nng_tls_config *cfg, nni_plat_tcp_pipe *tcp)
 
 	nni_mtx_lock(&cfg->lk);
 	// No more changes allowed to config.
-	if (cfg->active == false) {
-		nni_tls_certkey *ck;
-
-		rv = 0;
-
-		if (cfg->have_ca_certs || cfg->have_crl) {
-			mbedtls_ssl_conf_ca_chain(
-			    &cfg->cfg_ctx, &cfg->ca_certs, &cfg->crl);
-		}
-		NNI_LIST_FOREACH (&cfg->certkeys, ck) {
-			if (rv != 0) {
-				break;
-			}
-			if (rv == 0) {
-				rv = mbedtls_x509_crt_parse(
-				    &ck->mcrt, ck->crt, ck->crtlen);
-			}
-			if (rv == 0) {
-				rv = mbedtls_pk_parse_key(&ck->mpk, ck->key,
-				    ck->keylen, (uint8_t *) ck->pass,
-				    ck->pass != NULL ? strlen(ck->pass) : 0);
-			}
-			if (rv == 0) {
-				rv = mbedtls_ssl_conf_own_cert(
-				    &cfg->cfg_ctx, &ck->mcrt, &ck->mpk);
-			}
-
-			if (rv != 0) {
-				break;
-			}
-		}
-		if (rv != 0) {
-			nni_mtx_unlock(&cfg->lk);
-			nni_tls_fini(tp);
-			return (nni_tls_mkerr(rv));
-		}
-		cfg->active = true;
-	}
+	cfg->active = true;
 	cfg->refcnt++;
 	tp->cfg = cfg;
 	nni_mtx_unlock(&cfg->lk);
@@ -841,228 +790,93 @@ nng_tls_config_auth_mode(nng_tls_config *cfg, nng_tls_auth_mode mode)
 	return (0);
 }
 
-#define PEMSTART "-----BEGIN "
-
-// nni_tls_copy_key_cert_material copies either PEM or DER encoded
-// key material.  It allocates an extra byte for a NUL terminator
-// required by mbed TLS if the data is PEM and missing the terminator.
-// It is required that the key material passed in begins with the
-// PEM delimiter if it is actually PEM.
-static int
-nni_tls_copy_key_cert_material(
-    uint8_t **dstp, size_t *szp, const uint8_t *src, size_t sz)
-{
-	bool     addz = false;
-	uint8_t *dst;
-
-	if ((sz > strlen(PEMSTART)) &&
-	    (strncmp((const char *) src, PEMSTART, strlen(PEMSTART)) == 0) &&
-	    (src[sz - 1] != '\0')) {
-		addz = true;
-	}
-
-	if (addz) {
-		if ((dst = nni_alloc(sz + 1)) != NULL) {
-			memcpy(dst, src, sz);
-			dst[sz] = '\0';
-			sz++;
-		}
-	} else {
-		if ((dst = nni_alloc(sz)) != NULL) {
-			memcpy(dst, src, sz);
-		}
-	}
-	if (dst == NULL) {
-		return (NNG_ENOMEM);
-	}
-	*dstp = dst;
-	*szp  = sz;
-	return (0);
-}
-
 int
-nng_tls_config_cert(nng_tls_config *cfg, const uint8_t *key, size_t sz)
+nng_tls_config_ca_chain(
+    nng_tls_config *cfg, const char *certs, const char *crl)
 {
-	int              rv = 0;
-	nni_tls_certkey *ck;
-	bool             cknew;
-
-	if (sz < 1) {
-		return (NNG_EINVAL);
-	}
+	size_t         len;
+	const uint8_t *pem;
+	int            rv;
 
+	// Certs and CRL are in PEM data, with terminating NUL byte.
 	nni_mtx_lock(&cfg->lk);
 	if (cfg->active) {
 		rv = NNG_ESTATE;
 		goto err;
 	}
-	cknew = false;
-	if (((ck = nni_list_last(&cfg->certkeys)) == NULL) ||
-	    (ck->crt != NULL)) {
-		if ((ck = NNI_ALLOC_STRUCT(ck)) == NULL) {
-			rv = NNG_ENOMEM;
-			goto err;
-		}
-		mbedtls_pk_init(&ck->mpk);
-		mbedtls_x509_crt_init(&ck->mcrt);
-		cknew = true;
-	}
-
-	rv = nni_tls_copy_key_cert_material(&ck->crt, &ck->crtlen, key, sz);
-	if (rv != 0) {
-		goto err;
-	}
-	if (cknew) {
-		nni_list_append(&cfg->certkeys, ck);
-	}
-err:
-	nni_mtx_unlock(&cfg->lk);
-
-	return (rv);
-}
-
-int
-nng_tls_config_key(nng_tls_config *cfg, const uint8_t *key, size_t sz)
-{
-	int              rv = 0;
-	nni_tls_certkey *ck;
-	bool             cknew;
-
-	if (sz < 1) {
-		return (NNG_EINVAL);
-	}
-
-	nni_mtx_lock(&cfg->lk);
-	if (cfg->active) {
-		rv = NNG_ESTATE;
+	pem = (const uint8_t *) certs;
+	len = strlen(certs) + 1;
+	if ((rv = mbedtls_x509_crt_parse(&cfg->ca_certs, pem, len)) != 0) {
+		rv = nni_tls_mkerr(rv);
 		goto err;
 	}
-	cknew = false;
-	if (((ck = nni_list_last(&cfg->certkeys)) == NULL) ||
-	    (ck->key != NULL)) {
-		if ((ck = NNI_ALLOC_STRUCT(ck)) == NULL) {
-			rv = NNG_ENOMEM;
+	if (crl != NULL) {
+		pem = (const uint8_t *) crl;
+		len = strlen(crl) + 1;
+		if ((rv = mbedtls_x509_crl_parse(&cfg->crl, pem, len)) != 0) {
+			rv = nni_tls_mkerr(rv);
 			goto err;
 		}
-		cknew = true;
-	}
-
-	rv = nni_tls_copy_key_cert_material(&ck->key, &ck->keylen, key, sz);
-	if (rv != 0) {
-		goto err;
-	}
-	if (cknew) {
-		nni_list_append(&cfg->certkeys, ck);
+		cfg->have_crl = true;
 	}
 err:
 	nni_mtx_unlock(&cfg->lk);
-
 	return (rv);
 }
 
 int
-nng_tls_config_pass(nng_tls_config *cfg, const char *pass)
+nng_tls_config_own_cert(
+    nng_tls_config *cfg, const char *cert, const char *key, const char *pass)
 {
-	int              rv = 0;
+	size_t           len;
+	const uint8_t *  pem;
 	nni_tls_certkey *ck;
-	bool             cknew;
+	int              rv;
 
-	if (pass == NULL) {
-		return (NNG_EINVAL);
+	if ((ck = NNI_ALLOC_STRUCT(ck)) == NULL) {
+		return (NNG_ENOMEM);
 	}
+	mbedtls_x509_crt_init(&ck->crt);
+	mbedtls_pk_init(&ck->key);
 
-	nni_mtx_lock(&cfg->lk);
-	if (cfg->active) {
-		rv = NNG_ESTATE;
+	pem = (const uint8_t *) cert;
+	len = strlen(cert) + 1;
+	if ((rv = mbedtls_x509_crt_parse(&ck->crt, pem, len)) != 0) {
+		rv = nni_tls_mkerr(rv);
 		goto err;
 	}
-	cknew = false;
-	if (((ck = nni_list_last(&cfg->certkeys)) == NULL) ||
-	    (ck->pass != NULL)) {
-		if ((ck = NNI_ALLOC_STRUCT(ck)) == NULL) {
-			rv = NNG_ENOMEM;
-			goto err;
-		}
-		cknew = true;
-	}
 
-	if ((ck->pass = nni_strdup(pass)) != NULL) {
-		rv = NNG_ENOMEM;
+	pem = (const uint8_t *) key;
+	len = strlen(key) + 1;
+	rv  = mbedtls_pk_parse_key(&ck->key, pem, len, (const uint8_t *) pass,
+	    pass != NULL ? strlen(pass) : 0);
+	if (rv != 0) {
+		rv = nni_tls_mkerr(rv);
 		goto err;
 	}
-	rv = 0;
-	if (cknew) {
-		nni_list_append(&cfg->certkeys, ck);
-	}
-err:
-	nni_mtx_unlock(&cfg->lk);
-
-	return (rv);
-}
-
-int
-nng_tls_config_ca_cert(nng_tls_config *cfg, const uint8_t *data, size_t sz)
-{
-	uint8_t *tmp;
-	size_t   len = sz;
-	int      rv  = 0;
-
-	if (sz < 1) {
-		return (NNG_EINVAL);
-	}
-
-	if ((rv = nni_tls_copy_key_cert_material(&tmp, &len, data, sz)) != 0) {
-		return (NNG_ENOMEM);
-	}
 
 	nni_mtx_lock(&cfg->lk);
 	if (cfg->active) {
+		nni_mtx_unlock(&cfg->lk);
 		rv = NNG_ESTATE;
 		goto err;
 	}
-	if ((rv = mbedtls_x509_crt_parse(&cfg->ca_certs, tmp, len)) != 0) {
-		rv = nni_tls_mkerr(rv);
-	} else {
-		cfg->have_ca_certs = true;
-	}
-err:
-	nni_mtx_unlock(&cfg->lk);
-	nni_free(tmp, len);
+	rv = mbedtls_ssl_conf_own_cert(&cfg->cfg_ctx, &ck->crt, &ck->key);
 	if (rv != 0) {
-		nni_panic("panic:");
-	}
-	return (rv);
-}
-
-int
-nng_tls_config_crl(nng_tls_config *cfg, const uint8_t *data, size_t sz)
-{
-	int      rv;
-	uint8_t *tmp;
-	size_t   len;
-
-	if (sz < 1) {
-		return (NNG_EINVAL);
-	}
-
-	if ((rv = nni_tls_copy_key_cert_material(&tmp, &len, data, sz)) != 0) {
-		return (NNG_ENOMEM);
-	}
-
-	nni_mtx_lock(&cfg->lk);
-	if (cfg->active) {
-		rv = NNG_ESTATE;
+		nni_mtx_unlock(&cfg->lk);
+		rv = nni_tls_mkerr(rv);
 		goto err;
 	}
 
-	if ((rv = mbedtls_x509_crl_parse(&cfg->crl, tmp, len)) != 0) {
-		rv = nni_tls_mkerr(rv);
-	} else {
-		cfg->have_crl = true;
-	}
-err:
+	// Save this structure so we can free it with the context.
+	nni_list_append(&cfg->certkeys, ck);
 	nni_mtx_unlock(&cfg->lk);
-	nni_free(tmp, len);
+	return (0);
+
+err:
+	mbedtls_x509_crt_free(&ck->crt);
+	mbedtls_pk_free(&ck->key);
+	NNI_FREE_STRUCT(ck);
 	return (rv);
 }
 
diff --git a/src/transport/tls/tls.c b/src/transport/tls/tls.c
index 31426a78..408ff50c 100644
--- a/src/transport/tls/tls.c
+++ b/src/transport/tls/tls.c
@@ -843,93 +843,6 @@ tls_getopt_config(void *arg, void *v, size_t *szp)
 }
 
 static int
-tls_setopt_ca_cert(void *arg, const void *data, size_t sz)
-{
-	nni_tls_ep *ep = arg;
-
-	if (ep == NULL) {
-		return (0);
-	}
-	return (nng_tls_config_ca_cert(ep->cfg, data, sz));
-}
-
-static int
-tls_setopt_cert(void *arg, const void *data, size_t sz)
-{
-	nni_tls_ep *ep = arg;
-
-	if (ep == NULL) {
-		return (0);
-	}
-	return (nng_tls_config_cert(ep->cfg, data, sz));
-}
-
-static int
-tls_setopt_private_key(void *arg, const void *data, size_t sz)
-{
-	nni_tls_ep *ep = arg;
-
-	if (ep == NULL) {
-		return (0);
-	}
-	return (nng_tls_config_key(ep->cfg, data, sz));
-}
-
-static int
-tls_setopt_pass(void *arg, const void *data, size_t sz)
-{
-	nni_tls_ep *ep = arg;
-	size_t      len;
-
-	len = nni_strnlen(data, sz);
-	if (len >= sz) {
-		return (NNG_EINVAL);
-	}
-
-	if (ep == NULL) {
-		return (0);
-	}
-	return (nng_tls_config_pass(ep->cfg, data));
-}
-
-static int
-tls_getopt_auth_mode(void *arg, void *v, size_t *szp)
-{
-	nni_tls_ep *ep = arg;
-	return (nni_getopt_int(ep->authmode, v, szp));
-}
-
-static int
-tls_setopt_auth_mode(void *arg, const void *data, size_t sz)
-{
-	nni_tls_ep *ep = arg;
-	int         mode;
-	int         rv;
-
-	rv = nni_setopt_int(&mode, data, sz, -100, 100);
-	if (rv == 0) {
-		switch (mode) {
-		case NNG_TLS_AUTH_MODE_NONE:
-		case NNG_TLS_AUTH_MODE_OPTIONAL:
-		case NNG_TLS_AUTH_MODE_REQUIRED:
-			break;
-		default:
-			rv = NNG_EINVAL;
-			break;
-		}
-	}
-
-	if ((ep == NULL) || (rv != 0)) {
-		return (rv);
-	}
-
-	if ((rv = nng_tls_config_auth_mode(ep->cfg, mode)) == 0) {
-		ep->authmode = mode;
-	}
-	return (rv);
-}
-
-static int
 tls_getopt_verified(void *arg, void *v, size_t *szp)
 {
 	nni_tls_pipe *p = arg;
@@ -973,32 +886,6 @@ static nni_tran_ep_option nni_tls_ep_options[] = {
 	    .eo_getopt = tls_getopt_config,
 	    .eo_setopt = tls_setopt_config,
 	},
-	{
-	    .eo_name   = NNG_OPT_TLS_CA_CERT,
-	    .eo_getopt = NULL,
-	    .eo_setopt = tls_setopt_ca_cert,
-	},
-	{
-	    .eo_name   = NNG_OPT_TLS_CERT,
-	    .eo_getopt = NULL,
-	    .eo_setopt = tls_setopt_cert,
-	},
-	{
-	    .eo_name   = NNG_OPT_TLS_PRIVATE_KEY,
-	    .eo_getopt = NULL,
-	    .eo_setopt = tls_setopt_private_key,
-	},
-	{
-	    .eo_name   = NNG_OPT_TLS_PRIVATE_KEY_PASSWORD,
-	    .eo_getopt = NULL,
-	    .eo_setopt = tls_setopt_pass,
-	},
-	{
-	    .eo_name   = NNG_OPT_TLS_AUTH_MODE,
-	    .eo_getopt = tls_getopt_auth_mode,
-	    .eo_setopt = tls_setopt_auth_mode,
-	},
-
 	// terminate list
 	{ NULL, NULL, NULL },
 };
diff --git a/src/transport/tls/tls.h b/src/transport/tls/tls.h
index b36ee774..25edfa3a 100644
--- a/src/transport/tls/tls.h
+++ b/src/transport/tls/tls.h
@@ -1,6 +1,6 @@
 //
-// Copyright 2017 Staysail Systems, Inc. <info@staysail.tech>
-// Copyright 2017 Capitar IT Group BV <info@capitar.com>
+// Copyright 2018 Staysail Systems, Inc. <info@staysail.tech>
+// Copyright 2018 Capitar IT Group BV <info@capitar.com>
 //
 // This software is supplied under the terms of the MIT License, a
 // copy of which should be located in the distribution where this
@@ -19,42 +19,12 @@ NNG_DECL int nng_tls_register(void);
 // started.  Once started, it is no longer possible to alter the TLS
 // configuration.
 
-// NNG_OPT_TLS_CA_CERT is a string with one or more X.509 certificates,
-// representing the entire CA chain.  The content may be either PEM or DER
-// encoded.
-#define NNG_OPT_TLS_CA_CERT "tls:ca-cert"
-
-// NNG_OPT_TLS_CRL is a PEM encoded CRL (revocation list).  Multiple lists
-// may be loaded by using this option multiple times.
-#define NNG_OPT_TLS_CRL "tls:crl"
-
-// NNG_OPT_TLS_CERT is used to specify our own certificate. At present
-// only one certificate may be supplied.  (In the future it may be
-// possible to call this multiple times, for servers that select different
-// certificates depending upon client capabilities.)
-#define NNG_OPT_TLS_CERT "tls:cert"
-
-// NNG_OPT_TLS_PRIVATE_KEY is used to specify the private key used
-// with the given certificate.  This should be called after setting
-// the certificate.  The private key may be in PEM or DER format.
-// If in PEM encoded, a terminating ZERO byte should be included.
-#define NNG_OPT_TLS_PRIVATE_KEY "tls:private-key"
-
-// NNG_OPT_TLS_PRIVATE_KEY_PASSWORD is used to specify a password
-// used for the private key.  The value is an ASCIIZ string.
-#define NNG_OPT_TLS_PRIVATE_KEY_PASSWORD "tls:private-key-password"
-
-// NNG_OPT_TLS_AUTH_MODE is an integer indicating whether our
-// peer should be verified or not.  It is required on clients/dialers,
-// and off on servers/listeners, by default.
-#define NNG_OPT_TLS_AUTH_MODE "tls:auth-mode"
-
 // NNG_OPT_TLS_AUTH_VERIFIED is a boolean that can be read on pipes,
 // indicating whether the peer certificate is verified.
 #define NNG_OPT_TLS_AUTH_VERIFIED "tls:auth-verified"
 
+// NNG_OPT_TLS_CONFIG is used to access the underlying configuration
+// (an nng_tls_config *).
 #define NNG_OPT_TLS_CONFIG "tls:config"
 
-// XXX: TBD: Ciphersuite selection and reporting.  Session reuse?
-
 #endif // NNG_TRANSPORT_TLS_TLS_H