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
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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#49462): https://edk2.groups.io/g/devel/message/49462 Mute This Topic: https://groups.io/mt/37952584/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#49743): https://edk2.groups.io/g/devel/message/49743 Mute This Topic: https://groups.io/mt/37952584/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#49887): https://edk2.groups.io/g/devel/message/49887 Mute This Topic: https://groups.io/mt/37952584/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#49575): https://edk2.groups.io/g/devel/message/49575 Mute This Topic: https://groups.io/mt/37952584/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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 >> >> >> > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#49886): https://edk2.groups.io/g/devel/message/49886 Mute This Topic: https://groups.io/mt/37952584/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.