From patchwork Mon Feb 1 06:43:10 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1590 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director15.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net with LMTP id 2OtAFPE9GGBQXAAAIUCqbw (envelope-from ) for ; Mon, 01 Feb 2021 12:44:17 -0500 Received: from proxy16.mail.ord1d.rsapps.net ([172.30.191.6]) by director15.mail.ord1d.rsapps.net with LMTP id AOnzE/E9GGDzJgAAIcMcQg (envelope-from ) for ; Mon, 01 Feb 2021 12:44:17 -0500 Received: from smtp14.gate.ord1d ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy16.mail.ord1d.rsapps.net with LMTPS id qJCVE/E9GGA1AgAAetu3IA (envelope-from ) for ; Mon, 01 Feb 2021 12:44:17 -0500 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: smtp14.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=rfc2549.org X-Suspicious-Flag: YES X-Classification-ID: 1a59c704-64b5-11eb-aa0b-525400504bae-1-1 Received: from [216.105.38.7] ([216.105.38.7:52838] helo=lists.sourceforge.net) by smtp14.gate.ord1d.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id C1/9E-04334-0FD38106; Mon, 01 Feb 2021 12:44:16 -0500 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 1l6dEX-0001i3-S5; Mon, 01 Feb 2021 17:43:29 +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 1l6dEW-0001hq-OR for openvpn-devel@lists.sourceforge.net; Mon, 01 Feb 2021 17:43:28 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=References:In-Reply-To:Message-Id:Date:Subject:To: From:Sender:Reply-To:Cc:MIME-Version:Content-Type: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=o2jP4g+PhP6V0KY1j0RuvNeCiuoemaGSYRCqYmP4K8A=; b=DX7KYrsSPJKskKl1WesWn9kSiT MgYnhFZ0tWLpNBPqQ6mPRT4FVdaJbKlpGeFtxVGuQFeSBMTdyODtvQqcMN0azTymwdJMbqQ5LG0GO 3Xagg0mbI1/hynhvfWBAIE2CASxUkQ0Ls4aQreIZqP0IVLSbn+cChqJ7FeGteVC8eplk=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=References:In-Reply-To:Message-Id:Date:Subject:To:From:Sender:Reply-To:Cc :MIME-Version:Content-Type: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=o2jP4g+PhP6V0KY1j0RuvNeCiuoemaGSYRCqYmP4K8A=; b=dUseuv24HubQ3RwMyutzbuiAbJ t308EAeon/mYocJgFa7fQ6vfS5VNAU3950cvUa8Zj59fjHRPIaIiG+YHmWZuWA5vIXx731YtOZIqf D+Kn5eJRH1lJG2IlaG7vGJM/AvUdmYFIIaMmCTmQPdTqZWON1KVyHSzU7iARuAjJ1l18=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.2) id 1l6dEO-00BGHy-HJ for openvpn-devel@lists.sourceforge.net; Mon, 01 Feb 2021 17:43:28 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.94 (FreeBSD)) (envelope-from ) id 1l6dEE-000NVd-K9 for openvpn-devel@lists.sourceforge.net; Mon, 01 Feb 2021 18:43:10 +0100 Received: (nullmailer pid 22206 invoked by uid 10006); Mon, 01 Feb 2021 17:43:10 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Mon, 1 Feb 2021 18:43:10 +0100 Message-Id: <20210201174310.22153-4-arne@rfc2549.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20210201174310.22153-1-arne@rfc2549.org> References: <20210201174310.22153-1-arne@rfc2549.org> X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record X-Headers-End: 1l6dEO-00BGHy-HJ Subject: [Openvpn-devel] [PATCH v3 3/3] Handle the unlikely case that PRF generation fails 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: , MIME-Version: 1.0 Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox We never had handling of this failure condition. But should it happen we can now handle it. Signed-off-by: Arne Schwabe --- src/openvpn/crypto_backend.h | 4 +- src/openvpn/crypto_mbedtls.c | 17 ++++---- src/openvpn/crypto_openssl.c | 50 ++++++++++++++++++----- src/openvpn/ssl.c | 78 ++++++++++++++++++++++-------------- 4 files changed, 99 insertions(+), 50 deletions(-) diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h index d86d9a34..384ffc80 100644 --- a/src/openvpn/crypto_backend.h +++ b/src/openvpn/crypto_backend.h @@ -710,7 +710,9 @@ const char *translate_cipher_name_to_openvpn(const char *cipher_name); * @param secret_len length of the secret * @param output output destination * @param output_len length of output/number of bytes to generate + * + * @return true if successful, false on any error */ -void ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret, +bool ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret, int secret_len, uint8_t *output, int output_len); #endif /* CRYPTO_BACKEND_H_ */ diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c index 5ed88d6d..3b28ba26 100644 --- a/src/openvpn/crypto_mbedtls.c +++ b/src/openvpn/crypto_mbedtls.c @@ -987,11 +987,12 @@ memcmp_constant_time(const void *a, const void *b, size_t size) } /* mbedtls-2.18.0 or newer */ #ifdef HAVE_MBEDTLS_SSL_TLS_PRF -void ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret, +bool ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret, int secret_len, uint8_t *output, int output_len) { - mbedtls_ssl_tls_prf(MBEDTLS_SSL_TLS_PRF_TLS1, secret, secret_len, "", seed, - seed_len, output, output_len); + return mbed_ok(mbedtls_ssl_tls_prf(MBEDTLS_SSL_TLS_PRF_TLS1, secret, + secret_len, "", seed, seed_len, output, + output_len)); } #else /* @@ -1088,9 +1089,8 @@ tls1_P_hash(const md_kt_t *md_kt, * (1) key_block contains a full set of 4 keys. * (2) The pre-master secret is generated by the client. */ -void -ssl_tls1_PRF(const uint8_t *label, int label_len, const uint8_t *sec, int slen, - uint8_t *out1, int olen) +bool ssl_tls1_PRF(const uint8_t *label, int label_len, const uint8_t *sec, + int slen, uint8_t *out1, int olen) { struct gc_arena gc = gc_new(); const md_kt_t *md5 = md_kt_get("MD5"); @@ -1103,8 +1103,8 @@ ssl_tls1_PRF(const uint8_t *label, int label_len, const uint8_t *sec, int slen, const uint8_t *S2 = &(sec[len]); len += (slen&1); /* add for odd, make longer */ - tls1_P_hash(md5,S1,len,label,label_len,out1,olen); - tls1_P_hash(sha1,S2,len,label,label_len,out2,olen); + tls1_P_hash(md5, S1, len, label, label_len, out1, olen); + tls1_P_hash(sha1, S2, len, label, label_len, out2, olen); for (int i = 0; i= 0x10100000L -void ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret, +bool ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret, int secret_len, uint8_t *output, int output_len) { EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_TLS1_PRF, NULL); - ASSERT(EVP_PKEY_derive_init(pctx) == 1); + if(!EVP_PKEY_derive_init(pctx)) + { + return false; + } - ASSERT(EVP_PKEY_CTX_set_tls1_prf_md(pctx, EVP_md5_sha1()) == 1); - ASSERT(EVP_PKEY_CTX_set1_tls1_prf_secret(pctx, secret, secret_len) == 1); + if(!EVP_PKEY_CTX_set_tls1_prf_md(pctx, EVP_md5_sha1())) + { + return false; + } - ASSERT(EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, seed, seed_len) == 1); + if(!EVP_PKEY_CTX_set1_tls1_prf_secret(pctx, secret, secret_len)) + { + return false; + } + + if(!EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, seed, seed_len)) + { + return false; + } size_t out_len = output_len; - ASSERT (EVP_PKEY_derive(pctx, output, &out_len) == 1); - ASSERT (out_len == output_len); + if(!EVP_PKEY_derive(pctx, output, &out_len)) + { + return false; + } + if (out_len != output_len) + { + return false; + } + return true; } #else /* @@ -1258,9 +1278,10 @@ err: * (1) key_block contains a full set of 4 keys. * (2) The pre-master secret is generated by the client. */ -void ssl_tls1_PRF(const uint8_t *label, int label_len, const uint8_t *sec, +bool ssl_tls1_PRF(const uint8_t *label, int label_len, const uint8_t *sec, int slen, uint8_t *out1, int olen) { + bool ret = true; struct gc_arena gc = gc_new(); /* For some reason our md_kt_get("MD5") fails otherwise in the unit test */ const md_kt_t *md5 = EVP_md5(); @@ -1274,8 +1295,16 @@ void ssl_tls1_PRF(const uint8_t *label, int label_len, const uint8_t *sec, len += (slen&1); /* add for odd, make longer */ if(!tls1_P_hash(md5,S1,len,label,label_len,out1,olen)) + { + ret = false; + goto done; + } - ASSERT(tls1_P_hash(sha1,S2,len,label,label_len,out2,olen)); + if(!tls1_P_hash(sha1,S2,len,label,label_len,out2,olen)) + { + ret = false; + goto done; + } for (int i = 0; iserver, "Server"); } -static void +static bool openvpn_PRF(const uint8_t *secret, int secret_len, const char *label, @@ -1593,6 +1593,7 @@ openvpn_PRF(const uint8_t *secret, uint8_t *output, int output_len) { + bool ret = true; /* concatenate seed components */ struct buffer seed = alloc_buf(strlen(label) @@ -1614,12 +1615,16 @@ openvpn_PRF(const uint8_t *secret, } /* compute PRF */ - ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, output, output_len); + if (!ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, output, output_len)) + { + ret = false; + } buf_clear(&seed); free_buf(&seed); VALGRIND_MAKE_READABLE((void *)output, output_len); + return ret; } static void @@ -1654,8 +1659,8 @@ generate_key_expansion_tls_export(struct tls_session *session, struct key2 *key2 return true; } -static struct key2 -generate_key_expansion_openvpn_prf(const struct tls_session *session) +static bool +generate_key_expansion_openvpn_prf(const struct tls_session *session, struct key2 *key2) { uint8_t master[48] = { 0 }; @@ -1671,33 +1676,40 @@ generate_key_expansion_openvpn_prf(const struct tls_session *session) key_source2_print(key_src); /* compute master secret */ - openvpn_PRF(key_src->client.pre_master, - sizeof(key_src->client.pre_master), - KEY_EXPANSION_ID " master secret", - key_src->client.random1, - sizeof(key_src->client.random1), - key_src->server.random1, - sizeof(key_src->server.random1), - NULL, - NULL, - master, - sizeof(master)); + if(!openvpn_PRF(key_src->client.pre_master, + sizeof(key_src->client.pre_master), + KEY_EXPANSION_ID " master secret", + key_src->client.random1, + sizeof(key_src->client.random1), + key_src->server.random1, + sizeof(key_src->server.random1), + NULL, + NULL, + master, + sizeof(master))) + { + return false; + } - struct key2 key2; /* compute key expansion */ - openvpn_PRF(master, - sizeof(master), - KEY_EXPANSION_ID " key expansion", - key_src->client.random2, - sizeof(key_src->client.random2), - key_src->server.random2, - sizeof(key_src->server.random2), - client_sid, - server_sid, - (uint8_t *)key2.keys, - sizeof(key2.keys)); + if (!openvpn_PRF(master, + sizeof(master), + KEY_EXPANSION_ID " key expansion", + key_src->client.random2, + sizeof(key_src->client.random2), + key_src->server.random2, + sizeof(key_src->server.random2), + client_sid, + server_sid, + (uint8_t *)key2->keys, + sizeof(key2->keys))) + { + return false; + } secure_memzero(&master, sizeof(master)); + + /* * fixup_key only correctly sets DES parity bits if the cipher is a * DES variant. @@ -1712,11 +1724,11 @@ generate_key_expansion_openvpn_prf(const struct tls_session *session) */ for (int i = 0; i < 2; ++i) { - fixup_key(&key2.keys[i], &session->opt->key_type); + fixup_key(&key2->keys[i], &session->opt->key_type); } - key2.n = 2; + key2->n = 2; - return key2; + return true; } /* @@ -1748,7 +1760,11 @@ generate_key_expansion(struct key_ctx_bi *key, } else { - key2 = generate_key_expansion_openvpn_prf(session); + if (!generate_key_expansion_openvpn_prf(session, &key2)) + { + msg(D_TLS_ERRORS, "TLS Error: PRF calcuation failed"); + goto exit; + } } key2_print(&key2, &session->opt->key_type,