Skip to content

Commit

Permalink
CryptoPkg/TlsLib: fix tls cipher configuration
Browse files Browse the repository at this point in the history 
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.

Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=2541
Signed-off-by: Gerd Hoffmann <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Laszlo Ersek <[email protected]>
Reviewed-by: Yi Li <[email protected]>
  • Loading branch information
@kraxel @mergify
kraxel authored and mergify[bot] committed Oct 8, 2023
1 parent 5087a07 commit 4ddd8ac
Showing 1 changed file with 36 additions and 128 deletions.
164 changes: 36 additions & 128 deletions CryptoPkg/Library/TlsLib/TlsConfig.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 OpensslCipherNameLength;

TlsConn = (TLS_CONNECTION *)Tls;
if ((TlsConn == NULL) || (TlsConn->Ssl == NULL) || (CipherId == NULL)) {
Expand All @@ -315,18 +213,26 @@ 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.
//
MappedCipherCount = 0;
CipherStringSize = 0;
for (Index = 0; Index < CipherNum; Index++) {
for (Index = 0; OpensslCipherStack != NULL && Index < CipherNum; Index++) {
//
// 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",
Expand All @@ -343,7 +249,7 @@ TlsSetCipherList (
}

//
// Accumulate Mapping->OpensslCipherLength into CipherStringSize. If this
// Accumulate cipher name string length into CipherStringSize. If this
// is not the first successful mapping, account for a colon (":") prefix
// too.
//
Expand All @@ -357,7 +263,7 @@ TlsSetCipherList (

Status = SafeUintnAdd (
CipherStringSize,
Mapping->OpensslCipherLength,
AsciiStrLen (SSL_CIPHER_get_name (OpensslCipher)),
&CipherStringSize
);
if (EFI_ERROR (Status)) {
Expand All @@ -368,7 +274,7 @@ TlsSetCipherList (
//
// Record the mapping.
//
MappedCipher[MappedCipherCount++] = Mapping;
MappedCipher[MappedCipherCount++] = OpensslCipher;
}

//
Expand Down Expand Up @@ -403,21 +309,23 @@ TlsSetCipherList (
//
CipherStringPosition = CipherString;
for (Index = 0; Index < MappedCipherCount; Index++) {
Mapping = MappedCipher[Index];
OpensslCipher = MappedCipher[Index];
OpensslCipherName = SSL_CIPHER_get_name (OpensslCipher);
OpensslCipherNameLength = AsciiStrLen (OpensslCipherName);
//
// Append the colon (":") prefix except for the first mapping, then append
// Mapping->OpensslCipher.
// OpensslCipherName.
//
if (Index > 0) {
*(CipherStringPosition++) = ':';
}

CopyMem (
CipherStringPosition,
Mapping->OpensslCipher,
Mapping->OpensslCipherLength
OpensslCipherName,
OpensslCipherNameLength
);
CipherStringPosition += Mapping->OpensslCipherLength;
CipherStringPosition += OpensslCipherNameLength;
}

//
Expand Down

0 comments on commit 4ddd8ac

Please sign in to comment.