Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for ACME verifications on ip addresses #410

Closed
mquinnatlasroofingcom opened this issue Oct 27, 2020 · 19 comments
Closed

Support for ACME verifications on ip addresses #410

mquinnatlasroofingcom opened this issue Oct 27, 2020 · 19 comments
Labels
area/acme ACME enhancement roadmap An item for roadmap discussion

Comments

@mquinnatlasroofingcom
Copy link

What would you like to be added

There is a extension to ACME to allow it to verify ip addresses.

The extension is:
https://tools.ietf.org/html/rfc8738

Why this is needed

This isn't something that Let's Encrypt will ever do but for locally hosted ACME provisioners it would be useful to get rid of certificate errors for sites when accessed via their local ip address. In our organization, we have very little standardization of hostnames (mostly due to acquisitions of other companies and systems) so often we just remember the IPs.

It's currently possible to use templates to force an ACME provisioner to sign any IP address that is passed to it but it will not perform any challenge. This defeats the security of ACME.

@mquinnatlasroofingcom mquinnatlasroofingcom added enhancement needs triage Waiting for discussion / prioritization by team labels Oct 27, 2020
@dopey dopey added roadmap An item for roadmap discussion area/acme ACME and removed needs triage Waiting for discussion / prioritization by team labels Nov 17, 2020
@mholt
Copy link

mholt commented Jan 22, 2021

I also have a need for this, as we'll often be authenticating Caddy instances by their IP address - for internal/admin use only, so it's something we need signed by our own CA and not a public CA.

@RedSerenity
Copy link

@mquinnatlasroofingcom Can you share how you did this with a template?

@mquinnatlasroofingcom
Copy link
Author

I had a back and forth with one of the Smallstep developers on their community forum thing. It's on there up around the time that I submitted this feature request. Bottom line though, it doesn't make any sense to do it that way since the certificate will not do an ACME verification on the ip address, only on the hostname. As a result, you lose all the security that ACME offers.

I wound up creating a modified version of their ACME provisioner that accepts and does verification of SANs of IP address type. You'll want to have a look at the RFC for guidance on what to do. There were only a handful of changes that I had to make to enable this. It's been a while and I don't really remember them all. Check out the source code in acme/order.go, the finalize function is a good place to start. You'll probably have to mess around with your ACME client too in order to get it to make CSRs with ip address type SANs.

@RedSerenity
Copy link

Thanks, @mquinnatlasroofingcom!

@mholt
Copy link

mholt commented Apr 5, 2021

@mquinnatlasroofingcom Is this something you'd be able to make a pull request for? @dopey Do you know if you'd be able to accept such a PR (or when this might be on the roadmap for)?

@dopey
Copy link
Contributor

dopey commented Apr 5, 2021

This is still on the roadmap along with external account binding. It's on our short term roadmap but our roadmap is also constantly in flux #startupthings.

We'd definitely be interested in taking a look at the PR. And as @mquinnatlasroofingcom mentioned, we'll need accompanying integration from the cli side (although maybe not right away).

@mholt we've actually just done a complete rework of our ACME backend (it's still in a PR but will probably be merged this week). Very minor change for you if caddy wants to integrate with latest (just changing the name of a few types).

@mholt
Copy link

mholt commented Apr 5, 2021

Oh, awesome -- yeah, would be happy to use the latest and greatest! 🙂

@jack4it
Copy link

jack4it commented May 6, 2021

@mholt we've actually just done a complete rework of our ACME backend (it's still in a PR but will probably be merged this week). Very minor change for you if caddy wants to integrate with latest (just changing the name of a few types).

Does this PR include support for https://tools.ietf.org/html/rfc8738? Can we take a look at the PR? :)

@dopey
Copy link
Contributor

dopey commented May 6, 2021

Hey @jack4it the PR I was referencing was actually merged a few weeks back (#496). It was a rewrite of the ACME backend that will allow for hooking in a SQL db - so not directly related to the RFC you're asking about.

Support for this issue (IP verification) is still on the roadmap. We want to support it yesterday, we're just short on resources :]

@jack4it
Copy link

jack4it commented May 6, 2021

I can look by myself, but is there an established process/guideline that we can help? :) would be happy to contribute @dopey

