webpki/cert: extend verify_is_valid_for to iPAddress SAN

[lucab]

I currently have some certificates from a private CA where the subject identity is established via an iPAddress subject alternative name. I just checked that this crate only tries to match a dNSName against a CN or a SAN, which is the reason why validation fails. This is a feature request to add support for such SAN validation.

As I (or somebody else) may want to tackle this in the future, I'd like to get some input on the expected API for this:

• should a new dedicated is_valid_for_ip be introduced?
• should the current is_valid_for_dns be renamed/overloaded to accept more than one SAN types?
• should the consumer call is_valid_for multiple times, or just once with all DNSs/IPs/etc?
• should the Ok return type be amended to return back the matching entry?

What's the API of other TLS library in this regard? Should this crate stick to prior art?

(I was personally thinking about a single is_valid_for which takes a slice of subject names, and returns back the matching one on success)

[dignan]

I ran into the same issue and started going down the road of trying to find relevant RFCs for this. It looks like the DNS-based verification follows https://tools.ietf.org/html/rfc6125. However, that document explicitly marks IP-based verification as out-of-scope. It'd be useful to get input on what a "good" implementation would look like. It looks like nss converts the input string to a netaddr, and then does byte comparisons.

[lucab]

@dignan section 3.1.3.2 of that RFC define the decoding and comparison rules (but, that is out of scope according to the preamble, duh). Anyway, the ASN type of that OID is "octet string" so the only valid comparison I can think of is full byte-by-byte equality in network order. Did you have any more specific doubt/concern?

[briansmith]

I probably own at least some of the copyright for the mozilla::pkix IP address handling code so it might be easiest for me to transliterate it. Otherwise we'd need to write all-new code.

[lucab]

@briansmith do you have any input/insight on my original API questions? I see that CheckCertHostname in pkix takes a single non-specialized input and fallback-try multiple parsers, do you want to stick to that? (I didn't put this option in my initial list as I don't consider it safe/idiomatic)

[briansmith]

@lucab There are two questions:

1. Should webpki continue on its original intended trajectory, where it was intentionally limited in functionality (e.g. we intentionally omitted adding IP address support), or should we expand its scope?

2. If we should expand the scope, what should the API look like.

I won't have time to ponder either issue for a couple of weeks.

[briansmith]

I'm going to work on some things related to client auth support, which I think will help inform the work to extend webpki to other kinds of names, including IP addresses. In particular I want to add support for some non-dNSName forms for client auth purposes.

[djc]

@briansmith have you made up your mind as to whether you want to have support for this in webpki? We think this would be important for the kind of peer-to-peer connections that would be more common with QUIC's use of TLS.

[lucab]

Additionally, in the meanwhile I've found a public website with a trust chain to a well-known root CA and an iPAddress SAN: https://1.1.1.1/

[briansmith]

Update on my comment from August 2017: Back then, I was going to do some work along these lines but ultimately that project decided to use DNS names after considering all the factors.

If you are going to be at RWC this year, please email me @ brian@briansmith.org and let's set up a time to chat about this.

[djc]

Not sure if that was directed at me, but I won't be at RWC. Happy to do some video chat though, if that helps.

[nicklan]

We need this support to properly be able to connect to minikube clusters (which use autogenerated certs with IPAddress SANs) in click. See: databricks/click#98

Let me know if there's anything I can do to help move this along.

[briansmith]

Let me know if there's anything I can do to help move this along.

If your organization is interested in sponsoring the development of this, I would be happy to do it and I can do it quickly. I already implemented this exact functionality for another project (not webpki) in the past. Email me if interested: brian@briansmith.org.

[nicklan]

