Deep down inside OpenSSL, err... LibreSSL, RSA_set_ex_data attempts to

free() the external data when releasing the RSA object.  The
RSA_GET_EX_NEW_INDEX(3) manual page doesn't mention that this is the
default behaviour - it just describes the possible free_func()
callback - and the code path in libcrypto is hiding the fact behind
layers of abstraction.

Fix possible double free by allocating and copying the external data
reference that is used for RSA privsep (pkiname in smtpd's case).

ok eric@ gilles@

diff --git a/usr.sbin/smtpd/ssl.c b/usr.sbin/smtpd/ssl.c
index 99d2b63..6ea7c6b 100644 (file)
--- a/usr.sbin/smtpd/ssl.c
+++ b/usr.sbin/smtpd/ssl.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ssl.c,v 1.65 2014/05/10 21:34:07 reyk Exp $   */
+/*     $OpenBSD: ssl.c,v 1.66 2014/05/20 14:21:45 reyk Exp $   */
 
 /*
  * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@openbsd.org>
@@ -246,9 +246,10 @@ fail:
 }
 
 SSL_CTX *
-ssl_ctx_create(void *pkiname, char *cert, off_t cert_len)
+ssl_ctx_create(const char *pkiname, char *cert, off_t cert_len)
 {
        SSL_CTX *ctx;
+       size_t   pkinamelen = 0;
 
        ctx = SSL_CTX_new(SSLv23_method());
        if (ctx == NULL) {
@@ -269,11 +270,13 @@ ssl_ctx_create(void *pkiname, char *cert, off_t cert_len)
        }
 
        if (cert != NULL) {
+               if (pkiname != NULL)
+                       pkinamelen = strlen(pkiname) + 1;       
                if (!ssl_ctx_use_certificate_chain(ctx, cert, cert_len)) {
                        ssl_error("ssl_ctx_create");
                        fatal("ssl_ctx_create: invalid certificate chain");
                } else if (!ssl_ctx_fake_private_key(ctx,
-                   pkiname, cert, cert_len)) {
+                   pkiname, pkinamelen, cert, cert_len)) {
                        ssl_error("ssl_ctx_create");
                        fatal("ssl_ctx_create: could not fake private key");
                } else if (!SSL_CTX_check_private_key(ctx)) {
@@ -459,14 +462,12 @@ ssl_set_ecdh_curve(SSL_CTX *ctx, const char *curve)
 }
 
 int
-ssl_ctx_load_pkey(SSL_CTX *ctx, void *data, char *buf, off_t len,
+ssl_ctx_load_pkey(SSL_CTX *ctx, char *buf, off_t len,
     X509 **x509ptr, EVP_PKEY **pkeyptr)
 {
-       int              ret = 1;
        BIO             *in;
        X509            *x509 = NULL;
        EVP_PKEY        *pkey = NULL;
-       RSA             *rsa = NULL;
 
        if ((in = BIO_new_mem_buf(buf, len)) == NULL) {
                SSLerr(SSL_F_SSL_CTX_USE_PRIVATEKEY, ERR_R_BUF_LIB);
@@ -484,47 +485,50 @@ ssl_ctx_load_pkey(SSL_CTX *ctx, void *data, char *buf, off_t len,
                goto fail;
        }
 
+       BIO_free(in);
+
        *x509ptr = x509;
        *pkeyptr = pkey;
 
-       if (data == NULL)
-               goto done;
-
-       if ((rsa = EVP_PKEY_get1_RSA(pkey)) == NULL) {
-               SSLerr(SSL_F_SSL_CTX_USE_PRIVATEKEY, ERR_R_EVP_LIB);
-               goto fail;
-       }
-
-       RSA_set_ex_data(rsa, 0, data);
-       RSA_free(rsa); /* dereference, will be cleaned up with pkey */
-       goto done;
+       return (1);
 
  fail:
        ssl_error("ssl_ctx_load_pkey");
 
+       if (in != NULL)
+               BIO_free(in);
        if (pkey != NULL)
                EVP_PKEY_free(pkey);
        if (x509 != NULL)
                X509_free(x509);
-       ret = 0;
 
- done:
-       if (in != NULL)
-               BIO_free(in);
-
-       return ret;
+       return (0);
 }
 
 int