@dopey
Copy link
Contributor

dopey commented May 6, 2021

Yea! Help would be much appreciated. I'd be happy to schedule some time to give you a quick tour of the codebase with particular attention to the section where the IP verification code should/would be implemented. My email is max@smallstep.com - shoot me an email.

That said, it sounds like @mquinnatlasroofingcom may already have a working implementation. That would probably be a really useful place to start.

@hslatman
Copy link
Member

Started working on this. The http-01 challenge flow seems to work with Caddy as the client now, but have to improve on compliance with the RFC as well as tls-alpn-01.

(somewhat shortened) Step output:

INFO[0058] duration=4.928567ms duration-ns=4928567 fields.time="2021-05-21T01:27:17+02:00" method=POST name=ca nonce=VVRJQkZEdUVUSENLRFp4SWhnN0duRWpsR2RKQ0VWbWs path=/acme/acme-ip/order/3SUOkgaiSvisJUJZndWPMzrrVkTqpWIz/finalize protocol=HTTP/2.0 referer= remote-address=127.0.0.1 request-id=c2jf0lecie6rd4ti9m8g response="{\"id\":\"3SUOkgaiSvisJUJZndWPMzrrVkTqpWIz\",\"status\":\"valid\",\"expires\":\"2021-05-21T23:27:15Z\",\"identifiers\":[{\"type\":\"ip\",\"value\":\"127.0.0.1\"}],\"notBefore\":\"2021-05-20T23:26:15Z\",\"notAfter\":\"2021-05-21T23:27:15Z\",\"authorizations\":[\"https://127.0.0.1:8443/acme/acme-ip/authz/U9Q8i1YRvpqpY45TTZaLlte7SeHaW98Z\"],\"finalize\":\"https://127.0.0.1:8443/acme/acme-ip/order/3SUOkgaiSvisJUJZndWPMzrrVkTqpWIz/finalize\",\"certificate\":\"https://127.0.0.1:8443/acme/acme-ip/certificate/G4EGUe86fotdXWjFISafJf71OIoBwgLb\"}" size=501 status=200 user-agent="Caddy/2.4.1 CertMagic acmez (darwin; amd64)" user-id=
INFO[0058] certificate=MIIB2TCCAX+.....m7Kzr+ztA duration="931.877µs" duration-ns=931877 fields.time="2021-05-21T01:27:17+02:00" issuer="herman-ip-verification Intermediate CA" method=POST name=ca nonce=QXVRWDl5WGpwd0VPWWxnRHMxd3ZLcHFnNjFPQXR3T2I path=/acme/acme-ip/certificate/G4EGUe86fotdXWjFISafJf71OIoBwgLb protocol=HTTP/2.0 provisioner=acme-ip public-key="ECDSA P-256" referer= remote-address=127.0.0.1 request-id=c2jf0lecie6rd4ti9m90 serial=291202333297069808568327607252964312968 size=1368 status=200

Caddy output; configured to talk to the modified ACME provisioner:

2021/05/20 23:27:15.512	INFO	tls.issuance.acme.acme_client	trying to solve challenge	{"identifier": "127.0.0.1", "challenge_type": "http-01", "ca": "https://127.0.0.1:8443/2.0/acme/acme-ip/directory"}
2021/05/20 23:27:16.770	INFO	tls.issuance.acme	served key authentication	{"identifier": "127.0.0.1", "challenge": "http-01", "remote": "127.0.0.1:56799", "distributed": false}
2021/05/20 23:27:17.026	INFO	tls.issuance.acme.acme_client	validations succeeded; finalizing order	{"order": "https://127.0.0.1:8443/acme/acme-ip/order/3SUOkgaiSvisJUJZndWPMzrrVkTqpWIz"}
2021/05/20 23:27:17.034	INFO	tls.issuance.acme.acme_client	successfully downloaded available certificate chains	{"count": 1, "first_url": "https://127.0.0.1:8443/acme/acme-ip/certificate/G4EGUe86fotdXWjFISafJf71OIoBwgLb"}
2021/05/20 23:27:17.034	INFO	tls.obtain	certificate obtained successfully	{"identifier": "127.0.0.1"}

