From patchwork Sun Sep 19 06:29:55 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1959 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director15.mail.ord1d.rsapps.net ([172.31.255.6]) by backend30.mail.ord1d.rsapps.net with LMTP id KPTCB9llR2FxHgAAIUCqbw (envelope-from ) for ; Sun, 19 Sep 2021 12:31:21 -0400 Received: from proxy11.mail.iad3b.rsapps.net ([172.31.255.6]) by director15.mail.ord1d.rsapps.net with LMTP id oJB8B9llR2E2CAAAIcMcQg (envelope-from ) for ; Sun, 19 Sep 2021 12:31:21 -0400 Received: from smtp15.gate.iad3b ([172.31.255.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy11.mail.iad3b.rsapps.net with LMTPS id KJe3AdllR2HnMQAARNREpw (envelope-from ) for ; Sun, 19 Sep 2021 12:31:21 -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: smtp15.gate.iad3b.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: 050b2cf0-1967-11ec-8b02-5254003d6d3a-1-1 Received: from [216.105.38.7] ([216.105.38.7:44136] helo=lists.sourceforge.net) by smtp15.gate.iad3b.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 45/DB-09934-8D567416; Sun, 19 Sep 2021 12:31:20 -0400 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.90_1) (envelope-from ) id 1mRzht-0000DN-3C; Sun, 19 Sep 2021 16:30:21 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mRzhk-0000D9-84 for openvpn-devel@lists.sourceforge.net; Sun, 19 Sep 2021 16:30:12 +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=hhgqcJpDR83Fd8q59DD2yXf2kgHGoDyGZRFgFYx0+jA=; b=aswkgh9AxLJu6j/Og28LUtUj4G bQYhs7Xdfz17FLqx9geAnqmkA+e13fpLtktpqzhu5KkOjqZNI17T+qyqodIe6vh6Uav8jobmpdNei hwKYzRQ9kO9nYnN/4Zg282vbbHABeGcy7GzR2D7PdpE1n8evgkx1joVIfUm7KmK3bK0o=; 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=hhgqcJpDR83Fd8q59DD2yXf2kgHGoDyGZRFgFYx0+jA=; b=MhdBZpOTrr4HPAZhuUQ26zXJAG ZZmiWCQ6vwq4YjFCdXpX9Ve0m1BjcTuj6UNKYzT/gfBUhkglHC22EUr3MqkiIaLOHbjgbJZ4C9gkW ES9yQNtw9GdJn6BZxyRLIJxoS8CqZZ7RUJLsX4JaYeBJK6oPJKV8UDF09m9GtAgWSdXw=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.3) id 1mRzhf-00052h-6Q for openvpn-devel@lists.sourceforge.net; Sun, 19 Sep 2021 16:30:12 +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 1mRzhU-0002Ym-Py for openvpn-devel@lists.sourceforge.net; Sun, 19 Sep 2021 18:29:56 +0200 Received: (nullmailer pid 695567 invoked by uid 10006); Sun, 19 Sep 2021 16:29:57 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Sun, 19 Sep 2021 18:29:55 +0200 Message-Id: <20210919162956.695496-7-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210919162956.695496-1-arne@rfc2549.org> References: <20210919162956.695496-1-arne@rfc2549.org> MIME-Version: 1.0 X-Spam-Report: Spam detection software, running on the system "util-spamd-2.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 code mainly sets the parity bits in the DES keys. As mbed TLS and OpenSSL already ignore these bits in the DES key and since DES is deprecated, remove this special DES code that is not even neede [...] Content analysis details: (0.3 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 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 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record X-Headers-End: 1mRzhf-00052h-6Q Subject: [Openvpn-devel] [PATCH 7/8] [OSSL 3.0] Remove DES key fixup code 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 code mainly sets the parity bits in the DES keys. As mbed TLS and OpenSSL already ignore these bits in the DES key and since DES is deprecated, remove this special DES code that is not even needed by the libraries. --- src/openvpn/crypto.c | 46 ------------------------------------ src/openvpn/crypto.h | 2 -- src/openvpn/crypto_backend.h | 9 ------- src/openvpn/crypto_mbedtls.c | 19 --------------- src/openvpn/crypto_openssl.c | 21 ---------------- src/openvpn/ntlm.c | 1 - src/openvpn/ssl.c | 18 -------------- 7 files changed, 116 deletions(-) diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 1dfc760f9..ce041153f 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -956,45 +956,6 @@ check_key(struct key *key, const struct key_type *kt) return true; } -/* - * Make safe mutations to key to ensure it is valid, - * such as ensuring correct parity on DES keys. - * - * This routine cannot guarantee it will generate a good - * key. You must always call check_key after this routine - * to make sure. - */ -void -fixup_key(struct key *key, const struct key_type *kt) -{ - struct gc_arena gc = gc_new(); - if (kt->cipher) - { -#ifdef ENABLE_DEBUG - const struct key orig = *key; -#endif - const int ndc = key_des_num_cblocks(kt->cipher); - - if (ndc) - { - key_des_fixup(key->cipher, kt->cipher_length, ndc); - } - -#ifdef ENABLE_DEBUG - if (check_debug_level(D_CRYPTO_DEBUG)) - { - if (memcmp(orig.cipher, key->cipher, kt->cipher_length)) - { - dmsg(D_CRYPTO_DEBUG, "CRYPTO INFO: fixup_key: before=%s after=%s", - format_hex(orig.cipher, kt->cipher_length, 0, &gc), - format_hex(key->cipher, kt->cipher_length, 0, &gc)); - } - } -#endif - } - gc_free(&gc); -} - void check_replay_consistency(const struct key_type *kt, bool packet_id) { @@ -1043,10 +1004,6 @@ generate_key_random(struct key *key, const struct key_type *kt) dmsg(D_SHOW_KEY_SOURCE, "Cipher source entropy: %s", format_hex(key->cipher, cipher_len, 0, &gc)); dmsg(D_SHOW_KEY_SOURCE, "HMAC source entropy: %s", format_hex(key->hmac, hmac_len, 0, &gc)); - if (kt) - { - fixup_key(key, kt); - } } while (kt && !check_key(key, kt)); gc_free(&gc); @@ -1589,9 +1546,6 @@ verify_fix_key2(struct key2 *key2, const struct key_type *kt, const char *shared for (i = 0; i < key2->n; ++i) { - /* Fix parity for DES keys and make sure not a weak key */ - fixup_key(&key2->keys[i], kt); - /* This should be a very improbable failure */ if (!check_key(&key2->keys[i], kt)) { diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h index 759da4bfb..e9ba21ab2 100644 --- a/src/openvpn/crypto.h +++ b/src/openvpn/crypto.h @@ -288,8 +288,6 @@ void check_replay_consistency(const struct key_type *kt, bool packet_id); bool check_key(struct key *key, const struct key_type *kt); -void fixup_key(struct key *key, const struct key_type *kt); - bool write_key(const struct key *key, const struct key_type *kt, struct buffer *buf); diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h index e0bfdf585..cc897acf4 100644 --- a/src/openvpn/crypto_backend.h +++ b/src/openvpn/crypto_backend.h @@ -170,15 +170,6 @@ int key_des_num_cblocks(const cipher_kt_t *kt); */ bool key_des_check(uint8_t *key, int key_len, int ndc); -/* - * Fix the given DES key, setting its parity to odd. - * - * @param key Key to check - * @param key_len Length of the key, in bytes - * @param ndc Number of DES cblocks that the key is made up of. - */ -void key_des_fixup(uint8_t *key, int key_len, int ndc); - /** * Encrypt the given block, using DES ECB mode * diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c index e2f5f4012..2c4a1405c 100644 --- a/src/openvpn/crypto_mbedtls.c +++ b/src/openvpn/crypto_mbedtls.c @@ -434,25 +434,6 @@ err: return false; } -void -key_des_fixup(uint8_t *key, int key_len, int ndc) -{ - int i; - struct buffer b; - - buf_set_read(&b, key, key_len); - for (i = 0; i < ndc; ++i) - { - unsigned char *key = buf_read_alloc(&b, MBEDTLS_DES_KEY_SIZE); - if (!key) - { - msg(D_CRYPT_ERRORS, "CRYPTO INFO: fixup_key_DES: insufficient key material"); - return; - } - mbedtls_des_key_set_parity(key); - } -} - /* * * Generic cipher key type functions diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index 9df6da02c..8637be86d 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -564,27 +564,6 @@ err: #endif } -void -key_des_fixup(uint8_t *key, int key_len, int ndc) -{ - int i; - struct buffer b; - - buf_set_read(&b, key, key_len); - for (i = 0; i < ndc; ++i) - { - DES_cblock *dc = (DES_cblock *) buf_read_alloc(&b, sizeof(DES_cblock)); - if (!dc) - { - msg(D_CRYPT_ERRORS, "CRYPTO INFO: fixup_key_DES: insufficient key material"); - ERR_clear_error(); - return; - } - DES_set_odd_parity(dc); - } -} - - /* * * Generic cipher key type functions diff --git a/src/openvpn/ntlm.c b/src/openvpn/ntlm.c index 3abe3b7e3..28e68ded5 100644 --- a/src/openvpn/ntlm.c +++ b/src/openvpn/ntlm.c @@ -67,7 +67,6 @@ create_des_keys(const unsigned char *hash, unsigned char *key) key[5] = ((hash[4] & 31) << 3) | (hash[5] >> 5); key[6] = ((hash[5] & 63) << 2) | (hash[6] >> 6); key[7] = ((hash[6] & 127) << 1); - key_des_fixup(key, 8, 1); } static void diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index b2dc48be2..ee416a64c 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1739,24 +1739,6 @@ generate_key_expansion_openvpn_prf(const struct tls_session *session, struct key } secure_memzero(&master, sizeof(master)); - - - /* - * fixup_key only correctly sets DES parity bits if the cipher is a - * DES variant. - * - * The newer OpenSSL and mbed TLS libraries (those that support EKM) - * ignore these bits. - * - * We keep the DES fixup here as compatibility. - * OpenVPN3 never did this fixup anyway. So this code is *probably* not - * required but we keep it for compatibility until we remove DES support - * since it does not hurt either. - */ - for (int i = 0; i < 2; ++i) - { - fixup_key(&key2->keys[i], &session->opt->key_type); - } key2->n = 2; return true;