From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 6E4ED941AB0 for ; Fri, 29 Sep 2023 13:23:27 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=QySG8zoiMR/lOCdrjFP/5O1nlyFD9Em669H4LkXInww=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding:Content-Type; s=20140610; t=1695993806; v=1; b=h2gV3D6nFmVpWc72OYOTD8UQUpZy3yzMOqm/zx9xu4GG/rtjokMfYPlIIeqBDJ3PEKlHy3g9 UYlwMsWChkY/is2acMmTd7659Sr+c0DIxp2YGTxojf6WoLhJUiDuHvE/ag7cL9LLWs9qCFxa3RU eVjgmgDzbfrTZfzxcsUSMWZc= X-Received: by 127.0.0.2 with SMTP id zICNYY7687511xsxdJoW39AN; Fri, 29 Sep 2023 06:23:26 -0700 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.groups.io with SMTP id smtpd.web11.17069.1695993805352448423 for ; Fri, 29 Sep 2023 06:23:25 -0700 X-Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-269-NeVTBmXmNBW66vO-eR0MEg-1; Fri, 29 Sep 2023 09:23:21 -0400 X-MC-Unique: NeVTBmXmNBW66vO-eR0MEg-1 X-Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id DA49285A5BA; Fri, 29 Sep 2023 13:23:20 +0000 (UTC) X-Received: from sirius.home.kraxel.org (unknown [10.39.193.95]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5B90D40C2064; Fri, 29 Sep 2023 13:23:20 +0000 (UTC) X-Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id DF7E41800638; Fri, 29 Sep 2023 15:23:18 +0200 (CEST) From: "Gerd Hoffmann" To: devel@edk2.groups.io Cc: =?UTF-8?q?L=C3=A1szl=C3=B3=20=C3=89rsek?= , Yi Li , Guomin Jiang , Xiaoyu Lu , Jiewen Yao , Oliver Steffen , Gerd Hoffmann Subject: [edk2-devel] [PATCH 1/1] CryptoPkg/TlsLib: fix tls cipher configuration Date: Fri, 29 Sep 2023 15:23:18 +0200 Message-ID: <20230929132318.2901308-1-kraxel@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,kraxel@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: NFqBrFsKtj0Q9gBFbTLJ9Mybx7686176AA= Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=h2gV3D6n; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io Trying to configure the TLS ciphers can lead to TLS handshake failures because TlsCipherMappingTable is not in line with the ciphers actually supported by OpensslLib. Fix that by removing TlsCipherMappingTable altogether. Use SSL_get_ciphers() instead to get the stack of ciphers supported by openssl. Name and ID of the ciphers can be queried using the SSL_CIPHER_get_name() and SSL_CIPHER_get_protocol_id() functions, which allows us to map IDs to names without a hard-code table. Signed-off-by: Gerd Hoffmann --- CryptoPkg/Library/TlsLib/TlsConfig.c | 158 ++++++--------------------- 1 file changed, 33 insertions(+), 125 deletions(-) diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c b/CryptoPkg/Library/TlsLib/TlsConfig.c index f9333165a913..0ca6994b6586 100644 --- a/CryptoPkg/Library/TlsLib/TlsConfig.c +++ b/CryptoPkg/Library/TlsLib/TlsConfig.c @@ -9,65 +9,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include "InternalTlsLib.h" -typedef struct { - // - // IANA/IETF defined Cipher Suite ID - // - UINT16 IanaCipher; - // - // OpenSSL-used Cipher Suite String - // - CONST CHAR8 *OpensslCipher; - // - // Length of OpensslCipher - // - UINTN OpensslCipherLength; -} TLS_CIPHER_MAPPING; - -// -// Create a TLS_CIPHER_MAPPING initializer from IanaCipher and OpensslCipher so -// that OpensslCipherLength is filled in automatically. IanaCipher must be an -// integer constant expression, and OpensslCipher must be a string literal. -// -#define MAP(IanaCipher, OpensslCipher) \ - { (IanaCipher), (OpensslCipher), sizeof (OpensslCipher) - 1 } - -// -// The mapping table between IANA/IETF Cipher Suite definitions and -// OpenSSL-used Cipher Suite name. -// -// Keep the table uniquely sorted by the IanaCipher field, in increasing order. -// -STATIC CONST TLS_CIPHER_MAPPING TlsCipherMappingTable[] = { - MAP (0x0001, "NULL-MD5"), /// TLS_RSA_WITH_NULL_MD5 - MAP (0x0002, "NULL-SHA"), /// TLS_RSA_WITH_NULL_SHA - MAP (0x0004, "RC4-MD5"), /// TLS_RSA_WITH_RC4_128_MD5 - MAP (0x0005, "RC4-SHA"), /// TLS_RSA_WITH_RC4_128_SHA - MAP (0x000A, "DES-CBC3-SHA"), /// TLS_RSA_WITH_3DES_EDE_CBC_SHA, mandatory TLS 1.1 - MAP (0x0016, "DHE-RSA-DES-CBC3-SHA"), /// TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA - MAP (0x002F, "AES128-SHA"), /// TLS_RSA_WITH_AES_128_CBC_SHA, mandatory TLS 1.2 - MAP (0x0030, "DH-DSS-AES128-SHA"), /// TLS_DH_DSS_WITH_AES_128_CBC_SHA - MAP (0x0031, "DH-RSA-AES128-SHA"), /// TLS_DH_RSA_WITH_AES_128_CBC_SHA - MAP (0x0033, "DHE-RSA-AES128-SHA"), /// TLS_DHE_RSA_WITH_AES_128_CBC_SHA - MAP (0x0035, "AES256-SHA"), /// TLS_RSA_WITH_AES_256_CBC_SHA - MAP (0x0036, "DH-DSS-AES256-SHA"), /// TLS_DH_DSS_WITH_AES_256_CBC_SHA - MAP (0x0037, "DH-RSA-AES256-SHA"), /// TLS_DH_RSA_WITH_AES_256_CBC_SHA - MAP (0x0039, "DHE-RSA-AES256-SHA"), /// TLS_DHE_RSA_WITH_AES_256_CBC_SHA - MAP (0x003B, "NULL-SHA256"), /// TLS_RSA_WITH_NULL_SHA256 - MAP (0x003C, "AES128-SHA256"), /// TLS_RSA_WITH_AES_128_CBC_SHA256 - MAP (0x003D, "AES256-SHA256"), /// TLS_RSA_WITH_AES_256_CBC_SHA256 - MAP (0x003E, "DH-DSS-AES128-SHA256"), /// TLS_DH_DSS_WITH_AES_128_CBC_SHA256 - MAP (0x003F, "DH-RSA-AES128-SHA256"), /// TLS_DH_RSA_WITH_AES_128_CBC_SHA256 - MAP (0x0067, "DHE-RSA-AES128-SHA256"), /// TLS_DHE_RSA_WITH_AES_128_CBC_SHA256 - MAP (0x0068, "DH-DSS-AES256-SHA256"), /// TLS_DH_DSS_WITH_AES_256_CBC_SHA256 - MAP (0x0069, "DH-RSA-AES256-SHA256"), /// TLS_DH_RSA_WITH_AES_256_CBC_SHA256 - MAP (0x006B, "DHE-RSA-AES256-SHA256"), /// TLS_DHE_RSA_WITH_AES_256_CBC_SHA256 - MAP (0x009F, "DHE-RSA-AES256-GCM-SHA384"), /// TLS_DHE_RSA_WITH_AES_256_GCM_SHA384 - MAP (0xC02B, "ECDHE-ECDSA-AES128-GCM-SHA256"), /// TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 - MAP (0xC02C, "ECDHE-ECDSA-AES256-GCM-SHA384"), /// TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 - MAP (0xC030, "ECDHE-RSA-AES256-GCM-SHA384"), /// TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 -}; - typedef struct { // // TLS Algorithm @@ -96,54 +37,6 @@ STATIC CONST TLS_ALGO_TO_NAME TlsSignatureAlgoToName[] = { { TlsSignatureAlgoEcdsa, "ECDSA" }, }; -/** - Gets the OpenSSL cipher suite mapping for the supplied IANA TLS cipher suite. - - @param[in] CipherId The supplied IANA TLS cipher suite ID. - - @return The corresponding OpenSSL cipher suite mapping if found, - NULL otherwise. - -**/ -STATIC -CONST TLS_CIPHER_MAPPING * -TlsGetCipherMapping ( - IN UINT16 CipherId - ) -{ - INTN Left; - INTN Right; - INTN Middle; - - // - // Binary Search Cipher Mapping Table for IANA-OpenSSL Cipher Translation - // - Left = 0; - Right = ARRAY_SIZE (TlsCipherMappingTable) - 1; - - while (Right >= Left) { - Middle = (Left + Right) / 2; - - if (CipherId == TlsCipherMappingTable[Middle].IanaCipher) { - // - // Translate IANA cipher suite ID to OpenSSL name. - // - return &TlsCipherMappingTable[Middle]; - } - - if (CipherId < TlsCipherMappingTable[Middle].IanaCipher) { - Right = Middle - 1; - } else { - Left = Middle + 1; - } - } - - // - // No Cipher Mapping found, return NULL. - // - return NULL; -} - /** Set a new TLS/SSL method for a particular TLS object. @@ -281,16 +174,21 @@ TlsSetCipherList ( IN UINTN CipherNum ) { - TLS_CONNECTION *TlsConn; - EFI_STATUS Status; - CONST TLS_CIPHER_MAPPING **MappedCipher; - UINTN MappedCipherBytes; - UINTN MappedCipherCount; - UINTN CipherStringSize; - UINTN Index; - CONST TLS_CIPHER_MAPPING *Mapping; - CHAR8 *CipherString; - CHAR8 *CipherStringPosition; + TLS_CONNECTION *TlsConn; + EFI_STATUS Status; + CONST SSL_CIPHER **MappedCipher; + UINTN MappedCipherBytes; + UINTN MappedCipherCount; + UINTN CipherStringSize; + UINTN Index; + INT32 StackIdx; + CHAR8 *CipherString; + CHAR8 *CipherStringPosition; + + STACK_OF (SSL_CIPHER) *OpensslCipherStack; + CONST SSL_CIPHER *OpensslCipher; + CONST CHAR8 *OpensslCipherName; + UINTN OpensslCipherNameSize; TlsConn = (TLS_CONNECTION *)Tls; if ((TlsConn == NULL) || (TlsConn->Ssl == NULL) || (CipherId == NULL)) { @@ -315,6 +213,8 @@ TlsSetCipherList ( return EFI_OUT_OF_RESOURCES; } + OpensslCipherStack = SSL_get_ciphers (TlsConn->Ssl); + // // Map the cipher IDs, and count the number of bytes for the full // CipherString. @@ -325,8 +225,14 @@ TlsSetCipherList ( // // Look up the IANA-to-OpenSSL mapping. // - Mapping = TlsGetCipherMapping (CipherId[Index]); - if (Mapping == NULL) { + for (StackIdx = 0; StackIdx < sk_SSL_CIPHER_num (OpensslCipherStack); StackIdx++) { + OpensslCipher = sk_SSL_CIPHER_value (OpensslCipherStack, StackIdx); + if (CipherId[Index] == SSL_CIPHER_get_protocol_id (OpensslCipher)) { + break; + } + } + + if (StackIdx == sk_SSL_CIPHER_num (OpensslCipherStack)) { DEBUG (( DEBUG_VERBOSE, "%a:%a: skipping CipherId=0x%04x\n", @@ -357,7 +263,7 @@ TlsSetCipherList ( Status = SafeUintnAdd ( CipherStringSize, - Mapping->OpensslCipherLength, + AsciiStrnLenS (SSL_CIPHER_get_name (OpensslCipher), MAX_STRING_SIZE), &CipherStringSize ); if (EFI_ERROR (Status)) { @@ -368,7 +274,7 @@ TlsSetCipherList ( // // Record the mapping. // - MappedCipher[MappedCipherCount++] = Mapping; + MappedCipher[MappedCipherCount++] = OpensslCipher; } // @@ -403,7 +309,9 @@ TlsSetCipherList ( // CipherStringPosition = CipherString; for (Index = 0; Index < MappedCipherCount; Index++) { - Mapping = MappedCipher[Index]; + OpensslCipher = MappedCipher[Index]; + OpensslCipherName = SSL_CIPHER_get_name (OpensslCipher); + OpensslCipherNameSize = AsciiStrnLenS (OpensslCipherName, MAX_STRING_SIZE); // // Append the colon (":") prefix except for the first mapping, then append // Mapping->OpensslCipher. @@ -414,10 +322,10 @@ TlsSetCipherList ( CopyMem ( CipherStringPosition, - Mapping->OpensslCipher, - Mapping->OpensslCipherLength + OpensslCipherName, + OpensslCipherNameSize ); - CipherStringPosition += Mapping->OpensslCipherLength; + CipherStringPosition += OpensslCipherNameSize; } // -- 2.41.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109198): https://edk2.groups.io/g/devel/message/109198 Mute This Topic: https://groups.io/mt/101657203/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-