Re: [edk2-devel] [PATCH v1 0/4] Support HTTPS HostName validation feature(CVE-2019-14553)

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 11672 bytes --]

Below summary of setup from Laszlo are also the part of our test cases. Beside those verification that only focus on the subject Common Name (CN), we also created the certificate contains Subject Alternative Name (SAN) to cover more test cases related to the subjectAltName. That’s also the parts of hostname verification. With the subjectAltName, we can set the multiple subject names (including IP address format defined in RFC 5280) in one certificate to pass the different URIs (IPv4/IPv6) retrieved from the DHCP server.

> So, my take is that the comparison is done simply on the textual
> representation (with the IPv6 brackets stripped), not the numerical
> value.
> Is that bad? The textual comparison may certainly report a mismatch when
> the numerical values actually match (for an IPv4 example, "192.168.0.1"
> would not match "192.168.000.001"). But that errs in the safe direction,
> does it not?

I think it’s not the problem here by setting textual IP in hostname since the numerical IP is not the target to be vitrificated. And I also post my explain in the previous email:



> UEFI TLS only provides the *HostName* verification interface to upper driver (HttpDxe),

> not the IP/email verification capability. Please refer to UEFI Spec2.8 section 28.10.2:

>    "...TLS session hostname for validation which is used to verify whether the name

>     within the peer certificate matches a given host name..."

> In upper UEFI HTTP driver, we get the hostname from URI directly no matter it's the real

