Always load CA, key and certificate files at the time the configuration

function is called. This simplifies code and results in a single memory
based code path being used to provide data to libssl. Errors that occur
when accessing the specified file are now detected and propagated
immediately. Since the file access now occurs when the configuration
function is called, we now play nicely with privsep/pledge.

ok beck@ bluhm@ doug@

3 files changed, 72 insertions, 35 deletions

diff --git a/src/lib/libtls/tls.c b/src/lib/libtls/tls.c
index 783d320a9d..e0464ec8b1 100644
--- a/src/lib/libtls/tls.c
+++ b/src/lib/libtls/tls.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls.c,v 1.39 2016/07/06 02:32:57 jsing Exp $ */
+/* $OpenBSD: tls.c,v 1.40 2016/07/06 16:16:36 jsing Exp $ */
 /*
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
  *
@@ -216,9 +216,7 @@ tls_configure_keypair(struct tls *ctx, SSL_CTX *ssl_ctx,
 
         if (!required &&
             keypair->cert_mem == NULL &&
-            keypair->key_mem == NULL &&
-            keypair->cert_file == NULL &&
-            keypair->key_file == NULL)
+            keypair->key_mem == NULL)
                 return(0);
 
         if (keypair->cert_mem != NULL) {
@@ -260,21 +258,6 @@ tls_configure_keypair(struct tls *ctx, SSL_CTX *ssl_ctx,
                 pkey = NULL;
         }
 
-        if (keypair->cert_file != NULL) {
-                if (SSL_CTX_use_certificate_chain_file(ssl_ctx,
-                    keypair->cert_file) != 1) {
-                        tls_set_errorx(ctx, "failed to load certificate file");
-                        goto err;
-                }
-        }
-        if (keypair->key_file != NULL) {
-                if (SSL_CTX_use_PrivateKey_file(ssl_ctx,
-                    keypair->key_file, SSL_FILETYPE_PEM) != 1) {
-                        tls_set_errorx(ctx, "failed to load private key file");
-                        goto err;
-                }
-        }
-
         if (SSL_CTX_check_private_key(ssl_ctx) != 1) {
                 tls_set_errorx(ctx, "private/public key mismatch");
                 goto err;
@@ -346,7 +329,7 @@ tls_configure_ssl_verify(struct tls *ctx, int verify)
                         goto err;
                 }
         } else if (SSL_CTX_load_verify_locations(ctx->ssl_ctx,
-            ctx->config->ca_file, ctx->config->ca_path) != 1) {
+            NULL, ctx->config->ca_path) != 1) {
                 tls_set_errorx(ctx, "ssl verify setup failure");
                 goto err;
         }
diff --git a/src/lib/libtls/tls_config.c b/src/lib/libtls/tls_config.c
index 8f73a5a45b..cfd054b024 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.18 2016/05/27 14:38:40 jsing Exp $ */
+/* $OpenBSD: tls_config.c,v 1.19 2016/07/06 16:16:36 jsing Exp $ */
 /*
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
  *
@@ -15,9 +15,13 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include <sys/stat.h>
+
 #include <ctype.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <stdlib.h>
+#include <unistd.h>
 
 #include <tls.h>
 #include "tls_internal.h"
@@ -57,6 +61,53 @@ set_mem(char **dest, size_t *destlen, const void *src, size_t srclen)
         return 0;
 }
 
+static int
+load_file(struct tls_error *error, const char *filetype, const char *filename,
+    char **buf, size_t *len)
+{
+        struct stat st;
+        int fd = -1;
+
+        free(*buf);
+        *buf = NULL;
+        *len = 0;
+
+        if ((fd = open(filename, O_RDONLY)) == -1) {
+                tls_error_set(error, "failed to open %s file '%s'",
+                    filetype, filename);
+                goto fail;
+        }
+        if (fstat(fd, &st) != 0) {
+                tls_error_set(error, "failed to stat %s file '%s'",
+                    filetype, filename);
+                goto fail;
+        }
+        *len = (size_t)st.st_size;
+        if ((*buf = malloc(*len)) == NULL) {
+                tls_error_set(error, "failed to allocate buffer for "
+                    "%s file '%s'", filetype, filename);
+                goto fail;
+        }
+        if (read(fd, *buf, *len) != *len) {
+                tls_error_set(error, "failed to read %s file '%s'",
+                    filetype, filename);
+                goto fail;
+        }
+        close(fd);
+        return 0;
+
+ fail:
+        if (fd != -1)
+                close(fd);
+        if (*buf != NULL)
+                explicit_bzero(*buf, *len);
+        free(*buf);
+        *buf = NULL;
+        *len = 0;
+
+        return -1;
+}
+
 static struct tls_keypair *
 tls_keypair_new()
 {
@@ -64,9 +115,11 @@ tls_keypair_new()
 }
 
 static int
-tls_keypair_set_cert_file(struct tls_keypair *keypair, const char *cert_file)
+tls_keypair_set_cert_file(struct tls_keypair *keypair, struct tls_error *error,
+    const char *cert_file)
 {
-        return set_string(&keypair->cert_file, cert_file);
+        return load_file(error, "certificate", cert_file, &keypair->cert_mem,
+            &keypair->cert_len);
 }
 
 static int
@@ -77,9 +130,13 @@ tls_keypair_set_cert_mem(struct tls_keypair *keypair, const uint8_t *cert,
 }
 
 static int
-tls_keypair_set_key_file(struct tls_keypair *keypair, const char *key_file)
+tls_keypair_set_key_file(struct tls_keypair *keypair, struct tls_error *error,
+    const char *key_file)
 {
-        return set_string(&keypair->key_file, key_file);
+        if (keypair->key_mem != NULL)
+                explicit_bzero(keypair->key_mem, keypair->key_len);
+        return load_file(error, "key", key_file, &keypair->key_mem,
+            &keypair->key_len);
 }
 
 static int
@@ -106,9 +163,7 @@ tls_keypair_free(struct tls_keypair *keypair)
 
         tls_keypair_clear(keypair);
 
-        free((char *)keypair->cert_file);
         free(keypair->cert_mem);
-        free((char *)keypair->key_file);
         free(keypair->key_mem);
 
         free(keypair);
@@ -166,7 +221,6 @@ tls_config_free(struct tls_config *config)
 
         free(config->error.msg);
 
-        free((char *)config->ca_file);
         free((char *)config->ca_mem);
         free((char *)config->ca_path);
         free((char *)config->ciphers);
@@ -252,7 +306,8 @@ tls_config_parse_protocols(uint32_t *protocols, const char *protostr)
 int
 tls_config_set_ca_file(struct tls_config *config, const char *ca_file)
 {
-        return set_string(&config->ca_file, ca_file);
+        return load_file(&config->error, "CA", ca_file, &config->ca_mem,
+            &config->ca_len);
 }
 
 int
@@ -270,7 +325,8 @@ tls_config_set_ca_mem(struct tls_config *config, const uint8_t *ca, size_t len)
 int
 tls_config_set_cert_file(struct tls_config *config, const char *cert_file)
 {
-        return tls_keypair_set_cert_file(config->keypair, cert_file);
+        return tls_keypair_set_cert_file(config->keypair, &config->error,
+            cert_file);
 }
 
 int
@@ -337,7 +393,8 @@ tls_config_set_ecdhecurve(struct tls_config *config, const char *name)
 int
 tls_config_set_key_file(struct tls_config *config, const char *key_file)
 {
-        return tls_keypair_set_key_file(config->keypair, key_file);
+        return tls_keypair_set_key_file(config->keypair, &config->error,
+            key_file);
 }
 
 int
diff --git a/src/lib/libtls/tls_internal.h b/src/lib/libtls/tls_internal.h
index 745fb40c76..b7a1530c96 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.29 2016/05/27 14:38:40 jsing Exp $ */
+/* $OpenBSD: tls_internal.h,v 1.30 2016/07/06 16:16:36 jsing Exp $ */
 /*
  * Copyright (c) 2014 Jeremie Courreges-Anglas <jca@openbsd.org>
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
@@ -42,10 +42,8 @@ struct tls_error {
 struct tls_keypair {
         struct tls_keypair *next;
 
-        const char *cert_file;
         char *cert_mem;
         size_t cert_len;
-        const char *key_file;
         char *key_mem;
         size_t key_len;
 };
@@ -53,7 +51,6 @@ struct tls_keypair {
 struct tls_config {
         struct tls_error error;
 
-        const char *ca_file;
         const char *ca_path;
         char *ca_mem;
         size_t ca_len;