Always calculate the hash value of the x509 cert in ssl_load_pkey().

Check whether TLS server object is available before using it.  With
these fixes the ssl inspect regress test just fails and does not
crash relayd.
OK claudio@

4 files changed, 50 insertions, 47 deletions

diff --git a/usr.sbin/relayd/ca.c b/usr.sbin/relayd/ca.c
index 94e80f5c6b2..c4eb9162d17 100644
--- a/usr.sbin/relayd/ca.c
+++ b/usr.sbin/relayd/ca.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ca.c,v 1.26 2017/05/28 10:39:15 benno Exp $	*/
+/*	$OpenBSD: ca.c,v 1.27 2017/07/28 13:58:52 bluhm Exp $	*/
 
 /*
  * Copyright (c) 2014 Reyk Floeter <reyk@openbsd.org>
@@ -82,20 +82,25 @@ ca_init(struct privsep *ps, struct privsep_proc *p, void *arg)
 	env->sc_id = getpid() & 0xffff;
 }
 
-static void
-hash_string(char *d, size_t dlen, char *hash, size_t hashlen)
+void
+hash_x509(X509 *cert, char *hash, size_t hashlen)
 {
 	static const char	hex[] = "0123456789abcdef";
-	size_t			off, i;
+	size_t			off;
+	char			digest[EVP_MAX_MD_SIZE];
+	int		 	dlen, i;
+
+	if (X509_digest(cert, EVP_sha256(), digest, &dlen) != 1)
+		fatalx("%s: X509_digest failed", __func__);
 
 	if (hashlen < 2 * dlen + sizeof("SHA256:"))
-		fatalx("%s hash buffer to small", __func__);
+		fatalx("%s: hash buffer to small", __func__);
 
 	off = strlcpy(hash, "SHA256:", hashlen);
 
 	for (i = 0; i < dlen; i++) {
-		hash[off++] = hex[(d[i] >> 4) & 0x0f];
-		hash[off++] = hex[d[i] & 0x0f];
+		hash[off++] = hex[(digest[i] >> 4) & 0x0f];
+		hash[off++] = hex[digest[i] & 0x0f];
 	}
 	hash[off] = 0;
 }
@@ -103,12 +108,11 @@ hash_string(char *d, size_t dlen, char *hash, size_t hashlen)
 void
 ca_launch(void)
 {
-	char		 d[EVP_MAX_MD_SIZE], hash[TLS_CERT_HASH_SIZE];
+	char		 hash[TLS_CERT_HASH_SIZE];
 	BIO		*in = NULL;
 	EVP_PKEY	*pkey = NULL;
 	struct relay	*rlay;
 	X509		*cert = NULL;
-	int		 dlen;
 
 	TAILQ_FOREACH(rlay, env->sc_relays, rl_entry) {
 		if ((rlay->rl_conf.flags & (F_TLS|F_TLSCLIENT)) == 0)
@@ -123,10 +127,7 @@ ca_launch(void)
 			    NULL, NULL)) == NULL)
 				fatalx("ca_launch: cert");
 
-			if (X509_digest(cert, EVP_sha256(), d, &dlen) != 1)
-				fatalx("ca_launch: cert");
-
-			hash_string(d, dlen, hash, sizeof(hash));
+			hash_x509(cert, hash, sizeof(hash));
 
 			BIO_free(in);
 			X509_free(cert);
@@ -161,10 +162,7 @@ ca_launch(void)
 			    NULL, NULL)) == NULL)
 				fatalx("ca_launch: cacert");
 
-			if (X509_digest(cert, EVP_sha256(), d, &dlen) != 1)
-				fatalx("ca_launch: cacert");
-
-			hash_string(d, dlen, hash, sizeof(hash));
+			hash_x509(cert, hash, sizeof(hash));
 
 			BIO_free(in);
 			X509_free(cert);
@@ -234,9 +232,11 @@ ca_dispatch_relay(int fd, struct privsep_proc *p, struct imsg *imsg)
 			fatalx("%s: invalid relay proc", __func__);
 		if (IMSG_DATA_SIZE(imsg) != (sizeof(cko) + cko.cko_flen))
 			fatalx("%s: invalid key operation", __func__);
-		if ((pkey = pkey_find(env, cko.cko_hash)) == NULL ||
-		    (rsa = EVP_PKEY_get1_RSA(pkey)) == NULL)
-			fatalx("%s: invalid relay key or id", __func__);
+		if ((pkey = pkey_find(env, cko.cko_hash)) == NULL)
+			fatalx("%s: invalid relay hash '%s'",
+			    __func__, cko.cko_hash);
+		if ((rsa = EVP_PKEY_get1_RSA(pkey)) == NULL)
+			fatalx("%s: invalid relay key", __func__);
 
 		DPRINTF("%s:%d: key hash %s proc %d",
 		    __func__, __LINE__, cko.cko_hash, cko.cko_proc);
diff --git a/usr.sbin/relayd/relay.c b/usr.sbin/relayd/relay.c
index 5a251026e3c..32bba1f0cc0 100644
--- a/usr.sbin/relayd/relay.c
+++ b/usr.sbin/relayd/relay.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: relay.c,v 1.222 2017/07/04 19:59:51 benno Exp $	*/
+/*	$OpenBSD: relay.c,v 1.223 2017/07/28 13:58:52 bluhm Exp $	*/
 
 /*
  * Copyright (c) 2006 - 2014 Reyk Floeter <reyk@openbsd.org>
@@ -2100,12 +2100,12 @@ relay_tls_ctx_create(struct relay *rlay)
 
 		if (rlay->rl_conf.tls_cacert_len) {
 			log_debug("%s: loading CA certificate", __func__);
-			if (!ssl_load_pkey(&rlay->rl_conf.tls_cakeyid,
+			if (!ssl_load_pkey(
 			    rlay->rl_tls_cacert, rlay->rl_conf.tls_cacert_len,
 			    &rlay->rl_tls_cacertx509, &rlay->rl_tls_capkey))
 				goto err;
 			/* loading certificate public key */