> FQDN (www.xxx.com<http://www.xxx.com>) or IP address format string (1.2.3.4 or 2001:8b0:10b::5 (not "[2001:8b0:10b::5])),

> and set it to the TLS hostname filed via the interface -- EFI_TLS_VERIFY_HOST.

> That's implementation choice for HttpDxe to achieve the HTTPS HostName validation feature

> by following the standard TLS HostName verification capability.



If above is incorrect, please correct me.

Anyway, thanks Laszlo and David’s comments.

Jiaxin

From: David Woodhouse <dwmw2@infradead.org>
Sent: Thursday, October 10, 2019 4:34 AM
To: Laszlo Ersek <lersek@redhat.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Richard Levitte <levitte@openssl.org>
Subject: Re: [edk2-devel] [PATCH v1 0/4] Support HTTPS HostName validation feature(CVE-2019-14553)

Can you show result of 'openssl x509 -noout -text -in xxxxxx.pem' on your certs please.

Would like to check if you really have a cert for the hostname string "192.168.124.2" or to the IP address. They are different things.

On 9 October 2019 21:24:34 BST, Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>> wrote:

Hi All,

(multi-hour composition ahead...)

On 10/09/19 09:53, David Woodhouse wrote:

On Tue, 2019-10-08 at 06:19 +0000, Wu, Jiaxin wrote:

 Hi David,

 I just realized you have the comments on Bugzilla 960:

"...given that testing is failing and code inspection shows it
would never have been expected to work."

 Do you mean you didn't pass the verification if URLs with IPv6
 literals (https://[2001:8b0:10b:1236::1]/)?  Can you also show me
 where the code inspection indicated it would never have been expected
 to work? We do pass the testing for the URLs with IPv6 if the CN or
 SAN in certificate has the corresponding IPv6 address (at least
 working with openssl 1.1.0).

 I have not tested this, but I started looking when there was a message
 on the edk2 list from someone who was reporting that it didn't work
 for IPv6 URIs, IIRC.

 You are using SSL_set1_host(), and I believe you're just passing in
 the bare hostname part of the URI, be it "1.2.3.4" or
 "[2001:8b0:10b::5]".

 That just adds it to the 'hosts' list in the X509_VERIFY_PARAM for the
 SSL connection.

 In the check_hosts() function in openssl/crypto/x509/v509_vfy.c, the
 code simply iterates over the members of that list, calling
 X509_check_host() for each one. It never calls X509_check_ip().

 If you look in openssl/crypto/x509/v3_utl.c you can see the
 X509_check_host() really does only check hostnames. You'd need to call
 X509_check_ip_asc() to check hostnames. And something would need to
 have stripped the [] which surround an IPv6 literal.

 I can't see how this can work. Have you tested it since the report on
 the list that it wasn't working?

 cf. https://github.com/openssl/openssl/pull/9201 which is being
 ignored by the OpenSSL developers \x14 OpenSSL really doesn't make
 life easy for you here, which is a shame.


For the series patches here, we are intending to support the host
name validation, I think we can commit the series patches since we
pass the verification of IPV6 URL, what do you think?

 If it passes the verification of IPv6 literals, then all my analysis
 is broken and so was the report on the list that prompted me to start
 looking (or I'm misremembering that report). In that case, sure, go
 ahead and commit.

Here's a summary of my setup.

* I've generated a brand new CA certificate, and two HTTP server
  certificates, signed by the CA.

* One HTTP server certificate is for Common Name = 192.168.124.2

* The other HTTP server certificate is for Common Name =
  fd33:eb1b:9b36::2

* I have a "net-server" virtual machine that runs Apache on the above IP
  addresses (TCP port 443).

  - This virtual machine also runs DHCP (v4) and DHCP (v6) daemons.

  - The DHCP servers send the following boot file names:

    - "https://192.168.124.2/RHEL-7.4-20170711.0-Server-x86_64-boot.iso"       [IPv4]
    - "https://[fd33:eb1b:9b36::2]/RHEL-7.4-20170711.0-Server-x86_64-boot.iso" [IPv6]

* For sanity-checking the environment, I run the following two commands
  on the *host* (connecting to the "net-server" virtual machine):

  - curl           -I 'https://192.168.124.2/RHEL-7.4-20170711.0-Server-x86_64-boot.iso'
  - curl --globoff -I 'https://[fd33:eb1b:9b36::2]/RHEL-7.4-20170711.0-Server-x86_64-boot.iso'

  - The host is configured to trust the brand new test CA certificate
    (see near the top).

  - When the certificates are assigned *correctly* to the IP addresses
    in the Apache configuration, the above "curl" commands complete just
    fine. If I add the "-v" option to "curl", it confirms the right
    certificates are used, and it confirms the test CA as issuer too.

  - When the certificates are (intentionally) *cross-assigned* to the IP
    addresses in the Apache configuration, then both "curl" commands
    break with the following error message:

curl: (51) Unable to communicate securely with peer: requested domain
name does not match the server's certificate.

  - If I add the "-v" option, I also see

NSS error -12276 (SSL_ERROR_BAD_CERT_DOMAIN)

  - As a side comment: Apache itself warns about the misconfig, in
    "/var/log/httpd/ssl_error_log":

... [ssl:warn] ... AH01909: RSA certificate configured for ...:443
does NOT include an ID which matches the server name

* I have a "net-client" virtual machine, running OVMF.

  - The edk2 HTTPS/TLS client booting in this virtual machine is
    configured to trust the exact same set of CA certificates that the
    host trusts too.

  - In other words, HTTPS boot in the "net-client" VM accepts server
    certificates signed by the new test CA.

* The following is the test plan.

1. The patch set is *not* applied (that is, OVMF is built at current
   master, commit 976d0353a6ce).

  1. Properly assigned certificates:

    1. HTTPSv4 boot --> expect success (correct behavior, establishes
                                        baseline)

    2. HTTPSv6 boot --> expect success (correct behavior, establishes
                                        baseline)

  2. Cross-assigned certificates:

    1. HTTPSv4 boot --> expect success (for reproducing the bug)

    2. HTTPSv6 boot --> expect success (for reproducing the bug)

2. With the patch set applied:

  1. Properly assigned certificates:

    1. HTTPSv4 boot --> expect success (failure means a regression)

    2. HTTPSv6 boot --> expect success (failure means a regression)

  2. Cross-assigned certificates:

    1. HTTPSv4 boot --> expect failure (for verifying the bugfix)

    2. HTTPSv6 boot --> expect failure (for verifying the bugfix)

* Results:

- 1.1.1. as expected (HTTPSv4 baseline established)
- 1.1.2. as expected (HTTPSv6 baseline established)
- 1.2.1. as expected (HTTPSv4 MITM bug reproduced)
- 1.2.2. as expected (HTTPSv6 MITM bug reproduced)
- 2.1.1. as expected (HTTPSv4 not regressed by series)
- 2.1.2. as expected (HTTPSv6 not regressed by series)
- 2.2.1. as expected (HTTPSv4 MITM averted)
- 2.2.2. as expected (HTTPSv6 MITM averted)

* In cases 2.2.1. and 2.2.2.:

- The UEFI console contains, respectively:

Start HTTP Boot over IPv4....

   Station IP address is 192.168.124.106

   URI: https://192.168.124.2/RHEL-7.4-20170711.0-Server-x86_64-boot.iso

   Error: Could not retrieve NBP file size from HTTP server.

   Error: Unexpected network error.



Start HTTP Boot over IPv6....

   Station IPv6 address is FD33:EB1B:9B36:0:0:0:0:C8

   URI: https://[fd33:eb1b:9b36::2]/RHEL-7.4-20170711.0-Server-x86_64-boot.iso

   Error: Could not retrieve NBP file size from HTTP server.

   Error: Unexpected network error.

- The OVMF log contains (in both cases):

TlsDoHandshake SSL_HANDSHAKE_ERROR State=0x4 SSL_ERROR_SSL
TlsDoHandshake ERROR 0x1416F086=L14:F16F:R86

- Decoding:

  - Library   0x14 -> ERR_LIB_SSL
  - Function 0x16F -> SSL_F_TLS_PROCESS_SERVER_CERTIFICATE
  - Reason    0x86 -> SSL_R_CERTIFICATE_VERIFY_FAILED

  - So this means that the ssl_verify_cert_chain() call fails in the
    tls_process_server_certificate() function, in
    "CryptoPkg/Library/OpensslLib/openssl/ssl/statem/statem_clnt.c".


Normally the above would be sufficient for me to give a "Tested-by" for
this patch set.

But now I'm uncertain whether (a) my results contradict David's
analysis, or (b) I tested something that David's analysis doesn't
*apply* to. (IOW if my test plan doesn't actually verify "IPv6
literals".)


FWIW, the brackets in the IPv6 notation are stripped in EfiHttpRequest()
[NetworkPkg/HttpDxe/HttpImpl.c], using the "HostName" local variable.
(The stripping comes from earlier commit 7191827f90b4
("NetworkPkg/HttpDxe: Strip square brackets in IPv6 expressed
HostName.", 2018-08-03).) Later in EfiHttpRequest(),
"HttpInstance->RemoteHost" is assigned "HostName".

Further, in TlsConfigureSession() [NetworkPkg/HttpDxe/HttpsSupport.c],
we set "HttpInstance->TlsConfigData.VerifyHost.HostName" to
"HttpInstance->RemoteHost". This is done in patch#4.

Also in patch#4, in the same function, we pass
"HttpInstance->TlsConfigData.VerifyHost" to SetSessionData(), from
patch#3.

There we pass "TlsVerifyHost->HostName" to TlsSetVerifyHost(), which
resides in patch#2.

At that point, we pass the hostname -- the IPv6 address, with the
brackets stripped -- to SSL_set1_host().

So, my take is that the comparison is done simply on the textual
representation (with the IPv6 brackets stripped), not the numerical
value.

Is that bad? The textual comparison may certainly report a mismatch when
the numerical values actually match (for an IPv4 example, "192.168.0.1"
would not match "192.168.000.001"). But that errs in the safe direction,
does it not?

Thanks!
Laszlo

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[-- Attachment #2: Type: text/html, Size: 26018 bytes --]