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

["Laszlo Ersek" <lersek@redhat.com>]

On 10/11/19 04:24, Wu, Jiaxin wrote:
> Hi Laszlo & David,
> 
> I think I have *repeated* several times that we are targeting to fix the HostName validation issue, not the IP or email address. *But* even so,  the series patches for UEFI TLS is also allowable to specify IP as host name for CN or dNSName of SAN in the certificate. That's why I said "if the CN or SAN in the certificate are set correctly, it should be OK to pass the verification". The failure you mentioned here is to set the IP in iPAddress of SAN, I agree it's the routine and suggested setting, *but* obviously, it's not the target we are supported according the implementation/description of TlsSetVerifyHost. We are targeting to the hostname verification, and meanwhile compatible with the IP in the URI (But need the *correct* certificate setting).
> 
> IP addresses stored in the DNS names and CN are of cause ignored by X509_check_ip & X509_check_ip_asc().
> 
> Post my explain again: 
>> 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.
> 
> Now, let's talking the iPAddress setting in SAN (same as email address),  if you do need such feature that verify the IP in X509_check_ip & X509_check_ip_asc , please report new Bugzilla (need TLS Spec update the expose the setting interface), don't mix up the HTTPS hostname verification here.

(To clarify my stance.)

Given the above statement of scope, and given the test env details that
I included in my previous message:

series
Tested-by: Laszlo Ersek <lersek@redhat.com>

I understand that my testing does not cover the use case described by David.

So my Tested-by is certainly *not* intended as the "last word" in this
thread, or anything like that. I'm only saying this patch set is good
enough for me, not that everyone should find it good enough for them.

Thanks
Laszlo


>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Friday, October 11, 2019 2:04 AM
>> To: David Woodhouse <dwmw2@infradead.org>; 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)
>>
>> On 10/10/19 17:45, David Woodhouse wrote:
>>> On Thu, 2019-10-10 at 10:00 +0200, Laszlo Ersek wrote:
>>>>>          Subject: C=HU, ST=Pest, L=Budapest, O=Laszlo Ersek Home Office,
>> OU=IPv6 cert, CN=fd33:eb1b:9b36::2
>>>
>>> Yeah, you're not actually testing the case I'm talking about. You want
>>> a GEN_IP in the x509v3 Subject Alternative Name.
>>>
>>> Compare with...
>>>
>>> $ openssl s_client  -connect vpn-i-ha01.intel.com:443 2>/dev/null | openssl
>> x509 -noout -text  | grep -A1 Alternative
>>>             X509v3 Subject Alternative Name:
>>>                 DNS:vpn-int.intel.com, DNS:scsidcint01-a.intel.com, IP
>> Address:134.191.232.101
>>>
>>> $ curl https://134.191.232.101/
>>>
>>
>> OK, thank you.
>>
>> I can imagine two failure modes, with the patches applied.
>>
>> (1) Edk2 ignores the GEN_IP in the SAN, and rejects a matching server
>> certificate.
>>
>> (2) Edk2 is confused by the GEN_IP in the SAN, and accepts an invalid
>> (mismatched) server certificate.
>>
>> Can we tell which failure mode applies?
>>
>> (I can't test it easily myself, as I don't even know how to create a
>> server certificate with a SAN -- any kind of SAN, let alone GEN_IP.)
>>
>> Case (2) is clearly bad, and it would be a sign that the patch series
>> does not fully fix the issue.
>>
>> Case (1) would be tolerable, in my opinion. I assume a GEN_IP SAN is
>> pretty rare in practice. Thus regressing it (perhaps temporarily) should
>> be an acceptable trade-off for fixing the current gaping hole (= subject
>> name not checked at all).
>>
>> Thanks
>> Laszlo