I did discuss this internally, but since this is not an issue for our internal use of Click (we don't use minikube), I couldn't get support for it I'm afraid.

Regardless, I think this is a bad state to be in. The use of DNSNameRef is already proliferating (see here and here and here and here for example, and there are more). It seems unlikely that DNSNameRef is the right high level struct to be exposing if you want to also support ip addresses (probably more something like GeneralName), so whatever API is agreed upon here will now require changes through many parts of many projects to get proper ip address support. At the same time, it seems crazy that the main stack for "true rust tls" doesn't support connecting to an ip address.

The code here isn't too difficult (i've already implemented a verify_cert_ip_san method to check that, although i'm sure there are some subtleties that need to be addressed). The API is what's tricky to settle on.

@briansmith if you have a suggestion and/or opinion on what the API should look like here, I, or someone else, can probably find some time to work on it. Please chime in as you've worked on this stuff before and also will have to be the one to finally merge things.

[nicklan]

I opened #120 just to show the code for simple san ip verification. That can serve as a discussion point for more code oriented stuff if you prefer.

[briansmith]

Thanks all. The issue isn't the lack of code for this. I've already implemented it.

Originally I was hoping that the Web PKI would be able to kill off the use of IP addresses in certificates since, in my experience, must uses of IP addresses in certificates were just plain wrong--dangerously so. However, https://1.1.1.1/ and other things have basically forced us to support IP addresses in certificates in webpki.

So now it is just a matter of me having the free time to care about this when I'm not working and my kid is asleep and I have nothing better to do with my time. Unfortunately, I am getting pretty good at finding good things to do with my time lately.

[briansmith]

I started working on this. #125 is a prerequisite for it.

[briansmith]

Please see PR #218 which attempts to refactor the API in preparation for this, in a way that will allow us to decouple the release schedule for webpki from the implementation of this feature.

[AlessandroBono]

The code to implement this is very simple. The biggest challenge is the deficit we have in the testing of it. I appreciate the external validation that people did of webpki but we're really missing proper infrastructure to test even the existing functionality, let alone new features like this. Now this project is in much better shape w.r.t. CI/CD but we still don't have great infrastructure for adding new tests. The the extent I can afford to spend time on this project, my intent is to immediately focus on improving the testing/validation infrastructure further, so that when we add IP address support we can have a reasonable test suite. In particular, it is critical for this feature that we add comprehensive name constraint tests.

I wrote some tests here #239.

[farcaller]

Is there any plan to progress with this and/or #239? Currently this is a second major blocker for using kube-rs with some default configurations of k8s clusters.

[cromefire]

Originally I was hoping that the Web PKI would be able to kill off the use of IP addresses in certificates since, in my experience, must uses of IP addresses in certificates were just plain wrong--dangerously so.

I think it's biggest use case is in internal and testing infrastructure, where setting up a DNS server just to securely connect to a database an mqtt server or whatever seems really overkill and error prone. Not having this feature basically prevents all usages in, I would claim most, local infrastructure and if a rust library doesn't support openssl or some alternative you are stuck with the tedious configuration of either the hosts file (which introduces a new config file to edit), setup some DNS infrastructure or switch to a different library that doesn't use webpki/rustls.

[ssokolow]

On that note, in my case, my goal is to figure out how to replace RusTLS with a statically linked copy of OpenSSL for my variation on miniserve so that, once I've got UPnP and What Is My IP? integration, I can also try to get HTTP/2 Opportunistic Encryption working as a way to get some basic level of resistance to passive surveillance on a server that you may not want to make known to some third-party authority.

(I'm basically trying to push what can be achieved by non-technically-savvy users without relying on a big third party like like Dropbox. An experiment in returning to the ideals of the early days of the web.)

[ereslibre]

I have opened #260 in order to address this issue. Would love to have some feedback on it. Thank you!

[rwthompsonii]

So it's now February of 2023, and there's still no support for IP addresses in certificates in webpki as near as I can tell. I've worked as an engineer for 8 years now. I've never worked in a place that it ever made sense to configure DNS for internal only services that aren't even routed outside of the k8s cluster they're hosted on, or the firewall they're sitting behind. You point the service at the IP address it needs to connect to and you walk away.

I get that it seems ugly, but DNS is just another failure point for me. I don't need yet another critical failure node to bring my service to its knees when DNS falls over, especially when I don't need DNS for anything other than rustls/webpki deciding for me that IP addresses aren't valid enough for TLS certificates.

It's incredibly annoying that I have to go find a lib that supports openssl just to workaround this one simple issue, instead of being able to use the rustls stack.

[farcaller]

@rwthompsonii rustls made a fork of this repo (context) and the issue is fixed there.