Move the keypair pubkey hash handling code to during config.

The keypair pubkey hash was being generated and set in the keypair when the
TLS context was being configured. This code should not be messing around
with the keypair contents, since it is part of the config (and not the
context).

Instead, generate the pubkey hash and store it in the keypair when the
certificate is configured. This means that we are guaranteed to have the
pubkey hash and as a side benefit, we identify bad certificate content
when it is provided, instead of during the context configuration.

ok beck@

4 files changed, 95 insertions, 69 deletions

diff --git a/src/lib/libtls/tls.c b/src/lib/libtls/tls.c
index 0e206e2c7e..8f2c7dde05 100644
--- a/src/lib/libtls/tls.c
+++ b/src/lib/libtls/tls.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls.c,v 1.74 2018/02/08 10:19:31 jsing Exp $ */
+/* $OpenBSD: tls.c,v 1.75 2018/02/10 04:57:35 jsing Exp $ */
 /*
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
  *
@@ -291,6 +291,34 @@ tls_cert_hash(X509 *cert, char **hash)
 }
 
 int
+tls_cert_pubkey_hash(X509 *cert, char **hash)
+{
+        char d[EVP_MAX_MD_SIZE], *dhex = NULL;
+        int dlen, rv = -1;
+
+        free(*hash);
+        *hash = NULL;
+
+        if (X509_pubkey_digest(cert, EVP_sha256(), d, &dlen) != 1)
+                goto err;
+
+        if (tls_hex_string(d, dlen, &dhex, NULL) != 0)
+                goto err;
+
+        if (asprintf(hash, "SHA256:%s", dhex) == -1) {
+                *hash = NULL;
+                goto err;
+        }
+
+        rv = 0;
+
+ err:
+        free(dhex);
+
+        return (rv);
+}
+
+int
 tls_configure_ssl_keypair(struct tls *ctx, SSL_CTX *ssl_ctx,
     struct tls_keypair *keypair, int required)
 {
@@ -313,9 +341,6 @@ tls_configure_ssl_keypair(struct tls *ctx, SSL_CTX *ssl_ctx,
                         tls_set_errorx(ctx, "failed to load certificate");
                         goto err;
                 }
-                if (tls_keypair_pubkey_hash(keypair, &ctx->error,
-                    &keypair->pubkey_hash) == -1)
-                        goto err;
         }
 
         if (keypair->key_mem != NULL) {
diff --git a/src/lib/libtls/tls_config.c b/src/lib/libtls/tls_config.c
index 6dfebfaebf..2dab4fc7d8 100644
--- a/src/lib/libtls/tls_config.c
+++ b/src/lib/libtls/tls_config.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_config.c,v 1.48 2018/02/10 04:41:24 jsing Exp $ */
+/* $OpenBSD: tls_config.c,v 1.49 2018/02/10 04:57:35 jsing Exp $ */
 /*
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
  *
@@ -351,12 +351,13 @@ tls_config_add_keypair_mem_internal(struct tls_config *config, const uint8_t *ce
 
         if ((keypair = tls_keypair_new()) == NULL)
                 return (-1);
-        if (tls_keypair_set_cert_mem(keypair, cert, cert_len) != 0)
+        if (tls_keypair_set_cert_mem(keypair, &config->error, cert, cert_len) != 0)
                 goto err;
-        if (tls_keypair_set_key_mem(keypair, key, key_len) != 0)
+        if (tls_keypair_set_key_mem(keypair, &config->error, key, key_len) != 0)
                 goto err;
         if (staple != NULL &&
-            tls_keypair_set_ocsp_staple_mem(keypair, staple, staple_len) != 0)
+            tls_keypair_set_ocsp_staple_mem(keypair, &config->error, staple,
+                staple_len) != 0)
                 goto err;
 
         tls_config_keypair_add(config, keypair);
@@ -431,7 +432,8 @@ int
 tls_config_set_cert_mem(struct tls_config *config, const uint8_t *cert,
     size_t len)
 {
-        return tls_keypair_set_cert_mem(config->keypair, cert, len);
+        return tls_keypair_set_cert_mem(config->keypair, &config->error,
+            cert, len);
 }
 
 int
@@ -592,7 +594,8 @@ int
 tls_config_set_key_mem(struct tls_config *config, const uint8_t *key,
     size_t len)
 {
-        return tls_keypair_set_key_mem(config->keypair, key, len);
+        return tls_keypair_set_key_mem(config->keypair, &config->error,
+            key, len);
 }
 
 static int
@@ -789,7 +792,8 @@ int
 tls_config_set_ocsp_staple_mem(struct tls_config *config, const uint8_t *staple,
     size_t len)
 {
-        return tls_keypair_set_ocsp_staple_mem(config->keypair, staple, len);
+        return tls_keypair_set_ocsp_staple_mem(config->keypair, &config->error,
+            staple, len);
 }
 
 int
diff --git a/src/lib/libtls/tls_internal.h b/src/lib/libtls/tls_internal.h
index 14265037eb..f8b9e6118e 100644
--- a/src/lib/libtls/tls_internal.h
+++ b/src/lib/libtls/tls_internal.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_internal.h,v 1.69 2018/02/10 04:41:24 jsing Exp $ */
+/* $OpenBSD: tls_internal.h,v 1.70 2018/02/10 04:57:35 jsing Exp $ */
 /*
  * Copyright (c) 2014 Jeremie Courreges-Anglas <jca@openbsd.org>
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
@@ -199,25 +199,22 @@ int tls_set_mem(char **_dest, size_t *_destlen, const void *_src,
 int tls_set_string(const char **_dest, const char *_src);
 
 struct tls_keypair *tls_keypair_new(void);
-void tls_keypair_clear_key(struct tls_keypair *_keypair);
+void tls_keypair_clear(struct tls_keypair *_keypair);
+void tls_keypair_free(struct tls_keypair *_keypair);
 int tls_keypair_set_cert_file(struct tls_keypair *_keypair,
     struct tls_error *_error, const char *_cert_file);
-int tls_keypair_set_cert_mem(struct tls_keypair *_keypair, const uint8_t *_cert,
-    size_t _len);
+int tls_keypair_set_cert_mem(struct tls_keypair *_keypair,
+    struct tls_error *_error, const uint8_t *_cert, size_t _len);
 int tls_keypair_set_key_file(struct tls_keypair *_keypair,
     struct tls_error *_error, const char *_key_file);
-int tls_keypair_set_key_mem(struct tls_keypair *_keypair, const uint8_t *_key,
-    size_t _len);
+int tls_keypair_set_key_mem(struct tls_keypair *_keypair,
+    struct tls_error *_error, const uint8_t *_key, size_t _len);
 int tls_keypair_set_ocsp_staple_file(struct tls_keypair *_keypair,
     struct tls_error *_error, const char *_ocsp_file);
 int tls_keypair_set_ocsp_staple_mem(struct tls_keypair *_keypair,
-    const uint8_t *_staple, size_t _len);
-void tls_keypair_clear(struct tls_keypair *_keypair);
-void tls_keypair_free(struct tls_keypair *_keypair);
+    struct tls_error *_error, const uint8_t *_staple, size_t _len);
 int tls_keypair_load_cert(struct tls_keypair *_keypair,
     struct tls_error *_error, X509 **_cert);
-int tls_keypair_pubkey_hash(struct tls_keypair *_keypair,
-    struct tls_error *_error, char **_hash);
 
 struct tls_sni_ctx *tls_sni_ctx_new(void);
 void tls_sni_ctx_free(struct tls_sni_ctx *sni_ctx);
@@ -281,6 +278,7 @@ struct tls_ocsp *tls_ocsp_setup_from_peer(struct tls *ctx);
 int tls_hex_string(const unsigned char *_in, size_t _inlen, char **_out,
     size_t *_outlen);
 int tls_cert_hash(X509 *_cert, char **_hash);
+int tls_cert_pubkey_hash(X509 *_cert, char **_hash);
 
 int tls_password_cb(char *_buf, int _size, int _rwflag, void *_u);
 
diff --git a/src/lib/libtls/tls_keypair.c b/src/lib/libtls/tls_keypair.c
index 626a95853f..03e7f4ad76 100644
--- a/src/lib/libtls/tls_keypair.c
+++ b/src/lib/libtls/tls_keypair.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_keypair.c,v 1.4 2018/02/08 10:19:31 jsing Exp $ */
+/* $OpenBSD: tls_keypair.c,v 1.5 2018/02/10 04:57:35 jsing Exp $ */
 /*
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
  *
@@ -29,7 +29,7 @@ tls_keypair_new(void)
         return calloc(1, sizeof(struct tls_keypair));
 }
 
-void
+static void
 tls_keypair_clear_key(struct tls_keypair *keypair)
 {
         freezero(keypair->key_mem, keypair->key_len);
@@ -37,19 +37,50 @@ tls_keypair_clear_key(struct tls_keypair *keypair)
         keypair->key_len = 0;
 }
 
+static int
+tls_keypair_pubkey_hash(struct tls_keypair *keypair, struct tls_error *error)
+{
+        X509 *cert = NULL;
+        int rv = -1;
+
+        free(keypair->pubkey_hash);
+        keypair->pubkey_hash = NULL;
+
+        if (keypair->cert_mem == NULL) {
+                rv = 0;
+                goto done;
+        }
+
+        if (tls_keypair_load_cert(keypair, error, &cert) == -1)
+                goto err;
+        if (tls_cert_pubkey_hash(cert, &keypair->pubkey_hash) == -1)
+                goto err;
+
+        rv = 0;
+
+ err:
+        X509_free(cert);
+ done:
+        return (rv);
+}
+
 int
 tls_keypair_set_cert_file(struct tls_keypair *keypair, struct tls_error *error,
     const char *cert_file)
 {
-        return tls_config_load_file(error, "certificate", cert_file,
-            &keypair->cert_mem, &keypair->cert_len);
+        if (tls_config_load_file(error, "certificate", cert_file,
+            &keypair->cert_mem, &keypair->cert_len) == -1)
+                return -1;
+        return tls_keypair_pubkey_hash(keypair, error);
 }
 
 int
-tls_keypair_set_cert_mem(struct tls_keypair *keypair, const uint8_t *cert,
-    size_t len)
+tls_keypair_set_cert_mem(struct tls_keypair *keypair, struct tls_error *error,
+    const uint8_t *cert, size_t len)
 {
-        return tls_set_mem(&keypair->cert_mem, &keypair->cert_len, cert, len);
+        if (tls_set_mem(&keypair->cert_mem, &keypair->cert_len, cert, len) == -1)
+                return -1;
+        return tls_keypair_pubkey_hash(keypair, error);
 }
 
 int
@@ -62,8 +93,8 @@ tls_keypair_set_key_file(struct tls_keypair *keypair, struct tls_error *error,
 }
 
 int
-tls_keypair_set_key_mem(struct tls_keypair *keypair, const uint8_t *key,
-    size_t len)
+tls_keypair_set_key_mem(struct tls_keypair *keypair, struct tls_error *error,
+    const uint8_t *key, size_t len)
 {
         tls_keypair_clear_key(keypair);
         return tls_set_mem(&keypair->key_mem, &keypair->key_len, key, len);
@@ -79,7 +110,7 @@ tls_keypair_set_ocsp_staple_file(struct tls_keypair *keypair,
 
 int
 tls_keypair_set_ocsp_staple_mem(struct tls_keypair *keypair,
-    const uint8_t *staple, size_t len)
+    struct tls_error *error, const uint8_t *staple, size_t len)
 {
         return tls_set_mem(&keypair->ocsp_staple, &keypair->ocsp_staple_len,
             staple, len);
@@ -88,9 +119,11 @@ tls_keypair_set_ocsp_staple_mem(struct tls_keypair *keypair,
 void
 tls_keypair_clear(struct tls_keypair *keypair)
 {
-        tls_keypair_set_cert_mem(keypair, NULL, 0);
-        tls_keypair_set_key_mem(keypair, NULL, 0);
-        tls_keypair_set_ocsp_staple_mem(keypair, NULL, 0);
+        struct tls_error error;
+
+        tls_keypair_set_cert_mem(keypair, &error, NULL, 0);
+        tls_keypair_set_key_mem(keypair, &error, NULL, 0);
+        tls_keypair_set_ocsp_staple_mem(keypair, &error, NULL, 0);
 
         free(keypair->pubkey_hash);
         keypair->pubkey_hash = NULL;
@@ -143,37 +176,3 @@ tls_keypair_load_cert(struct tls_keypair *keypair, struct tls_error *error,
 
         return (rv);
 }
-
-int
-tls_keypair_pubkey_hash(struct tls_keypair *keypair, struct tls_error *error,
-    char **hash)
-{
-        X509 *cert = NULL;
-        char d[EVP_MAX_MD_SIZE], *dhex = NULL;
-        int dlen, rv = -1;
-
-        free(*hash);
-        *hash = NULL;
-
-        if (tls_keypair_load_cert(keypair, error, &cert) == -1)
-                goto err;
-
-        if (X509_pubkey_digest(cert, EVP_sha256(), d, &dlen) != 1)
-                goto err;
-
-        if (tls_hex_string(d, dlen, &dhex, NULL) != 0)
-                goto err;
-
-        if (asprintf(hash, "SHA256:%s", dhex) == -1) {
-                *hash = NULL;
-                goto err;
-        }
-
-        rv = 0;
-
- err:
-        X509_free(cert);
-        free(dhex);
-
-        return (rv);
-}