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 2145DD80CA2 for ; Fri, 29 Sep 2023 10:19:37 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=34xlXQyuQSCjpt3R0V5kAChqbWchU3gvYCW/oxQkMvQ=; c=relaxed/simple; d=groups.io; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Disposition; s=20140610; t=1695982776; v=1; b=iO/PWo5J7hPMRq65PbLUeG4TSBilMLDctg4YxDxmJAF5o7/C8oXzkboqG/uOoGhSKBe9Xjb7 ZuZGYcL+VK8wiW4rD74s6dAFdkepV00Jfz2o0GG7PCczUFCh4I/1KqK11W4yTRsoBVsNDbKj6ss 6XtIoY3IkH/EJQIrVYjvIuo8= X-Received: by 127.0.0.2 with SMTP id TTVcYY7687511x1FBTxqOfvf; Fri, 29 Sep 2023 03:19:36 -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.web10.14112.1695982775957643942 for ; Fri, 29 Sep 2023 03:19:36 -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-211-fvh15AqcONehILt7xhmaQA-1; Fri, 29 Sep 2023 06:19:29 -0400 X-MC-Unique: fvh15AqcONehILt7xhmaQA-1 X-Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (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 C8743101A53B; Fri, 29 Sep 2023 10:19:28 +0000 (UTC) X-Received: from sirius.home.kraxel.org (unknown [10.39.193.95]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2580F492B16; Fri, 29 Sep 2023 10:19:28 +0000 (UTC) X-Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id 804C21800638; Fri, 29 Sep 2023 12:19:26 +0200 (CEST) Date: Fri, 29 Sep 2023 12:19:26 +0200 From: "Gerd Hoffmann" To: Laszlo Ersek Cc: edk2-devel-groups-io , "Li, Yi" , Jiewen Yao Subject: Re: [edk2-devel] setting TLS ciphers is broken (openssl 3?) Message-ID: References: <27kjaqdrgubri6i3vvickznsmdqnuo6h3tbxfmb3hr76n75gjf@cah3opindcnc> <273c853d-e70b-ddda-4387-35b825fdfebc@redhat.com> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 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: RrwzzX42g1QzFqSvuCmAovzFx7686176AA= Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b="iO/PWo5J"; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none) Hi, > > (2) TlsCipherMappingTable [CryptoPkg/Library/TlsLib/TlsConfig.c] > That seems to be the case. Maybe (2) needs an update to enable newer > ciphers, when logging the mappings I see there are quite a few IDs which > don't get mapped: Turns out we don't need TlsCipherMappingTable at all, we can just query the openssl library instead. take care, Gerd --------------------------- cut here --------------------------- >From 47f94a52d8e7a21ec3f0ff05ac7d146beff5be3b Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 29 Sep 2023 12:03:18 +0200 Subject: [PATCH 1/2] CryptoPkg: fix tls config Signed-off-by: Gerd Hoffmann --- CryptoPkg/Library/TlsLib/TlsConfig.c | 157 ++++++--------------------- 1 file changed, 32 insertions(+), 125 deletions(-) diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c b/CryptoPkg/Library/TlsLib/TlsConfig.c index f9333165a913..2e5210193c6b 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,20 @@ 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, 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 +212,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 +224,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 +262,7 @@ TlsSetCipherList ( Status = SafeUintnAdd ( CipherStringSize, - Mapping->OpensslCipherLength, + AsciiStrnLenS (SSL_CIPHER_get_name (OpensslCipher), MAX_STRING_SIZE), &CipherStringSize ); if (EFI_ERROR (Status)) { @@ -368,7 +273,7 @@ TlsSetCipherList ( // // Record the mapping. // - MappedCipher[MappedCipherCount++] = Mapping; + MappedCipher[MappedCipherCount++] = OpensslCipher; } // @@ -403,7 +308,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 +321,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 (#109191): https://edk2.groups.io/g/devel/message/109191 Mute This Topic: https://groups.io/mt/101613778/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-