From patchwork Thu Sep 13 23:14:17 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steffan Karger X-Patchwork-Id: 462 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director11.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net (Dovecot) with LMTP id q7YKN1V8m1s7eAAAIUCqbw for ; Fri, 14 Sep 2018 05:16:05 -0400 Received: from proxy19.mail.ord1d.rsapps.net ([172.30.191.6]) by director11.mail.ord1d.rsapps.net with LMTP id gKUHN1V8m1v4HQAAvGGmqA ; Fri, 14 Sep 2018 05:16:05 -0400 Received: from smtp2.gate.ord1d ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy19.mail.ord1d.rsapps.net with LMTP id wOAyNlV8m1sdCAAAyH2SIw ; Fri, 14 Sep 2018 05:16:05 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp2.gate.ord1d.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=fox-it.com X-Suspicious-Flag: YES X-Classification-ID: ce9d7a1e-b7fe-11e8-94e1-5254004a0287-1-1 Received: from [216.105.38.7] ([216.105.38.7:7010] helo=lists.sourceforge.net) by smtp2.gate.ord1d.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id D9/46-14295-55C7B9B5; Fri, 14 Sep 2018 05:16:05 -0400 Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1g0kBh-0001Od-A6; Fri, 14 Sep 2018 09:14:53 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1g0kBg-0001OR-0l for openvpn-devel@lists.sourceforge.net; Fri, 14 Sep 2018 09:14:52 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Type:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:CC:To:From:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=zA4XPpG+wZmvyr4s7AphEvgkndgE1R5gq1NQoDNzjMI=; b=T/wWGLa3+4ArcQghIjnCf5X3tO 7fpv6QhhE2IKh5wppHf1jdZwM8/XBUTukgNZ8P9oeW5DB4w0zjKzk0NqnhxjjZrtIUo46PVF+QxNj 2Z8AlOuacxzYzWJlNtNPVGYmMyDtC9mJejujgapVlpzh9JBhJAAteVF8W7nqX4iyd7/I=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Type:MIME-Version:References:In-Reply-To:Message-ID:Date:Subject: CC:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=zA4XPpG+wZmvyr4s7AphEvgkndgE1R5gq1NQoDNzjMI=; b=hnsloNBbie67EOHzv+ZKW7ur1Q paxBexX9qZnBBisgdDS9oeHMF8S1E5SpHe2VBRprpyBE+Uezpp6HZcgzWo4ShiRYVDF5GTGUWldi+ 42xjscxjSmmvfYWNNWOS43ybkAY7ng6WJ8mfEVAbtQ7m5eW12JljscTEog61m/wc+gds=; Received: from ns2.fox-it.com ([178.250.144.131]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLSv1:ECDHE-RSA-AES256-SHA:256) (Exim 4.90_1) id 1g0kBd-00BBl4-RX for openvpn-devel@lists.sourceforge.net; Fri, 14 Sep 2018 09:14:51 +0000 Received: from FOXDFT52.FOX.local (unknown [10.0.0.129]) by ns2.fox-it.com (Postfix) with ESMTPS id 1D4911AF871 for ; Fri, 14 Sep 2018 11:14:42 +0200 (CEST) Received: from steffan-fox.fox.local (172.16.5.172) by FOXDFT52.FOX.local (10.0.0.129) with Microsoft SMTP Server (TLS) id 15.0.1293.2; Fri, 14 Sep 2018 11:14:41 +0200 From: Steffan Karger To: Date: Fri, 14 Sep 2018 11:14:17 +0200 Message-ID: <1536916459-25900-1-git-send-email-steffan.karger@fox-it.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1505166722-5409-1-git-send-email-steffan.karger@fox-it.com> References: <1505166722-5409-1-git-send-email-steffan.karger@fox-it.com> MIME-Version: 1.0 X-ClientProxiedBy: FOXDFT52.FOX.local (10.0.0.129) To FOXDFT52.FOX.local (10.0.0.129) X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -0.0 SPF_PASS SPF: sender matches SPF record X-Headers-End: 1g0kBd-00BBl4-RX Subject: [Openvpn-devel] [PATCH v2 1/3] Do not load certificate from tls_ctx_use_external_private_key() X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox The cert and key loading logic surrounding management-external-key and management-external cert was somewhat intertwined. Untangle these to prepare for making the external key code more reusable. The best part is that this even reduces the number of lines of code. Signed-off-by: Steffan Karger Acked-By: Arne Schwabe --- v2: rebase onto current master src/openvpn/ssl.c | 48 ++++++++++++++++------------------- src/openvpn/ssl_backend.h | 15 +++-------- src/openvpn/ssl_mbedtls.c | 6 ++--- src/openvpn/ssl_openssl.c | 64 +++++++++++++++++++++++------------------------ 4 files changed, 59 insertions(+), 74 deletions(-) diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index dcb5445..4257c33 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -657,41 +657,37 @@ init_ssl(const struct options *options, struct tls_root_ctx *new_ctx) } #endif #ifdef MANAGMENT_EXTERNAL_KEY - else if ((options->management_flags & MF_EXTERNAL_KEY) - && (options->cert_file || options->management_flags & MF_EXTERNAL_CERT)) + else if (options->management_flags & MF_EXTERNAL_CERT) { - if (options->cert_file) - { - tls_ctx_use_external_private_key(new_ctx, options->cert_file, - options->cert_file_inline); - } - else - { - char *external_certificate = management_query_cert(management, - options->management_certificate); - tls_ctx_use_external_private_key(new_ctx, INLINE_FILE_TAG, - external_certificate); - free(external_certificate); - } + char *cert = management_query_cert(management, + options->management_certificate); + tls_ctx_load_cert_file(new_ctx, INLINE_FILE_TAG, cert); + free(cert); } #endif - else + else if (options->cert_file) + { + tls_ctx_load_cert_file(new_ctx, options->cert_file, options->cert_file_inline); + } + + if (options->priv_key_file) { - /* Load Certificate */ - if (options->cert_file) + if (0 != tls_ctx_load_priv_file(new_ctx, options->priv_key_file, + options->priv_key_file_inline)) { - tls_ctx_load_cert_file(new_ctx, options->cert_file, options->cert_file_inline); + goto err; } - - /* Load Private Key */ - if (options->priv_key_file) + } +#ifdef MANAGMENT_EXTERNAL_KEY + else if (options->management_flags & MF_EXTERNAL_KEY) + { + if (tls_ctx_use_management_external_key(new_ctx)) { - if (0 != tls_ctx_load_priv_file(new_ctx, options->priv_key_file, options->priv_key_file_inline)) - { - goto err; - } + msg (M_WARN, "Cannot initialize mamagement-external-key"); + goto err; } } +#endif if (options->ca_file || options->ca_path) { diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h index 202fe3f..5023c02 100644 --- a/src/openvpn/ssl_backend.h +++ b/src/openvpn/ssl_backend.h @@ -270,8 +270,7 @@ void tls_ctx_load_cert_file(struct tls_root_ctx *ctx, const char *cert_file, * successful. */ int tls_ctx_load_priv_file(struct tls_root_ctx *ctx, const char *priv_key_file, - const char *priv_key_file_inline - ); + const char *priv_key_file_inline); #ifdef MANAGMENT_EXTERNAL_KEY @@ -280,18 +279,12 @@ int tls_ctx_load_priv_file(struct tls_root_ctx *ctx, const char *priv_key_file, * private key matching the given certificate. * * @param ctx TLS context to use - * @param cert_file The file name to load the certificate from, or - * "[[INLINE]]" in the case of inline files. - * @param cert_file_inline A string containing the certificate * - * @return 1 if an error occurred, 0 if parsing was - * successful. + * @return 1 if an error occurred, 0 if successful. */ -int tls_ctx_use_external_private_key(struct tls_root_ctx *ctx, - const char *cert_file, const char *cert_file_inline); - -#endif +int tls_ctx_use_management_external_key(struct tls_root_ctx *ctx); +#endif /* MANAGMENT_EXTERNAL_KEY */ /** * Load certificate authority certificates from the given file or path. diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index ef83e65..3c6d872 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -621,15 +621,13 @@ external_key_len(void *vctx) } int -tls_ctx_use_external_private_key(struct tls_root_ctx *ctx, - const char *cert_file, const char *cert_file_inline) +tls_ctx_use_management_external_key(struct tls_root_ctx *ctx) { ASSERT(NULL != ctx); - tls_ctx_load_cert_file(ctx, cert_file, cert_file_inline); - if (ctx->crt_chain == NULL) { + msg (M_WARN, "ERROR: external key requires a certificate."); return 1; } diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 012668a..bf00a38 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -795,11 +795,9 @@ tls_ctx_add_extra_certs(struct tls_root_ctx *ctx, BIO *bio) } } -/* Like tls_ctx_load_cert, but returns a copy of the certificate in **X509 */ -static void -tls_ctx_load_cert_file_and_copy(struct tls_root_ctx *ctx, - const char *cert_file, const char *cert_file_inline, X509 **x509 - ) +void +tls_ctx_load_cert_file(struct tls_root_ctx *ctx, const char *cert_file, + const char *cert_file_inline) { BIO *in = NULL; X509 *x = NULL; @@ -807,10 +805,6 @@ tls_ctx_load_cert_file_and_copy(struct tls_root_ctx *ctx, bool inline_file = false; ASSERT(NULL != ctx); - if (NULL != x509) - { - ASSERT(NULL == *x509); - } inline_file = (strcmp(cert_file, INLINE_FILE_TAG) == 0); @@ -861,23 +855,12 @@ end: { BIO_free(in); } - if (x509) - { - *x509 = x; - } else if (x) { X509_free(x); } } -void -tls_ctx_load_cert_file(struct tls_root_ctx *ctx, const char *cert_file, - const char *cert_file_inline) -{ - tls_ctx_load_cert_file_and_copy(ctx, cert_file, cert_file_inline, NULL); -} - int tls_ctx_load_priv_file(struct tls_root_ctx *ctx, const char *priv_key_file, const char *priv_key_file_inline @@ -1039,7 +1022,7 @@ rsa_priv_dec(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, i static int openvpn_extkey_rsa_finish(RSA *rsa) { - /* meth was allocated in tls_ctx_use_external_private_key() ; since + /* meth was allocated in tls_ctx_use_management_external_key() ; since * this function is called when the parent RSA object is destroyed, * it is no longer used after this point so kill it. */ const RSA_METHOD *meth = RSA_get_method(rsa); @@ -1288,14 +1271,20 @@ err: #endif /* OPENSSL_VERSION_NUMBER > 1.1.0 dev */ int -tls_ctx_use_external_private_key(struct tls_root_ctx *ctx, - const char *cert_file, const char *cert_file_inline) +tls_ctx_use_management_external_key(struct tls_root_ctx *ctx) { - X509 *cert = NULL; + int ret = 1; ASSERT(NULL != ctx); - tls_ctx_load_cert_file_and_copy(ctx, cert_file, cert_file_inline, &cert); +#if OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(LIBRESSL_VERSION_NUMBER) + /* OpenSSL 1.0.2 and up */ + X509 *cert = SSL_CTX_get0_certificate(ctx->ctx); +#else + /* OpenSSL 1.0.1 and earlier need an SSL object to get at the certificate */ + SSL *ssl = SSL_new(ctx->ctx); + X509 *cert = SSL_get_certificate(ssl); +#endif ASSERT(NULL != cert); @@ -1308,7 +1297,7 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx, { if (!tls_ctx_use_external_rsa_key(ctx, pkey)) { - goto err; + goto cleanup; } } #if OPENSSL_VERSION_NUMBER > 0x10100000L && !defined(OPENSSL_NO_EC) && !defined(LIBRESSL_VERSION_NUMBER) @@ -1316,26 +1305,35 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx, { if (!tls_ctx_use_external_ec_key(ctx, pkey)) { - goto err; + goto cleanup; } } else { crypto_msg(M_WARN, "management-external-key requires an RSA or EC certificate"); - goto err; + goto cleanup; } #else else { crypto_msg(M_WARN, "management-external-key requires an RSA certificate"); - goto err; + goto cleanup; } #endif /* OPENSSL_VERSION_NUMBER > 1.1.0 dev */ - return 0; -err: - crypto_msg(M_FATAL, "Cannot enable SSL external private key capability"); - return 1; + ret = 0; +cleanup: +#if OPENSSL_VERSION_NUMBER < 0x10002000L || defined(LIBRESSL_VERSION_NUMBER) + if (ssl) + { + SSL_free(ssl); + } +#endif + if (ret) + { + crypto_msg(M_FATAL, "Cannot enable SSL external private key capability"); + } + return ret; } #endif /* ifdef MANAGMENT_EXTERNAL_KEY */