Load CA, certificate and key files into memory when the appropriate

tls_config_set_*_file() function is called. This allows us to immediately
propagate useful error messages, play more nicely with privsep/pledge and
have a single code path. Instead of always loading the default CA when
tls_config_new() is called, defer and only load the default CA when
tls_configure() is invoked, if a CA has not already been specified.

ok beck@ bluhm@

diff --git a/lib/libtls/tls.c b/lib/libtls/tls.c
index 4d4910d..429881d 100644 (file)
--- a/lib/libtls/tls.c
+++ b/lib/libtls/tls.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls.c,v 1.44 2016/08/12 15:10:59 jsing Exp $ */
+/* $OpenBSD: tls.c,v 1.45 2016/08/13 13:05:51 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;
@@ -340,31 +323,46 @@ tls_configure_ssl(struct tls *ctx)
 int
 tls_configure_ssl_verify(struct tls *ctx, int verify)
 {
+       size_t ca_len = ctx->config->ca_len;
+       char *ca_mem = ctx->config->ca_mem;
+       char *ca_free = NULL;
+
        SSL_CTX_set_verify(ctx->ssl_ctx, verify, NULL);
 
-       if (ctx->config->ca_mem != NULL) {
-               /* XXX do this in set. */
-               if (ctx->config->ca_len > INT_MAX) {
+       /* If no CA has been specified, attempt to load the default. */
+       if (ctx->config->ca_mem == NULL && ctx->config->ca_path == NULL) {
+               if (tls_config_load_file(&ctx->error, "CA", _PATH_SSL_CA_FILE,
+                   &ca_mem, &ca_len) != 0)
+                       goto err;
+               ca_free = ca_mem;
+       }
+
+       if (ca_mem != NULL) {
+               if (ca_len > INT_MAX) {
                        tls_set_errorx(ctx, "ca too long");
                        goto err;
                }
-               if (SSL_CTX_load_verify_mem(ctx->ssl_ctx,
-                   ctx->config->ca_mem, ctx->config->ca_len) != 1) {
+               if (SSL_CTX_load_verify_mem(ctx->ssl_ctx, ca_mem,
+                   ca_len) != 1) {
                        tls_set_errorx(ctx, "ssl verify memory setup failure");
                        goto err;
                }
-       } else if (SSL_CTX_load_verify_locations(ctx->ssl_ctx,
-           ctx->config->ca_file, ctx->config->ca_path) != 1) {
-               tls_set_errorx(ctx, "ssl verify setup failure");
+       } else if (SSL_CTX_load_verify_locations(ctx->ssl_ctx, NULL,
+           ctx->config->ca_path) != 1) {
+               tls_set_errorx(ctx, "ssl verify locations failure");
                goto err;
        }
        if (ctx->config->verify_depth >= 0)
                SSL_CTX_set_verify_depth(ctx->ssl_ctx,
                    ctx->config->verify_depth);
 
+       free(ca_free);
+
        return (0);
 
  err:
+       free(ca_free);
+
        return (-1);
 }
 

diff --git a/lib/libtls/tls_config.c b/lib/libtls/tls_config.c
index e690b9e..cd2a04c 100644 (file)
--- a/lib/libtls/tls_config.c
+++ b/lib/libtls/tls_config.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_config.c,v 1.25 2016/08/12 15:10:59 jsing Exp $ */
+/* $OpenBSD: tls_config.c,v 1.26 2016/08/13 13:05:51 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"
@@ -64,9 +68,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 tls_config_load_file(error, "certificate", cert_file,
+           &keypair->cert_mem, &keypair->cert_len);
 }
 
 static int
@@ -77,9 +83,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 tls_config_load_file(error, "key", key_file,
+           &keypair->key_mem, &keypair->key_len);
 }
 
 static int
@@ -106,14 +116,59 @@ 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);
 }
 
+int
+tls_config_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", filetype);
+               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;
+}
+
 struct tls_config *
 tls_config_new(void)
 {
@@ -128,8 +183,6 @@ tls_config_new(void)
        /*
         * Default configuration.
         */
-       if (tls_config_set_ca_file(config, _PATH_SSL_CA_FILE) != 0)
-               goto err;
        if (tls_config_set_dheparams(config, "none") != 0)
                goto err;
        if (tls_config_set_ecdhecurve(config, "auto") != 0)
@@ -167,7 +220,6 @@ tls_config_free(struct tls_config *config)
        free(config->error.msg);
 
        free(config->alpn);
-       free((char *)config->ca_file);
        free((char *)config->ca_mem);
        free((char *)config->ca_path);
        free((char *)config->ciphers);
@@ -319,7 +371,8 @@ tls_config_set_alpn(struct tls_config *config, const char *alpn)
 int
 tls_config_set_ca_file(struct tls_config *config, const char *ca_file)
 {
-       return set_string(&config->ca_file, ca_file);
+       return tls_config_load_file(&config->error, "CA", ca_file,
+           &config->ca_mem, &config->ca_len);
 }
 
 int
@@ -337,7 +390,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
@@ -424,7 +478,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/lib/libtls/tls_internal.h b/lib/libtls/tls_internal.h
index 1ef95ad..fa972bb 100644 (file)
--- a/lib/libtls/tls_internal.h
+++ b/lib/libtls/tls_internal.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_internal.h,v 1.35 2016/08/12 15:10:59 jsing Exp $ */
+/* $OpenBSD: tls_internal.h,v 1.36 2016/08/13 13:05:51 jsing Exp $ */
 /*
  * Copyright (c) 2014 Jeremie Courreges-Anglas <jca@openbsd.org>
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
@@ -44,10 +44,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;
 };
@@ -57,7 +55,6 @@ struct tls_config {
 
        char *alpn;
        size_t alpn_len;
-       const char *ca_file;
        const char *ca_path;
        char *ca_mem;
        size_t ca_len;
@@ -120,8 +117,12 @@ int tls_configure_keypair(struct tls *ctx, SSL_CTX *ssl_ctx,
 int tls_configure_server(struct tls *ctx);
 int tls_configure_ssl(struct tls *ctx);
 int tls_configure_ssl_verify(struct tls *ctx, int verify);
+
 int tls_handshake_client(struct tls *ctx);
 int tls_handshake_server(struct tls *ctx);
+
+int tls_config_load_file(struct tls_error *error, const char *filetype,
+    const char *filename, char **buf, size_t *len);
 int tls_host_port(const char *hostport, char **host, char **port);
 
 int tls_error_set(struct tls_error *error, const char *fmt, ...)