Re: [edk2-devel] [RFC v1 5/4] CryptoPkg/TlsLib: accept peer certs via both DNS names and IP addresses

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

Jiaxin,

On 10/16/19 17:25, David Woodhouse wrote:
> On Wed, 2019-10-16 at 16:43 +0200, Laszlo Ersek wrote:
>> Regarding the current edk2 patch set, I think we should do the following:
>>
>> - use X509_VERIFY_PARAM_set1_ip() rather than
>> X509_VERIFY_PARAM_set1_ip_asc()
>>
>> - incorporate "StdLib/BsdSocketLib/inet_pton.c" from the edk2-libc
>> project (which used to be part of edk2 itself) into TlsLib, and call
>> inet_pton() for parsing the address as both IPv4 and IPv6.
> 
> That makes sense.

Please wait a little before starting work on this. I've been made aware,
in <https://hackerone.com/reports/715413>, of the practices of various
certificate authorities:

[1] https://www.geocerts.com/support/ip-address-in-ssl-certificate
[2]
https://www.leaderssl.com/articles/381-issuing-ssl-certificate-for-an-ip-address
[3]
https://support.globalsign.com/customer/en/portal/articles/1216536-securing-a-public-ip-address---ssl-certificates

What's most worrisome is [3], which writes:

    If you are targeting Windows 10 and later, you can populate the IP
    address in either field. If however, you are targeting Windows 8.1
    and earlier, you should only specify the IP address as the common
    name.

Keyword being "only".

Assuming the above quote precisely reflects reality: if we made edk2
strictly insist on the IP address being in the SAN.iPAddress field, then
edk2 could not HTTPS-boot from such web servers that intend to serve
Windows clients up to 8.1.

Reference [2] advises to put the IP address in both CN and SAN.iPAddress
for best compatibility, and that would be fine, for
X509_VERIFY_PARAM_set1_ip(). But the word "only" in [3] is really bad
for X509_VERIFY_PARAM_set1_ip().

Thanks
Laszlo