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 <dw...@infradead.org> Sent: Thursday, October 10, 2019 4:34 AM To: Laszlo Ersek <ler...@redhat.com>; Wu, Jiaxin <jiaxin...@intel.com>; devel@edk2.groups.io; Wang, Jian J <jian.j.w...@intel.com>; Bret Barkelew <bret.barke...@microsoft.com> Cc: Richard Levitte <levi...@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 <ler...@redhat.com<mailto:ler...@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 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. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48686): https://edk2.groups.io/g/devel/message/48686 Mute This Topic: https://groups.io/mt/34307578/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
