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

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

On 10/16/19 11:19, David Woodhouse wrote:
> On Wed, 2019-10-16 at 09:36 +0200, Laszlo Ersek wrote:
>> On 10/16/19 07:18, Wu, Jiaxin wrote:
>>>    In some cases, the URI is specified as an IP address rather than
>>>    a Hostname .  In this case, the iPAddress subjectAltName must be
>>>    present in the certificate and must exactly match the IP in the
>>>    URI.
>>
>> Wow!
>>
>> This seems to prove David right, and it suggests that symptom (2)
>> encountered with my patch is actually *valid* behavior -- the
>> certificate I generated with "genkey" is *not* valid for a URI that
>> specifies an IP address!
>>
>> This is not good news: the "curl" utility also accepts such a
>> certificate as valid (IP address in URL, but the certificate only
>> contains a CN,  matching the IP address in textual form). Is that a
>> bug in "curl" then?
>
> Yes, I believe so.
> Please report it at https://github.com/curl/curl/issues

<https://github.com/curl/curl/issues/new> directs me to
<https://hackerone.com/curl>, for security-related issues, so I've now
registered on that site, and started writing up a report.

However. When I wanted to include a reference to RFC6125, I looked more
closely at the section that Jiaxin quoted. More specifically, the
surroundings of that section.

It turns out that the quoted section "3.1. Server Identity" is actually
part of Appendix B.2 ("HTTP (2000)"):

  https://tools.ietf.org/html/rfc6125#appendix-B.2

which is further part of Appendix B ("Prior Art"):

  https://tools.ietf.org/html/rfc6125#appendix-B

Appendix B starts with the statement

> (This section is non-normative.)

and Appendix B.2 starts with

>    In 2000, [HTTP-TLS] specified the following text regarding
>    application service identity verification in HTTP:

Thus, Appendix B of RFC6125 doesn't require us to do anything! (Other
specs may still require us, of course.)

Now, does RFC6125 say anything about IP addresses? Yes, it does, in
section 1.7.2, "Out of Scope":

https://tools.ietf.org/html/rfc6125#section-1.7.2

>    o  Identifiers other than fully qualified DNS domain names.
>
>       Some certification authorities issue server certificates based on
>       IP addresses, but preliminary evidence indicates that such
>       certificates are a very small percentage (less than 1%) of issued
>       certificates.  Furthermore, IP addresses are not necessarily
>       reliable identifiers for application services because of the
>       existence of private internets [PRIVATE], host mobility, multiple
>       interfaces on a given host, Network Address Translators (NATs)
>       resulting in different addresses for a host from different
>       locations on the network, the practice of grouping many hosts
>       together behind a single IP address, etc.  Most fundamentally,
>       most users find DNS domain names much easier to work with than IP
>       addresses, which is why the domain name system was designed in the
>       first place.  We prefer to define best practices for the much more
>       common use case and not to complicate the rules in this
>       specification.
>
>       Furthermore, we focus here on application service identities, not
>       specific resources located at such services.  Therefore this
>       document discusses Uniform Resource Identifiers [URI] only as a
>       way to communicate a DNS domain name (via the URI "host" component
>       or its equivalent), not as a way to communicate other aspects of a
>       service such as a specific resource (via the URI "path" component)
>       or parameters (via the URI "query" component).
>
>       We also do not discuss attributes unrelated to DNS domain names,
>       such as those defined in [X.520] and other such specifications
>       (e.g., organizational attributes, geographical attributes, company
>       logos, and the like).

So... I think we can (and should) proceed just the same, but we should
reference:

  https://tools.ietf.org/html/rfc2818#section-3.1

rather than RFC6125.

Accordingly, I've filed the curl report with reference to RFC-2818:

  https://hackerone.com/reports/715413

[...]

On 10/16/19 11:19, David Woodhouse wrote:
> So what did we say about false modesty, Laszlo?

It *was* hard; and in the first place, I truly hadn't expected myself to
send an edk2 RFC, at all... It was made possible by your code.

> In the end you did actually solve it all for yourself -- based on the
> pointer I'd given in bugzilla, and then ignoring my subsequent
> misdirection about callbacks and my overly complex attempt at doing it
> myself :)

You're too kind; I followed your code closely. I only updated the coding
style, added a bunch of comments, and supplied some error checks and
debug messages. In addition, Jiaxin suggested we should try to set the
smart verification params in TlsSetVerifyHost() at once.

Anyway: we still have the issue that X509_VERIFY_PARAM_set_ip_asc()
appears to reject IPv4 address literals. Could you check that please?

(Using a hosted (Linux userspace) program like "sconnect", it must be
easier to debug. I tried connecting gdb to QEMU, running OVMF, but it
crashed gdb. :)

I also looked at the OpenSSL code parsing the IPv4 literal, and nothing
struck me as obviously wrong. (Well, sscanf() is generally improper for
parsing untrusted data, but that's beside the point, for my test case
with a valid IPv4 literal.))

Thanks!
Laszlo