From patchwork Thu Sep 13 23:14:19 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steffan Karger X-Patchwork-Id: 463 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director8.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net (Dovecot) with LMTP id rNtaFVd8m1tBeAAAIUCqbw for ; Fri, 14 Sep 2018 05:16:07 -0400 Received: from proxy12.mail.ord1d.rsapps.net ([172.30.191.6]) by director8.mail.ord1d.rsapps.net with LMTP id cCz9M1d8m1vfEgAAfY0hYg ; Fri, 14 Sep 2018 05:16:07 -0400 Received: from smtp6.gate.ord1d ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy12.mail.ord1d.rsapps.net with LMTP id sD//M1d8m1tWFAAA7PHxkg ; Fri, 14 Sep 2018 05:16:07 -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: smtp6.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: cf9ed048-b7fe-11e8-a772-52540050e3e0-1-1 Received: from [216.105.38.7] ([216.105.38.7:2398] helo=lists.sourceforge.net) by smtp6.gate.ord1d.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 1B/07-08843-75C7B9B5; Fri, 14 Sep 2018 05:16:07 -0400 Received: from [127.0.0.1] (helo=sfs-ml-1.v29.lw.sourceforge.com) by sfs-ml-1.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1g0kBi-0001UJ-Nn; Fri, 14 Sep 2018 09:14:54 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1g0kBh-0001UC-9U for openvpn-devel@lists.sourceforge.net; Fri, 14 Sep 2018 09:14:53 +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=Ow88uVTB0Z3bF+xUs9nj3NV3cNrQRbSzdYqYTO6DC18=; b=Gr46gtta2XyxtuPZM94+Fcj7o2 gXxnH4AiAyUe0aa7ToxWrJkVBHTiViiXK8ovomUSp9lLtehszbXvvCVtCePkvjfbiknyGkegvoEy3 A8VIDNyOtOgvVUq2Q/LwD8XK+3Ku7pyhvlIGIitCAwV7CGsFOSVsk3TSLJuCKf2DZdnM=; 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=Ow88uVTB0Z3bF+xUs9nj3NV3cNrQRbSzdYqYTO6DC18=; b=CRvGF4hzArVVT/1ytvh+ygILxU e5pf9936H7Y93tnmVSVjMWYqbVBXCjr7JJn2Wz1X8AP4sSkj3o1FrYPvGw0l8unwvnFSTkx5h2Vmf Aienh0MecLYmmtEEpENKqUeDIdQhzfIrSEYhXq8bI2gC6zaP/nxFyEGcgOWFwjnx7nqs=; Received: from ns2.fox-it.com ([178.250.144.131]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLSv1:ECDHE-RSA-AES256-SHA:256) (Exim 4.90_1) id 1g0kBe-001Zu2-HU for openvpn-devel@lists.sourceforge.net; Fri, 14 Sep 2018 09:14:52 +0000 Received: from FOXDFT52.FOX.local (unknown [10.0.0.129]) by ns2.fox-it.com (Postfix) with ESMTPS id 7ABCB1AF87C 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:42 +0200 From: Steffan Karger To: Date: Fri, 14 Sep 2018 11:14:19 +0200 Message-ID: <1536916459-25900-3-git-send-email-steffan.karger@fox-it.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1536916459-25900-1-git-send-email-steffan.karger@fox-it.com> References: <1505166722-5409-1-git-send-email-steffan.karger@fox-it.com> <1536916459-25900-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: 1g0kBe-001Zu2-HU Subject: [Openvpn-devel] [PATCH v2 3/3] mbedtls: remove dependency on mbedtls pkcs11 module 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 Instead of using mbedtls's pkcs11 module, reuse the code we already have for management-external-key to also do pkcs11 signatures. As far as mbed is concerned, we simply provide an external signature. This has the following advantages: * We no longer need mbed TLS to be compiled with the pkcs11 modules enabled (which is not enabled by default). This makes it easier to use a system/distribution-provided mbed shared library. * We no longer have a dependency on pkcs11-helper through mbed TLS. So if we want to migrate to some other pkcs11 lib (see e.g. trac #491, #538 and #549 for reason why), this will be easier. While touching this code, switch from M_FATAL to M_WARN and proper error handling. This improves the error reporting, and helps prevent potential future DoS attacks if someone starts using these functions on peer input. Signed-off-by: Steffan Karger Acked-By: Arne Schwabe --- v2: rebase onto current master configure.ac | 29 --------------- src/openvpn/pkcs11_mbedtls.c | 87 +++++++++++++++++++++++++++++--------------- src/openvpn/ssl_mbedtls.c | 7 +--- src/openvpn/ssl_mbedtls.h | 6 +-- 4 files changed, 62 insertions(+), 67 deletions(-) diff --git a/configure.ac b/configure.ac index 9c31435..3d8e15b 100644 --- a/configure.ac +++ b/configure.ac @@ -994,35 +994,6 @@ elif test "${with_crypto_library}" = "mbedtls"; then [AC_MSG_ERROR([mbed TLS 2.y.z required])] ) - mbedtls_with_pkcs11="no" - AC_COMPILE_IFELSE( - [AC_LANG_PROGRAM( - [[ -#include - ]], - [[ -#ifndef MBEDTLS_PKCS11_C -#error pkcs11 wrapper missing -#endif - ]] - )], - mbedtls_with_pkcs11="yes") - - AC_MSG_CHECKING([mbedtls pkcs11 support]) - if test "${enable_pkcs11}" = "yes"; then - if test "${mbedtls_with_pkcs11}" = "yes"; then - AC_MSG_RESULT([ok]) - else - AC_MSG_ERROR([mbedtls has no pkcs11 wrapper compiled in]) - fi - else - if test "${mbedtls_with_pkcs11}" != "yes"; then - AC_MSG_RESULT([ok]) - else - AC_MSG_ERROR([mbed TLS compiled with PKCS11, while OpenVPN is not]) - fi - fi - have_crypto_aead_modes="yes" AC_CHECK_FUNCS( [ \ diff --git a/src/openvpn/pkcs11_mbedtls.c b/src/openvpn/pkcs11_mbedtls.c index 7620624..bd704e0 100644 --- a/src/openvpn/pkcs11_mbedtls.c +++ b/src/openvpn/pkcs11_mbedtls.c @@ -39,60 +39,89 @@ #include "errlevel.h" #include "pkcs11_backend.h" #include "ssl_verify_backend.h" -#include #include -int -pkcs11_init_tls_session(pkcs11h_certificate_t certificate, - struct tls_root_ctx *const ssl_ctx) +static bool +pkcs11_get_x509_cert(pkcs11h_certificate_t pkcs11_cert, mbedtls_x509_crt *cert) { - int ret = 1; + unsigned char *cert_blob = NULL; + size_t cert_blob_size = 0; + bool ret = false; - ASSERT(NULL != ssl_ctx); - - ALLOC_OBJ_CLEAR(ssl_ctx->crt_chain, mbedtls_x509_crt); - if (mbedtls_pkcs11_x509_cert_bind(ssl_ctx->crt_chain, certificate)) + if (pkcs11h_certificate_getCertificateBlob(pkcs11_cert, NULL, + &cert_blob_size) != CKR_OK) { - msg(M_FATAL, "PKCS#11: Cannot retrieve mbed TLS certificate object"); + msg(M_WARN, "PKCS#11: Cannot retrieve certificate object size"); goto cleanup; } - ALLOC_OBJ_CLEAR(ssl_ctx->priv_key_pkcs11, mbedtls_pkcs11_context); - if (mbedtls_pkcs11_priv_key_bind(ssl_ctx->priv_key_pkcs11, certificate)) + check_malloc_return((cert_blob = calloc(1, cert_blob_size))); + if (pkcs11h_certificate_getCertificateBlob(pkcs11_cert, cert_blob, + &cert_blob_size) != CKR_OK) { - msg(M_FATAL, "PKCS#11: Cannot initialize mbed TLS private key object"); + msg(M_WARN, "PKCS#11: Cannot retrieve certificate object"); goto cleanup; } - ALLOC_OBJ_CLEAR(ssl_ctx->priv_key, mbedtls_pk_context); - if (!mbed_ok(mbedtls_pk_setup_rsa_alt(ssl_ctx->priv_key, - ssl_ctx->priv_key_pkcs11, mbedtls_ssl_pkcs11_decrypt, - mbedtls_ssl_pkcs11_sign, mbedtls_ssl_pkcs11_key_len))) + if (!mbed_ok(mbedtls_x509_crt_parse(cert, cert_blob, cert_blob_size))) { + msg(M_WARN, "PKCS#11: Could not parse certificate"); goto cleanup; } - ret = 0; - + ret = true; cleanup: + free(cert_blob); return ret; } +static bool +pkcs11_sign(void *pkcs11_cert, const void *src, size_t src_len, + void *dst, size_t dst_len) +{ + return CKR_OK == pkcs11h_certificate_signAny(pkcs11_cert, CKM_RSA_PKCS, + src, src_len, dst, &dst_len); +} + +int +pkcs11_init_tls_session(pkcs11h_certificate_t certificate, + struct tls_root_ctx *const ssl_ctx) +{ + ASSERT(NULL != ssl_ctx); + + ssl_ctx->pkcs11_cert = certificate; + + ALLOC_OBJ_CLEAR(ssl_ctx->crt_chain, mbedtls_x509_crt); + if (!pkcs11_get_x509_cert(certificate, ssl_ctx->crt_chain)) + { + msg(M_WARN, "PKCS#11: Cannot initialize certificate"); + return 1; + } + + if (tls_ctx_use_external_signing_func(ssl_ctx, pkcs11_sign, certificate)) + { + msg(M_WARN, "PKCS#11: Cannot register signing function"); + return 1; + } + + return 0; +} + char * pkcs11_certificate_dn(pkcs11h_certificate_t cert, struct gc_arena *gc) { char *ret = NULL; - mbedtls_x509_crt mbed_crt = {0}; + mbedtls_x509_crt mbed_crt = { 0 }; - if (mbedtls_pkcs11_x509_cert_bind(&mbed_crt, cert)) + if (!pkcs11_get_x509_cert(cert, &mbed_crt)) { - msg(M_FATAL, "PKCS#11: Cannot retrieve mbed TLS certificate object"); + msg(M_WARN, "PKCS#11: Cannot retrieve mbed TLS certificate object"); goto cleanup; } if (!(ret = x509_get_subject(&mbed_crt, gc))) { - msg(M_FATAL, "PKCS#11: mbed TLS cannot parse subject"); + msg(M_WARN, "PKCS#11: mbed TLS cannot parse subject"); goto cleanup; } @@ -107,23 +136,21 @@ pkcs11_certificate_serial(pkcs11h_certificate_t cert, char *serial, size_t serial_len) { int ret = 1; + mbedtls_x509_crt mbed_crt = { 0 }; - mbedtls_x509_crt mbed_crt = {0}; - - if (mbedtls_pkcs11_x509_cert_bind(&mbed_crt, cert)) + if (!pkcs11_get_x509_cert(cert, &mbed_crt)) { - msg(M_FATAL, "PKCS#11: Cannot retrieve mbed TLS certificate object"); + msg(M_WARN, "PKCS#11: Cannot retrieve mbed TLS certificate object"); goto cleanup; } - if (-1 == mbedtls_x509_serial_gets(serial, serial_len, &mbed_crt.serial)) + if (mbedtls_x509_serial_gets(serial, serial_len, &mbed_crt.serial) < 0) { - msg(M_FATAL, "PKCS#11: mbed TLS cannot parse serial"); + msg(M_WARN, "PKCS#11: mbed TLS cannot parse serial"); goto cleanup; } ret = 0; - cleanup: mbedtls_x509_crt_free(&mbed_crt); diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 4c39cf3..e4850cb 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -43,6 +43,7 @@ #include "buffer.h" #include "misc.h" #include "manage.h" +#include "pkcs11_backend.h" #include "ssl_common.h" #include @@ -167,11 +168,7 @@ tls_ctx_free(struct tls_root_ctx *ctx) } #if defined(ENABLE_PKCS11) - if (ctx->priv_key_pkcs11 != NULL) - { - mbedtls_pkcs11_priv_key_free(ctx->priv_key_pkcs11); - free(ctx->priv_key_pkcs11); - } + pkcs11h_certificate_freeCertificate(ctx->pkcs11_cert); #endif if (ctx->allowed_ciphers) diff --git a/src/openvpn/ssl_mbedtls.h b/src/openvpn/ssl_mbedtls.h index 73b4c1a..998d6f2 100644 --- a/src/openvpn/ssl_mbedtls.h +++ b/src/openvpn/ssl_mbedtls.h @@ -35,7 +35,7 @@ #include #if defined(ENABLE_PKCS11) -#include +#include #endif typedef struct _buffer_entry buffer_entry; @@ -99,8 +99,8 @@ struct tls_root_ctx { mbedtls_x509_crl *crl; /**< Certificate Revocation List */ time_t crl_last_mtime; /**< CRL last modification time */ off_t crl_last_size; /**< size of last loaded CRL */ -#if defined(ENABLE_PKCS11) - mbedtls_pkcs11_context *priv_key_pkcs11; /**< PKCS11 private key */ +#ifdef ENABLE_PKCS11 + pkcs11h_certificate_t pkcs11_cert; /**< PKCS11 certificate */ #endif struct external_context external_key; /**< External key context */ int *allowed_ciphers; /**< List of allowed ciphers for this connection */