@mholt
Copy link

mholt commented May 21, 2021

That's awesome - great progress!

@mholt
Copy link

mholt commented Jun 3, 2021

@hslatman Is this something you could make a PR for while you finish it up so reviews could begin?

@hslatman
Copy link
Member

hslatman commented Jun 3, 2021

@mholt My plan is to have the PR ready within 24 hours.

@hslatman
Copy link
Member

hslatman commented Jun 3, 2021

The PR is here: #602.

Got a couple of tests to fix and have to add some for the the IP type of identifier. There are also some improvements that I'd like to do soon.

So far I've tested with a small tool I wrote based on acmez as well as with Caddy. Both HTTP-01 and TLS-ALPN-01 work with my tool. Caddy is not able to solve the TLS-ALPN-01 challenge for an IP identifier, because it looks for the IP (i.e. 127.0.0.1) in its challenge store based on the ClientHello ServerName. The RFC states that IPs should be their ARPA address in the ServerName (i.e. 1.0.0.127.in-addr.arpa), which is what I implemented. Not doing this conversion will result in an empty identifier on the Caddy side, which might be due to how Go processes the ServerName.

Reversing the ARPA to the IP as the identifier to use in the lookup OR storing the ARPA in the lookup are fixes for that issue and have verified that in a quick local POC.

@mholt
Copy link

mholt commented Jun 4, 2021

@hslatman

Caddy is not able to solve the TLS-ALPN-01 challenge for an IP identifier, because it looks for the IP (i.e. 127.0.0.1) in its challenge store based on the ClientHello ServerName

That's true; are you saying that the ServerName comes in the form 1.0.0.127.in-addr.arpa and that the .in-addr.arpa suffix needs to be stripped by Caddy first? I'm open to changing this behavior.

will result in an empty identifier on the Caddy side

What do you mean by this exactly?

@hslatman
Copy link
Member

hslatman commented Jun 4, 2021

That's true; are you saying that the ServerName comes in the form 1.0.0.127.in-addr.arpa and that the .in-addr.arpa suffix needs to be stripped by Caddy first? I'm open to changing this behavior.

Indeed, for IP identifiers the ServerName should be formatted as its ARPA representation. At least, that's what I concluded from the RFC and it made sense to me for the server -> client direction. I have a potential fix for Caddy that first checks if the ServerName suffix is .in-addr.arpa or .ip6.arpa and, if so, reverses it back to the IP and uses that to do the lookup. Using the ARPA domain as the key for the lookup is another solution. The latter seems cleaner to me, the ARPA representation being a proper domain and Caddy refers to the identifiers as domains too, but in the end it's "just" a different representation. I can open a PR with the first solution, if you want. I would need some time to try the second one, if you prefer that one.

What do you mean by this exactly?

Sorry, my explanation was a bit terse. I meant to say that, in case step-ca doesn't format the IP as its ARPA representation, the ServerName on the Caddy side will be empty. I think that's due to how Go handles the field, but I haven't investigated that further yet. Caddy logs it like this:

2021/06/03 21:46:00.240	ERROR	tls	tls-alpn challenge	{"server_name": "", "error": "no information found to solve challenge for identifier: "}

While having it formatted as ARPA looks like this:

2021/06/03 21:55:25.224	INFO	tls	served key authentication certificate	{"server_name": "1.0.0.127.in-addr.arpa", "challenge": "tls-alpn-01", "remote": "127.0.0.1:63671", "distributed": false}

@mholt
Copy link

mholt commented Jun 4, 2021

Ahh gotcha. That makes more sense, thank you. I would happily accept a PR! I also think the latter option would be cleaner. What's the diff on the first option though? (feel free to discuss in caddyserver/certmagic#133)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/acme ACME enhancement roadmap An item for roadmap discussion
Projects
None yet
Development

No branches or pull requests

6 participants