-ssl_ctx_fake_private_key(SSL_CTX *ctx, void *data, char *buf, off_t len)
+ssl_ctx_fake_private_key(SSL_CTX *ctx, const void *data, size_t datalen,
+    char *buf, off_t len)
 {
        int              ret = 0;
        EVP_PKEY        *pkey = NULL;
        X509            *x509 = NULL;
+       RSA             *rsa = NULL;
+       void            *exdata = NULL;
 
-       if (!ssl_ctx_load_pkey(ctx, data, buf, len, &x509, &pkey))
+       if (!ssl_ctx_load_pkey(ctx, buf, len, &x509, &pkey))
                return (0);
 
+       if (data != NULL && datalen) {
+               if ((rsa = EVP_PKEY_get1_RSA(pkey)) == NULL ||
+                   (exdata = malloc(datalen)) == NULL) {
+                       SSLerr(SSL_F_SSL_CTX_USE_PRIVATEKEY, ERR_R_EVP_LIB);
+                       goto done;
+               }
+
+               memcpy(exdata, data, datalen);
+               RSA_set_ex_data(rsa, 0, exdata);
+       }
+
        /*
         * Use the public key as the "private" key - the secret key
         * parameters are hidden in an extra process that will be
@@ -537,6 +541,9 @@ ssl_ctx_fake_private_key(SSL_CTX *ctx, void *data, char *buf, off_t len)
                ssl_error("ssl_ctx_fake_private_key");
        }
 
+ done:
+       if (rsa != NULL)
+               RSA_free(rsa);
        if (pkey != NULL)
                EVP_PKEY_free(pkey);
        if (x509 != NULL)

diff --git a/usr.sbin/smtpd/ssl.h b/usr.sbin/smtpd/ssl.h
index c2df38a..923746e 100644 (file)
--- a/usr.sbin/smtpd/ssl.h
+++ b/usr.sbin/smtpd/ssl.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ssl.h,v 1.7 2014/04/29 19:13:14 reyk Exp $    */
+/*     $OpenBSD: ssl.h,v 1.8 2014/05/20 14:21:46 reyk Exp $    */
 /*
  * Copyright (c) 2013 Gilles Chehade <gilles@poolp.org>
  *
@@ -44,7 +44,7 @@ struct pki {
 /* ssl.c */
 void           ssl_init(void);
 int            ssl_setup(SSL_CTX **, struct pki *);
-SSL_CTX               *ssl_ctx_create(void *, char *, off_t);
+SSL_CTX               *ssl_ctx_create(const char *, char *, off_t);
 int            ssl_cmp(struct pki *, struct pki *);
 DH            *get_dh1024(void);
 DH            *get_dh_from_memory(char *, size_t);
@@ -62,9 +62,10 @@ int          ssl_load_keyfile(struct pki *, const char *, const char *);
 int            ssl_load_cafile(struct pki *, const char *);
 int            ssl_load_dhparams(struct pki *, const char *);
 
-int            ssl_ctx_load_pkey(SSL_CTX *, void *, char *, off_t,
+int            ssl_ctx_load_pkey(SSL_CTX *, char *, off_t,
                    X509 **, EVP_PKEY **);
-int            ssl_ctx_fake_private_key(SSL_CTX *, void *, char *, off_t);
+int            ssl_ctx_fake_private_key(SSL_CTX *, const void *, size_t,
+                   char *, off_t);
 
 /* ssl_privsep.c */
 int            ssl_ctx_use_certificate_chain(SSL_CTX *, char *, off_t);