From aded3101de1fa7dd31f5c7e11c67f6f85a4f6211 Mon Sep 17 00:00:00 2001 From: reyk Date: Tue, 20 May 2014 14:21:45 +0000 Subject: [PATCH] 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@ --- usr.sbin/smtpd/ssl.c | 57 +++++++++++++++++++++++++------------------- usr.sbin/smtpd/ssl.h | 9 +++---- 2 files changed, 37 insertions(+), 29 deletions(-) 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 @@ -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 * @@ -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); -- 2.20.1