From patchwork Wed Dec 1 07:07:24 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 2099 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director10.mail.ord1d.rsapps.net ([172.28.255.1]) by backend30.mail.ord1d.rsapps.net with LMTP id cHV5BSa6p2EHbAAAIUCqbw (envelope-from ) for ; Wed, 01 Dec 2021 13:08:38 -0500 Received: from proxy7.mail.ord1c.rsapps.net ([172.28.255.1]) by director10.mail.ord1d.rsapps.net with LMTP id QBRMBSa6p2E0dQAApN4f7A (envelope-from ) for ; Wed, 01 Dec 2021 13:08:38 -0500 Received: from smtp33.gate.ord1c ([172.28.255.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy7.mail.ord1c.rsapps.net with LMTPS id ONGNBCa6p2EWJAAAknS3pQ (envelope-from ) for ; Wed, 01 Dec 2021 13:08:38 -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: smtp33.gate.ord1c.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: b4542376-52d1-11ec-9058-54520067fec4-1-1 Received: from [216.105.38.7] ([216.105.38.7:34104] helo=lists.sourceforge.net) by smtp33.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 3D/9B-00371-52AB7A16; Wed, 01 Dec 2021 13:08:37 -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.94.2) (envelope-from ) id 1msU1A-0005pX-Tp; Wed, 01 Dec 2021 18:07:44 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1msU18-0005p8-3d for openvpn-devel@lists.sourceforge.net; Wed, 01 Dec 2021 18:07:42 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-Id:Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: 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=vVy2k4hHxToAH3L02VVOaLQRJTAJ6kM4XMMbIRVzDhQ=; b=E/OZpHtld2pMegpMnS4Ubk6Y6l 996xFHSCdkXWXn5Wq6PPXx/tEi9Fg1e2mjlYqrSPQUVVc9KPWalP9fUO4dXdlUb7c4+Z8xCgOEqlo MyxADhpBxe7cZYKCb5IDyI7TMi6ElhGTJN4Lhm1AEAPGcDx1b91mUqVYBD6GX2fs12nc=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-Id: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: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=vVy2k4hHxToAH3L02VVOaLQRJTAJ6kM4XMMbIRVzDhQ=; b=E4QrJHh7cuqNJNbgUpExjl9/vM MjKOfem+foKRWEFpd3ZBE4S09CgREqWMCc+Ah/Y9h9DREKtW7D8D78zMAY2P3WkBkiQduwyqfX7P1 LsiXVTIBXlLMSWw/NbVDYdJM9hONkN1/wfovGukp3nDGITtvphlfVq3nB4tBvXajulzU=; 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.3) id 1msU15-000ZiM-PM for openvpn-devel@lists.sourceforge.net; Wed, 01 Dec 2021 18:07:41 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.94.2 (FreeBSD)) (envelope-from ) id 1msU0t-0000KT-Qx for openvpn-devel@lists.sourceforge.net; Wed, 01 Dec 2021 19:07:27 +0100 Received: (nullmailer pid 2496964 invoked by uid 10006); Wed, 01 Dec 2021 18:07:28 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Wed, 1 Dec 2021 19:07:24 +0100 Message-Id: <20211201180727.2496903-6-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211201180727.2496903-1-arne@rfc2549.org> References: <20211201180727.2496903-1-arne@rfc2549.org> MIME-Version: 1.0 X-Spam-Report: Spam detection software, running on the system "util-spamd-1.v13.lw.sourceforge.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: This field is only set once with md_kt_size and then only read. Remove this field and replace the read accesses with md_kt_size. Signed-off-by: Arne Schwabe --- src/openvpn/auth_token.c | 2 -- src/openvpn/crypto.c | 35 +++++++++++++++ src/openvpn/crypto.h | 1 - src/openvpn/crypto_backend.h | 4 +-- [...] Content analysis details: (0.3 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different X-Headers-End: 1msU15-000ZiM-PM Subject: [Openvpn-devel] [PATCH 6/9] Remove key_type->hmac_length 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 This field is only set once with md_kt_size and then only read. Remove this field and replace the read accesses with md_kt_size. Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/auth_token.c | 2 -- src/openvpn/crypto.c | 35 +++++++++++++++----------- src/openvpn/crypto.h | 1 - src/openvpn/crypto_backend.h | 4 +-- src/openvpn/crypto_mbedtls.c | 14 ++++++++--- src/openvpn/crypto_openssl.c | 8 +++--- src/openvpn/init.c | 2 -- src/openvpn/ntlm.c | 8 +++--- src/openvpn/openvpn.h | 2 +- src/openvpn/tls_crypt.c | 2 -- tests/unit_tests/openvpn/test_crypto.c | 2 +- 11 files changed, 42 insertions(+), 38 deletions(-) diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c index c6c37ea86..5d5cea7f6 100644 --- a/src/openvpn/auth_token.c +++ b/src/openvpn/auth_token.c @@ -44,8 +44,6 @@ auth_token_kt(void) return (struct key_type) { 0 }; } - kt.hmac_length = md_kt_size(kt.digest); - return kt; } diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index c85a75319..fd730668f 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -693,7 +693,7 @@ crypto_adjust_frame_parameters(struct frame *frame, crypto_overhead += cipher_kt_block_size(kt->cipher); } - crypto_overhead += kt->hmac_length; + crypto_overhead += md_kt_size(kt->digest); frame_add_to_extra_frame(frame, crypto_overhead); @@ -780,9 +780,9 @@ init_key_type(struct key_type *kt, const char *ciphername, if (!aead_cipher) /* Ignore auth for AEAD ciphers */ { kt->digest = md_kt_get(authname); - kt->hmac_length = md_kt_size(kt->digest); + int hmac_length = md_kt_size(kt->digest); - if (OPENVPN_MAX_HMAC_SIZE < kt->hmac_length) + if (OPENVPN_MAX_HMAC_SIZE < hmac_length) { msg(M_FATAL, "HMAC '%s' not allowed: digest size too big.", authname); } @@ -828,17 +828,17 @@ init_key_ctx(struct key_ctx *ctx, const struct key *key, cipher_kt_iv_size(kt->cipher)); warn_insecure_key_type(ciphername, kt->cipher); } - if (kt->digest && kt->hmac_length > 0) + if (kt->digest) { ctx->hmac = hmac_ctx_new(); - hmac_ctx_init(ctx->hmac, key->hmac, kt->hmac_length, kt->digest); + hmac_ctx_init(ctx->hmac, key->hmac, kt->digest); msg(D_HANDSHAKE, "%s: Using %d bit message hash '%s' for HMAC authentication", prefix, md_kt_size(kt->digest) * 8, md_kt_name(kt->digest)); dmsg(D_SHOW_KEYS, "%s: HMAC KEY: %s", prefix, - format_hex(key->hmac, kt->hmac_length, 0, &gc)); + format_hex(key->hmac, md_kt_size(kt->digest), 0, &gc)); dmsg(D_CRYPTO_DEBUG, "%s: HMAC size=%d block_size=%d", prefix, @@ -958,9 +958,11 @@ generate_key_random(struct key *key, const struct key_type *kt) { cipher_len = cipher_kt_key_size(kt->cipher); - if (kt->digest && kt->hmac_length > 0 && kt->hmac_length <= hmac_len) + int kt_hmac_length = md_kt_size(kt->digest); + + if (kt->digest && kt_hmac_length > 0 && kt_hmac_length <= hmac_len) { - hmac_len = kt->hmac_length; + hmac_len = kt_hmac_length; } } if (!rand_bytes(key->cipher, cipher_len) @@ -993,13 +995,13 @@ key2_print(const struct key2 *k, format_hex(k->keys[0].cipher, cipher_kt_key_size(kt->cipher), 0, &gc)); dmsg(D_SHOW_KEY_SOURCE, "%s (hmac): %s", prefix0, - format_hex(k->keys[0].hmac, kt->hmac_length, 0, &gc)); + format_hex(k->keys[0].hmac, md_kt_size(kt->digest), 0, &gc)); dmsg(D_SHOW_KEY_SOURCE, "%s (cipher): %s", prefix1, format_hex(k->keys[1].cipher, cipher_kt_key_size(kt->cipher), 0, &gc)); dmsg(D_SHOW_KEY_SOURCE, "%s (hmac): %s", prefix1, - format_hex(k->keys[1].hmac, kt->hmac_length, 0, &gc)); + format_hex(k->keys[1].hmac, md_kt_size(kt->digest), 0, &gc)); gc_free(&gc); } @@ -1527,14 +1529,17 @@ write_key(const struct key *key, const struct key_type *kt, struct buffer *buf) { ASSERT(cipher_kt_key_size(kt->cipher) <= MAX_CIPHER_KEY_LENGTH - && kt->hmac_length <= MAX_HMAC_KEY_LENGTH); + && md_kt_size(kt->digest) <= MAX_HMAC_KEY_LENGTH); const uint8_t cipher_length = cipher_kt_key_size(kt->cipher); if (!buf_write(buf, &cipher_length, 1)) { return false; } - if (!buf_write(buf, &kt->hmac_length, 1)) + + uint8_t hmac_length = md_kt_size(kt->digest); + + if (!buf_write(buf, &hmac_length, 1)) { return false; } @@ -1542,7 +1547,7 @@ write_key(const struct key *key, const struct key_type *kt, { return false; } - if (!buf_write(buf, key->hmac, kt->hmac_length)) + if (!buf_write(buf, key->hmac, hmac_length)) { return false; } @@ -1572,7 +1577,7 @@ read_key(struct key *key, const struct key_type *kt, struct buffer *buf) goto read_err; } - if (cipher_length != cipher_kt_key_size(kt->cipher) || hmac_length != kt->hmac_length) + if (cipher_length != cipher_kt_key_size(kt->cipher) || hmac_length != md_kt_size(kt->digest)) { goto key_len_err; } @@ -1595,7 +1600,7 @@ read_err: key_len_err: msg(D_TLS_ERRORS, "TLS Error: key length mismatch, local cipher/hmac %d/%d, remote cipher/hmac %d/%d", - cipher_kt_key_size(kt->cipher), kt->hmac_length, cipher_length, hmac_length); + cipher_kt_key_size(kt->cipher), md_kt_size(kt->digest), cipher_length, hmac_length); return 0; } diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h index 8998a74f9..1e2ca3cb0 100644 --- a/src/openvpn/crypto.h +++ b/src/openvpn/crypto.h @@ -138,7 +138,6 @@ struct sha256_digest { */ struct key_type { - uint8_t hmac_length; /**< HMAC length, in bytes */ const cipher_kt_t *cipher; /**< Cipher static parameters */ const md_kt_t *digest; /**< Message digest static parameters */ }; diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h index d4dd93c3a..cc3e40400 100644 --- a/src/openvpn/crypto_backend.h +++ b/src/openvpn/crypto_backend.h @@ -616,12 +616,10 @@ void hmac_ctx_free(hmac_ctx_t *ctx); * * @param ctx HMAC context to initialise * @param key The key to use for the HMAC - * @param key_len The key length to use * @param kt Static message digest parameters * */ -void hmac_ctx_init(hmac_ctx_t *ctx, const uint8_t *key, int key_length, - const md_kt_t *kt); +void hmac_ctx_init(hmac_ctx_t *ctx, const uint8_t *key, const md_kt_t *kt); /* * Free the given HMAC context. diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c index feb97bc94..8acf0e184 100644 --- a/src/openvpn/crypto_mbedtls.c +++ b/src/openvpn/crypto_mbedtls.c @@ -867,12 +867,13 @@ hmac_ctx_free(mbedtls_md_context_t *ctx) } void -hmac_ctx_init(mbedtls_md_context_t *ctx, const uint8_t *key, int key_len, +hmac_ctx_init(mbedtls_md_context_t *ctx, const uint8_t *key, const mbedtls_md_info_t *kt) { ASSERT(NULL != kt && NULL != ctx); mbedtls_md_init(ctx); + int key_len = mbedtls_md_get_size(kt); ASSERT(0 == mbedtls_md_setup(ctx, kt, 1)); ASSERT(0 == mbedtls_md_hmac_starts(ctx, key, key_len)); @@ -978,8 +979,15 @@ tls1_P_hash(const md_kt_t *md_kt, const uint8_t *sec, int sec_len, int chunk = md_kt_size(md_kt); unsigned int A1_len = md_kt_size(md_kt); - hmac_ctx_init(ctx, sec, sec_len, md_kt); - hmac_ctx_init(ctx_tmp, sec, sec_len, md_kt); + /* This is the only place where we init an HMAC with a key that is not + * equal to its size, therefore we init the hmac ctx manually here */ + mbedtls_md_init(ctx); + ASSERT(0 == mbedtls_md_setup(ctx, md_kt, 1)); + ASSERT(0 == mbedtls_md_hmac_starts(ctx, sec, sec_len)); + + mbedtls_md_init(ctx_tmp); + ASSERT(0 == mbedtls_md_setup(ctx_tmp, md_kt, 1)); + ASSERT(0 == mbedtls_md_hmac_starts(ctx_tmp, sec, sec_len)); hmac_ctx_update(ctx,seed,seed_len); hmac_ctx_final(ctx, A1); diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index 8b53b2ce8..e28e2f43a 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -1079,11 +1079,11 @@ hmac_ctx_free(HMAC_CTX *ctx) } void -hmac_ctx_init(HMAC_CTX *ctx, const uint8_t *key, int key_len, - const EVP_MD *kt) +hmac_ctx_init(HMAC_CTX *ctx, const uint8_t *key, const EVP_MD *kt) { ASSERT(NULL != kt && NULL != ctx); + int key_len = EVP_MD_size(kt); HMAC_CTX_reset(ctx); if (!HMAC_Init_ex(ctx, key, key_len, kt, NULL)) { @@ -1152,10 +1152,10 @@ hmac_ctx_free(hmac_ctx_t *ctx) } void -hmac_ctx_init(hmac_ctx_t *ctx, const uint8_t *key, int key_len, - const EVP_MD *kt) +hmac_ctx_init(hmac_ctx_t *ctx, const uint8_t *key, const EVP_MD *kt) { ASSERT(NULL != kt && NULL != ctx && ctx->ctx != NULL); + int key_len = EVP_MD_size(kt); ASSERT(key_len <= EVP_MAX_KEY_LENGTH); /* We need to make a copy of the key since the OSSL parameters diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 0645a08df..4fee7f49f 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2661,8 +2661,6 @@ do_init_tls_wrap_key(struct context *c) if (!streq(options->authname, "none")) { c->c1.ks.tls_auth_key_type.digest = md_kt_get(options->authname); - c->c1.ks.tls_auth_key_type.hmac_length = - md_kt_size(c->c1.ks.tls_auth_key_type.digest); } else { diff --git a/src/openvpn/ntlm.c b/src/openvpn/ntlm.c index 28e68ded5..8fc9fbd6a 100644 --- a/src/openvpn/ntlm.c +++ b/src/openvpn/ntlm.c @@ -81,13 +81,13 @@ gen_md4_hash(const uint8_t *data, int data_len, uint8_t *result) } static void -gen_hmac_md5(const uint8_t *data, int data_len, const uint8_t *key, int key_len, +gen_hmac_md5(const uint8_t *data, int data_len, const uint8_t *key, uint8_t *result) { const md_kt_t *md5_kt = md_kt_get("MD5"); hmac_ctx_t *hmac_ctx = hmac_ctx_new(); - hmac_ctx_init(hmac_ctx, key, key_len, md5_kt); + hmac_ctx_init(hmac_ctx, key, md5_kt); hmac_ctx_update(hmac_ctx, data, data_len); hmac_ctx_final(hmac_ctx, result); hmac_ctx_cleanup(hmac_ctx); @@ -287,7 +287,7 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, } unicodize(userdomain_u, userdomain); gen_hmac_md5((uint8_t *)userdomain_u, 2 * strlen(userdomain), md4_hash, - MD5_DIGEST_LENGTH, ntlmv2_hash); + ntlmv2_hash); /* NTLMv2 Blob */ memset(ntlmv2_blob, 0, 128); /* Clear blob buffer */ @@ -352,7 +352,7 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, /* hmac-md5 */ gen_hmac_md5(&ntlmv2_response[8], ntlmv2_blob_size + 8, ntlmv2_hash, - MD5_DIGEST_LENGTH, ntlmv2_hmacmd5); + ntlmv2_hmacmd5); /* Add hmac-md5 result to the blob. * Note: This overwrites challenge previously written at diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h index df6bc9df2..84477837e 100644 --- a/src/openvpn/openvpn.h +++ b/src/openvpn/openvpn.h @@ -526,7 +526,7 @@ struct context #define PROTO_DUMP(buf, gc) protocol_dump((buf), \ PROTO_DUMP_FLAGS \ |(c->c2.tls_multi ? PD_TLS : 0) \ - |(c->options.tls_auth_file ? c->c1.ks.key_type.hmac_length : 0), \ + |(c->options.tls_auth_file ? md_kt_size(c->c1.ks.key_type.digest) : 0), \ gc) #define CIPHER_ENABLED(c) (c->c1.ks.key_type.cipher != NULL) diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c index 8403363e2..80ed9684e 100644 --- a/src/openvpn/tls_crypt.c +++ b/src/openvpn/tls_crypt.c @@ -65,8 +65,6 @@ tls_crypt_kt(void) return (struct key_type) { 0 }; } - kt.hmac_length = md_kt_size(kt.digest); - return kt; } diff --git a/tests/unit_tests/openvpn/test_crypto.c b/tests/unit_tests/openvpn/test_crypto.c index d3ce2d6f5..42632c72b 100644 --- a/tests/unit_tests/openvpn/test_crypto.c +++ b/tests/unit_tests/openvpn/test_crypto.c @@ -181,7 +181,7 @@ crypto_test_hmac(void **state) uint8_t key[20]; memcpy(key, testkey, sizeof(key)); - hmac_ctx_init(hmac, key, 20, sha1); + hmac_ctx_init(hmac, key, sha1); hmac_ctx_update(hmac, (const uint8_t *)ipsumlorem, (int) strlen(ipsumlorem)); hmac_ctx_update(hmac, (const uint8_t *)ipsumlorem, (int) strlen(ipsumlorem));