Skip to content

Commit

Permalink
CryptoPkg/TlsLib: TlsSetVerifyHost: parse IP address literals as such (
Browse files Browse the repository at this point in the history
…CVE-2019-14553)

Using the inet_pton() function that we imported in the previous patches,
recognize if "HostName" is an IP address literal, and then parse it into
binary representation. Passing the latter to OpenSSL for server
certificate validation is important, per RFC-2818
<https://tools.ietf.org/html/rfc2818#section-3.1>:

> In some cases, the URI is specified as an IP address rather than a
> hostname. In this case, the iPAddress subjectAltName must be present in
> the certificate and must exactly match the IP in the URI.

Note: we cannot use X509_VERIFY_PARAM_set1_ip_asc() because in the OpenSSL
version that is currently consumed by edk2, said function depends on
sscanf() for parsing IPv4 literals. In
"CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c", we only provide an
empty -- always failing -- stub for sscanf(), however.

Cc: David Woodhouse <[email protected]>
Cc: Jian J Wang <[email protected]>
Cc: Jiaxin Wu <[email protected]>
Cc: Sivaraman Nainar <[email protected]>
Cc: Xiaoyu Lu <[email protected]>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
CVE: CVE-2019-14553
Suggested-by: David Woodhouse <[email protected]>
Signed-off-by: Laszlo Ersek <[email protected]>
Acked-by: Jian J Wang <[email protected]>
Reviewed-by: Jiaxin Wu <[email protected]>
  • Loading branch information
lersek committed Nov 2, 2019
1 parent 8d16ef8 commit 1e72b1f
Showing 1 changed file with 24 additions and 4 deletions.
28 changes: 24 additions & 4 deletions CryptoPkg/Library/TlsLib/TlsConfig.c
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,11 @@ TlsSetVerifyHost (
IN CHAR8 *HostName
)
{
TLS_CONNECTION *TlsConn;
TLS_CONNECTION *TlsConn;
X509_VERIFY_PARAM *VerifyParam;
UINTN BinaryAddressSize;
UINT8 BinaryAddress[MAX (NS_INADDRSZ, NS_IN6ADDRSZ)];
INTN ParamStatus;

TlsConn = (TLS_CONNECTION *) Tls;
if (TlsConn == NULL || TlsConn->Ssl == NULL || HostName == NULL) {
Expand All @@ -526,11 +530,27 @@ TlsSetVerifyHost (

SSL_set_hostflags(TlsConn->Ssl, Flags);

if (SSL_set1_host(TlsConn->Ssl, HostName) == 0) {
return EFI_ABORTED;
VerifyParam = SSL_get0_param (TlsConn->Ssl);
ASSERT (VerifyParam != NULL);

BinaryAddressSize = 0;
if (inet_pton (AF_INET6, HostName, BinaryAddress) == 1) {
BinaryAddressSize = NS_IN6ADDRSZ;
} else if (inet_pton (AF_INET, HostName, BinaryAddress) == 1) {
BinaryAddressSize = NS_INADDRSZ;
}

return EFI_SUCCESS;
if (BinaryAddressSize > 0) {
DEBUG ((DEBUG_VERBOSE, "%a:%a: parsed \"%a\" as an IPv%c address "
"literal\n", gEfiCallerBaseName, __FUNCTION__, HostName,
(UINTN)((BinaryAddressSize == NS_IN6ADDRSZ) ? '6' : '4')));
ParamStatus = X509_VERIFY_PARAM_set1_ip (VerifyParam, BinaryAddress,
BinaryAddressSize);
} else {
ParamStatus = X509_VERIFY_PARAM_set1_host (VerifyParam, HostName, 0);
}

return (ParamStatus == 1) ? EFI_SUCCESS : EFI_ABORTED;
}

/**
Expand Down

0 comments on commit 1e72b1f

Please sign in to comment.