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...@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 <dw...@infradead.org>; Wang, Jian J > <jian.j.w...@intel.com>; Wu, Jiaxin <jiaxin...@intel.com>; Sivaraman > Nainar <sivaram...@amiindia.co.in>; Lu, XiaoyuX <xiaoyux...@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 > 20190927034441.3096-1-Jiaxin.wu@intel.com">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 <dw...@infradead.org> > Cc: Jian J Wang <jian.j.w...@intel.com> > Cc: Jiaxin Wu <jiaxin...@intel.com> > Cc: Sivaraman Nainar <sivaram...@amiindia.co.in> > Cc: Xiaoyu Lu <xiaoyux...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
