From: reyk <reyk@openbsd.org>
Date: Tue, 20 May 2014 14:21:45 +0000 (+0000)
Subject: Deep down inside OpenSSL, err... LibreSSL, RSA_set_ex_data attempts to
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=aded3101de1fa7dd31f5c7e11c67f6f85a4f6211;p=openbsd

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 99d2b633ba8..6ea7c6b7df0 100644
--- 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 c2df38a66ab..923746eefd6 100644
--- 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);