* [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553)
@ 2019-10-26 5:37 Laszlo Ersek
2019-10-26 5:37 ` [PATCH v2 1/8] MdePkg/Include/Protocol/Tls.h: Add the data type of EfiTlsVerifyHost (CVE-2019-14553) Laszlo Ersek
` (9 more replies)
0 siblings, 10 replies; 26+ messages in thread
From: Laszlo Ersek @ 2019-10-26 5:37 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: David Woodhouse, Jian J Wang, Jiaxin Wu, Sivaraman Nainar,
Xiaoyu Lu
Repo: https://github.com/lersek/edk2.git
Branch: bz960_with_inet_pton_v2
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
Previous posting from Jiaxin:
[edk2-devel] [PATCH v1 0/4]
Support HTTPS HostName validation feature(CVE-2019-14553)
https://edk2.groups.io/g/devel/message/48183
http://mid.mail-archive.com/20190927034441.3096-1-Jiaxin.wu@intel.com
In v2, I have inserted 4 new patches in the middle, to satisfy two
additional requirements raised by Siva and David:
- If the Subject Alternative Name in the server certificate contains an
IP address in binary representation, and the URL specifies an IP
address in literal form for "hostname", then both of those things
should be compared against each other, after converting the literal
from the URL to binary representation. In other words, a server
certificate with an IP address SAN should be recognized.
- If the URL specifies an IP address literal, then, according to
RFC-2818, "the iPAddress subjectAltName must be present in the
certificate and must exactly match the IP in the URI". In other words,
if a certificate matches the IP address literal from the URL via
Common Name only, then the certificate must be rejected.
I've also fixed two commit message warts in Jiaxin's patches (see the
Notes sections on the patches).
I've tested the series painstakingly. Here's the script I wrote for
certificate generation:
> ## @file
> # Bash shell script for generating test certificates, for
> # <https://bugzilla.tianocore.org/show_bug.cgi?id=960>.
> #
> # Copyright (C) 2019, Red Hat, Inc.
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> # Customize te variables in section "Configuration", then run the script with
> # "bash gencerts.sh".
> #
> # The script creates 17 files in the current working directory:
> # - one CA certificate (note: key is discarded);
> #
> # - for the (IPv4 domain name, IPv4 address) pair, one keypair (that is, a
> # CA-issued certificate, plus the private key) for each case below:
> # - Common Name = IPv4 domain name, no subjectAltName,
> # - Common Name = IPv4 domain name, IPv4 address in subjectAltName,
> # - Common Name = IPv4 address literal, no subjectAltName,
> # - Common Name = IPv4 address literal, IPv4 address in subjectAltName;
> #
> # - for the (IPv6 domain name, IPv6 address) pair, a similar set of files.
> #
> # Finally, the script prints some commands for the root user that are related
> # to the following OVMF feature: OVMF can HTTPS boot while trusting the same
> # set of CA certificates that the virt host trusts. The commands install the
> # new CA certificate on the host (note: this should never be done in
> # production, in spite of the CA key being discarded), and also extract all CA
> # certs in the format that OVMF expects. (This edk2-specific extraction is
> # normally performed by the "update-ca-trust" command, but if yours isn't
> # up-to-date enough for that, build and install p11-kit from source, and set
> # MY_P11_KIT_PREFIX, before invoking this script.) See "OvmfPkg/README" for
> # passing the extracted CA certs to OVMF on the QEMU cmdline.
> ##
> set -e -u -C
>
> # Configuration.
> CA_NAME=TianoCore_BZ_960_CA
> IPV4_NAME=ipv4-server
> IPV4_ADDR=192.168.124.2
> IPV6_NAME=ipv6-server
> IPV6_ADDR=fd33:eb1b:9b36::2
>
> # Create a temporary directory for transient files.
> TMP_D=$(mktemp -d)
> trap 'rm -f -r -- "$TMP_D"' EXIT
>
> # Set some helper variables.
> TMP_EXT=$TMP_D/ext # OpenSSL extensions
> TMP_CSR=$TMP_D/csr # certificate request
> TMP_CA_KEY=$TMP_D/ca.key # CA key
> TMP_CA_SRL=$TMP_D/ca.srl # CA serial number
>
> # Generate the CA certificate.
> openssl req -x509 -nodes \
> -subj /CN="$CA_NAME" \
> -out "$CA_NAME".crt \
> -keyout "$TMP_CA_KEY"
>
> # Create a CA-issued certificate.
> # Parameters:
> # $1: Common Name
> # $2: IPv4 or IPv6 address literal, to be used in SAN; or empty string
> gencrt()
> {
> local CN="$1"
> local SANIP="$2"
> local STEM
> local EXT
>
> if test -z "$SANIP"; then
> # File name stem consists of Common Name only. No certificate extensions.
> STEM=svr_$CN
> EXT=
> else
> # File name stem includes Common Name and IP address literal.
> STEM=svr_${CN}_${SANIP}
>
> # SAN IP extension in the certificate. Rewrite the ad-hoc extensions file
> # with the current SAN IP.
> echo "subjectAltName=IP:$SANIP" >| "$TMP_EXT"
> EXT="-extfile $TMP_EXT"
> fi
> STEM=${STEM//[:.]/_}
>
> # Generate CSR.
> openssl req -new -nodes \
> -subj /CN="$CN" \
> -out "$TMP_CSR" \
> -keyout "$STEM".key
>
> # Sign the certificate request, potentially adding the SAN IP.
> openssl x509 -req -CAcreateserial $EXT \
> -in "$TMP_CSR" \
> -out "$STEM".crt \
> -CA "$CA_NAME".crt \
> -CAkey "$TMP_CA_KEY" \
> -CAserial "$TMP_CA_SRL"
> }
>
> # Generate all certificates.
> gencrt "$IPV4_NAME" "" # domain name in CN, no SAN IPv4
> gencrt "$IPV4_NAME" "$IPV4_ADDR" # domain name in CN, SAN IPv4
> gencrt "$IPV4_ADDR" "" # IPv4 literal in CN, no SAN IPv4
> gencrt "$IPV4_ADDR" "$IPV4_ADDR" # IPv4 literal in CN, SAN IPv4
> gencrt "$IPV6_NAME" "" # domain name in CN, no SAN IPv6
> gencrt "$IPV6_NAME" "$IPV6_ADDR" # domain name in CN, SAN IPv6
> gencrt "$IPV6_ADDR" "" # IPv6 literal in CN, no SAN IPv6
> gencrt "$IPV6_ADDR" "$IPV6_ADDR" # IPv6 literal in CN, SAN IPv6
>
> # Print commands for the root user:
> # - for making the CA a trusted CA
> echo
> echo install -o root -g root -m 644 -t /etc/pki/ca-trust/source/anchors \
> "$PWD/$CA_NAME".crt
> echo restorecon -Fvv /etc/pki/ca-trust/source/anchors/"$CA_NAME".crt
> echo update-ca-trust extract
>
> # - and for extracting the CA certificates for OVMF.
> if test -v MY_P11_KIT_PREFIX; then
> echo mkdir -p -v /etc/pki/ca-trust/extracted/edk2
> echo chmod -c --reference=/etc/pki/ca-trust/extracted/java \
> /etc/pki/ca-trust/extracted/edk2
> echo "$MY_P11_KIT_PREFIX/bin/p11-kit" extract --overwrite \
> --format=edk2-cacerts \
> --filter=ca-anchors \
> --purpose=server-auth \
> /etc/pki/ca-trust/extracted/edk2/cacerts.bin
> echo chmod -c --reference=/etc/pki/ca-trust/extracted/java/cacerts \
> /etc/pki/ca-trust/extracted/edk2/cacerts.bin
> echo restorecon -FvvR /etc/pki/ca-trust/extracted/edk2
> fi
And here's the test matrix:
> Server Certificate URL cURL edk2 unpatched edk2 patched
> --------------------- -------------------- ---------------- ---------------- ----------------
> Common Subject hostname resolves status expected status expected status expected
> Name Alt. Name to IPvX
> -------------------------------------------------------------------------------------------------
> IP-literal - IP-literal IPv4 accept COMPAT/1 accept NO/2 reject yes
> IP-literal - IP-literal IPv6 accept COMPAT/1 accept NO/2 reject yes
> IP-literal - domainname IPv4 reject yes accept NO/2 reject yes
> IP-literal - domainname IPv6 reject yes accept NO/2 reject yes
> IP-literal IP IP-literal IPv4 accept yes accept yes accept yes
> IP-literal IP IP-literal IPv6 accept yes accept yes accept yes
> IP-literal IP domainname IPv4 reject yes accept NO/2 reject yes
> IP-literal IP domainname IPv6 reject yes accept NO/2 reject yes
> domainname - IP-literal IPv4 reject yes accept NO/2 reject yes
> domainname - IP-literal IPv6 reject yes accept NO/2 reject yes
> domainname - domainname IPv4 accept yes accept yes accept yes
> domainname - domainname IPv6 accept yes accept yes accept yes
> domainname IP IP-literal IPv4 accept yes accept yes accept yes
> domainname IP IP-literal IPv6 accept yes accept yes accept yes
> domainname IP domainname IPv4 accept yes accept yes accept yes
> domainname IP domainname IPv6 accept yes accept yes accept yes
>
> #1 -- should not be accepted: an IP literal in the URL must match the IP
> address in the SAN, regardless of the Common Name; but cURL accepts it
> for compatibility
>
> #2 -- this is (or exemplifies) CVE-2019-14553
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Thanks,
Laszlo
Laszlo Ersek (4):
CryptoPkg/Crt: turn strchr() into a function (CVE-2019-14553)
CryptoPkg/Crt: satisfy "inet_pton.c" dependencies (CVE-2019-14553)
CryptoPkg/Crt: import "inet_pton.c" (CVE-2019-14553)
CryptoPkg/TlsLib: TlsSetVerifyHost: parse IP address literals as such
(CVE-2019-14553)
Wu, Jiaxin (4):
MdePkg/Include/Protocol/Tls.h: Add the data type of EfiTlsVerifyHost
(CVE-2019-14553)
CryptoPkg/TlsLib: Add the new API "TlsSetVerifyHost" (CVE-2019-14553)
NetworkPkg/TlsDxe: Add the support of host validation to TlsDxe driver
(CVE-2019-14553)
NetworkPkg/HttpDxe: Set the HostName for the verification
(CVE-2019-14553)
CryptoPkg/Include/Library/TlsLib.h | 20 ++
CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf | 1 +
CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c | 5 +
CryptoPkg/Library/BaseCryptLib/SysCall/inet_pton.c | 257 ++++++++++++++++++++
CryptoPkg/Library/Include/CrtLibSupport.h | 19 +-
CryptoPkg/Library/Include/arpa/inet.h | 9 +
CryptoPkg/Library/Include/arpa/nameser.h | 9 +
CryptoPkg/Library/Include/netinet/in.h | 9 +
CryptoPkg/Library/Include/sys/param.h | 9 +
CryptoPkg/Library/Include/sys/socket.h | 9 +
CryptoPkg/Library/TlsLib/TlsConfig.c | 58 ++++-
MdePkg/Include/Protocol/Tls.h | 68 +++++-
NetworkPkg/HttpDxe/HttpProto.h | 1 +
NetworkPkg/HttpDxe/HttpsSupport.c | 21 +-
NetworkPkg/TlsDxe/TlsProtocol.c | 44 +++-
15 files changed, 519 insertions(+), 20 deletions(-)
create mode 100644 CryptoPkg/Library/Include/arpa/inet.h
create mode 100644 CryptoPkg/Library/Include/arpa/nameser.h
create mode 100644 CryptoPkg/Library/Include/netinet/in.h
create mode 100644 CryptoPkg/Library/Include/sys/param.h
create mode 100644 CryptoPkg/Library/Include/sys/socket.h
create mode 100644 CryptoPkg/Library/BaseCryptLib/SysCall/inet_pton.c
--
2.19.1.3.g30247aa5d201
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 1/8] MdePkg/Include/Protocol/Tls.h: Add the data type of EfiTlsVerifyHost (CVE-2019-14553)
2019-10-26 5:37 [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553) Laszlo Ersek
@ 2019-10-26 5:37 ` Laszlo Ersek
2019-10-28 8:12 ` [edk2-devel] " Liming Gao
2019-10-26 5:37 ` [PATCH v2 2/8] CryptoPkg/TlsLib: Add the new API "TlsSetVerifyHost" (CVE-2019-14553) Laszlo Ersek
` (8 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Laszlo Ersek @ 2019-10-26 5:37 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: David Woodhouse, Jian J Wang, Jiaxin Wu, Sivaraman Nainar,
Xiaoyu Lu
From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=960
CVE: CVE-2019-14553
In the patch, we add the new data type named "EfiTlsVerifyHost" and
the EFI_TLS_VERIFY_HOST_FLAG for the TLS protocol consumer (HTTP)
to enable the host name check so as to avoid the potential
Man-In-The-Middle attack.
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Ye Ting <ting.ye@intel.com>
Reviewed-by: Long Qin <qin.long@intel.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20190927034441.3096-2-Jiaxin.wu@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- fix whitespace in subject line
- drop Contributed-under line per BZ#1373
MdePkg/Include/Protocol/Tls.h | 68 ++++++++++++++++----
1 file changed, 57 insertions(+), 11 deletions(-)
diff --git a/MdePkg/Include/Protocol/Tls.h b/MdePkg/Include/Protocol/Tls.h
index bf1b6727a1e9..af524ae2a60e 100644
--- a/MdePkg/Include/Protocol/Tls.h
+++ b/MdePkg/Include/Protocol/Tls.h
@@ -40,12 +40,8 @@ typedef struct _EFI_TLS_PROTOCOL EFI_TLS_PROTOCOL;
///
/// EFI_TLS_SESSION_DATA_TYPE
///
typedef enum {
- ///
- /// Session Configuration
- ///
-
///
/// TLS session Version. The corresponding Data is of type EFI_TLS_VERSION.
///
EfiTlsVersion,
@@ -85,13 +81,8 @@ typedef enum {
/// TLS session data session state.
/// The corresponding Data is of type EFI_TLS_SESSION_STATE.
///
EfiTlsSessionState,
-
- ///
- /// Session information
- ///
-
///
/// TLS session data client random.
/// The corresponding Data is of type EFI_TLS_RANDOM.
///
@@ -105,11 +96,17 @@ typedef enum {
/// TLS session data key material.
/// The corresponding Data is of type EFI_TLS_MASTER_SECRET.
///
EfiTlsKeyMaterial,
+ ///
+ /// TLS session hostname for validation which is used to verify whether the name
+ /// within the peer certificate matches a given host name.
+ /// This parameter is invalid when EfiTlsVerifyMethod is EFI_TLS_VERIFY_NONE.
+ /// The corresponding Data is of type EFI_TLS_VERIFY_HOST.
+ ///
+ EfiTlsVerifyHost,
EfiTlsSessionDataTypeMaximum
-
} EFI_TLS_SESSION_DATA_TYPE;
///
/// EFI_TLS_VERSION
@@ -177,17 +174,66 @@ typedef UINT32 EFI_TLS_VERIFY;
/// the reason for the certificate verification failure.
///
#define EFI_TLS_VERIFY_PEER 0x1
///
-/// TLS session will fail peer certificate is absent.
+/// EFI_TLS_VERIFY_FAIL_IF_NO_PEER_CERT is only meaningful in the server mode.
+/// TLS session will fail if client certificate is absent.
///
#define EFI_TLS_VERIFY_FAIL_IF_NO_PEER_CERT 0x2
///
/// TLS session only verify client once, and doesn't request certificate during
/// re-negotiation.
///
#define EFI_TLS_VERIFY_CLIENT_ONCE 0x4
+///
+/// EFI_TLS_VERIFY_HOST_FLAG
+///
+typedef UINT32 EFI_TLS_VERIFY_HOST_FLAG;
+///
+/// There is no additional flags set for hostname validation.
+/// Wildcards are supported and they match only in the left-most label.
+///
+#define EFI_TLS_VERIFY_FLAG_NONE 0x00
+///
+/// Always check the Subject Distinguished Name (DN) in the peer certificate even if the
+/// certificate contains Subject Alternative Name (SAN).
+///
+#define EFI_TLS_VERIFY_FLAG_ALWAYS_CHECK_SUBJECT 0x01
+///
+/// Disable the match of all wildcards.
+///
+#define EFI_TLS_VERIFY_FLAG_NO_WILDCARDS 0x02
+///
+/// Disable the "*" as wildcard in labels that have a prefix or suffix (e.g. "www*" or "*www").
+///
+#define EFI_TLS_VERIFY_FLAG_NO_PARTIAL_WILDCARDS 0x04
+///
+/// Allow the "*" to match more than one labels. Otherwise, only matches a single label.
+///
+#define EFI_TLS_VERIFY_FLAG_MULTI_LABEL_WILDCARDS 0x08
+///
+/// Restrict to only match direct child sub-domains which start with ".".
+/// For example, a name of ".example.com" would match "www.example.com" with this flag,
+/// but would not match "www.sub.example.com".
+///
+#define EFI_TLS_VERIFY_FLAG_SINGLE_LABEL_SUBDOMAINS 0x10
+///
+/// Never check the Subject Distinguished Name (DN) even there is no
+/// Subject Alternative Name (SAN) in the certificate.
+///
+#define EFI_TLS_VERIFY_FLAG_NEVER_CHECK_SUBJECT 0x20
+
+///
+/// EFI_TLS_VERIFY_HOST
+///
+#pragma pack (1)
+typedef struct {
+ EFI_TLS_VERIFY_HOST_FLAG Flags;
+ CHAR8 *HostName;
+} EFI_TLS_VERIFY_HOST;
+#pragma pack ()
+
///
/// EFI_TLS_RANDOM
/// Note: The definition of EFI_TLS_RANDOM is from "RFC 5246 A.4.1.
/// Hello Messages".
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 2/8] CryptoPkg/TlsLib: Add the new API "TlsSetVerifyHost" (CVE-2019-14553)
2019-10-26 5:37 [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553) Laszlo Ersek
2019-10-26 5:37 ` [PATCH v2 1/8] MdePkg/Include/Protocol/Tls.h: Add the data type of EfiTlsVerifyHost (CVE-2019-14553) Laszlo Ersek
@ 2019-10-26 5:37 ` Laszlo Ersek
2019-10-26 11:51 ` [edk2-devel] " Philippe Mathieu-Daudé
2019-10-28 5:28 ` Wang, Jian J
2019-10-26 5:37 ` [PATCH v2 3/8] CryptoPkg/Crt: turn strchr() into a function (CVE-2019-14553) Laszlo Ersek
` (7 subsequent siblings)
9 siblings, 2 replies; 26+ messages in thread
From: Laszlo Ersek @ 2019-10-26 5:37 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: David Woodhouse, Jian J Wang, Jiaxin Wu, Sivaraman Nainar,
Xiaoyu Lu
From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=960
CVE: CVE-2019-14553
In the patch, we add the new API "TlsSetVerifyHost" for the TLS
protocol to set the specified host name that need to be verified.
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Ye Ting <ting.ye@intel.com>
Reviewed-by: Long Qin <qin.long@intel.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20190927034441.3096-3-Jiaxin.wu@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- fix whitespace in subject line
- drop Contributed-under line per BZ#1373
CryptoPkg/Include/Library/TlsLib.h | 20 +++++++++++
CryptoPkg/Library/TlsLib/TlsConfig.c | 38 +++++++++++++++++++-
2 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/CryptoPkg/Include/Library/TlsLib.h b/CryptoPkg/Include/Library/TlsLib.h
index 9875cb6e746b..3af7d4bc095e 100644
--- a/CryptoPkg/Include/Library/TlsLib.h
+++ b/CryptoPkg/Include/Library/TlsLib.h
@@ -395,8 +395,28 @@ TlsSetVerify (
IN VOID *Tls,
IN UINT32 VerifyMode
);
+/**
+ Set the specified host name to be verified.
+
+ @param[in] Tls Pointer to the TLS object.
+ @param[in] Flags The setting flags during the validation.
+ @param[in] HostName The specified host name to be verified.
+
+ @retval EFI_SUCCESS The HostName setting was set successfully.
+ @retval EFI_INVALID_PARAMETER The parameter is invalid.
+ @retval EFI_ABORTED Invalid HostName setting.
+
+**/
+EFI_STATUS
+EFIAPI
+TlsSetVerifyHost (
+ IN VOID *Tls,
+ IN UINT32 Flags,
+ IN CHAR8 *HostName
+ );
+
/**
Sets a TLS/SSL session ID to be used during TLS/SSL connect.
This function sets a session ID to be used when the TLS/SSL connection is
diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c b/CryptoPkg/Library/TlsLib/TlsConfig.c
index 74b577d60ee3..2bf5aee7c093 100644
--- a/CryptoPkg/Library/TlsLib/TlsConfig.c
+++ b/CryptoPkg/Library/TlsLib/TlsConfig.c
@@ -1,8 +1,8 @@
/** @file
SSL/TLS Configuration Library Wrapper Implementation over OpenSSL.
-Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -496,8 +496,44 @@ TlsSetVerify (
//
SSL_set_verify (TlsConn->Ssl, VerifyMode, NULL);
}
+/**
+ Set the specified host name to be verified.
+
+ @param[in] Tls Pointer to the TLS object.
+ @param[in] Flags The setting flags during the validation.
+ @param[in] HostName The specified host name to be verified.
+
+ @retval EFI_SUCCESS The HostName setting was set successfully.
+ @retval EFI_INVALID_PARAMETER The parameter is invalid.
+ @retval EFI_ABORTED Invalid HostName setting.
+
+**/
+EFI_STATUS
+EFIAPI
+TlsSetVerifyHost (
+ IN VOID *Tls,
+ IN UINT32 Flags,
+ IN CHAR8 *HostName
+ )
+{
+ TLS_CONNECTION *TlsConn;
+
+ TlsConn = (TLS_CONNECTION *) Tls;
+ if (TlsConn == NULL || TlsConn->Ssl == NULL || HostName == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ SSL_set_hostflags(TlsConn->Ssl, Flags);
+
+ if (SSL_set1_host(TlsConn->Ssl, HostName) == 0) {
+ return EFI_ABORTED;
+ }
+
+ return EFI_SUCCESS;
+}
+
/**
Sets a TLS/SSL session ID to be used during TLS/SSL connect.
This function sets a session ID to be used when the TLS/SSL connection is
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 3/8] CryptoPkg/Crt: turn strchr() into a function (CVE-2019-14553)
2019-10-26 5:37 [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553) Laszlo Ersek
2019-10-26 5:37 ` [PATCH v2 1/8] MdePkg/Include/Protocol/Tls.h: Add the data type of EfiTlsVerifyHost (CVE-2019-14553) Laszlo Ersek
2019-10-26 5:37 ` [PATCH v2 2/8] CryptoPkg/TlsLib: Add the new API "TlsSetVerifyHost" (CVE-2019-14553) Laszlo Ersek
@ 2019-10-26 5:37 ` Laszlo Ersek
2019-10-26 11:47 ` [edk2-devel] " Philippe Mathieu-Daudé
2019-10-28 5:12 ` Wang, Jian J
2019-10-26 5:37 ` [PATCH v2 4/8] CryptoPkg/Crt: satisfy "inet_pton.c" dependencies (CVE-2019-14553) Laszlo Ersek
` (6 subsequent siblings)
9 siblings, 2 replies; 26+ messages in thread
From: Laszlo Ersek @ 2019-10-26 5:37 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: David Woodhouse, Jian J Wang, Jiaxin Wu, Sivaraman Nainar,
Xiaoyu Lu
According to the ISO C standard, strchr() is a function. We #define it as
a macro. Unfortunately, our macro evaluates the first argument ("str")
twice. If the expression passed for "str" has side effects, the behavior
may be undefined.
In a later patch in this series, we're going to resurrect "inet_pton.c"
(originally from the StdLib package), which calls strchr() just like that:
strchr((xdigits = xdigits_l), ch)
strchr((xdigits = xdigits_u), ch)
To enable this kind of function call, turn strchr() into a function.
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
CVE: CVE-2019-14553
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- new patch
CryptoPkg/Library/Include/CrtLibSupport.h | 2 +-
CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c | 5 +++++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h b/CryptoPkg/Library/Include/CrtLibSupport.h
index 5806f50f7485..b90da20ff7e7 100644
--- a/CryptoPkg/Library/Include/CrtLibSupport.h
+++ b/CryptoPkg/Library/Include/CrtLibSupport.h
@@ -146,8 +146,9 @@ int isalnum (int);
int isupper (int);
int tolower (int);
int strcmp (const char *, const char *);
int strncasecmp (const char *, const char *, size_t);
+char *strchr (const char *, int);
char *strrchr (const char *, int);
unsigned long strtoul (const char *, char **, int);
long strtol (const char *, char **, int);
char *strerror (int);
@@ -187,9 +188,8 @@ void abort (void);
#define strlen(str) (size_t)(AsciiStrnLenS(str,MAX_STRING_SIZE))
#define strcpy(strDest,strSource) AsciiStrCpyS(strDest,MAX_STRING_SIZE,strSource)
#define strncpy(strDest,strSource,count) AsciiStrnCpyS(strDest,MAX_STRING_SIZE,strSource,(UINTN)count)
#define strcat(strDest,strSource) AsciiStrCatS(strDest,MAX_STRING_SIZE,strSource)
-#define strchr(str,ch) ScanMem8((VOID *)(str),AsciiStrSize(str),(UINT8)ch)
#define strncmp(string1,string2,count) (int)(AsciiStrnCmp(string1,string2,(UINTN)(count)))
#define strcasecmp(str1,str2) (int)AsciiStriCmp(str1,str2)
#define sprintf(buf,...) AsciiSPrint(buf,MAX_STRING_SIZE,__VA_ARGS__)
#define localtime(timer) NULL
diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
index 71a2ef34ed2b..42235ab96ac3 100644
--- a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
+++ b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
@@ -114,8 +114,13 @@ QuickSortWorker (
//
// -- String Manipulation Routines --
//
+char *strchr(const char *str, int ch)
+{
+ return ScanMem8 (str, AsciiStrSize (str), (UINT8)ch);
+}
+
/* Scan a string for the last occurrence of a character */
char *strrchr (const char *str, int c)
{
char * save;
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 4/8] CryptoPkg/Crt: satisfy "inet_pton.c" dependencies (CVE-2019-14553)
2019-10-26 5:37 [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553) Laszlo Ersek
` (2 preceding siblings ...)
2019-10-26 5:37 ` [PATCH v2 3/8] CryptoPkg/Crt: turn strchr() into a function (CVE-2019-14553) Laszlo Ersek
@ 2019-10-26 5:37 ` Laszlo Ersek
2019-10-28 5:34 ` Wang, Jian J
2019-10-28 13:06 ` David Woodhouse
2019-10-26 5:37 ` [PATCH v2 5/8] CryptoPkg/Crt: import "inet_pton.c" (CVE-2019-14553) Laszlo Ersek
` (5 subsequent siblings)
9 siblings, 2 replies; 26+ messages in thread
From: Laszlo Ersek @ 2019-10-26 5:37 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: David Woodhouse, Jian J Wang, Jiaxin Wu, Sivaraman Nainar,
Xiaoyu Lu
In a later patch in this series, we're going to resurrect "inet_pton.c"
(originally from the StdLib package). That source file has a number of
standard C and BSD socket dependencies. Provide those dependencies here:
- The header files below will simply #include <CrtLibSupport.h>:
- arpa/inet.h
- arpa/nameser.h
- netinet/in.h
- sys/param.h
- sys/socket.h
- EAFNOSUPPORT comes from "StdLib/Include/errno.h", at commit
e2d3a25f1a31; which is the commit immediately preceding the removal of
StdLib from edk2 (964f432b9b0a).
Note that the other error macro, which we alread #define, namely EINVAL,
has a value (22) that also matches "StdLib/Include/errno.h".
- The AF_INET and AF_INET6 address family macros come from
"StdLib/Include/sys/socket.h".
- The NS_INT16SZ, NS_INADDRSZ and NS_IN6ADDRSZ macros come from
"StdLib/Include/arpa/nameser.h".
- The "u_int" and "u_char" types come from "StdLib/Include/sys/types.h".
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
CVE: CVE-2019-14553
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- new patch
CryptoPkg/Library/Include/CrtLibSupport.h | 16 ++++++++++++++++
CryptoPkg/Library/Include/arpa/inet.h | 9 +++++++++
CryptoPkg/Library/Include/arpa/nameser.h | 9 +++++++++
CryptoPkg/Library/Include/netinet/in.h | 9 +++++++++
CryptoPkg/Library/Include/sys/param.h | 9 +++++++++
CryptoPkg/Library/Include/sys/socket.h | 9 +++++++++
6 files changed, 61 insertions(+)
diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h b/CryptoPkg/Library/Include/CrtLibSupport.h
index b90da20ff7e7..e603fad763f9 100644
--- a/CryptoPkg/Library/Include/CrtLibSupport.h
+++ b/CryptoPkg/Library/Include/CrtLibSupport.h
@@ -73,22 +73,38 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
//
// Definitions for global constants used by CRT library routines
//
#define EINVAL 22 /* Invalid argument */
+#define EAFNOSUPPORT 47 /* Address family not supported by protocol family */
#define INT_MAX 0x7FFFFFFF /* Maximum (signed) int value */
#define LONG_MAX 0X7FFFFFFFL /* max value for a long */
#define LONG_MIN (-LONG_MAX-1) /* min value for a long */
#define ULONG_MAX 0xFFFFFFFF /* Maximum unsigned long value */
#define CHAR_BIT 8 /* Number of bits in a char */
+//
+// Address families.
+//
+#define AF_INET 2 /* internetwork: UDP, TCP, etc. */
+#define AF_INET6 24 /* IP version 6 */
+
+//
+// Define constants based on RFC0883, RFC1034, RFC 1035
+//
+#define NS_INT16SZ 2 /*%< #/bytes of data in a u_int16_t */
+#define NS_INADDRSZ 4 /*%< IPv4 T_A */
+#define NS_IN6ADDRSZ 16 /*%< IPv6 T_AAAA */
+
//
// Basic types mapping
//
typedef UINTN size_t;
+typedef UINTN u_int;
typedef INTN ssize_t;
typedef INT32 time_t;
typedef UINT8 __uint8_t;
typedef UINT8 sa_family_t;
+typedef UINT8 u_char;
typedef UINT32 uid_t;
typedef UINT32 gid_t;
//
diff --git a/CryptoPkg/Library/Include/arpa/inet.h b/CryptoPkg/Library/Include/arpa/inet.h
new file mode 100644
index 000000000000..988e4e0a73e3
--- /dev/null
+++ b/CryptoPkg/Library/Include/arpa/inet.h
@@ -0,0 +1,9 @@
+/** @file
+ Include file to support building third-party standard C / BSD sockets code.
+
+ Copyright (C) 2019, Red Hat, Inc.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <CrtLibSupport.h>
diff --git a/CryptoPkg/Library/Include/arpa/nameser.h b/CryptoPkg/Library/Include/arpa/nameser.h
new file mode 100644
index 000000000000..988e4e0a73e3
--- /dev/null
+++ b/CryptoPkg/Library/Include/arpa/nameser.h
@@ -0,0 +1,9 @@
+/** @file
+ Include file to support building third-party standard C / BSD sockets code.
+
+ Copyright (C) 2019, Red Hat, Inc.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <CrtLibSupport.h>
diff --git a/CryptoPkg/Library/Include/netinet/in.h b/CryptoPkg/Library/Include/netinet/in.h
new file mode 100644
index 000000000000..988e4e0a73e3
--- /dev/null
+++ b/CryptoPkg/Library/Include/netinet/in.h
@@ -0,0 +1,9 @@
+/** @file
+ Include file to support building third-party standard C / BSD sockets code.
+
+ Copyright (C) 2019, Red Hat, Inc.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <CrtLibSupport.h>
diff --git a/CryptoPkg/Library/Include/sys/param.h b/CryptoPkg/Library/Include/sys/param.h
new file mode 100644
index 000000000000..988e4e0a73e3
--- /dev/null
+++ b/CryptoPkg/Library/Include/sys/param.h
@@ -0,0 +1,9 @@
+/** @file
+ Include file to support building third-party standard C / BSD sockets code.
+
+ Copyright (C) 2019, Red Hat, Inc.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <CrtLibSupport.h>
diff --git a/CryptoPkg/Library/Include/sys/socket.h b/CryptoPkg/Library/Include/sys/socket.h
new file mode 100644
index 000000000000..988e4e0a73e3
--- /dev/null
+++ b/CryptoPkg/Library/Include/sys/socket.h
@@ -0,0 +1,9 @@
+/** @file
+ Include file to support building third-party standard C / BSD sockets code.
+
+ Copyright (C) 2019, Red Hat, Inc.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <CrtLibSupport.h>
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 5/8] CryptoPkg/Crt: import "inet_pton.c" (CVE-2019-14553)
2019-10-26 5:37 [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553) Laszlo Ersek
` (3 preceding siblings ...)
2019-10-26 5:37 ` [PATCH v2 4/8] CryptoPkg/Crt: satisfy "inet_pton.c" dependencies (CVE-2019-14553) Laszlo Ersek
@ 2019-10-26 5:37 ` Laszlo Ersek
2019-10-28 6:16 ` Wang, Jian J
2019-10-26 5:37 ` [PATCH v2 6/8] CryptoPkg/TlsLib: TlsSetVerifyHost: parse IP address literals as such (CVE-2019-14553) Laszlo Ersek
` (4 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Laszlo Ersek @ 2019-10-26 5:37 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: David Woodhouse, Jian J Wang, Jiaxin Wu, Sivaraman Nainar,
Xiaoyu Lu
For TianoCore BZ#1734, StdLib has been moved from the edk2 project to the
edk2-libc project, in commit 964f432b9b0a ("edk2: Remove AppPkg, StdLib,
StdLibPrivateInternalFiles", 2019-04-29).
We'd like to use the inet_pton() function in CryptoPkg. Resurrect the
"inet_pton.c" file from just before the StdLib removal, as follows:
$ git show \
964f432b9b0a^:StdLib/BsdSocketLib/inet_pton.c \
> CryptoPkg/Library/BaseCryptLib/SysCall/inet_pton.c
The inet_pton() function is only intended for the DXE phase at this time,
therefore only the "BaseCryptLib" instance INF file receives the new file.
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
CVE: CVE-2019-14553
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- new patch
CryptoPkg/Library/Include/CrtLibSupport.h | 1 +
CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf | 1 +
CryptoPkg/Library/BaseCryptLib/SysCall/inet_pton.c | 257 ++++++++++++++++++++
3 files changed, 259 insertions(+)
diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h b/CryptoPkg/Library/Include/CrtLibSupport.h
index e603fad763f9..5a20ba636fff 100644
--- a/CryptoPkg/Library/Include/CrtLibSupport.h
+++ b/CryptoPkg/Library/Include/CrtLibSupport.h
@@ -191,8 +191,9 @@ char *secure_getenv (const char *);
void abort (void) __attribute__((__noreturn__));
#else
void abort (void);
#endif
+int inet_pton (int, const char *, void *);
//
// Macros that directly map functions to BaseLib, BaseMemoryLib, and DebugLib functions
//
diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
index a98be2cd9590..dc9e6e5d45f9 100644
--- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -57,8 +57,9 @@ [Sources]
SysCall/CrtWrapper.c
SysCall/TimerWrapper.c
SysCall/BaseMemAllocation.c
+ SysCall/inet_pton.c
[Sources.Ia32]
Rand/CryptRandTsc.c
diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/inet_pton.c b/CryptoPkg/Library/BaseCryptLib/SysCall/inet_pton.c
new file mode 100644
index 000000000000..32e1ab8690e6
--- /dev/null
+++ b/CryptoPkg/Library/BaseCryptLib/SysCall/inet_pton.c
@@ -0,0 +1,257 @@
+/* Copyright (c) 1996 by Internet Software Consortium.
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SOFTWARE CONSORTIUM DISCLAIMS
+ * ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL INTERNET SOFTWARE
+ * CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL
+ * DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR
+ * PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS
+ * ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS
+ * SOFTWARE.
+ */
+
+/*
+ * Portions copyright (c) 1999, 2000
+ * Intel Corporation.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * 3. All advertising materials mentioning features or use of this software
+ * must display the following acknowledgement:
+ *
+ * This product includes software developed by Intel Corporation and
+ * its contributors.
+ *
+ * 4. Neither the name of Intel Corporation or its contributors may be
+ * used to endorse or promote products derived from this software
+ * without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY INTEL CORPORATION AND CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL INTEL CORPORATION OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#if defined(LIBC_SCCS) && !defined(lint)
+static char rcsid[] = "$Id: inet_pton.c,v 1.1.1.1 2003/11/19 01:51:30 kyu3 Exp $";
+#endif /* LIBC_SCCS and not lint */
+
+#include <sys/param.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <arpa/nameser.h>
+#include <string.h>
+#include <errno.h>
+
+/*
+ * WARNING: Don't even consider trying to compile this on a system where
+ * sizeof(int) < 4. sizeof(int) > 4 is fine; all the world's not a VAX.
+ */
+
+static int inet_pton4 (const char *src, u_char *dst);
+static int inet_pton6 (const char *src, u_char *dst);
+
+/* int
+ * inet_pton(af, src, dst)
+ * convert from presentation format (which usually means ASCII printable)
+ * to network format (which is usually some kind of binary format).
+ * return:
+ * 1 if the address was valid for the specified address family
+ * 0 if the address wasn't valid (`dst' is untouched in this case)
+ * -1 if some other error occurred (`dst' is untouched in this case, too)
+ * author:
+ * Paul Vixie, 1996.
+ */
+int
+inet_pton(
+ int af,
+ const char *src,
+ void *dst
+ )
+{
+ switch (af) {
+ case AF_INET:
+ return (inet_pton4(src, dst));
+ case AF_INET6:
+ return (inet_pton6(src, dst));
+ default:
+ errno = EAFNOSUPPORT;
+ return (-1);
+ }
+ /* NOTREACHED */
+}
+
+/* int
+ * inet_pton4(src, dst)
+ * like inet_aton() but without all the hexadecimal and shorthand.
+ * return:
+ * 1 if `src' is a valid dotted quad, else 0.
+ * notice:
+ * does not touch `dst' unless it's returning 1.
+ * author:
+ * Paul Vixie, 1996.
+ */
+static int
+inet_pton4(
+ const char *src,
+ u_char *dst
+ )
+{
+ static const char digits[] = "0123456789";
+ int saw_digit, octets, ch;
+ u_char tmp[NS_INADDRSZ], *tp;
+
+ saw_digit = 0;
+ octets = 0;
+ *(tp = tmp) = 0;
+ while ((ch = *src++) != '\0') {
+ const char *pch;
+
+ if ((pch = strchr(digits, ch)) != NULL) {
+ u_int new = *tp * 10 + (u_int)(pch - digits);
+
+ if (new > 255)
+ return (0);
+ *tp = (u_char)new;
+ if (! saw_digit) {
+ if (++octets > 4)
+ return (0);
+ saw_digit = 1;
+ }
+ } else if (ch == '.' && saw_digit) {
+ if (octets == 4)
+ return (0);
+ *++tp = 0;
+ saw_digit = 0;
+ } else
+ return (0);
+ }
+ if (octets < 4)
+ return (0);
+
+ memcpy(dst, tmp, NS_INADDRSZ);
+ return (1);
+}
+
+/* int
+ * inet_pton6(src, dst)
+ * convert presentation level address to network order binary form.
+ * return:
+ * 1 if `src' is a valid [RFC1884 2.2] address, else 0.
+ * notice:
+ * (1) does not touch `dst' unless it's returning 1.
+ * (2) :: in a full address is silently ignored.
+ * credit:
+ * inspired by Mark Andrews.
+ * author:
+ * Paul Vixie, 1996.
+ */
+static int
+inet_pton6(
+ const char *src,
+ u_char *dst
+ )
+{
+ static const char xdigits_l[] = "0123456789abcdef",
+ xdigits_u[] = "0123456789ABCDEF";
+ u_char tmp[NS_IN6ADDRSZ], *tp, *endp, *colonp;
+ const char *xdigits, *curtok;
+ int ch, saw_xdigit;
+ u_int val;
+
+ memset((tp = tmp), '\0', NS_IN6ADDRSZ);
+ endp = tp + NS_IN6ADDRSZ;
+ colonp = NULL;
+ /* Leading :: requires some special handling. */
+ if (*src == ':')
+ if (*++src != ':')
+ return (0);
+ curtok = src;
+ saw_xdigit = 0;
+ val = 0;
+ while ((ch = *src++) != '\0') {
+ const char *pch;
+
+ if ((pch = strchr((xdigits = xdigits_l), ch)) == NULL)
+ pch = strchr((xdigits = xdigits_u), ch);
+ if (pch != NULL) {
+ val <<= 4;
+ val |= (pch - xdigits);
+ if (val > 0xffff)
+ return (0);
+ saw_xdigit = 1;
+ continue;
+ }
+ if (ch == ':') {
+ curtok = src;
+ if (!saw_xdigit) {
+ if (colonp)
+ return (0);
+ colonp = tp;
+ continue;
+ }
+ if (tp + NS_INT16SZ > endp)
+ return (0);
+ *tp++ = (u_char) (val >> 8) & 0xff;
+ *tp++ = (u_char) val & 0xff;
+ saw_xdigit = 0;
+ val = 0;
+ continue;
+ }
+ if (ch == '.' && ((tp + NS_INADDRSZ) <= endp) &&
+ inet_pton4(curtok, tp) > 0) {
+ tp += NS_INADDRSZ;
+ saw_xdigit = 0;
+ break; /* '\0' was seen by inet_pton4(). */
+ }
+ return (0);
+ }
+ if (saw_xdigit) {
+ if (tp + NS_INT16SZ > endp)
+ return (0);
+ *tp++ = (u_char) (val >> 8) & 0xff;
+ *tp++ = (u_char) val & 0xff;
+ }
+ if (colonp != NULL) {
+ /*
+ * Since some memmove()'s erroneously fail to handle
+ * overlapping regions, we'll do the shift by hand.
+ */
+ const int n = (int)(tp - colonp);
+ int i;
+
+ for (i = 1; i <= n; i++) {
+ endp[- i] = colonp[n - i];
+ colonp[n - i] = 0;
+ }
+ tp = endp;
+ }
+ if (tp != endp)
+ return (0);
+ memcpy(dst, tmp, NS_IN6ADDRSZ);
+ return (1);
+}
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 6/8] CryptoPkg/TlsLib: TlsSetVerifyHost: parse IP address literals as such (CVE-2019-14553)
2019-10-26 5:37 [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553) Laszlo Ersek
` (4 preceding siblings ...)
2019-10-26 5:37 ` [PATCH v2 5/8] CryptoPkg/Crt: import "inet_pton.c" (CVE-2019-14553) Laszlo Ersek
@ 2019-10-26 5:37 ` Laszlo Ersek
2019-10-28 6:12 ` Wang, Jian J
2019-10-26 5:37 ` [PATCH v2 7/8] NetworkPkg/TlsDxe: Add the support of host validation to TlsDxe driver (CVE-2019-14553) Laszlo Ersek
` (3 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Laszlo Ersek @ 2019-10-26 5:37 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: David Woodhouse, Jian J Wang, Jiaxin Wu, Sivaraman Nainar,
Xiaoyu Lu
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 <dwmw2@infradead.org>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
CVE: CVE-2019-14553
Suggested-by: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- new patch
CryptoPkg/Library/TlsLib/TlsConfig.c | 28 +++++++++++++++++---
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c b/CryptoPkg/Library/TlsLib/TlsConfig.c
index 2bf5aee7c093..307eb57896dc 100644
--- a/CryptoPkg/Library/TlsLib/TlsConfig.c
+++ b/CryptoPkg/Library/TlsLib/TlsConfig.c
@@ -516,22 +516,42 @@ TlsSetVerifyHost (
IN UINT32 Flags,
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) {
return EFI_INVALID_PARAMETER;
}
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;
+ }
+
+ 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 EFI_SUCCESS;
+ return (ParamStatus == 1) ? EFI_SUCCESS : EFI_ABORTED;
}
/**
Sets a TLS/SSL session ID to be used during TLS/SSL connect.
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 7/8] NetworkPkg/TlsDxe: Add the support of host validation to TlsDxe driver (CVE-2019-14553)
2019-10-26 5:37 [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553) Laszlo Ersek
` (5 preceding siblings ...)
2019-10-26 5:37 ` [PATCH v2 6/8] CryptoPkg/TlsLib: TlsSetVerifyHost: parse IP address literals as such (CVE-2019-14553) Laszlo Ersek
@ 2019-10-26 5:37 ` Laszlo Ersek
2019-10-26 5:37 ` [PATCH v2 8/8] NetworkPkg/HttpDxe: Set the HostName for the verification (CVE-2019-14553) Laszlo Ersek
` (2 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Laszlo Ersek @ 2019-10-26 5:37 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: David Woodhouse, Jian J Wang, Jiaxin Wu, Sivaraman Nainar,
Xiaoyu Lu
From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=960
CVE: CVE-2019-14553
The new data type named "EfiTlsVerifyHost" and the
EFI_TLS_VERIFY_HOST_FLAG are supported in TLS protocol.
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Ye Ting <ting.ye@intel.com>
Reviewed-by: Long Qin <qin.long@intel.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20190927034441.3096-4-Jiaxin.wu@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- fix whitespace in subject line
- drop Contributed-under line per BZ#1373
NetworkPkg/TlsDxe/TlsProtocol.c | 44 ++++++++++++++++++--
1 file changed, 41 insertions(+), 3 deletions(-)
diff --git a/NetworkPkg/TlsDxe/TlsProtocol.c b/NetworkPkg/TlsDxe/TlsProtocol.c
index a7a993fc6fc5..001e5400d00f 100644
--- a/NetworkPkg/TlsDxe/TlsProtocol.c
+++ b/NetworkPkg/TlsDxe/TlsProtocol.c
@@ -1,8 +1,8 @@
/** @file
Implementation of EFI TLS Protocol Interfaces.
- Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -55,14 +55,18 @@ TlsSetSessionData (
TLS_INSTANCE *Instance;
UINT16 *CipherId;
CONST EFI_TLS_CIPHER *TlsCipherList;
UINTN CipherCount;
+ CONST EFI_TLS_VERIFY_HOST *TlsVerifyHost;
+ EFI_TLS_VERIFY VerifyMethod;
+ UINTN VerifyMethodSize;
UINTN Index;
EFI_TPL OldTpl;
- Status = EFI_SUCCESS;
- CipherId = NULL;
+ Status = EFI_SUCCESS;
+ CipherId = NULL;
+ VerifyMethodSize = sizeof (EFI_TLS_VERIFY);
if (This == NULL || Data == NULL || DataSize == 0) {
return EFI_INVALID_PARAMETER;
}
@@ -147,8 +151,42 @@ TlsSetSessionData (
goto ON_EXIT;
}
TlsSetVerify (Instance->TlsConn, *((UINT32 *) Data));
+ break;
+ case EfiTlsVerifyHost:
+ if (DataSize != sizeof (EFI_TLS_VERIFY_HOST)) {
+ Status = EFI_INVALID_PARAMETER;
+ goto ON_EXIT;
+ }
+
+ TlsVerifyHost = (CONST EFI_TLS_VERIFY_HOST *) Data;
+
+ if ((TlsVerifyHost->Flags & EFI_TLS_VERIFY_FLAG_ALWAYS_CHECK_SUBJECT) != 0 &&
+ (TlsVerifyHost->Flags & EFI_TLS_VERIFY_FLAG_NEVER_CHECK_SUBJECT) != 0) {
+ Status = EFI_INVALID_PARAMETER;
+ goto ON_EXIT;
+ }
+
+ if ((TlsVerifyHost->Flags & EFI_TLS_VERIFY_FLAG_NO_WILDCARDS) != 0 &&
+ ((TlsVerifyHost->Flags & EFI_TLS_VERIFY_FLAG_NO_PARTIAL_WILDCARDS) != 0 ||
+ (TlsVerifyHost->Flags & EFI_TLS_VERIFY_FLAG_MULTI_LABEL_WILDCARDS) != 0)) {
+ Status = EFI_INVALID_PARAMETER;
+ goto ON_EXIT;
+ }
+
+ Status = This->GetSessionData (This, EfiTlsVerifyMethod, &VerifyMethod, &VerifyMethodSize);
+ if (EFI_ERROR (Status)) {
+ goto ON_EXIT;
+ }
+
+ if ((VerifyMethod & EFI_TLS_VERIFY_PEER) == 0) {
+ Status = EFI_INVALID_PARAMETER;
+ goto ON_EXIT;
+ }
+
+ Status = TlsSetVerifyHost (Instance->TlsConn, TlsVerifyHost->Flags, TlsVerifyHost->HostName);
+
break;
case EfiTlsSessionID:
if (DataSize != sizeof (EFI_TLS_SESSION_ID)) {
Status = EFI_INVALID_PARAMETER;
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 8/8] NetworkPkg/HttpDxe: Set the HostName for the verification (CVE-2019-14553)
2019-10-26 5:37 [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553) Laszlo Ersek
` (6 preceding siblings ...)
2019-10-26 5:37 ` [PATCH v2 7/8] NetworkPkg/TlsDxe: Add the support of host validation to TlsDxe driver (CVE-2019-14553) Laszlo Ersek
@ 2019-10-26 5:37 ` Laszlo Ersek
2019-10-29 2:37 ` [edk2-devel] [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553) Wu, Jiaxin
2019-10-31 9:28 ` Laszlo Ersek
9 siblings, 0 replies; 26+ messages in thread
From: Laszlo Ersek @ 2019-10-26 5:37 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: David Woodhouse, Jian J Wang, Jiaxin Wu, Sivaraman Nainar,
Xiaoyu Lu
From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=960
CVE: CVE-2019-14553
Set the HostName by consuming TLS protocol to enable the host name
check so as to avoid the potential Man-In-The-Middle attack.
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Ye Ting <ting.ye@intel.com>
Reviewed-by: Long Qin <qin.long@intel.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20190927034441.3096-5-Jiaxin.wu@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- fix whitespace in subject line
- drop Contributed-under line per BZ#1373
NetworkPkg/HttpDxe/HttpProto.h | 1 +
NetworkPkg/HttpDxe/HttpsSupport.c | 21 ++++++++++++++++----
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/NetworkPkg/HttpDxe/HttpProto.h b/NetworkPkg/HttpDxe/HttpProto.h
index 6e1f51748a73..34308e016d3e 100644
--- a/NetworkPkg/HttpDxe/HttpProto.h
+++ b/NetworkPkg/HttpDxe/HttpProto.h
@@ -81,8 +81,9 @@ typedef struct {
typedef struct {
EFI_TLS_VERSION Version;
EFI_TLS_CONNECTION_END ConnectionEnd;
EFI_TLS_VERIFY VerifyMethod;
+ EFI_TLS_VERIFY_HOST VerifyHost;
EFI_TLS_SESSION_STATE SessionState;
} TLS_CONFIG_DATA;
//
diff --git a/NetworkPkg/HttpDxe/HttpsSupport.c b/NetworkPkg/HttpDxe/HttpsSupport.c
index 988bbcbce7d8..5dfb13bd6021 100644
--- a/NetworkPkg/HttpDxe/HttpsSupport.c
+++ b/NetworkPkg/HttpDxe/HttpsSupport.c
@@ -622,15 +622,18 @@ TlsConfigureSession (
//
// TlsConfigData initialization
//
- HttpInstance->TlsConfigData.ConnectionEnd = EfiTlsClient;
- HttpInstance->TlsConfigData.VerifyMethod = EFI_TLS_VERIFY_PEER;
- HttpInstance->TlsConfigData.SessionState = EfiTlsSessionNotStarted;
+ HttpInstance->TlsConfigData.ConnectionEnd = EfiTlsClient;
+ HttpInstance->TlsConfigData.VerifyMethod = EFI_TLS_VERIFY_PEER;
+ HttpInstance->TlsConfigData.VerifyHost.Flags = EFI_TLS_VERIFY_FLAG_NO_WILDCARDS;
+ HttpInstance->TlsConfigData.VerifyHost.HostName = HttpInstance->RemoteHost;
+ HttpInstance->TlsConfigData.SessionState = EfiTlsSessionNotStarted;
//
// EfiTlsConnectionEnd,
- // EfiTlsVerifyMethod
+ // EfiTlsVerifyMethod,
+ // EfiTlsVerifyHost,
// EfiTlsSessionState
//
Status = HttpInstance->Tls->SetSessionData (
HttpInstance->Tls,
@@ -651,8 +654,18 @@ TlsConfigureSession (
if (EFI_ERROR (Status)) {
return Status;
}
+ Status = HttpInstance->Tls->SetSessionData (
+ HttpInstance->Tls,
+ EfiTlsVerifyHost,
+ &HttpInstance->TlsConfigData.VerifyHost,
+ sizeof (EFI_TLS_VERIFY_HOST)
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
Status = HttpInstance->Tls->SetSessionData (
HttpInstance->Tls,
EfiTlsSessionState,
&(HttpInstance->TlsConfigData.SessionState),
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [edk2-devel] [PATCH v2 3/8] CryptoPkg/Crt: turn strchr() into a function (CVE-2019-14553)
2019-10-26 5:37 ` [PATCH v2 3/8] CryptoPkg/Crt: turn strchr() into a function (CVE-2019-14553) Laszlo Ersek
@ 2019-10-26 11:47 ` Philippe Mathieu-Daudé
2019-10-28 5:12 ` Wang, Jian J
1 sibling, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-10-26 11:47 UTC (permalink / raw)
To: devel, lersek
Cc: David Woodhouse, Jian J Wang, Jiaxin Wu, Sivaraman Nainar,
Xiaoyu Lu
On 10/26/19 7:37 AM, Laszlo Ersek wrote:
> According to the ISO C standard, strchr() is a function. We #define it as
> a macro. Unfortunately, our macro evaluates the first argument ("str")
> twice. If the expression passed for "str" has side effects, the behavior
> may be undefined.
>
> In a later patch in this series, we're going to resurrect "inet_pton.c"
> (originally from the StdLib package), which calls strchr() just like that:
>
> strchr((xdigits = xdigits_l), ch)
> strchr((xdigits = xdigits_u), ch)
>
> To enable this kind of function call, turn strchr() into a function.
>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
> CVE: CVE-2019-14553
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> v2:
> - new patch
>
> CryptoPkg/Library/Include/CrtLibSupport.h | 2 +-
> CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c | 5 +++++
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h b/CryptoPkg/Library/Include/CrtLibSupport.h
> index 5806f50f7485..b90da20ff7e7 100644
> --- a/CryptoPkg/Library/Include/CrtLibSupport.h
> +++ b/CryptoPkg/Library/Include/CrtLibSupport.h
> @@ -146,8 +146,9 @@ int isalnum (int);
> int isupper (int);
> int tolower (int);
> int strcmp (const char *, const char *);
> int strncasecmp (const char *, const char *, size_t);
> +char *strchr (const char *, int);
> char *strrchr (const char *, int);
> unsigned long strtoul (const char *, char **, int);
> long strtol (const char *, char **, int);
> char *strerror (int);
> @@ -187,9 +188,8 @@ void abort (void);
> #define strlen(str) (size_t)(AsciiStrnLenS(str,MAX_STRING_SIZE))
> #define strcpy(strDest,strSource) AsciiStrCpyS(strDest,MAX_STRING_SIZE,strSource)
> #define strncpy(strDest,strSource,count) AsciiStrnCpyS(strDest,MAX_STRING_SIZE,strSource,(UINTN)count)
> #define strcat(strDest,strSource) AsciiStrCatS(strDest,MAX_STRING_SIZE,strSource)
> -#define strchr(str,ch) ScanMem8((VOID *)(str),AsciiStrSize(str),(UINT8)ch)
> #define strncmp(string1,string2,count) (int)(AsciiStrnCmp(string1,string2,(UINTN)(count)))
> #define strcasecmp(str1,str2) (int)AsciiStriCmp(str1,str2)
> #define sprintf(buf,...) AsciiSPrint(buf,MAX_STRING_SIZE,__VA_ARGS__)
> #define localtime(timer) NULL
> diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> index 71a2ef34ed2b..42235ab96ac3 100644
> --- a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> +++ b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> @@ -114,8 +114,13 @@ QuickSortWorker (
> //
> // -- String Manipulation Routines --
> //
>
> +char *strchr(const char *str, int ch)
> +{
> + return ScanMem8 (str, AsciiStrSize (str), (UINT8)ch);
> +}
> +
> /* Scan a string for the last occurrence of a character */
> char *strrchr (const char *str, int c)
> {
> char * save;
>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/8] CryptoPkg/TlsLib: Add the new API "TlsSetVerifyHost" (CVE-2019-14553)
2019-10-26 5:37 ` [PATCH v2 2/8] CryptoPkg/TlsLib: Add the new API "TlsSetVerifyHost" (CVE-2019-14553) Laszlo Ersek
@ 2019-10-26 11:51 ` Philippe Mathieu-Daudé
2019-11-02 11:01 ` Laszlo Ersek
2019-10-28 5:28 ` Wang, Jian J
1 sibling, 1 reply; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-10-26 11:51 UTC (permalink / raw)
To: devel, lersek
Cc: David Woodhouse, Jian J Wang, Jiaxin Wu, Sivaraman Nainar,
Xiaoyu Lu
On 10/26/19 7:37 AM, Laszlo Ersek wrote:
> From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=960
> CVE: CVE-2019-14553
> In the patch, we add the new API "TlsSetVerifyHost" for the TLS
> protocol to set the specified host name that need to be verified.
>
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> Reviewed-by: Ye Ting <ting.ye@intel.com>
> Reviewed-by: Long Qin <qin.long@intel.com>
> Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> Message-Id: <20190927034441.3096-3-Jiaxin.wu@intel.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> v2:
> - fix whitespace in subject line
> - drop Contributed-under line per BZ#1373
>
> CryptoPkg/Include/Library/TlsLib.h | 20 +++++++++++
> CryptoPkg/Library/TlsLib/TlsConfig.c | 38 +++++++++++++++++++-
> 2 files changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/CryptoPkg/Include/Library/TlsLib.h b/CryptoPkg/Include/Library/TlsLib.h
> index 9875cb6e746b..3af7d4bc095e 100644
> --- a/CryptoPkg/Include/Library/TlsLib.h
> +++ b/CryptoPkg/Include/Library/TlsLib.h
> @@ -395,8 +395,28 @@ TlsSetVerify (
> IN VOID *Tls,
> IN UINT32 VerifyMode
> );
>
> +/**
> + Set the specified host name to be verified.
> +
> + @param[in] Tls Pointer to the TLS object.
> + @param[in] Flags The setting flags during the validation.
> + @param[in] HostName The specified host name to be verified.
> +
> + @retval EFI_SUCCESS The HostName setting was set successfully.
> + @retval EFI_INVALID_PARAMETER The parameter is invalid.
> + @retval EFI_ABORTED Invalid HostName setting.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +TlsSetVerifyHost (
> + IN VOID *Tls,
> + IN UINT32 Flags,
> + IN CHAR8 *HostName
> + );
> +
> /**
> Sets a TLS/SSL session ID to be used during TLS/SSL connect.
>
> This function sets a session ID to be used when the TLS/SSL connection is
> diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c b/CryptoPkg/Library/TlsLib/TlsConfig.c
> index 74b577d60ee3..2bf5aee7c093 100644
> --- a/CryptoPkg/Library/TlsLib/TlsConfig.c
> +++ b/CryptoPkg/Library/TlsLib/TlsConfig.c
> @@ -1,8 +1,8 @@
> /** @file
> SSL/TLS Configuration Library Wrapper Implementation over OpenSSL.
>
> -Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
> (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -496,8 +496,44 @@ TlsSetVerify (
> //
> SSL_set_verify (TlsConn->Ssl, VerifyMode, NULL);
> }
>
> +/**
> + Set the specified host name to be verified.
> +
> + @param[in] Tls Pointer to the TLS object.
> + @param[in] Flags The setting flags during the validation.
> + @param[in] HostName The specified host name to be verified.
> +
> + @retval EFI_SUCCESS The HostName setting was set successfully.
> + @retval EFI_INVALID_PARAMETER The parameter is invalid.
> + @retval EFI_ABORTED Invalid HostName setting.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +TlsSetVerifyHost (
> + IN VOID *Tls,
> + IN UINT32 Flags,
> + IN CHAR8 *HostName
> + )
> +{
> + TLS_CONNECTION *TlsConn;
> +
> + TlsConn = (TLS_CONNECTION *) Tls;
> + if (TlsConn == NULL || TlsConn->Ssl == NULL || HostName == NULL) {
Nitpicking, I'd check HostName first.
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + SSL_set_hostflags(TlsConn->Ssl, Flags);
> +
> + if (SSL_set1_host(TlsConn->Ssl, HostName) == 0) {
> + return EFI_ABORTED;
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> /**
> Sets a TLS/SSL session ID to be used during TLS/SSL connect.
>
> This function sets a session ID to be used when the TLS/SSL connection is
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [edk2-devel] [PATCH v2 3/8] CryptoPkg/Crt: turn strchr() into a function (CVE-2019-14553)
2019-10-26 5:37 ` [PATCH v2 3/8] CryptoPkg/Crt: turn strchr() into a function (CVE-2019-14553) Laszlo Ersek
2019-10-26 11:47 ` [edk2-devel] " Philippe Mathieu-Daudé
@ 2019-10-28 5:12 ` Wang, Jian J
1 sibling, 0 replies; 26+ messages in thread
From: Wang, Jian J @ 2019-10-28 5:12 UTC (permalink / raw)
To: devel@edk2.groups.io, lersek@redhat.com
Cc: David Woodhouse, Wu, Jiaxin, Sivaraman Nainar, Lu, XiaoyuX
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Regards,
Jian
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
> Sent: Saturday, October 26, 2019 1:37 PM
> To: edk2-devel-groups-io <devel@edk2.groups.io>
> Cc: David Woodhouse <dwmw2@infradead.org>; Wang, Jian J
> <jian.j.wang@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Sivaraman Nainar
> <sivaramann@amiindia.co.in>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Subject: [edk2-devel] [PATCH v2 3/8] CryptoPkg/Crt: turn strchr() into a function
> (CVE-2019-14553)
>
> According to the ISO C standard, strchr() is a function. We #define it as
> a macro. Unfortunately, our macro evaluates the first argument ("str")
> twice. If the expression passed for "str" has side effects, the behavior
> may be undefined.
>
> In a later patch in this series, we're going to resurrect "inet_pton.c"
> (originally from the StdLib package), which calls strchr() just like that:
>
> strchr((xdigits = xdigits_l), ch)
> strchr((xdigits = xdigits_u), ch)
>
> To enable this kind of function call, turn strchr() into a function.
>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
> CVE: CVE-2019-14553
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> v2:
> - new patch
>
> CryptoPkg/Library/Include/CrtLibSupport.h | 2 +-
> CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c | 5 +++++
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h
> b/CryptoPkg/Library/Include/CrtLibSupport.h
> index 5806f50f7485..b90da20ff7e7 100644
> --- a/CryptoPkg/Library/Include/CrtLibSupport.h
> +++ b/CryptoPkg/Library/Include/CrtLibSupport.h
> @@ -146,8 +146,9 @@ int isalnum (int);
> int isupper (int);
> int tolower (int);
> int strcmp (const char *, const char *);
> int strncasecmp (const char *, const char *, size_t);
> +char *strchr (const char *, int);
> char *strrchr (const char *, int);
> unsigned long strtoul (const char *, char **, int);
> long strtol (const char *, char **, int);
> char *strerror (int);
> @@ -187,9 +188,8 @@ void abort (void);
> #define strlen(str) (size_t)(AsciiStrnLenS(str,MAX_STRING_SIZE))
> #define strcpy(strDest,strSource)
> AsciiStrCpyS(strDest,MAX_STRING_SIZE,strSource)
> #define strncpy(strDest,strSource,count)
> AsciiStrnCpyS(strDest,MAX_STRING_SIZE,strSource,(UINTN)count)
> #define strcat(strDest,strSource)
> AsciiStrCatS(strDest,MAX_STRING_SIZE,strSource)
> -#define strchr(str,ch) ScanMem8((VOID
> *)(str),AsciiStrSize(str),(UINT8)ch)
> #define strncmp(string1,string2,count)
> (int)(AsciiStrnCmp(string1,string2,(UINTN)(count)))
> #define strcasecmp(str1,str2) (int)AsciiStriCmp(str1,str2)
> #define sprintf(buf,...)
> AsciiSPrint(buf,MAX_STRING_SIZE,__VA_ARGS__)
> #define localtime(timer) NULL
> diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> index 71a2ef34ed2b..42235ab96ac3 100644
> --- a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> +++ b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> @@ -114,8 +114,13 @@ QuickSortWorker (
> //
> // -- String Manipulation Routines --
> //
>
> +char *strchr(const char *str, int ch)
> +{
> + return ScanMem8 (str, AsciiStrSize (str), (UINT8)ch);
> +}
> +
> /* Scan a string for the last occurrence of a character */
> char *strrchr (const char *str, int c)
> {
> char * save;
> --
> 2.19.1.3.g30247aa5d201
>
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/8] CryptoPkg/TlsLib: Add the new API "TlsSetVerifyHost" (CVE-2019-14553)
2019-10-26 5:37 ` [PATCH v2 2/8] CryptoPkg/TlsLib: Add the new API "TlsSetVerifyHost" (CVE-2019-14553) Laszlo Ersek
2019-10-26 11:51 ` [edk2-devel] " Philippe Mathieu-Daudé
@ 2019-10-28 5:28 ` Wang, Jian J
1 sibling, 0 replies; 26+ messages in thread
From: Wang, Jian J @ 2019-10-28 5:28 UTC (permalink / raw)
To: devel@edk2.groups.io, lersek@redhat.com
Cc: David Woodhouse, Wu, Jiaxin, Sivaraman Nainar, Lu, XiaoyuX
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Regards,
Jian
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
> Sent: Saturday, October 26, 2019 1:37 PM
> To: edk2-devel-groups-io <devel@edk2.groups.io>
> Cc: David Woodhouse <dwmw2@infradead.org>; Wang, Jian J
> <jian.j.wang@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Sivaraman Nainar
> <sivaramann@amiindia.co.in>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Subject: [edk2-devel] [PATCH v2 2/8] CryptoPkg/TlsLib: Add the new API
> "TlsSetVerifyHost" (CVE-2019-14553)
>
> From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=960
> CVE: CVE-2019-14553
> In the patch, we add the new API "TlsSetVerifyHost" for the TLS
> protocol to set the specified host name that need to be verified.
>
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> Reviewed-by: Ye Ting <ting.ye@intel.com>
> Reviewed-by: Long Qin <qin.long@intel.com>
> Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> Message-Id: <20190927034441.3096-3-Jiaxin.wu@intel.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> v2:
> - fix whitespace in subject line
> - drop Contributed-under line per BZ#1373
>
> CryptoPkg/Include/Library/TlsLib.h | 20 +++++++++++
> CryptoPkg/Library/TlsLib/TlsConfig.c | 38 +++++++++++++++++++-
> 2 files changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/CryptoPkg/Include/Library/TlsLib.h
> b/CryptoPkg/Include/Library/TlsLib.h
> index 9875cb6e746b..3af7d4bc095e 100644
> --- a/CryptoPkg/Include/Library/TlsLib.h
> +++ b/CryptoPkg/Include/Library/TlsLib.h
> @@ -395,8 +395,28 @@ TlsSetVerify (
> IN VOID *Tls,
> IN UINT32 VerifyMode
> );
>
> +/**
> + Set the specified host name to be verified.
> +
> + @param[in] Tls Pointer to the TLS object.
> + @param[in] Flags The setting flags during the validation.
> + @param[in] HostName The specified host name to be verified.
> +
> + @retval EFI_SUCCESS The HostName setting was set successfully.
> + @retval EFI_INVALID_PARAMETER The parameter is invalid.
> + @retval EFI_ABORTED Invalid HostName setting.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +TlsSetVerifyHost (
> + IN VOID *Tls,
> + IN UINT32 Flags,
> + IN CHAR8 *HostName
> + );
> +
> /**
> Sets a TLS/SSL session ID to be used during TLS/SSL connect.
>
> This function sets a session ID to be used when the TLS/SSL connection is
> diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c
> b/CryptoPkg/Library/TlsLib/TlsConfig.c
> index 74b577d60ee3..2bf5aee7c093 100644
> --- a/CryptoPkg/Library/TlsLib/TlsConfig.c
> +++ b/CryptoPkg/Library/TlsLib/TlsConfig.c
> @@ -1,8 +1,8 @@
> /** @file
> SSL/TLS Configuration Library Wrapper Implementation over OpenSSL.
>
> -Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
> (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -496,8 +496,44 @@ TlsSetVerify (
> //
> SSL_set_verify (TlsConn->Ssl, VerifyMode, NULL);
> }
>
> +/**
> + Set the specified host name to be verified.
> +
> + @param[in] Tls Pointer to the TLS object.
> + @param[in] Flags The setting flags during the validation.
> + @param[in] HostName The specified host name to be verified.
> +
> + @retval EFI_SUCCESS The HostName setting was set successfully.
> + @retval EFI_INVALID_PARAMETER The parameter is invalid.
> + @retval EFI_ABORTED Invalid HostName setting.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +TlsSetVerifyHost (
> + IN VOID *Tls,
> + IN UINT32 Flags,
> + IN CHAR8 *HostName
> + )
> +{
> + TLS_CONNECTION *TlsConn;
> +
> + TlsConn = (TLS_CONNECTION *) Tls;
> + if (TlsConn == NULL || TlsConn->Ssl == NULL || HostName == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + SSL_set_hostflags(TlsConn->Ssl, Flags);
> +
> + if (SSL_set1_host(TlsConn->Ssl, HostName) == 0) {
> + return EFI_ABORTED;
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> /**
> Sets a TLS/SSL session ID to be used during TLS/SSL connect.
>
> This function sets a session ID to be used when the TLS/SSL connection is
> --
> 2.19.1.3.g30247aa5d201
>
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 4/8] CryptoPkg/Crt: satisfy "inet_pton.c" dependencies (CVE-2019-14553)
2019-10-26 5:37 ` [PATCH v2 4/8] CryptoPkg/Crt: satisfy "inet_pton.c" dependencies (CVE-2019-14553) Laszlo Ersek
@ 2019-10-28 5:34 ` Wang, Jian J
2019-10-28 13:06 ` David Woodhouse
1 sibling, 0 replies; 26+ messages in thread
From: Wang, Jian J @ 2019-10-28 5:34 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: David Woodhouse, Wu, Jiaxin, Sivaraman Nainar, Lu, XiaoyuX
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Regards,
Jian
> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Saturday, October 26, 2019 1:37 PM
> To: edk2-devel-groups-io <devel@edk2.groups.io>
> Cc: David Woodhouse <dwmw2@infradead.org>; Wang, Jian J
> <jian.j.wang@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Sivaraman Nainar
> <sivaramann@amiindia.co.in>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Subject: [PATCH v2 4/8] CryptoPkg/Crt: satisfy "inet_pton.c" dependencies (CVE-
> 2019-14553)
>
> In a later patch in this series, we're going to resurrect "inet_pton.c"
> (originally from the StdLib package). That source file has a number of
> standard C and BSD socket dependencies. Provide those dependencies here:
>
> - The header files below will simply #include <CrtLibSupport.h>:
>
> - arpa/inet.h
> - arpa/nameser.h
> - netinet/in.h
> - sys/param.h
> - sys/socket.h
>
> - EAFNOSUPPORT comes from "StdLib/Include/errno.h", at commit
> e2d3a25f1a31; which is the commit immediately preceding the removal of
> StdLib from edk2 (964f432b9b0a).
>
> Note that the other error macro, which we alread #define, namely EINVAL,
> has a value (22) that also matches "StdLib/Include/errno.h".
>
> - The AF_INET and AF_INET6 address family macros come from
> "StdLib/Include/sys/socket.h".
>
> - The NS_INT16SZ, NS_INADDRSZ and NS_IN6ADDRSZ macros come from
> "StdLib/Include/arpa/nameser.h".
>
> - The "u_int" and "u_char" types come from "StdLib/Include/sys/types.h".
>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
> CVE: CVE-2019-14553
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> v2:
> - new patch
>
> CryptoPkg/Library/Include/CrtLibSupport.h | 16 ++++++++++++++++
> CryptoPkg/Library/Include/arpa/inet.h | 9 +++++++++
> CryptoPkg/Library/Include/arpa/nameser.h | 9 +++++++++
> CryptoPkg/Library/Include/netinet/in.h | 9 +++++++++
> CryptoPkg/Library/Include/sys/param.h | 9 +++++++++
> CryptoPkg/Library/Include/sys/socket.h | 9 +++++++++
> 6 files changed, 61 insertions(+)
>
> diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h
> b/CryptoPkg/Library/Include/CrtLibSupport.h
> index b90da20ff7e7..e603fad763f9 100644
> --- a/CryptoPkg/Library/Include/CrtLibSupport.h
> +++ b/CryptoPkg/Library/Include/CrtLibSupport.h
> @@ -73,22 +73,38 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> //
> // Definitions for global constants used by CRT library routines
> //
> #define EINVAL 22 /* Invalid argument */
> +#define EAFNOSUPPORT 47 /* Address family not supported by
> protocol family */
> #define INT_MAX 0x7FFFFFFF /* Maximum (signed) int value */
> #define LONG_MAX 0X7FFFFFFFL /* max value for a long */
> #define LONG_MIN (-LONG_MAX-1) /* min value for a long */
> #define ULONG_MAX 0xFFFFFFFF /* Maximum unsigned long value */
> #define CHAR_BIT 8 /* Number of bits in a char */
>
> +//
> +// Address families.
> +//
> +#define AF_INET 2 /* internetwork: UDP, TCP, etc. */
> +#define AF_INET6 24 /* IP version 6 */
> +
> +//
> +// Define constants based on RFC0883, RFC1034, RFC 1035
> +//
> +#define NS_INT16SZ 2 /*%< #/bytes of data in a u_int16_t */
> +#define NS_INADDRSZ 4 /*%< IPv4 T_A */
> +#define NS_IN6ADDRSZ 16 /*%< IPv6 T_AAAA */
> +
> //
> // Basic types mapping
> //
> typedef UINTN size_t;
> +typedef UINTN u_int;
> typedef INTN ssize_t;
> typedef INT32 time_t;
> typedef UINT8 __uint8_t;
> typedef UINT8 sa_family_t;
> +typedef UINT8 u_char;
> typedef UINT32 uid_t;
> typedef UINT32 gid_t;
>
> //
> diff --git a/CryptoPkg/Library/Include/arpa/inet.h
> b/CryptoPkg/Library/Include/arpa/inet.h
> new file mode 100644
> index 000000000000..988e4e0a73e3
> --- /dev/null
> +++ b/CryptoPkg/Library/Include/arpa/inet.h
> @@ -0,0 +1,9 @@
> +/** @file
> + Include file to support building third-party standard C / BSD sockets code.
> +
> + Copyright (C) 2019, Red Hat, Inc.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <CrtLibSupport.h>
> diff --git a/CryptoPkg/Library/Include/arpa/nameser.h
> b/CryptoPkg/Library/Include/arpa/nameser.h
> new file mode 100644
> index 000000000000..988e4e0a73e3
> --- /dev/null
> +++ b/CryptoPkg/Library/Include/arpa/nameser.h
> @@ -0,0 +1,9 @@
> +/** @file
> + Include file to support building third-party standard C / BSD sockets code.
> +
> + Copyright (C) 2019, Red Hat, Inc.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <CrtLibSupport.h>
> diff --git a/CryptoPkg/Library/Include/netinet/in.h
> b/CryptoPkg/Library/Include/netinet/in.h
> new file mode 100644
> index 000000000000..988e4e0a73e3
> --- /dev/null
> +++ b/CryptoPkg/Library/Include/netinet/in.h
> @@ -0,0 +1,9 @@
> +/** @file
> + Include file to support building third-party standard C / BSD sockets code.
> +
> + Copyright (C) 2019, Red Hat, Inc.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <CrtLibSupport.h>
> diff --git a/CryptoPkg/Library/Include/sys/param.h
> b/CryptoPkg/Library/Include/sys/param.h
> new file mode 100644
> index 000000000000..988e4e0a73e3
> --- /dev/null
> +++ b/CryptoPkg/Library/Include/sys/param.h
> @@ -0,0 +1,9 @@
> +/** @file
> + Include file to support building third-party standard C / BSD sockets code.
> +
> + Copyright (C) 2019, Red Hat, Inc.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <CrtLibSupport.h>
> diff --git a/CryptoPkg/Library/Include/sys/socket.h
> b/CryptoPkg/Library/Include/sys/socket.h
> new file mode 100644
> index 000000000000..988e4e0a73e3
> --- /dev/null
> +++ b/CryptoPkg/Library/Include/sys/socket.h
> @@ -0,0 +1,9 @@
> +/** @file
> + Include file to support building third-party standard C / BSD sockets code.
> +
> + Copyright (C) 2019, Red Hat, Inc.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <CrtLibSupport.h>
> --
> 2.19.1.3.g30247aa5d201
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 6/8] CryptoPkg/TlsLib: TlsSetVerifyHost: parse IP address literals as such (CVE-2019-14553)
2019-10-26 5:37 ` [PATCH v2 6/8] CryptoPkg/TlsLib: TlsSetVerifyHost: parse IP address literals as such (CVE-2019-14553) Laszlo Ersek
@ 2019-10-28 6:12 ` Wang, Jian J
0 siblings, 0 replies; 26+ messages in thread
From: Wang, Jian J @ 2019-10-28 6:12 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: David Woodhouse, Wu, Jiaxin, Sivaraman Nainar, Lu, XiaoyuX
I'm aware of the change is necessary for the solution. But I'm not expert on TLS.
I'd others to give more professional opinions.
Acked-by: Jian J Wang <jian.j.wang@intel.com>
Regards,
Jian
> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Saturday, October 26, 2019 1:37 PM
> To: edk2-devel-groups-io <devel@edk2.groups.io>
> Cc: David Woodhouse <dwmw2@infradead.org>; Wang, Jian J
> <jian.j.wang@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Sivaraman Nainar
> <sivaramann@amiindia.co.in>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Subject: [PATCH v2 6/8] CryptoPkg/TlsLib: TlsSetVerifyHost: parse IP address
> literals as such (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 <dwmw2@infradead.org>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
> CVE: CVE-2019-14553
> Suggested-by: David Woodhouse <dwmw2@infradead.org>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> v2:
> - new patch
>
> CryptoPkg/Library/TlsLib/TlsConfig.c | 28 +++++++++++++++++---
> 1 file changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c
> b/CryptoPkg/Library/TlsLib/TlsConfig.c
> index 2bf5aee7c093..307eb57896dc 100644
> --- a/CryptoPkg/Library/TlsLib/TlsConfig.c
> +++ b/CryptoPkg/Library/TlsLib/TlsConfig.c
> @@ -516,22 +516,42 @@ TlsSetVerifyHost (
> IN UINT32 Flags,
> 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) {
> return EFI_INVALID_PARAMETER;
> }
>
> 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;
> + }
> +
> + 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 EFI_SUCCESS;
> + return (ParamStatus == 1) ? EFI_SUCCESS : EFI_ABORTED;
> }
>
> /**
> Sets a TLS/SSL session ID to be used during TLS/SSL connect.
> --
> 2.19.1.3.g30247aa5d201
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 5/8] CryptoPkg/Crt: import "inet_pton.c" (CVE-2019-14553)
2019-10-26 5:37 ` [PATCH v2 5/8] CryptoPkg/Crt: import "inet_pton.c" (CVE-2019-14553) Laszlo Ersek
@ 2019-10-28 6:16 ` Wang, Jian J
0 siblings, 0 replies; 26+ messages in thread
From: Wang, Jian J @ 2019-10-28 6:16 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: David Woodhouse, Wu, Jiaxin, Sivaraman Nainar, Lu, XiaoyuX
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Regards,
Jian
> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Saturday, October 26, 2019 1:37 PM
> To: edk2-devel-groups-io <devel@edk2.groups.io>
> Cc: David Woodhouse <dwmw2@infradead.org>; Wang, Jian J
> <jian.j.wang@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Sivaraman Nainar
> <sivaramann@amiindia.co.in>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Subject: [PATCH v2 5/8] CryptoPkg/Crt: import "inet_pton.c" (CVE-2019-14553)
>
> For TianoCore BZ#1734, StdLib has been moved from the edk2 project to the
> edk2-libc project, in commit 964f432b9b0a ("edk2: Remove AppPkg, StdLib,
> StdLibPrivateInternalFiles", 2019-04-29).
>
> We'd like to use the inet_pton() function in CryptoPkg. Resurrect the
> "inet_pton.c" file from just before the StdLib removal, as follows:
>
> $ git show \
> 964f432b9b0a^:StdLib/BsdSocketLib/inet_pton.c \
> > CryptoPkg/Library/BaseCryptLib/SysCall/inet_pton.c
>
> The inet_pton() function is only intended for the DXE phase at this time,
> therefore only the "BaseCryptLib" instance INF file receives the new file.
>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
> CVE: CVE-2019-14553
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> v2:
> - new patch
>
> CryptoPkg/Library/Include/CrtLibSupport.h | 1 +
> CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf | 1 +
> CryptoPkg/Library/BaseCryptLib/SysCall/inet_pton.c | 257
> ++++++++++++++++++++
> 3 files changed, 259 insertions(+)
>
> diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h
> b/CryptoPkg/Library/Include/CrtLibSupport.h
> index e603fad763f9..5a20ba636fff 100644
> --- a/CryptoPkg/Library/Include/CrtLibSupport.h
> +++ b/CryptoPkg/Library/Include/CrtLibSupport.h
> @@ -191,8 +191,9 @@ char *secure_getenv (const char *);
> void abort (void) __attribute__((__noreturn__));
> #else
> void abort (void);
> #endif
> +int inet_pton (int, const char *, void *);
>
> //
> // Macros that directly map functions to BaseLib, BaseMemoryLib, and
> DebugLib functions
> //
> diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> index a98be2cd9590..dc9e6e5d45f9 100644
> --- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> +++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> @@ -57,8 +57,9 @@ [Sources]
>
> SysCall/CrtWrapper.c
> SysCall/TimerWrapper.c
> SysCall/BaseMemAllocation.c
> + SysCall/inet_pton.c
>
> [Sources.Ia32]
> Rand/CryptRandTsc.c
>
> diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/inet_pton.c
> b/CryptoPkg/Library/BaseCryptLib/SysCall/inet_pton.c
> new file mode 100644
> index 000000000000..32e1ab8690e6
> --- /dev/null
> +++ b/CryptoPkg/Library/BaseCryptLib/SysCall/inet_pton.c
> @@ -0,0 +1,257 @@
> +/* Copyright (c) 1996 by Internet Software Consortium.
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SOFTWARE
> CONSORTIUM DISCLAIMS
> + * ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
> IMPLIED WARRANTIES
> + * OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL INTERNET
> SOFTWARE
> + * CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
> CONSEQUENTIAL
> + * DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
> DATA OR
> + * PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> TORTIOUS
> + * ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
> PERFORMANCE OF THIS
> + * SOFTWARE.
> + */
> +
> +/*
> + * Portions copyright (c) 1999, 2000
> + * Intel Corporation.
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + *
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + *
> + * 3. All advertising materials mentioning features or use of this software
> + * must display the following acknowledgement:
> + *
> + * This product includes software developed by Intel Corporation and
> + * its contributors.
> + *
> + * 4. Neither the name of Intel Corporation or its contributors may be
> + * used to endorse or promote products derived from this software
> + * without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY INTEL CORPORATION AND CONTRIBUTORS
> ``AS IS''
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL INTEL CORPORATION OR
> CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
> WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
> OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
> ADVISED OF
> + * THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + */
> +
> +#if defined(LIBC_SCCS) && !defined(lint)
> +static char rcsid[] = "$Id: inet_pton.c,v 1.1.1.1 2003/11/19 01:51:30 kyu3 Exp
> $";
> +#endif /* LIBC_SCCS and not lint */
> +
> +#include <sys/param.h>
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +#include <netinet/in.h>
> +#include <arpa/inet.h>
> +#include <arpa/nameser.h>
> +#include <string.h>
> +#include <errno.h>
> +
> +/*
> + * WARNING: Don't even consider trying to compile this on a system where
> + * sizeof(int) < 4. sizeof(int) > 4 is fine; all the world's not a VAX.
> + */
> +
> +static int inet_pton4 (const char *src, u_char *dst);
> +static int inet_pton6 (const char *src, u_char *dst);
> +
> +/* int
> + * inet_pton(af, src, dst)
> + * convert from presentation format (which usually means ASCII printable)
> + * to network format (which is usually some kind of binary format).
> + * return:
> + * 1 if the address was valid for the specified address family
> + * 0 if the address wasn't valid (`dst' is untouched in this case)
> + * -1 if some other error occurred (`dst' is untouched in this case, too)
> + * author:
> + * Paul Vixie, 1996.
> + */
> +int
> +inet_pton(
> + int af,
> + const char *src,
> + void *dst
> + )
> +{
> + switch (af) {
> + case AF_INET:
> + return (inet_pton4(src, dst));
> + case AF_INET6:
> + return (inet_pton6(src, dst));
> + default:
> + errno = EAFNOSUPPORT;
> + return (-1);
> + }
> + /* NOTREACHED */
> +}
> +
> +/* int
> + * inet_pton4(src, dst)
> + * like inet_aton() but without all the hexadecimal and shorthand.
> + * return:
> + * 1 if `src' is a valid dotted quad, else 0.
> + * notice:
> + * does not touch `dst' unless it's returning 1.
> + * author:
> + * Paul Vixie, 1996.
> + */
> +static int
> +inet_pton4(
> + const char *src,
> + u_char *dst
> + )
> +{
> + static const char digits[] = "0123456789";
> + int saw_digit, octets, ch;
> + u_char tmp[NS_INADDRSZ], *tp;
> +
> + saw_digit = 0;
> + octets = 0;
> + *(tp = tmp) = 0;
> + while ((ch = *src++) != '\0') {
> + const char *pch;
> +
> + if ((pch = strchr(digits, ch)) != NULL) {
> + u_int new = *tp * 10 + (u_int)(pch - digits);
> +
> + if (new > 255)
> + return (0);
> + *tp = (u_char)new;
> + if (! saw_digit) {
> + if (++octets > 4)
> + return (0);
> + saw_digit = 1;
> + }
> + } else if (ch == '.' && saw_digit) {
> + if (octets == 4)
> + return (0);
> + *++tp = 0;
> + saw_digit = 0;
> + } else
> + return (0);
> + }
> + if (octets < 4)
> + return (0);
> +
> + memcpy(dst, tmp, NS_INADDRSZ);
> + return (1);
> +}
> +
> +/* int
> + * inet_pton6(src, dst)
> + * convert presentation level address to network order binary form.
> + * return:
> + * 1 if `src' is a valid [RFC1884 2.2] address, else 0.
> + * notice:
> + * (1) does not touch `dst' unless it's returning 1.
> + * (2) :: in a full address is silently ignored.
> + * credit:
> + * inspired by Mark Andrews.
> + * author:
> + * Paul Vixie, 1996.
> + */
> +static int
> +inet_pton6(
> + const char *src,
> + u_char *dst
> + )
> +{
> + static const char xdigits_l[] = "0123456789abcdef",
> + xdigits_u[] = "0123456789ABCDEF";
> + u_char tmp[NS_IN6ADDRSZ], *tp, *endp, *colonp;
> + const char *xdigits, *curtok;
> + int ch, saw_xdigit;
> + u_int val;
> +
> + memset((tp = tmp), '\0', NS_IN6ADDRSZ);
> + endp = tp + NS_IN6ADDRSZ;
> + colonp = NULL;
> + /* Leading :: requires some special handling. */
> + if (*src == ':')
> + if (*++src != ':')
> + return (0);
> + curtok = src;
> + saw_xdigit = 0;
> + val = 0;
> + while ((ch = *src++) != '\0') {
> + const char *pch;
> +
> + if ((pch = strchr((xdigits = xdigits_l), ch)) == NULL)
> + pch = strchr((xdigits = xdigits_u), ch);
> + if (pch != NULL) {
> + val <<= 4;
> + val |= (pch - xdigits);
> + if (val > 0xffff)
> + return (0);
> + saw_xdigit = 1;
> + continue;
> + }
> + if (ch == ':') {
> + curtok = src;
> + if (!saw_xdigit) {
> + if (colonp)
> + return (0);
> + colonp = tp;
> + continue;
> + }
> + if (tp + NS_INT16SZ > endp)
> + return (0);
> + *tp++ = (u_char) (val >> 8) & 0xff;
> + *tp++ = (u_char) val & 0xff;
> + saw_xdigit = 0;
> + val = 0;
> + continue;
> + }
> + if (ch == '.' && ((tp + NS_INADDRSZ) <= endp) &&
> + inet_pton4(curtok, tp) > 0) {
> + tp += NS_INADDRSZ;
> + saw_xdigit = 0;
> + break; /* '\0' was seen by inet_pton4(). */
> + }
> + return (0);
> + }
> + if (saw_xdigit) {
> + if (tp + NS_INT16SZ > endp)
> + return (0);
> + *tp++ = (u_char) (val >> 8) & 0xff;
> + *tp++ = (u_char) val & 0xff;
> + }
> + if (colonp != NULL) {
> + /*
> + * Since some memmove()'s erroneously fail to handle
> + * overlapping regions, we'll do the shift by hand.
> + */
> + const int n = (int)(tp - colonp);
> + int i;
> +
> + for (i = 1; i <= n; i++) {
> + endp[- i] = colonp[n - i];
> + colonp[n - i] = 0;
> + }
> + tp = endp;
> + }
> + if (tp != endp)
> + return (0);
> + memcpy(dst, tmp, NS_IN6ADDRSZ);
> + return (1);
> +}
> --
> 2.19.1.3.g30247aa5d201
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [edk2-devel] [PATCH v2 1/8] MdePkg/Include/Protocol/Tls.h: Add the data type of EfiTlsVerifyHost (CVE-2019-14553)
2019-10-26 5:37 ` [PATCH v2 1/8] MdePkg/Include/Protocol/Tls.h: Add the data type of EfiTlsVerifyHost (CVE-2019-14553) Laszlo Ersek
@ 2019-10-28 8:12 ` Liming Gao
0 siblings, 0 replies; 26+ messages in thread
From: Liming Gao @ 2019-10-28 8:12 UTC (permalink / raw)
To: devel@edk2.groups.io, lersek@redhat.com
Cc: David Woodhouse, Wang, Jian J, Wu, Jiaxin, Sivaraman Nainar,
Lu, XiaoyuX
Reviewed-by: Liming Gao <liming.gao@intel.com>
>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Laszlo Ersek
>Sent: Saturday, October 26, 2019 1:37 PM
>To: edk2-devel-groups-io <devel@edk2.groups.io>
>Cc: David Woodhouse <dwmw2@infradead.org>; Wang, Jian J
><jian.j.wang@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Sivaraman
>Nainar <sivaramann@amiindia.co.in>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
>Subject: [edk2-devel] [PATCH v2 1/8] MdePkg/Include/Protocol/Tls.h: Add
>the data type of EfiTlsVerifyHost (CVE-2019-14553)
>
>From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
>
>REF: https://bugzilla.tianocore.org/show_bug.cgi?id=960
>CVE: CVE-2019-14553
>In the patch, we add the new data type named "EfiTlsVerifyHost" and
>the EFI_TLS_VERIFY_HOST_FLAG for the TLS protocol consumer (HTTP)
>to enable the host name check so as to avoid the potential
>Man-In-The-Middle attack.
>
>Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
>Reviewed-by: Ye Ting <ting.ye@intel.com>
>Reviewed-by: Long Qin <qin.long@intel.com>
>Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
>Acked-by: Laszlo Ersek <lersek@redhat.com>
>Message-Id: <20190927034441.3096-2-Jiaxin.wu@intel.com>
>Cc: David Woodhouse <dwmw2@infradead.org>
>Cc: Jian J Wang <jian.j.wang@intel.com>
>Cc: Jiaxin Wu <jiaxin.wu@intel.com>
>Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
>Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
>Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>---
>
>Notes:
> v2:
> - fix whitespace in subject line
> - drop Contributed-under line per BZ#1373
>
> MdePkg/Include/Protocol/Tls.h | 68 ++++++++++++++++----
> 1 file changed, 57 insertions(+), 11 deletions(-)
>
>diff --git a/MdePkg/Include/Protocol/Tls.h b/MdePkg/Include/Protocol/Tls.h
>index bf1b6727a1e9..af524ae2a60e 100644
>--- a/MdePkg/Include/Protocol/Tls.h
>+++ b/MdePkg/Include/Protocol/Tls.h
>@@ -40,12 +40,8 @@ typedef struct _EFI_TLS_PROTOCOL
>EFI_TLS_PROTOCOL;
> ///
> /// EFI_TLS_SESSION_DATA_TYPE
> ///
> typedef enum {
>- ///
>- /// Session Configuration
>- ///
>-
> ///
> /// TLS session Version. The corresponding Data is of type EFI_TLS_VERSION.
> ///
> EfiTlsVersion,
>@@ -85,13 +81,8 @@ typedef enum {
> /// TLS session data session state.
> /// The corresponding Data is of type EFI_TLS_SESSION_STATE.
> ///
> EfiTlsSessionState,
>-
>- ///
>- /// Session information
>- ///
>-
> ///
> /// TLS session data client random.
> /// The corresponding Data is of type EFI_TLS_RANDOM.
> ///
>@@ -105,11 +96,17 @@ typedef enum {
> /// TLS session data key material.
> /// The corresponding Data is of type EFI_TLS_MASTER_SECRET.
> ///
> EfiTlsKeyMaterial,
>+ ///
>+ /// TLS session hostname for validation which is used to verify whether the
>name
>+ /// within the peer certificate matches a given host name.
>+ /// This parameter is invalid when EfiTlsVerifyMethod is
>EFI_TLS_VERIFY_NONE.
>+ /// The corresponding Data is of type EFI_TLS_VERIFY_HOST.
>+ ///
>+ EfiTlsVerifyHost,
>
> EfiTlsSessionDataTypeMaximum
>-
> } EFI_TLS_SESSION_DATA_TYPE;
>
> ///
> /// EFI_TLS_VERSION
>@@ -177,17 +174,66 @@ typedef UINT32 EFI_TLS_VERIFY;
> /// the reason for the certificate verification failure.
> ///
> #define EFI_TLS_VERIFY_PEER 0x1
> ///
>-/// TLS session will fail peer certificate is absent.
>+/// EFI_TLS_VERIFY_FAIL_IF_NO_PEER_CERT is only meaningful in the server
>mode.
>+/// TLS session will fail if client certificate is absent.
> ///
> #define EFI_TLS_VERIFY_FAIL_IF_NO_PEER_CERT 0x2
> ///
> /// TLS session only verify client once, and doesn't request certificate during
> /// re-negotiation.
> ///
> #define EFI_TLS_VERIFY_CLIENT_ONCE 0x4
>
>+///
>+/// EFI_TLS_VERIFY_HOST_FLAG
>+///
>+typedef UINT32 EFI_TLS_VERIFY_HOST_FLAG;
>+///
>+/// There is no additional flags set for hostname validation.
>+/// Wildcards are supported and they match only in the left-most label.
>+///
>+#define EFI_TLS_VERIFY_FLAG_NONE 0x00
>+///
>+/// Always check the Subject Distinguished Name (DN) in the peer certificate
>even if the
>+/// certificate contains Subject Alternative Name (SAN).
>+///
>+#define EFI_TLS_VERIFY_FLAG_ALWAYS_CHECK_SUBJECT 0x01
>+///
>+/// Disable the match of all wildcards.
>+///
>+#define EFI_TLS_VERIFY_FLAG_NO_WILDCARDS 0x02
>+///
>+/// Disable the "*" as wildcard in labels that have a prefix or suffix (e.g.
>"www*" or "*www").
>+///
>+#define EFI_TLS_VERIFY_FLAG_NO_PARTIAL_WILDCARDS 0x04
>+///
>+/// Allow the "*" to match more than one labels. Otherwise, only matches a
>single label.
>+///
>+#define EFI_TLS_VERIFY_FLAG_MULTI_LABEL_WILDCARDS 0x08
>+///
>+/// Restrict to only match direct child sub-domains which start with ".".
>+/// For example, a name of ".example.com" would match
>"www.example.com" with this flag,
>+/// but would not match "www.sub.example.com".
>+///
>+#define EFI_TLS_VERIFY_FLAG_SINGLE_LABEL_SUBDOMAINS 0x10
>+///
>+/// Never check the Subject Distinguished Name (DN) even there is no
>+/// Subject Alternative Name (SAN) in the certificate.
>+///
>+#define EFI_TLS_VERIFY_FLAG_NEVER_CHECK_SUBJECT 0x20
>+
>+///
>+/// EFI_TLS_VERIFY_HOST
>+///
>+#pragma pack (1)
>+typedef struct {
>+ EFI_TLS_VERIFY_HOST_FLAG Flags;
>+ CHAR8 *HostName;
>+} EFI_TLS_VERIFY_HOST;
>+#pragma pack ()
>+
> ///
> /// EFI_TLS_RANDOM
> /// Note: The definition of EFI_TLS_RANDOM is from "RFC 5246 A.4.1.
> /// Hello Messages".
>--
>2.19.1.3.g30247aa5d201
>
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 4/8] CryptoPkg/Crt: satisfy "inet_pton.c" dependencies (CVE-2019-14553)
2019-10-26 5:37 ` [PATCH v2 4/8] CryptoPkg/Crt: satisfy "inet_pton.c" dependencies (CVE-2019-14553) Laszlo Ersek
2019-10-28 5:34 ` Wang, Jian J
@ 2019-10-28 13:06 ` David Woodhouse
2019-10-29 0:47 ` Laszlo Ersek
1 sibling, 1 reply; 26+ messages in thread
From: David Woodhouse @ 2019-10-28 13:06 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: Jian J Wang, Jiaxin Wu, Sivaraman Nainar, Xiaoyu Lu
[-- Attachment #1: Type: text/plain, Size: 1471 bytes --]
On Sat, 2019-10-26 at 07:37 +0200, Laszlo Ersek wrote:
> In a later patch in this series, we're going to resurrect "inet_pton.c"
> (originally from the StdLib package). That source file has a number of
> standard C and BSD socket dependencies. Provide those dependencies here:
>
> - The header files below will simply #include <CrtLibSupport.h>:
>
> - arpa/inet.h
> - arpa/nameser.h
> - netinet/in.h
> - sys/param.h
> - sys/socket.h
>
> - EAFNOSUPPORT comes from "StdLib/Include/errno.h", at commit
> e2d3a25f1a31; which is the commit immediately preceding the removal of
> StdLib from edk2 (964f432b9b0a).
>
> Note that the other error macro, which we alread #define, namely EINVAL,
> has a value (22) that also matches "StdLib/Include/errno.h".
>
> - The AF_INET and AF_INET6 address family macros come from
> "StdLib/Include/sys/socket.h".
>
> - The NS_INT16SZ, NS_INADDRSZ and NS_IN6ADDRSZ macros come from
> "StdLib/Include/arpa/nameser.h".
>
> - The "u_int" and "u_char" types come from "StdLib/Include/sys/types.h".
Hm.
If you're porting a whole standard C library to EDK2 then I suppose it
makes sense to build up all this infrastructure for it.
But in this case when it's only the single inet_pton() function that
you need, perhaps it makes more sense to 'port' that one function to
UEFI (or just reimplement it looking like EDK2 code), instead of
bringing all this stuff along with it?
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 4/8] CryptoPkg/Crt: satisfy "inet_pton.c" dependencies (CVE-2019-14553)
2019-10-28 13:06 ` David Woodhouse
@ 2019-10-29 0:47 ` Laszlo Ersek
2019-10-29 2:44 ` [edk2-devel] " Wu, Jiaxin
0 siblings, 1 reply; 26+ messages in thread
From: Laszlo Ersek @ 2019-10-29 0:47 UTC (permalink / raw)
To: David Woodhouse, edk2-devel-groups-io
Cc: Jian J Wang, Jiaxin Wu, Sivaraman Nainar, Xiaoyu Lu
On 10/28/19 14:06, David Woodhouse wrote:
> On Sat, 2019-10-26 at 07:37 +0200, Laszlo Ersek wrote:
>> In a later patch in this series, we're going to resurrect "inet_pton.c"
>> (originally from the StdLib package). That source file has a number of
>> standard C and BSD socket dependencies. Provide those dependencies here:
>>
>> - The header files below will simply #include <CrtLibSupport.h>:
>>
>> - arpa/inet.h
>> - arpa/nameser.h
>> - netinet/in.h
>> - sys/param.h
>> - sys/socket.h
>>
>> - EAFNOSUPPORT comes from "StdLib/Include/errno.h", at commit
>> e2d3a25f1a31; which is the commit immediately preceding the removal of
>> StdLib from edk2 (964f432b9b0a).
>>
>> Note that the other error macro, which we alread #define, namely EINVAL,
>> has a value (22) that also matches "StdLib/Include/errno.h".
>>
>> - The AF_INET and AF_INET6 address family macros come from
>> "StdLib/Include/sys/socket.h".
>>
>> - The NS_INT16SZ, NS_INADDRSZ and NS_IN6ADDRSZ macros come from
>> "StdLib/Include/arpa/nameser.h".
>>
>> - The "u_int" and "u_char" types come from "StdLib/Include/sys/types.h".
>
> Hm.
>
> If you're porting a whole standard C library to EDK2 then I suppose it
> makes sense to build up all this infrastructure for it.
>
> But in this case when it's only the single inet_pton() function that
> you need, perhaps it makes more sense to 'port' that one function to
> UEFI (or just reimplement it looking like EDK2 code), instead of
> bringing all this stuff along with it?
I didn't want to take responsibility for touching any of that code -- I
wanted it to be a piece of the puzzle that we'd just drop in. Its coding
style is very foreign to edk2 norms, so once we started, we wouldn't
stop before rewriting it more or less completely. (For example it quite
frequently consumes the values that assignment expressions evaluate to,
which is a huge no-no in edk2, as far as I understand.) I have no
capacity for such a rework (or additional ownership / responsibility),
sorry.
I worked from Friday evening to Saturday ~6-7AM as my "second sprint" on
this code and its testing, until I was satisfied with the test coverage.
I apologize but I simply cannot repeat that. This is all I can
contribute code-wise (and testing-wise) to fixing this issue.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553)
2019-10-26 5:37 [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553) Laszlo Ersek
` (7 preceding siblings ...)
2019-10-26 5:37 ` [PATCH v2 8/8] NetworkPkg/HttpDxe: Set the HostName for the verification (CVE-2019-14553) Laszlo Ersek
@ 2019-10-29 2:37 ` Wu, Jiaxin
2019-11-02 11:15 ` Laszlo Ersek
2019-10-31 9:28 ` Laszlo Ersek
9 siblings, 1 reply; 26+ messages in thread
From: Wu, Jiaxin @ 2019-10-29 2:37 UTC (permalink / raw)
To: devel@edk2.groups.io, lersek@redhat.com
Cc: David Woodhouse, Wang, Jian J, Sivaraman Nainar, Lu, XiaoyuX
Test matrix - that's a great summary! The result is also good to me.
Thanks Laszlo's patches to fix the gap.
Series Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
> Ersek
> Sent: Saturday, October 26, 2019 1:37 PM
> To: edk2-devel-groups-io <devel@edk2.groups.io>
> Cc: David Woodhouse <dwmw2@infradead.org>; Wang, Jian J
> <jian.j.wang@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Sivaraman
> Nainar <sivaramann@amiindia.co.in>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Subject: [edk2-devel] [PATCH v2 0/8] support server identity validation in
> HTTPS Boot (CVE-2019-14553)
>
> Repo: https://github.com/lersek/edk2.git
> Branch: bz960_with_inet_pton_v2
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
>
> Previous posting from Jiaxin:
>
> [edk2-devel] [PATCH v1 0/4]
> Support HTTPS HostName validation feature(CVE-2019-14553)
>
> https://edk2.groups.io/g/devel/message/48183
> http://mid.mail-archive.com/20190927034441.3096-1-Jiaxin.wu@intel.com
>
> In v2, I have inserted 4 new patches in the middle, to satisfy two
> additional requirements raised by Siva and David:
>
> - If the Subject Alternative Name in the server certificate contains an
> IP address in binary representation, and the URL specifies an IP
> address in literal form for "hostname", then both of those things
> should be compared against each other, after converting the literal
> from the URL to binary representation. In other words, a server
> certificate with an IP address SAN should be recognized.
>
> - If the URL specifies an IP address literal, then, according to
> RFC-2818, "the iPAddress subjectAltName must be present in the
> certificate and must exactly match the IP in the URI". In other words,
> if a certificate matches the IP address literal from the URL via
> Common Name only, then the certificate must be rejected.
>
> I've also fixed two commit message warts in Jiaxin's patches (see the
> Notes sections on the patches).
>
> I've tested the series painstakingly. Here's the script I wrote for
> certificate generation:
>
> > ## @file
> > # Bash shell script for generating test certificates, for
> > # <https://bugzilla.tianocore.org/show_bug.cgi?id=960>.
> > #
> > # Copyright (C) 2019, Red Hat, Inc.
> > #
> > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > #
> > # Customize te variables in section "Configuration", then run the script with
> > # "bash gencerts.sh".
> > #
> > # The script creates 17 files in the current working directory:
> > # - one CA certificate (note: key is discarded);
> > #
> > # - for the (IPv4 domain name, IPv4 address) pair, one keypair (that is, a
> > # CA-issued certificate, plus the private key) for each case below:
> > # - Common Name = IPv4 domain name, no subjectAltName,
> > # - Common Name = IPv4 domain name, IPv4 address in
> subjectAltName,
> > # - Common Name = IPv4 address literal, no subjectAltName,
> > # - Common Name = IPv4 address literal, IPv4 address in subjectAltName;
> > #
> > # - for the (IPv6 domain name, IPv6 address) pair, a similar set of files.
> > #
> > # Finally, the script prints some commands for the root user that are
> related
> > # to the following OVMF feature: OVMF can HTTPS boot while trusting the
> same
> > # set of CA certificates that the virt host trusts. The commands install the
> > # new CA certificate on the host (note: this should never be done in
> > # production, in spite of the CA key being discarded), and also extract all CA
> > # certs in the format that OVMF expects. (This edk2-specific extraction is
> > # normally performed by the "update-ca-trust" command, but if yours isn't
> > # up-to-date enough for that, build and install p11-kit from source, and set
> > # MY_P11_KIT_PREFIX, before invoking this script.) See
> "OvmfPkg/README" for
> > # passing the extracted CA certs to OVMF on the QEMU cmdline.
> > ##
> > set -e -u -C
> >
> > # Configuration.
> > CA_NAME=TianoCore_BZ_960_CA
> > IPV4_NAME=ipv4-server
> > IPV4_ADDR=192.168.124.2
> > IPV6_NAME=ipv6-server
> > IPV6_ADDR=fd33:eb1b:9b36::2
> >
> > # Create a temporary directory for transient files.
> > TMP_D=$(mktemp -d)
> > trap 'rm -f -r -- "$TMP_D"' EXIT
> >
> > # Set some helper variables.
> > TMP_EXT=$TMP_D/ext # OpenSSL extensions
> > TMP_CSR=$TMP_D/csr # certificate request
> > TMP_CA_KEY=$TMP_D/ca.key # CA key
> > TMP_CA_SRL=$TMP_D/ca.srl # CA serial number
> >
> > # Generate the CA certificate.
> > openssl req -x509 -nodes \
> > -subj /CN="$CA_NAME" \
> > -out "$CA_NAME".crt \
> > -keyout "$TMP_CA_KEY"
> >
> > # Create a CA-issued certificate.
> > # Parameters:
> > # $1: Common Name
> > # $2: IPv4 or IPv6 address literal, to be used in SAN; or empty string
> > gencrt()
> > {
> > local CN="$1"
> > local SANIP="$2"
> > local STEM
> > local EXT
> >
> > if test -z "$SANIP"; then
> > # File name stem consists of Common Name only. No certificate
> extensions.
> > STEM=svr_$CN
> > EXT=
> > else
> > # File name stem includes Common Name and IP address literal.
> > STEM=svr_${CN}_${SANIP}
> >
> > # SAN IP extension in the certificate. Rewrite the ad-hoc extensions file
> > # with the current SAN IP.
> > echo "subjectAltName=IP:$SANIP" >| "$TMP_EXT"
> > EXT="-extfile $TMP_EXT"
> > fi
> > STEM=${STEM//[:.]/_}
> >
> > # Generate CSR.
> > openssl req -new -nodes \
> > -subj /CN="$CN" \
> > -out "$TMP_CSR" \
> > -keyout "$STEM".key
> >
> > # Sign the certificate request, potentially adding the SAN IP.
> > openssl x509 -req -CAcreateserial $EXT \
> > -in "$TMP_CSR" \
> > -out "$STEM".crt \
> > -CA "$CA_NAME".crt \
> > -CAkey "$TMP_CA_KEY" \
> > -CAserial "$TMP_CA_SRL"
> > }
> >
> > # Generate all certificates.
> > gencrt "$IPV4_NAME" "" # domain name in CN, no SAN IPv4
> > gencrt "$IPV4_NAME" "$IPV4_ADDR" # domain name in CN, SAN IPv4
> > gencrt "$IPV4_ADDR" "" # IPv4 literal in CN, no SAN IPv4
> > gencrt "$IPV4_ADDR" "$IPV4_ADDR" # IPv4 literal in CN, SAN IPv4
> > gencrt "$IPV6_NAME" "" # domain name in CN, no SAN IPv6
> > gencrt "$IPV6_NAME" "$IPV6_ADDR" # domain name in CN, SAN IPv6
> > gencrt "$IPV6_ADDR" "" # IPv6 literal in CN, no SAN IPv6
> > gencrt "$IPV6_ADDR" "$IPV6_ADDR" # IPv6 literal in CN, SAN IPv6
> >
> > # Print commands for the root user:
> > # - for making the CA a trusted CA
> > echo
> > echo install -o root -g root -m 644 -t /etc/pki/ca-trust/source/anchors \
> > "$PWD/$CA_NAME".crt
> > echo restorecon -Fvv /etc/pki/ca-trust/source/anchors/"$CA_NAME".crt
> > echo update-ca-trust extract
> >
> > # - and for extracting the CA certificates for OVMF.
> > if test -v MY_P11_KIT_PREFIX; then
> > echo mkdir -p -v /etc/pki/ca-trust/extracted/edk2
> > echo chmod -c --reference=/etc/pki/ca-trust/extracted/java \
> > /etc/pki/ca-trust/extracted/edk2
> > echo "$MY_P11_KIT_PREFIX/bin/p11-kit" extract --overwrite \
> > --format=edk2-cacerts \
> > --filter=ca-anchors \
> > --purpose=server-auth \
> > /etc/pki/ca-trust/extracted/edk2/cacerts.bin
> > echo chmod -c --reference=/etc/pki/ca-trust/extracted/java/cacerts \
> > /etc/pki/ca-trust/extracted/edk2/cacerts.bin
> > echo restorecon -FvvR /etc/pki/ca-trust/extracted/edk2
> > fi
>
> And here's the test matrix:
>
> > Server Certificate URL cURL edk2 unpatched edk2
> patched
> > --------------------- -------------------- ---------------- ---------------- --------------
> --
> > Common Subject hostname resolves status expected status
> expected status expected
> > Name Alt. Name to IPvX
> > -------------------------------------------------------------------------------------------
> ------
> > IP-literal - IP-literal IPv4 accept COMPAT/1 accept NO/2 reject
> yes
> > IP-literal - IP-literal IPv6 accept COMPAT/1 accept NO/2 reject
> yes
> > IP-literal - domainname IPv4 reject yes accept NO/2 reject
> yes
> > IP-literal - domainname IPv6 reject yes accept NO/2 reject
> yes
> > IP-literal IP IP-literal IPv4 accept yes accept yes accept yes
> > IP-literal IP IP-literal IPv6 accept yes accept yes accept yes
> > IP-literal IP domainname IPv4 reject yes accept NO/2 reject
> yes
> > IP-literal IP domainname IPv6 reject yes accept NO/2 reject
> yes
> > domainname - IP-literal IPv4 reject yes accept NO/2 reject
> yes
> > domainname - IP-literal IPv6 reject yes accept NO/2 reject
> yes
> > domainname - domainname IPv4 accept yes accept yes
> accept yes
> > domainname - domainname IPv6 accept yes accept yes
> accept yes
> > domainname IP IP-literal IPv4 accept yes accept yes accept
> yes
> > domainname IP IP-literal IPv6 accept yes accept yes accept
> yes
> > domainname IP domainname IPv4 accept yes accept yes
> accept yes
> > domainname IP domainname IPv6 accept yes accept yes
> accept yes
> >
> > #1 -- should not be accepted: an IP literal in the URL must match the IP
> > address in the SAN, regardless of the Common Name; but cURL accepts it
> > for compatibility
> >
> > #2 -- this is (or exemplifies) CVE-2019-14553
>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
>
> Thanks,
> Laszlo
>
> Laszlo Ersek (4):
> CryptoPkg/Crt: turn strchr() into a function (CVE-2019-14553)
> CryptoPkg/Crt: satisfy "inet_pton.c" dependencies (CVE-2019-14553)
> CryptoPkg/Crt: import "inet_pton.c" (CVE-2019-14553)
> CryptoPkg/TlsLib: TlsSetVerifyHost: parse IP address literals as such
> (CVE-2019-14553)
>
> Wu, Jiaxin (4):
> MdePkg/Include/Protocol/Tls.h: Add the data type of EfiTlsVerifyHost
> (CVE-2019-14553)
> CryptoPkg/TlsLib: Add the new API "TlsSetVerifyHost" (CVE-2019-14553)
> NetworkPkg/TlsDxe: Add the support of host validation to TlsDxe driver
> (CVE-2019-14553)
> NetworkPkg/HttpDxe: Set the HostName for the verification
> (CVE-2019-14553)
>
> CryptoPkg/Include/Library/TlsLib.h | 20 ++
> CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf | 1 +
> CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c | 5 +
> CryptoPkg/Library/BaseCryptLib/SysCall/inet_pton.c | 257
> ++++++++++++++++++++
> CryptoPkg/Library/Include/CrtLibSupport.h | 19 +-
> CryptoPkg/Library/Include/arpa/inet.h | 9 +
> CryptoPkg/Library/Include/arpa/nameser.h | 9 +
> CryptoPkg/Library/Include/netinet/in.h | 9 +
> CryptoPkg/Library/Include/sys/param.h | 9 +
> CryptoPkg/Library/Include/sys/socket.h | 9 +
> CryptoPkg/Library/TlsLib/TlsConfig.c | 58 ++++-
> MdePkg/Include/Protocol/Tls.h | 68 +++++-
> NetworkPkg/HttpDxe/HttpProto.h | 1 +
> NetworkPkg/HttpDxe/HttpsSupport.c | 21 +-
> NetworkPkg/TlsDxe/TlsProtocol.c | 44 +++-
> 15 files changed, 519 insertions(+), 20 deletions(-)
> create mode 100644 CryptoPkg/Library/Include/arpa/inet.h
> create mode 100644 CryptoPkg/Library/Include/arpa/nameser.h
> create mode 100644 CryptoPkg/Library/Include/netinet/in.h
> create mode 100644 CryptoPkg/Library/Include/sys/param.h
> create mode 100644 CryptoPkg/Library/Include/sys/socket.h
> create mode 100644 CryptoPkg/Library/BaseCryptLib/SysCall/inet_pton.c
>
> --
> 2.19.1.3.g30247aa5d201
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [edk2-devel] [PATCH v2 4/8] CryptoPkg/Crt: satisfy "inet_pton.c" dependencies (CVE-2019-14553)
2019-10-29 0:47 ` Laszlo Ersek
@ 2019-10-29 2:44 ` Wu, Jiaxin
2019-10-29 3:19 ` Wang, Jian J
0 siblings, 1 reply; 26+ messages in thread
From: Wu, Jiaxin @ 2019-10-29 2:44 UTC (permalink / raw)
To: devel@edk2.groups.io, lersek@redhat.com, David Woodhouse
Cc: Wang, Jian J, Sivaraman Nainar, Lu, XiaoyuX
> > Hm.
> >
> > If you're porting a whole standard C library to EDK2 then I suppose it
> > makes sense to build up all this infrastructure for it.
> >
> > But in this case when it's only the single inet_pton() function that
> > you need, perhaps it makes more sense to 'port' that one function to
> > UEFI (or just reimplement it looking like EDK2 code), instead of
> > bringing all this stuff along with it?
>
> I didn't want to take responsibility for touching any of that code -- I
> wanted it to be a piece of the puzzle that we'd just drop in. Its coding
> style is very foreign to edk2 norms, so once we started, we wouldn't
> stop before rewriting it more or less completely. (For example it quite
> frequently consumes the values that assignment expressions evaluate to,
> which is a huge no-no in edk2, as far as I understand.) I have no
> capacity for such a rework (or additional ownership / responsibility),
> sorry.
>
> I worked from Friday evening to Saturday ~6-7AM as my "second sprint" on
> this code and its testing, until I was satisfied with the test coverage.
> I apologize but I simply cannot repeat that. This is all I can
> contribute code-wise (and testing-wise) to fixing this issue.
Jian,
do you think it makes sense to keep the exiting coding style of inet_pton() in edk2\CryptoPkg\Library\BaseCryptLib\SysCall? (Personally, I can accept that).
>
> Thanks
> Laszlo
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [edk2-devel] [PATCH v2 4/8] CryptoPkg/Crt: satisfy "inet_pton.c" dependencies (CVE-2019-14553)
2019-10-29 2:44 ` [edk2-devel] " Wu, Jiaxin
@ 2019-10-29 3:19 ` Wang, Jian J
0 siblings, 0 replies; 26+ messages in thread
From: Wang, Jian J @ 2019-10-29 3:19 UTC (permalink / raw)
To: Wu, Jiaxin, devel@edk2.groups.io, lersek@redhat.com,
David Woodhouse
Cc: Sivaraman Nainar, Lu, XiaoyuX
Hi Jiaxin,
> -----Original Message-----
> From: Wu, Jiaxin <jiaxin.wu@intel.com>
> Sent: Tuesday, October 29, 2019 10:45 AM
> To: devel@edk2.groups.io; lersek@redhat.com; David Woodhouse
> <dwmw2@infradead.org>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Sivaraman Nainar
> <sivaramann@amiindia.co.in>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Subject: RE: [edk2-devel] [PATCH v2 4/8] CryptoPkg/Crt: satisfy "inet_pton.c"
> dependencies (CVE-2019-14553)
>
> > > Hm.
> > >
> > > If you're porting a whole standard C library to EDK2 then I suppose it
> > > makes sense to build up all this infrastructure for it.
> > >
> > > But in this case when it's only the single inet_pton() function that
> > > you need, perhaps it makes more sense to 'port' that one function to
> > > UEFI (or just reimplement it looking like EDK2 code), instead of
> > > bringing all this stuff along with it?
> >
> > I didn't want to take responsibility for touching any of that code -- I
> > wanted it to be a piece of the puzzle that we'd just drop in. Its coding
> > style is very foreign to edk2 norms, so once we started, we wouldn't
> > stop before rewriting it more or less completely. (For example it quite
> > frequently consumes the values that assignment expressions evaluate to,
> > which is a huge no-no in edk2, as far as I understand.) I have no
> > capacity for such a rework (or additional ownership / responsibility),
> > sorry.
> >
> > I worked from Friday evening to Saturday ~6-7AM as my "second sprint" on
> > this code and its testing, until I was satisfied with the test coverage.
> > I apologize but I simply cannot repeat that. This is all I can
> > contribute code-wise (and testing-wise) to fixing this issue.
>
>
> Jian,
>
> do you think it makes sense to keep the exiting coding style of inet_pton() in
> edk2\CryptoPkg\Library\BaseCryptLib\SysCall? (Personally, I can accept that).
>
As long as it's kept in SysCall, I'm ok with it.
Regards,
Jian
> >
> > Thanks
> > Laszlo
> >
> >
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553)
2019-10-26 5:37 [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553) Laszlo Ersek
` (8 preceding siblings ...)
2019-10-29 2:37 ` [edk2-devel] [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553) Wu, Jiaxin
@ 2019-10-31 9:28 ` Laszlo Ersek
2019-11-02 11:23 ` Laszlo Ersek
9 siblings, 1 reply; 26+ messages in thread
From: Laszlo Ersek @ 2019-10-31 9:28 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: David Woodhouse, Jian J Wang, Jiaxin Wu, Sivaraman Nainar,
Xiaoyu Lu
On 10/26/19 07:37, Laszlo Ersek wrote:
> Repo: https://github.com/lersek/edk2.git
> Branch: bz960_with_inet_pton_v2
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
> In v2, I have inserted 4 new patches in the middle, to satisfy two
> additional requirements raised by Siva and David:
>
> - If the Subject Alternative Name in the server certificate contains an
> IP address in binary representation, and the URL specifies an IP
> address in literal form for "hostname", then both of those things
> should be compared against each other, after converting the literal
> from the URL to binary representation. In other words, a server
> certificate with an IP address SAN should be recognized.
>
> - If the URL specifies an IP address literal, then, according to
> RFC-2818, "the iPAddress subjectAltName must be present in the
> certificate and must exactly match the IP in the URI". In other words,
> if a certificate matches the IP address literal from the URL via
> Common Name only, then the certificate must be rejected.
>
> I've also fixed two commit message warts in Jiaxin's patches (see the
> Notes sections on the patches).
>
> I've tested the series painstakingly. [...]
> And here's the test matrix:
>
>> Server Certificate URL cURL edk2 unpatched edk2 patched
>> --------------------- -------------------- ---------------- ---------------- ----------------
>> Common Subject hostname resolves status expected status expected status expected
>> Name Alt. Name to IPvX
>> -------------------------------------------------------------------------------------------------
>> IP-literal - IP-literal IPv4 accept COMPAT/1 accept NO/2 reject yes
>> IP-literal - IP-literal IPv6 accept COMPAT/1 accept NO/2 reject yes
>> IP-literal - domainname IPv4 reject yes accept NO/2 reject yes
>> IP-literal - domainname IPv6 reject yes accept NO/2 reject yes
>> IP-literal IP IP-literal IPv4 accept yes accept yes accept yes
>> IP-literal IP IP-literal IPv6 accept yes accept yes accept yes
>> IP-literal IP domainname IPv4 reject yes accept NO/2 reject yes
>> IP-literal IP domainname IPv6 reject yes accept NO/2 reject yes
>> domainname - IP-literal IPv4 reject yes accept NO/2 reject yes
>> domainname - IP-literal IPv6 reject yes accept NO/2 reject yes
>> domainname - domainname IPv4 accept yes accept yes accept yes
>> domainname - domainname IPv6 accept yes accept yes accept yes
>> domainname IP IP-literal IPv4 accept yes accept yes accept yes
>> domainname IP IP-literal IPv6 accept yes accept yes accept yes
>> domainname IP domainname IPv4 accept yes accept yes accept yes
>> domainname IP domainname IPv6 accept yes accept yes accept yes
>>
>> #1 -- should not be accepted: an IP literal in the URL must match the IP
>> address in the SAN, regardless of the Common Name; but cURL accepts it
>> for compatibility
>>
>> #2 -- this is (or exemplifies) CVE-2019-14553
Based on the feedback thus far, I'm planning to push this set on
Saturday (that is, after 1 week of list-time), or perhaps next Monday
(depends on how my Saturday will look).
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/8] CryptoPkg/TlsLib: Add the new API "TlsSetVerifyHost" (CVE-2019-14553)
2019-10-26 11:51 ` [edk2-devel] " Philippe Mathieu-Daudé
@ 2019-11-02 11:01 ` Laszlo Ersek
0 siblings, 0 replies; 26+ messages in thread
From: Laszlo Ersek @ 2019-11-02 11:01 UTC (permalink / raw)
To: devel, philmd
Cc: David Woodhouse, Jian J Wang, Jiaxin Wu, Sivaraman Nainar,
Xiaoyu Lu
On 10/26/19 13:51, Philippe Mathieu-Daudé wrote:
> On 10/26/19 7:37 AM, Laszlo Ersek wrote:
>> From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=960
>> CVE: CVE-2019-14553
>> In the patch, we add the new API "TlsSetVerifyHost" for the TLS
>> protocol to set the specified host name that need to be verified.
>>
>> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
>> Reviewed-by: Ye Ting <ting.ye@intel.com>
>> Reviewed-by: Long Qin <qin.long@intel.com>
>> Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
>> Acked-by: Laszlo Ersek <lersek@redhat.com>
>> Message-Id: <20190927034441.3096-3-Jiaxin.wu@intel.com>
>> Cc: David Woodhouse <dwmw2@infradead.org>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
>> Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
>> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>> v2:
>> - fix whitespace in subject line
>> - drop Contributed-under line per BZ#1373
>>
>> CryptoPkg/Include/Library/TlsLib.h | 20 +++++++++++
>> CryptoPkg/Library/TlsLib/TlsConfig.c | 38 +++++++++++++++++++-
>> 2 files changed, 57 insertions(+), 1 deletion(-)
>>
>> diff --git a/CryptoPkg/Include/Library/TlsLib.h
>> b/CryptoPkg/Include/Library/TlsLib.h
>> index 9875cb6e746b..3af7d4bc095e 100644
>> --- a/CryptoPkg/Include/Library/TlsLib.h
>> +++ b/CryptoPkg/Include/Library/TlsLib.h
>> @@ -395,8 +395,28 @@ TlsSetVerify (
>> IN VOID *Tls,
>> IN UINT32 VerifyMode
>> );
>> +/**
>> + Set the specified host name to be verified.
>> +
>> + @param[in] Tls Pointer to the TLS object.
>> + @param[in] Flags The setting flags during the validation.
>> + @param[in] HostName The specified host name to be verified.
>> +
>> + @retval EFI_SUCCESS The HostName setting was set
>> successfully.
>> + @retval EFI_INVALID_PARAMETER The parameter is invalid.
>> + @retval EFI_ABORTED Invalid HostName setting.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +TlsSetVerifyHost (
>> + IN VOID *Tls,
>> + IN UINT32 Flags,
>> + IN CHAR8 *HostName
>> + );
>> +
>> /**
>> Sets a TLS/SSL session ID to be used during TLS/SSL connect.
>> This function sets a session ID to be used when the TLS/SSL
>> connection is
>> diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c
>> b/CryptoPkg/Library/TlsLib/TlsConfig.c
>> index 74b577d60ee3..2bf5aee7c093 100644
>> --- a/CryptoPkg/Library/TlsLib/TlsConfig.c
>> +++ b/CryptoPkg/Library/TlsLib/TlsConfig.c
>> @@ -1,8 +1,8 @@
>> /** @file
>> SSL/TLS Configuration Library Wrapper Implementation over OpenSSL.
>> -Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
>> +Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
>> (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>> SPDX-License-Identifier: BSD-2-Clause-Patent
>> **/
>> @@ -496,8 +496,44 @@ TlsSetVerify (
>> //
>> SSL_set_verify (TlsConn->Ssl, VerifyMode, NULL);
>> }
>> +/**
>> + Set the specified host name to be verified.
>> +
>> + @param[in] Tls Pointer to the TLS object.
>> + @param[in] Flags The setting flags during the validation.
>> + @param[in] HostName The specified host name to be verified.
>> +
>> + @retval EFI_SUCCESS The HostName setting was set
>> successfully.
>> + @retval EFI_INVALID_PARAMETER The parameter is invalid.
>> + @retval EFI_ABORTED Invalid HostName setting.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +TlsSetVerifyHost (
>> + IN VOID *Tls,
>> + IN UINT32 Flags,
>> + IN CHAR8 *HostName
>> + )
>> +{
>> + TLS_CONNECTION *TlsConn;
>> +
>> + TlsConn = (TLS_CONNECTION *) Tls;
>> + if (TlsConn == NULL || TlsConn->Ssl == NULL || HostName == NULL) {
>
> Nitpicking, I'd check HostName first.
Valid point... but this BZ / CVE has been in flight for ~1.5 years
now... Let me just run with this patch please.
> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Thanks!
Laszlo
>
>> + return EFI_INVALID_PARAMETER;
>> + }
>> +
>> + SSL_set_hostflags(TlsConn->Ssl, Flags);
>> +
>> + if (SSL_set1_host(TlsConn->Ssl, HostName) == 0) {
>> + return EFI_ABORTED;
>> + }
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>> /**
>> Sets a TLS/SSL session ID to be used during TLS/SSL connect.
>> This function sets a session ID to be used when the TLS/SSL
>> connection is
>>
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553)
2019-10-29 2:37 ` [edk2-devel] [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553) Wu, Jiaxin
@ 2019-11-02 11:15 ` Laszlo Ersek
0 siblings, 0 replies; 26+ messages in thread
From: Laszlo Ersek @ 2019-11-02 11:15 UTC (permalink / raw)
To: devel, jiaxin.wu
Cc: David Woodhouse, Wang, Jian J, Sivaraman Nainar, Lu, XiaoyuX
On 10/29/19 03:37, Wu, Jiaxin wrote:
> Test matrix - that's a great summary! The result is also good to me.
>
> Thanks Laszlo's patches to fix the gap.
>
> Series Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Thanks, I've added this tag to the middle four patches (the ones that I
wrote).
I'm not adding this tag to the other four patches (the first two and the
last two), because you authored those, and I didn't modify them (apart
from the trivial commit message changes that I listed in the Notes
sections of the individual patch emails). The idea is that a patch
authored by a particular contributor does not benefit from a review by
the same contributor. (It would amount to saying "I agree with the code
that I wrote" -- it's tautological.)
Thanks!
Laszlo
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
>> Ersek
>> Sent: Saturday, October 26, 2019 1:37 PM
>> To: edk2-devel-groups-io <devel@edk2.groups.io>
>> Cc: David Woodhouse <dwmw2@infradead.org>; Wang, Jian J
>> <jian.j.wang@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Sivaraman
>> Nainar <sivaramann@amiindia.co.in>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
>> Subject: [edk2-devel] [PATCH v2 0/8] support server identity validation in
>> HTTPS Boot (CVE-2019-14553)
>>
>> Repo: https://github.com/lersek/edk2.git
>> Branch: bz960_with_inet_pton_v2
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
>>
>> Previous posting from Jiaxin:
>>
>> [edk2-devel] [PATCH v1 0/4]
>> Support HTTPS HostName validation feature(CVE-2019-14553)
>>
>> https://edk2.groups.io/g/devel/message/48183
>> http://mid.mail-archive.com/20190927034441.3096-1-Jiaxin.wu@intel.com
>>
>> In v2, I have inserted 4 new patches in the middle, to satisfy two
>> additional requirements raised by Siva and David:
>>
>> - If the Subject Alternative Name in the server certificate contains an
>> IP address in binary representation, and the URL specifies an IP
>> address in literal form for "hostname", then both of those things
>> should be compared against each other, after converting the literal
>> from the URL to binary representation. In other words, a server
>> certificate with an IP address SAN should be recognized.
>>
>> - If the URL specifies an IP address literal, then, according to
>> RFC-2818, "the iPAddress subjectAltName must be present in the
>> certificate and must exactly match the IP in the URI". In other words,
>> if a certificate matches the IP address literal from the URL via
>> Common Name only, then the certificate must be rejected.
>>
>> I've also fixed two commit message warts in Jiaxin's patches (see the
>> Notes sections on the patches).
>>
>> I've tested the series painstakingly. Here's the script I wrote for
>> certificate generation:
>>
>>> ## @file
>>> # Bash shell script for generating test certificates, for
>>> # <https://bugzilla.tianocore.org/show_bug.cgi?id=960>.
>>> #
>>> # Copyright (C) 2019, Red Hat, Inc.
>>> #
>>> # SPDX-License-Identifier: BSD-2-Clause-Patent
>>> #
>>> # Customize te variables in section "Configuration", then run the script with
>>> # "bash gencerts.sh".
>>> #
>>> # The script creates 17 files in the current working directory:
>>> # - one CA certificate (note: key is discarded);
>>> #
>>> # - for the (IPv4 domain name, IPv4 address) pair, one keypair (that is, a
>>> # CA-issued certificate, plus the private key) for each case below:
>>> # - Common Name = IPv4 domain name, no subjectAltName,
>>> # - Common Name = IPv4 domain name, IPv4 address in
>> subjectAltName,
>>> # - Common Name = IPv4 address literal, no subjectAltName,
>>> # - Common Name = IPv4 address literal, IPv4 address in subjectAltName;
>>> #
>>> # - for the (IPv6 domain name, IPv6 address) pair, a similar set of files.
>>> #
>>> # Finally, the script prints some commands for the root user that are
>> related
>>> # to the following OVMF feature: OVMF can HTTPS boot while trusting the
>> same
>>> # set of CA certificates that the virt host trusts. The commands install the
>>> # new CA certificate on the host (note: this should never be done in
>>> # production, in spite of the CA key being discarded), and also extract all CA
>>> # certs in the format that OVMF expects. (This edk2-specific extraction is
>>> # normally performed by the "update-ca-trust" command, but if yours isn't
>>> # up-to-date enough for that, build and install p11-kit from source, and set
>>> # MY_P11_KIT_PREFIX, before invoking this script.) See
>> "OvmfPkg/README" for
>>> # passing the extracted CA certs to OVMF on the QEMU cmdline.
>>> ##
>>> set -e -u -C
>>>
>>> # Configuration.
>>> CA_NAME=TianoCore_BZ_960_CA
>>> IPV4_NAME=ipv4-server
>>> IPV4_ADDR=192.168.124.2
>>> IPV6_NAME=ipv6-server
>>> IPV6_ADDR=fd33:eb1b:9b36::2
>>>
>>> # Create a temporary directory for transient files.
>>> TMP_D=$(mktemp -d)
>>> trap 'rm -f -r -- "$TMP_D"' EXIT
>>>
>>> # Set some helper variables.
>>> TMP_EXT=$TMP_D/ext # OpenSSL extensions
>>> TMP_CSR=$TMP_D/csr # certificate request
>>> TMP_CA_KEY=$TMP_D/ca.key # CA key
>>> TMP_CA_SRL=$TMP_D/ca.srl # CA serial number
>>>
>>> # Generate the CA certificate.
>>> openssl req -x509 -nodes \
>>> -subj /CN="$CA_NAME" \
>>> -out "$CA_NAME".crt \
>>> -keyout "$TMP_CA_KEY"
>>>
>>> # Create a CA-issued certificate.
>>> # Parameters:
>>> # $1: Common Name
>>> # $2: IPv4 or IPv6 address literal, to be used in SAN; or empty string
>>> gencrt()
>>> {
>>> local CN="$1"
>>> local SANIP="$2"
>>> local STEM
>>> local EXT
>>>
>>> if test -z "$SANIP"; then
>>> # File name stem consists of Common Name only. No certificate
>> extensions.
>>> STEM=svr_$CN
>>> EXT=
>>> else
>>> # File name stem includes Common Name and IP address literal.
>>> STEM=svr_${CN}_${SANIP}
>>>
>>> # SAN IP extension in the certificate. Rewrite the ad-hoc extensions file
>>> # with the current SAN IP.
>>> echo "subjectAltName=IP:$SANIP" >| "$TMP_EXT"
>>> EXT="-extfile $TMP_EXT"
>>> fi
>>> STEM=${STEM//[:.]/_}
>>>
>>> # Generate CSR.
>>> openssl req -new -nodes \
>>> -subj /CN="$CN" \
>>> -out "$TMP_CSR" \
>>> -keyout "$STEM".key
>>>
>>> # Sign the certificate request, potentially adding the SAN IP.
>>> openssl x509 -req -CAcreateserial $EXT \
>>> -in "$TMP_CSR" \
>>> -out "$STEM".crt \
>>> -CA "$CA_NAME".crt \
>>> -CAkey "$TMP_CA_KEY" \
>>> -CAserial "$TMP_CA_SRL"
>>> }
>>>
>>> # Generate all certificates.
>>> gencrt "$IPV4_NAME" "" # domain name in CN, no SAN IPv4
>>> gencrt "$IPV4_NAME" "$IPV4_ADDR" # domain name in CN, SAN IPv4
>>> gencrt "$IPV4_ADDR" "" # IPv4 literal in CN, no SAN IPv4
>>> gencrt "$IPV4_ADDR" "$IPV4_ADDR" # IPv4 literal in CN, SAN IPv4
>>> gencrt "$IPV6_NAME" "" # domain name in CN, no SAN IPv6
>>> gencrt "$IPV6_NAME" "$IPV6_ADDR" # domain name in CN, SAN IPv6
>>> gencrt "$IPV6_ADDR" "" # IPv6 literal in CN, no SAN IPv6
>>> gencrt "$IPV6_ADDR" "$IPV6_ADDR" # IPv6 literal in CN, SAN IPv6
>>>
>>> # Print commands for the root user:
>>> # - for making the CA a trusted CA
>>> echo
>>> echo install -o root -g root -m 644 -t /etc/pki/ca-trust/source/anchors \
>>> "$PWD/$CA_NAME".crt
>>> echo restorecon -Fvv /etc/pki/ca-trust/source/anchors/"$CA_NAME".crt
>>> echo update-ca-trust extract
>>>
>>> # - and for extracting the CA certificates for OVMF.
>>> if test -v MY_P11_KIT_PREFIX; then
>>> echo mkdir -p -v /etc/pki/ca-trust/extracted/edk2
>>> echo chmod -c --reference=/etc/pki/ca-trust/extracted/java \
>>> /etc/pki/ca-trust/extracted/edk2
>>> echo "$MY_P11_KIT_PREFIX/bin/p11-kit" extract --overwrite \
>>> --format=edk2-cacerts \
>>> --filter=ca-anchors \
>>> --purpose=server-auth \
>>> /etc/pki/ca-trust/extracted/edk2/cacerts.bin
>>> echo chmod -c --reference=/etc/pki/ca-trust/extracted/java/cacerts \
>>> /etc/pki/ca-trust/extracted/edk2/cacerts.bin
>>> echo restorecon -FvvR /etc/pki/ca-trust/extracted/edk2
>>> fi
>>
>> And here's the test matrix:
>>
>>> Server Certificate URL cURL edk2 unpatched edk2
>> patched
>>> --------------------- -------------------- ---------------- ---------------- --------------
>> --
>>> Common Subject hostname resolves status expected status
>> expected status expected
>>> Name Alt. Name to IPvX
>>> -------------------------------------------------------------------------------------------
>> ------
>>> IP-literal - IP-literal IPv4 accept COMPAT/1 accept NO/2 reject
>> yes
>>> IP-literal - IP-literal IPv6 accept COMPAT/1 accept NO/2 reject
>> yes
>>> IP-literal - domainname IPv4 reject yes accept NO/2 reject
>> yes
>>> IP-literal - domainname IPv6 reject yes accept NO/2 reject
>> yes
>>> IP-literal IP IP-literal IPv4 accept yes accept yes accept yes
>>> IP-literal IP IP-literal IPv6 accept yes accept yes accept yes
>>> IP-literal IP domainname IPv4 reject yes accept NO/2 reject
>> yes
>>> IP-literal IP domainname IPv6 reject yes accept NO/2 reject
>> yes
>>> domainname - IP-literal IPv4 reject yes accept NO/2 reject
>> yes
>>> domainname - IP-literal IPv6 reject yes accept NO/2 reject
>> yes
>>> domainname - domainname IPv4 accept yes accept yes
>> accept yes
>>> domainname - domainname IPv6 accept yes accept yes
>> accept yes
>>> domainname IP IP-literal IPv4 accept yes accept yes accept
>> yes
>>> domainname IP IP-literal IPv6 accept yes accept yes accept
>> yes
>>> domainname IP domainname IPv4 accept yes accept yes
>> accept yes
>>> domainname IP domainname IPv6 accept yes accept yes
>> accept yes
>>>
>>> #1 -- should not be accepted: an IP literal in the URL must match the IP
>>> address in the SAN, regardless of the Common Name; but cURL accepts it
>>> for compatibility
>>>
>>> #2 -- this is (or exemplifies) CVE-2019-14553
>>
>> Cc: David Woodhouse <dwmw2@infradead.org>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
>> Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
>> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
>>
>> Thanks,
>> Laszlo
>>
>> Laszlo Ersek (4):
>> CryptoPkg/Crt: turn strchr() into a function (CVE-2019-14553)
>> CryptoPkg/Crt: satisfy "inet_pton.c" dependencies (CVE-2019-14553)
>> CryptoPkg/Crt: import "inet_pton.c" (CVE-2019-14553)
>> CryptoPkg/TlsLib: TlsSetVerifyHost: parse IP address literals as such
>> (CVE-2019-14553)
>>
>> Wu, Jiaxin (4):
>> MdePkg/Include/Protocol/Tls.h: Add the data type of EfiTlsVerifyHost
>> (CVE-2019-14553)
>> CryptoPkg/TlsLib: Add the new API "TlsSetVerifyHost" (CVE-2019-14553)
>> NetworkPkg/TlsDxe: Add the support of host validation to TlsDxe driver
>> (CVE-2019-14553)
>> NetworkPkg/HttpDxe: Set the HostName for the verification
>> (CVE-2019-14553)
>>
>> CryptoPkg/Include/Library/TlsLib.h | 20 ++
>> CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf | 1 +
>> CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c | 5 +
>> CryptoPkg/Library/BaseCryptLib/SysCall/inet_pton.c | 257
>> ++++++++++++++++++++
>> CryptoPkg/Library/Include/CrtLibSupport.h | 19 +-
>> CryptoPkg/Library/Include/arpa/inet.h | 9 +
>> CryptoPkg/Library/Include/arpa/nameser.h | 9 +
>> CryptoPkg/Library/Include/netinet/in.h | 9 +
>> CryptoPkg/Library/Include/sys/param.h | 9 +
>> CryptoPkg/Library/Include/sys/socket.h | 9 +
>> CryptoPkg/Library/TlsLib/TlsConfig.c | 58 ++++-
>> MdePkg/Include/Protocol/Tls.h | 68 +++++-
>> NetworkPkg/HttpDxe/HttpProto.h | 1 +
>> NetworkPkg/HttpDxe/HttpsSupport.c | 21 +-
>> NetworkPkg/TlsDxe/TlsProtocol.c | 44 +++-
>> 15 files changed, 519 insertions(+), 20 deletions(-)
>> create mode 100644 CryptoPkg/Library/Include/arpa/inet.h
>> create mode 100644 CryptoPkg/Library/Include/arpa/nameser.h
>> create mode 100644 CryptoPkg/Library/Include/netinet/in.h
>> create mode 100644 CryptoPkg/Library/Include/sys/param.h
>> create mode 100644 CryptoPkg/Library/Include/sys/socket.h
>> create mode 100644 CryptoPkg/Library/BaseCryptLib/SysCall/inet_pton.c
>>
>> --
>> 2.19.1.3.g30247aa5d201
>>
>>
>>
>
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553)
2019-10-31 9:28 ` Laszlo Ersek
@ 2019-11-02 11:23 ` Laszlo Ersek
0 siblings, 0 replies; 26+ messages in thread
From: Laszlo Ersek @ 2019-11-02 11:23 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: David Woodhouse, Jian J Wang, Jiaxin Wu, Sivaraman Nainar,
Xiaoyu Lu
On 10/31/19 10:28, Laszlo Ersek wrote:
> On 10/26/19 07:37, Laszlo Ersek wrote:
>> Repo: https://github.com/lersek/edk2.git
>> Branch: bz960_with_inet_pton_v2
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
>
>> In v2, I have inserted 4 new patches in the middle, to satisfy two
>> additional requirements raised by Siva and David:
>>
>> - If the Subject Alternative Name in the server certificate contains an
>> IP address in binary representation, and the URL specifies an IP
>> address in literal form for "hostname", then both of those things
>> should be compared against each other, after converting the literal
>> from the URL to binary representation. In other words, a server
>> certificate with an IP address SAN should be recognized.
>>
>> - If the URL specifies an IP address literal, then, according to
>> RFC-2818, "the iPAddress subjectAltName must be present in the
>> certificate and must exactly match the IP in the URI". In other words,
>> if a certificate matches the IP address literal from the URL via
>> Common Name only, then the certificate must be rejected.
>>
>> I've also fixed two commit message warts in Jiaxin's patches (see the
>> Notes sections on the patches).
>>
>> I've tested the series painstakingly. [...]
>
>> And here's the test matrix:
>>
>>> Server Certificate URL cURL edk2 unpatched edk2 patched
>>> --------------------- -------------------- ---------------- ---------------- ----------------
>>> Common Subject hostname resolves status expected status expected status expected
>>> Name Alt. Name to IPvX
>>> -------------------------------------------------------------------------------------------------
>>> IP-literal - IP-literal IPv4 accept COMPAT/1 accept NO/2 reject yes
>>> IP-literal - IP-literal IPv6 accept COMPAT/1 accept NO/2 reject yes
>>> IP-literal - domainname IPv4 reject yes accept NO/2 reject yes
>>> IP-literal - domainname IPv6 reject yes accept NO/2 reject yes
>>> IP-literal IP IP-literal IPv4 accept yes accept yes accept yes
>>> IP-literal IP IP-literal IPv6 accept yes accept yes accept yes
>>> IP-literal IP domainname IPv4 reject yes accept NO/2 reject yes
>>> IP-literal IP domainname IPv6 reject yes accept NO/2 reject yes
>>> domainname - IP-literal IPv4 reject yes accept NO/2 reject yes
>>> domainname - IP-literal IPv6 reject yes accept NO/2 reject yes
>>> domainname - domainname IPv4 accept yes accept yes accept yes
>>> domainname - domainname IPv6 accept yes accept yes accept yes
>>> domainname IP IP-literal IPv4 accept yes accept yes accept yes
>>> domainname IP IP-literal IPv6 accept yes accept yes accept yes
>>> domainname IP domainname IPv4 accept yes accept yes accept yes
>>> domainname IP domainname IPv6 accept yes accept yes accept yes
>>>
>>> #1 -- should not be accepted: an IP literal in the URL must match the IP
>>> address in the SAN, regardless of the Common Name; but cURL accepts it
>>> for compatibility
>>>
>>> #2 -- this is (or exemplifies) CVE-2019-14553
>
> Based on the feedback thus far, I'm planning to push this set on
> Saturday (that is, after 1 week of list-time), or perhaps next Monday
> (depends on how my Saturday will look).
Pushed as commit range b15646484eaf..e2fc50812895.
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2019-11-02 11:24 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-26 5:37 [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553) Laszlo Ersek
2019-10-26 5:37 ` [PATCH v2 1/8] MdePkg/Include/Protocol/Tls.h: Add the data type of EfiTlsVerifyHost (CVE-2019-14553) Laszlo Ersek
2019-10-28 8:12 ` [edk2-devel] " Liming Gao
2019-10-26 5:37 ` [PATCH v2 2/8] CryptoPkg/TlsLib: Add the new API "TlsSetVerifyHost" (CVE-2019-14553) Laszlo Ersek
2019-10-26 11:51 ` [edk2-devel] " Philippe Mathieu-Daudé
2019-11-02 11:01 ` Laszlo Ersek
2019-10-28 5:28 ` Wang, Jian J
2019-10-26 5:37 ` [PATCH v2 3/8] CryptoPkg/Crt: turn strchr() into a function (CVE-2019-14553) Laszlo Ersek
2019-10-26 11:47 ` [edk2-devel] " Philippe Mathieu-Daudé
2019-10-28 5:12 ` Wang, Jian J
2019-10-26 5:37 ` [PATCH v2 4/8] CryptoPkg/Crt: satisfy "inet_pton.c" dependencies (CVE-2019-14553) Laszlo Ersek
2019-10-28 5:34 ` Wang, Jian J
2019-10-28 13:06 ` David Woodhouse
2019-10-29 0:47 ` Laszlo Ersek
2019-10-29 2:44 ` [edk2-devel] " Wu, Jiaxin
2019-10-29 3:19 ` Wang, Jian J
2019-10-26 5:37 ` [PATCH v2 5/8] CryptoPkg/Crt: import "inet_pton.c" (CVE-2019-14553) Laszlo Ersek
2019-10-28 6:16 ` Wang, Jian J
2019-10-26 5:37 ` [PATCH v2 6/8] CryptoPkg/TlsLib: TlsSetVerifyHost: parse IP address literals as such (CVE-2019-14553) Laszlo Ersek
2019-10-28 6:12 ` Wang, Jian J
2019-10-26 5:37 ` [PATCH v2 7/8] NetworkPkg/TlsDxe: Add the support of host validation to TlsDxe driver (CVE-2019-14553) Laszlo Ersek
2019-10-26 5:37 ` [PATCH v2 8/8] NetworkPkg/HttpDxe: Set the HostName for the verification (CVE-2019-14553) Laszlo Ersek
2019-10-29 2:37 ` [edk2-devel] [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553) Wu, Jiaxin
2019-11-02 11:15 ` Laszlo Ersek
2019-10-31 9:28 ` Laszlo Ersek
2019-11-02 11:23 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox