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...@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 <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 (#49886): https://edk2.groups.io/g/devel/message/49886 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] -=-=-=-=-=-=-=-=-=-=-=-