-			if (!ssl_load_pkey(NULL,
+			if (!ssl_load_pkey(
 			    rlay->rl_tls_cert, rlay->rl_conf.tls_cert_len,
 			    NULL, &rlay->rl_tls_pkey))
 				goto err;
@@ -2207,6 +2207,10 @@ relay_tls_transaction(struct rsession *con, struct ctl_relay_event *cre)
 			tls_server = relay_tls_inspect_create(rlay, cre);
 		else
 			tls_server = rlay->rl_tls_ctx;
+		if (tls_server == NULL) {
+			errstr = "no TLS server context available";
+			goto err;
+		}
 
 		if (tls_accept_socket(tls_server, &cre->tls, cre->s) == -1) {
 			errstr = "could not accept the TLS connection";
diff --git a/usr.sbin/relayd/relayd.h b/usr.sbin/relayd/relayd.h
index 243a9c27bae..6d1ed6e1b0a 100644
--- a/usr.sbin/relayd/relayd.h
+++ b/usr.sbin/relayd/relayd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: relayd.h,v 1.241 2017/07/04 19:59:51 benno Exp $	*/
+/*	$OpenBSD: relayd.h,v 1.242 2017/07/28 13:58:52 bluhm Exp $	*/
 
 /*
  * Copyright (c) 2006 - 2016 Reyk Floeter <reyk@openbsd.org>
@@ -1245,12 +1245,13 @@ void	 ssl_init(struct relayd *);
 char	*ssl_load_key(struct relayd *, const char *, off_t *, char *);
 uint8_t *ssl_update_certificate(const uint8_t *, size_t, EVP_PKEY *,
 	    EVP_PKEY *, X509 *, size_t *);
-int	 ssl_load_pkey(void *, char *, off_t, X509 **, EVP_PKEY **);
+int	 ssl_load_pkey(char *, off_t, X509 **, EVP_PKEY **);
 int	 ssl_ctx_fake_private_key(char *, off_t, const char **);
 
 /* ca.c */
 void	 ca(struct privsep *, struct privsep_proc *);
 void	 ca_engine_init(struct relayd *);
+void	 hash_x509(X509 *cert, char *hash, size_t hashlen);
 
 /* relayd.c */
 struct host	*host_find(struct relayd *, objid_t);
diff --git a/usr.sbin/relayd/ssl.c b/usr.sbin/relayd/ssl.c
index cf576fbb359..623ca8ac802 100644
--- a/usr.sbin/relayd/ssl.c
+++ b/usr.sbin/relayd/ssl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ssl.c,v 1.33 2017/05/28 10:39:15 benno Exp $	*/
+/*	$OpenBSD: ssl.c,v 1.34 2017/07/28 13:58:52 bluhm Exp $	*/
 
 /*
  * Copyright (c) 2007 - 2014 Reyk Floeter <reyk@openbsd.org>
@@ -205,62 +205,60 @@ done:
 }
 
 int
-ssl_load_pkey(void *data, char *buf, off_t len,
-    X509 **x509ptr, EVP_PKEY **pkeyptr)
+ssl_load_pkey(char *buf, off_t len, X509 **x509ptr, EVP_PKEY **pkeyptr)
 {
 	BIO		*in;
 	X509		*x509 = NULL;
 	EVP_PKEY	*pkey = NULL;
 	RSA		*rsa = NULL;
-	void		*exdata = NULL;
+	char		*hash = NULL;
 
 	if ((in = BIO_new_mem_buf(buf, len)) == NULL) {
 		log_warnx("%s: BIO_new_mem_buf failed", __func__);
 		return (0);
 	}
-
 	if ((x509 = PEM_read_bio_X509(in, NULL,
 	    ssl_password_cb, NULL)) == NULL) {
 		log_warnx("%s: PEM_read_bio_X509 failed", __func__);
 		goto fail;
 	}
-
 	if ((pkey = X509_get_pubkey(x509)) == NULL) {
 		log_warnx("%s: X509_get_pubkey failed", __func__);
 		goto fail;
 	}
-
-	BIO_free(in);
-
-	if (data != NULL) {
-		if ((rsa = EVP_PKEY_get1_RSA(pkey)) == NULL) {
-			log_warnx("%s: failed to extract RSA", __func__);
-			goto fail;
-		}
-
-		RSA_set_ex_data(rsa, 0, data);
-		RSA_free(rsa); /* dereference, will be cleaned up with pkey */
+	if ((rsa = EVP_PKEY_get1_RSA(pkey)) == NULL) {
+		log_warnx("%s: failed to extract RSA", __func__);
+		goto fail;
+	}
+	if ((hash = malloc(TLS_CERT_HASH_SIZE)) == NULL) {
+		log_warn("%s: allocate hash failed", __func__);
+		goto fail;
+	}
+	hash_x509(x509, hash, TLS_CERT_HASH_SIZE);
+	if (RSA_set_ex_data(rsa, 0, hash) != 1) {
+		log_warnx("%s: failed to set hash as exdata", __func__);
+		goto fail;
 	}
 
+	RSA_free(rsa); /* dereference, will be cleaned up with pkey */
+	*pkeyptr = pkey;
 	if (x509ptr != NULL)
 		*x509ptr = x509;
 	else
 		X509_free(x509);
-
-	*pkeyptr = pkey;
+	BIO_free(in);
 
 	return (1);
 
  fail:
+	free(hash);
 	if (rsa != NULL)
 		RSA_free(rsa);
-	if (in != NULL)
-		BIO_free(in);
 	if (pkey != NULL)
 		EVP_PKEY_free(pkey);
 	if (x509 != NULL)
 		X509_free(x509);
-	free(exdata);
+	BIO_free(in);
 
 	return (0);
 }