feat: add 6-days-long and IP address certificates - #91
Conversation
|
@FlorianUekermann 🥰 In addition, I’d like to replace that portion of the ACME client code in the codebase with https://github.com/djc/instant-acme. |
|
Sorry for the slow response. I was on vacation for a month. Thank you for the PR. I'll take a look at this asap, but need to catch up on some other stuff first.
That would mean depending on their http client (hyper), right? I'm very attached to our runtime agnostic non-spawning http client. |
The hyper client implementation is optional and behind a feature flag that can be avoided with
Would it fit into the shape of our |
| pub async fn new_order(&self, client_config: &Arc<ClientConfig>, domains: Vec<String>) -> Result<(String, Order), AcmeError> { | ||
| let domains: Vec<Identifier> = domains.into_iter().map(Identifier::Dns).collect(); | ||
| let payload = format!("{{\"identifiers\":{}}}", serde_json::to_string(&domains)?); | ||
| let mut has_ip = false; | ||
| let domains: Vec<Identifier> = domains |
There was a problem hiding this comment.
Let's use the IpAddr type for this. We should probably add an ips argument to AcmeConfig::new().
| .collect(); | ||
| let payload = if has_ip { | ||
| serde_json::to_string(&serde_json::json!({ | ||
| "identifiers": domains, |
There was a problem hiding this comment.
I think we need to use some canonicalized text form here (see RFCs for details). Using the IpAddr type throughout the crate would probably resolve that issue.
There was a problem hiding this comment.
I changed into IpAddr::to_string
| match client_hello.server_name() { | ||
| None => { | ||
| log::debug!("client did not supply SNI"); | ||
| None | ||
| match &self.inner.lock().unwrap().challenge_data { | ||
| Some(ChallengeData::TlsAlpn01 { sni, cert }) => { | ||
| if sni.parse::<std::net::IpAddr>().is_ok() { | ||
| log::debug!("returning IP challenge cert for {}", sni); | ||
| Some(cert.clone()) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
| _ => None, | ||
| } | ||
| } |
There was a problem hiding this comment.
Shouldn't the cert authority specify a server name like 1.2.0.192.in-addr.arpa, so we would never hit this branch? I would expect that no changes are needed here if sni is stored in the correct format in the challenge data.
DDR verified discovery (RFC 9462 §4) requires the encrypted
resolver's TLS certificate to assert the resolver's IP address as an
iPAddress SAN. Upstream rustls-acme 0.15 models only DNS identifiers
(`enum Identifier { Dns(String) }`), so it cannot request such a
cert.
Pin to the rust-proxy fork's `feat/ip` branch (PR
FlorianUekermann/rustls-acme#91), which adds `Identifier::Ip` and
selects Let's Encrypt's `shortlived` profile when any identifier is
an IP. The public API is unchanged — IP-address strings in the
existing `dns.tls.acme.domains` list are auto-detected — so no
src/acme/ code change is needed. Revisit once #91 merges and ships.
|
@FlorianUekermann check the latest changes, let me know if it's still not proper |
c7f180c to
d5ed825
Compare
3 participants
Adds support for ACME orders for IP addresses in addition to DNS names.
Related Issue #89