From patchwork Mon Nov 11 01:59:50 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "plaisthos (Code Review)" X-Patchwork-Id: 3930 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:41ba:b0:5d9:9f4c:3bc7 with SMTP id a26csp2278089mad; Sun, 10 Nov 2024 18:00:15 -0800 (PST) X-Forwarded-Encrypted: i=2; AJvYcCUtqPvc33pPKJuWy+W2eIgvW470A7mVoNgyuVWJHorAb9UrGddEZM1+Z4JrlsxNy0DPPW8q9E1bEXE=@openvpn.net X-Google-Smtp-Source: AGHT+IG3V1uhGuf6pn6jEtRgypUlf/93z85NpSaRxaofKn/tuC0VdF0PMm3lP79RJfeUWKGJKiJ3 X-Received: by 2002:a05:6602:158e:b0:83a:a305:d9f3 with SMTP id ca18e2360f4ac-83e03351215mr1280540139f.12.1731290415553; Sun, 10 Nov 2024 18:00:15 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1731290415; cv=none; d=google.com; s=arc-20240605; b=iqBwSDiLPtUHM3KMxbhcJQVowQj7uV0ojdJwXBbf7VDZK/+4fJqqGfRvMXVaDxMbs2 +TbyGaEdpHj+CMUfZlcjSkyEPuCHNle9UKTcG8HuXAkA6LPRjln9JCl/Ptk4W7U60kXe R8TQiuPb+FYwUwq6fKk6eRBq59NUAZjYuXcZgPQjM1N9HNokhPGoDBYKpopu9MZvN+r+ o3qfU7CHbjcdcpe2b6P2L7PVPs+27tFtV0JsmCMpxive51AZfeJOwrc6PIrhz9OFDd4g EzkjaNymMw6X4Dw41Ay3rvFFMZUu+hkvi2Fr1k6eVJoS44X4mK52IkeWtD+pOh4mIN3l r75w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=errors-to:cc:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:user-agent :mime-version:message-id:references:auto-submitted:to:date:from :dkim-signature:dkim-signature:dkim-signature; bh=RcVz3AJC7FO1H4iDevLR733OByCM5624AQZu1IrlYVo=; fh=lm0MLPW7DntlrDqRECIiC9JlE1uPxhepE0URYHIf+eE=; b=Mlb7zdcriLslMjheeEwM0pOe9L8W/g/kc7aUFFjIAsGvc0+FN3DBvZH29Z0/l1hkSx VKdhPFxQfDgRwGQwyRBUoz3+QPmPJB9/ut7dDuP2hOurlC9dXoB6fET6FKGvmuz/Itzz UZd8M4MsM3HNyxq4abvML7TYShmHly+uMYRB7yTdmoS6ElGqmBfqtRIwJhMYZoKm3fKQ EhImf75T6L/ny7sh+th1I05eLb+xvPfsOE/OYtailFd1St0i32yHZY4Sc/ZbRK8febG/ +5kQO4NPdaxWKuBeKr3VS2zzQQ9TAtSDvAhrWDFcDMg42RJKWuY5Vtodd/pufkb2H1kh 9wdg==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=UNbz1Is1; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b="cnX5/H05"; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=JihV7jfr; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=openvpn.net; dara=fail header.i=@openvpn.net Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id 8926c6da1cb9f-4de787d45b8si2764377173.105.2024.11.10.18.00.15 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 10 Nov 2024 18:00:15 -0800 (PST) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=UNbz1Is1; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b="cnX5/H05"; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=JihV7jfr; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=openvpn.net; dara=fail header.i=@openvpn.net Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1tAJil-0003TW-7j; Mon, 11 Nov 2024 02:00:03 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1tAJij-0003TI-7w for openvpn-devel@lists.sourceforge.net; Mon, 11 Nov 2024 02:00:01 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Type:Content-Transfer-Encoding:MIME-Version :Message-ID:Reply-To:References:Subject:List-Unsubscribe:List-Id:Cc:To:Date: From:Sender:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help: List-Subscribe:List-Post:List-Owner:List-Archive; bh=ICkrD6rXuT7B0CuoWMGZdhiqFlzG2PKWzMFRmSoSkd4=; b=UNbz1Is170xtdcUUOffkcxgyum mmS4hCTSHyjn+7Imv5T3vuz2mXZgmjkfDYyaS7QaWkL1ae3EbNTLC2JdtsmfXhIcu84cdJhecCN3g KdODy3xRX2fsG/JVQ9Ugw89tVqaHD0Yzt4tqVYZ5Q3bU2foGbcB2Kk5fYcU1Al8uP6zg=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Type:Content-Transfer-Encoding:MIME-Version:Message-ID:Reply-To: References:Subject:List-Unsubscribe:List-Id:Cc:To:Date:From:Sender:Content-ID :Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To: Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help:List-Subscribe:List-Post: List-Owner:List-Archive; bh=ICkrD6rXuT7B0CuoWMGZdhiqFlzG2PKWzMFRmSoSkd4=; b=c nX5/H05xe3ufVCmOlvPLuHyaS9xk59yK/6/WFAlsAxmSnFLcPgOQBq2kBOrmL7ulbJJ+vdZ68m0rb SXPNMG2qBHztMMgvUt3j4tkML1yAWwgnaEZjeq/XqpkNV+6i2UDsfTtVzHm5MoyWM3x2oXXAEuYVo o3Rbb0q/P4uTaevY=; Received: from mail-wm1-f52.google.com ([209.85.128.52]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1tAJih-0001Yu-OA for openvpn-devel@lists.sourceforge.net; Mon, 11 Nov 2024 02:00:01 +0000 Received: by mail-wm1-f52.google.com with SMTP id 5b1f17b1804b1-4315eeb2601so51171055e9.2 for ; Sun, 10 Nov 2024 17:59:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=openvpn.net; s=google; t=1731290393; x=1731895193; darn=lists.sourceforge.net; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from:from:to:cc :subject:date:message-id:reply-to; bh=ICkrD6rXuT7B0CuoWMGZdhiqFlzG2PKWzMFRmSoSkd4=; b=JihV7jfroinKYzizNzkRdfJ3osjRWQt/FNnwe5yQdsInABdugo/hvLdhuhuXVxUdZd iGPbAZFX8BFJbL4dmn39z+uCNPt98QB9soqzi3yxJDRnpCNxngVDcxFnzBY1/FN9DFFF 6Tx7k0rESB9tk3nfG+tN1us13ZKq4lxfczUC77AALsnJhip2KQFKB0b6dEND7z3RpmkN hMz9uQmOwJbOFgRY/hecN4JxkYq8o26L4d/gpJRUUTtnfnhBthRux7+xsI+lqpfvc2b8 SThuprHtryIHshjWsyO0n7bKB2y4+JF4fuIMx1eA7jifBlnGPTLOZ4j0bePIb2GDlOWN igXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731290393; x=1731895193; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ICkrD6rXuT7B0CuoWMGZdhiqFlzG2PKWzMFRmSoSkd4=; b=sfHJZoK1J771tBaDPI4Lii5B4HmwXPSSlygieqeSWlARldWCyDy3uByw9H41eWAcDf YZCZhX3PQMr5U7JxfK3MjAiAKcMT/6EvjOWsi6coi/I2rhffgJjqqe+xvMZWooJzP9g8 DC5noKHg6/RIw6aq6xJ34q1U0d4VN5FYpTZxLY3UXzo6Yof5mMPR9iclOrpaBDquziWn Z1L5NVtTM0ysBe6l3kMVco+5pB62zf/XIp2B0tfCcE/Os5VLH7wX6zTrxyERJA2bUYoB o4dBYG0KdOWM8zAyb6PaH6sGCGC7neLEbnVLnh+JPCbmNRmR6GsAV9upEHE88KNaZChS V7cg== X-Gm-Message-State: AOJu0YxrTITpaMeyeLWxrgv/KZPigCaWUGdlJayMJaHMfwWkr7Qx6aE9 5LO2VOEbH+MTFmH286+EpHDWmZRvKs02K1lNSTvIMUv32G/kEjcEslPP1Uill7gPG4kPpUZRsrr c X-Received: by 2002:a05:6000:1f87:b0:37c:c51b:8d9c with SMTP id ffacd0b85a97d-381f1884871mr11375169f8f.38.1731290392970; Sun, 10 Nov 2024 17:59:52 -0800 (PST) Received: from gerrit.openvpn.in (ec2-18-159-0-78.eu-central-1.compute.amazonaws.com. [18.159.0.78]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-381ed97db25sm11695803f8f.41.2024.11.10.17.59.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 10 Nov 2024 17:59:51 -0800 (PST) From: "plaisthos (Code Review)" X-Google-Original-From: "plaisthos (Code Review)" X-Gerrit-PatchSet: 1 Date: Mon, 11 Nov 2024 01:59:50 +0000 To: flichtenheld Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange X-Gerrit-Change-Id: Id223612c7bcab91d49c013fb775024bd64ab0836 X-Gerrit-Change-Number: 800 X-Gerrit-Project: openvpn X-Gerrit-ChangeURL: X-Gerrit-Commit: 6194cea3e935f752dc53e217865cf301101d7076 References: Message-ID: <1f3d1369ef5f4f20c559c21112f737d5dc72c22c-HTML@gerrit.openvpn.net> MIME-Version: 1.0 User-Agent: Gerrit/3.8.2 X-Spam-Score: -0.9 (/) 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: Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit Content analysis details: (-0.9 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [209.85.128.52 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.7 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.128.52 listed in wl.mailspike.net] 0.0 WEIRD_PORT URI: Uses non-standard port number for HTTP 0.0 HTML_MESSAGE BODY: HTML included in message -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.0 T_KAM_HTML_FONT_INVALID Test for Invalidly Named or Formatted Colors in HTML X-Headers-End: 1tAJih-0001Yu-OA Subject: [Openvpn-devel] [M] Change in openvpn[master]: Move initialisation of implicit IVs to init_key_ctx_bi methods 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: , Reply-To: arne-openvpn@rfc2549.org, openvpn-devel@lists.sourceforge.net, frank@lichtenheld.com Cc: openvpn-devel Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1815389578319431422?= X-GMAIL-MSGID: =?utf-8?q?1815389578319431422?= X-getmail-filter-classifier: gerrit message type newchange Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/800?usp=email to review the following change. Change subject: Move initialisation of implicit IVs to init_key_ctx_bi methods ...................................................................... Move initialisation of implicit IVs to init_key_ctx_bi methods This is really more a function of initialising the data cipher and key context and putting it into the init_key_ctx_bi makes more sense. It will allow calling init_key_ctx_bi to fully initialise a data channel key without calling some extra functions after that which will make the (upcoming) epoch key implementation cleaner. Also ensure that free_ctx_bi actually also sets initialized to false. Change-Id: Id223612c7bcab91d49c013fb775024bd64ab0836 Signed-off-by: Arne Schwabe --- M src/openvpn/crypto.c M src/openvpn/ssl.c M tests/unit_tests/openvpn/test_ssl.c 3 files changed, 30 insertions(+), 59 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/00/800/1 diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index f0b60a3..44c226c 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -892,6 +892,33 @@ } } +/** + * Update the implicit IV for a key_ctx_bi based on TLS session ids and cipher + * used. + * + * Note that the implicit IV is based on the HMAC key, but only in AEAD modes + * where the HMAC key is not used for an actual HMAC. + * + * @param ctx Encrypt/decrypt key context + * @param key key, hmac part used to calculate implicit IV + */ +static void +key_ctx_update_implicit_iv(struct key_ctx *ctx, const struct key *key) +{ + /* Only use implicit IV in AEAD cipher mode, where HMAC key is not used */ + if (cipher_ctx_mode_aead(ctx->cipher)) + { + size_t impl_iv_len = 0; + ASSERT(cipher_ctx_iv_length(ctx->cipher) >= OPENVPN_AEAD_MIN_IV_LEN); + impl_iv_len = cipher_ctx_iv_length(ctx->cipher) - sizeof(packet_id_type); + ASSERT(impl_iv_len <= OPENVPN_MAX_IV_LENGTH); + ASSERT(impl_iv_len <= MAX_HMAC_KEY_LENGTH); + CLEAR(ctx->implicit_iv); + /* The first bytes of the IV are filled with the packet id */ + memcpy(ctx->implicit_iv + sizeof(packet_id_type), key->hmac, impl_iv_len); + } +} + /* given a key and key_type, build a key_ctx */ void init_key_ctx(struct key_ctx *ctx, const struct key *key, @@ -950,7 +977,7 @@ snprintf(log_prefix, sizeof(log_prefix), "Outgoing %s", name); init_key_ctx(ctx, &key2->keys[kds.out_key], kt, OPENVPN_OP_ENCRYPT, log_prefix); - + key_ctx_update_implicit_iv(ctx, &key2->keys[kds.out_key]); } void @@ -965,7 +992,7 @@ snprintf(log_prefix, sizeof(log_prefix), "Incoming %s", name); init_key_ctx(ctx, &key2->keys[kds.in_key], kt, OPENVPN_OP_DECRYPT, log_prefix); - + key_ctx_update_implicit_iv(ctx, &key2->keys[kds.in_key]); } void @@ -1000,6 +1027,7 @@ { free_key_ctx(&ctx->encrypt); free_key_ctx(&ctx->decrypt); + ctx->initialized = false; } static bool diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 2d6cf64..9eaf3ac 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -96,21 +96,6 @@ #endif /* ifdef MEASURE_TLS_HANDSHAKE_STATS */ /** - * Update the implicit IV for a key_ctx_bi based on TLS session ids and cipher - * used. - * - * Note that the implicit IV is based on the HMAC key, but only in AEAD modes - * where the HMAC key is not used for an actual HMAC. - * - * @param ctx Encrypt/decrypt key context - * @param key HMAC key, used to calculate implicit IV - * @param key_len HMAC key length - */ -static void -key_ctx_update_implicit_iv(struct key_ctx *ctx, uint8_t *key, size_t key_len); - - -/** * Limit the reneg_bytes value when using a small-block (<128 bytes) cipher. * * @param cipher The current cipher (may be NULL). @@ -1409,12 +1394,6 @@ else { init_key_ctx_bi(key, key2, key_direction, key_type, "Data Channel"); - /* Initialize implicit IVs */ - key_ctx_update_implicit_iv(&key->encrypt, key2->keys[(int)server].hmac, - MAX_HMAC_KEY_LENGTH); - key_ctx_update_implicit_iv(&key->decrypt, - key2->keys[1 - (int)server].hmac, - MAX_HMAC_KEY_LENGTH); } } @@ -1551,23 +1530,6 @@ return ret; } -static void -key_ctx_update_implicit_iv(struct key_ctx *ctx, uint8_t *key, size_t key_len) -{ - /* Only use implicit IV in AEAD cipher mode, where HMAC key is not used */ - if (cipher_ctx_mode_aead(ctx->cipher)) - { - size_t impl_iv_len = 0; - ASSERT(cipher_ctx_iv_length(ctx->cipher) >= OPENVPN_AEAD_MIN_IV_LEN); - impl_iv_len = cipher_ctx_iv_length(ctx->cipher) - sizeof(packet_id_type); - ASSERT(impl_iv_len <= OPENVPN_MAX_IV_LENGTH); - ASSERT(impl_iv_len <= key_len); - CLEAR(ctx->implicit_iv); - /* The first bytes of the IV are filled with the packet id */ - memcpy(ctx->implicit_iv + sizeof(packet_id_type), key, impl_iv_len); - } -} - /** * Generate data channel keys for the supplied TLS session. * diff --git a/tests/unit_tests/openvpn/test_ssl.c b/tests/unit_tests/openvpn/test_ssl.c index ae33cc6..caacd9e 100644 --- a/tests/unit_tests/openvpn/test_ssl.c +++ b/tests/unit_tests/openvpn/test_ssl.c @@ -277,24 +277,6 @@ #endif /* HAVE_OPENSSL_STORE */ } -static void -init_implicit_iv(struct crypto_options *co) -{ - cipher_ctx_t *cipher = co->key_ctx_bi.encrypt.cipher; - - if (cipher_ctx_mode_aead(cipher)) - { - ASSERT(cipher_ctx_iv_length(cipher) <= OPENVPN_MAX_IV_LENGTH); - ASSERT(cipher_ctx_iv_length(cipher) >= OPENVPN_AEAD_MIN_IV_LEN); - - /* Generate dummy implicit IV */ - ASSERT(rand_bytes(co->key_ctx_bi.encrypt.implicit_iv, - OPENVPN_MAX_IV_LENGTH)); - - memcpy(co->key_ctx_bi.decrypt.implicit_iv, - co->key_ctx_bi.encrypt.implicit_iv, OPENVPN_MAX_IV_LENGTH); - } -} static void init_frame_parameters(struct frame *frame) @@ -346,7 +328,6 @@ /* init work */ ASSERT(buf_init(&work, frame.buf.headroom)); - init_implicit_iv(co); update_time(); /* Test encryption, decryption for all packet sizes */