public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Laszlo Ersek" <lersek@redhat.com>
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: David Woodhouse <dwmw2@infradead.org>,
	Jian J Wang <jian.j.wang@intel.com>,
	Jiaxin Wu <jiaxin.wu@intel.com>,
	Sivaraman Nainar <sivaramann@amiindia.co.in>,
	Xiaoyu Lu <xiaoyux.lu@intel.com>
Subject: [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553)
Date: Sat, 26 Oct 2019 07:37:11 +0200	[thread overview]
Message-ID: <20191026053719.10453-1-lersek@redhat.com> (raw)

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


             reply	other threads:[~2019-10-26  5:37 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-26  5:37 Laszlo Ersek [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191026053719.10453-1-lersek@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox