From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web11.6070.1572693334066426567 for ; Sat, 02 Nov 2019 04:15:34 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=IrpymYoQ; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1572693333; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=OZx99XVbCWTdfGPE6XqFAQtph4uDShQ1up7WGaBeCxw=; b=IrpymYoQBUqQUkQlTtvtSXR+unJlhrV4hqjBM6848k/3U05OjMD0cQ0TzyjS4xa7ibo/tc I3RrRrAVNUN1S9l3jqk6rVxsgDk4eFrnsQsygty07qImfhCEMmyFypUCIb3REGRw3i8ELU Y6lhZPbOVXSMWBS0GLuybNwGzljur6o= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-93-syplzDS9N4Wv4F3hjAnzyg-1; Sat, 02 Nov 2019 07:15:30 -0400 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 7D84C2B8; Sat, 2 Nov 2019 11:15:29 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-108.ams2.redhat.com [10.36.116.108]) by smtp.corp.redhat.com (Postfix) with ESMTP id B137E1001B23; Sat, 2 Nov 2019 11:15:26 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553) To: devel@edk2.groups.io, jiaxin.wu@intel.com Cc: David Woodhouse , "Wang, Jian J" , Sivaraman Nainar , "Lu, XiaoyuX" References: <20191026053719.10453-1-lersek@redhat.com> <895558F6EA4E3B41AC93A00D163B727416F915C1@SHSMSX107.ccr.corp.intel.com> From: "Laszlo Ersek" Message-ID: <08326907-55cd-7d6e-4ff2-5c0bdafa1eb8@redhat.com> Date: Sat, 2 Nov 2019 12:15:25 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <895558F6EA4E3B41AC93A00D163B727416F915C1@SHSMSX107.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-MC-Unique: syplzDS9N4Wv4F3hjAnzyg-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 10/29/19 03:37, Wu, Jiaxin wrote: > Test matrix - that's a great summary! The result is also good to me. >=20 > Thanks Laszlo's patches to fix the gap. >=20 > Series Reviewed-by: Jiaxin Wu 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 On Behalf Of Laszlo >> Ersek >> Sent: Saturday, October 26, 2019 1:37 PM >> To: edk2-devel-groups-io >> Cc: David Woodhouse ; Wang, Jian J >> ; Wu, Jiaxin ; Sivaraman >> Nainar ; Lu, XiaoyuX >> 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=3D960 >> >> 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 >>> # . >>> # >>> # Copyright (C) 2019, Red Hat, Inc. >>> # >>> # SPDX-License-Identifier: BSD-2-Clause-Patent >>> # >>> # Customize te variables in section "Configuration", then run the scri= pt 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 i= s, a >>> # CA-issued certificate, plus the private key) for each case below: >>> # - Common Name =3D IPv4 domain name, no subjectAltName, >>> # - Common Name =3D IPv4 domain name, IPv4 address in >> subjectAltName, >>> # - Common Name =3D IPv4 address literal, no subjectAltName, >>> # - Common Name =3D IPv4 address literal, IPv4 address in subjectAlt= Name; >>> # >>> # - for the (IPv6 domain name, IPv6 address) pair, a similar set of fi= les. >>> # >>> # Finally, the script prints some commands for the root user that are >> related >>> # to the following OVMF feature: OVMF can HTTPS boot while trusting th= e >> same >>> # set of CA certificates that the virt host trusts. The commands insta= ll 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 extrac= t all CA >>> # certs in the format that OVMF expects. (This edk2-specific extractio= n is >>> # normally performed by the "update-ca-trust" command, but if yours is= n't >>> # up-to-date enough for that, build and install p11-kit from source, a= nd 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=3DTianoCore_BZ_960_CA >>> IPV4_NAME=3Dipv4-server >>> IPV4_ADDR=3D192.168.124.2 >>> IPV6_NAME=3Dipv6-server >>> IPV6_ADDR=3Dfd33:eb1b:9b36::2 >>> >>> # Create a temporary directory for transient files. >>> TMP_D=3D$(mktemp -d) >>> trap 'rm -f -r -- "$TMP_D"' EXIT >>> >>> # Set some helper variables. >>> TMP_EXT=3D$TMP_D/ext # OpenSSL extensions >>> TMP_CSR=3D$TMP_D/csr # certificate request >>> TMP_CA_KEY=3D$TMP_D/ca.key # CA key >>> TMP_CA_SRL=3D$TMP_D/ca.srl # CA serial number >>> >>> # Generate the CA certificate. >>> openssl req -x509 -nodes \ >>> -subj /CN=3D"$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=3D"$1" >>> local SANIP=3D"$2" >>> local STEM >>> local EXT >>> >>> if test -z "$SANIP"; then >>> # File name stem consists of Common Name only. No certificate >> extensions. >>> STEM=3Dsvr_$CN >>> EXT=3D >>> else >>> # File name stem includes Common Name and IP address literal. >>> STEM=3Dsvr_${CN}_${SANIP} >>> >>> # SAN IP extension in the certificate. Rewrite the ad-hoc extensio= ns file >>> # with the current SAN IP. >>> echo "subjectAltName=3DIP:$SANIP" >| "$TMP_EXT" >>> EXT=3D"-extfile $TMP_EXT" >>> fi >>> STEM=3D${STEM//[:.]/_} >>> >>> # Generate CSR. >>> openssl req -new -nodes \ >>> -subj /CN=3D"$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/anchor= s \ >>> "$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=3D/etc/pki/ca-trust/extracted/java \ >>> /etc/pki/ca-trust/extracted/edk2 >>> echo "$MY_P11_KIT_PREFIX/bin/p11-kit" extract --overwrite \ >>> --format=3Dedk2-cacerts \ >>> --filter=3Dca-anchors \ >>> --purpose=3Dserver-auth \ >>> /etc/pki/ca-trust/extracted/edk2/cacerts.bin >>> echo chmod -c --reference=3D/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 un= patched 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 >> Cc: Jian J Wang >> Cc: Jiaxin Wu >> Cc: Sivaraman Nainar >> Cc: Xiaoyu Lu >> >> 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 drive= r >> (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 >> >> >> >=20 >=20 >=20 >=20