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

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

> 
> 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]".

Here, I want to highlight is that 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) 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.
  
> 
> That just adds it to the 'hosts' list in the X509_VERIFY_PARAM for the
> SSL connection.

Yes.

> 
> 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().

Yes.

> 
> If you look in openssl/crypto/x509/v3_utl.c you can see the
> X509_check_host() really does only check hostnames. 

Yes.

> 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.
> 

Disagree, why need check the IP here since we only focus on the hostname verification? For HttpDxe driver, it's the implementation choice to treat the IP in URI as hostname string format. As I said before in the email, if the CN or SAN (Seems only in X509 V3) in the certificate are set correctly, it should be OK to pass the verification. Laszlo and I already have verified that.

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

Sorry, I can't remember there is any failure of Ipv6 URI reported from edk2. If you can find it, that will be better.

> 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.
> 
> > Thanks,
> > Jiaxin