Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,24 @@ In both cases, the key is expected to be encoded in

[RFC8555#eab]: https://datatracker.ietf.org/doc/html/rfc8555#section-7.3.4

### preferred_chain

**Syntax:** **`preferred_chain`** _`issuer name`_

**Default:** -

**Context:** acme_issuer

_This directive appeared in version 0.3.0._

Specifies the preferred certificate chain.

If the ACME issuer offers multiple certificate chains,
prefer the chain with the topmost certificate issued from the
Subject Common Name _`issuer name`_.

If no matches, the default chain will be used.

### profile

**Syntax:** **`profile`** _`name`_ \[`require`]
Expand All @@ -298,6 +316,8 @@ In both cases, the key is expected to be encoded in

**Context:** acme_issuer

_This directive appeared in version 0.3.0._

Requests the supported [certificate profile][draft-ietf-acme-profiles]
_`name`_ from the ACME server.

Expand Down
88 changes: 68 additions & 20 deletions src/acme.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,20 @@ use ngx::async_::sleep;
use ngx::collections::Vec;
use ngx::ngx_log_debug;
use openssl::pkey::{PKey, PKeyRef, Private};
use openssl::x509::{self, extension as x509_ext, X509Req};
use openssl::x509::{self, extension as x509_ext, X509Req, X509};
use types::{AccountStatus, ProblemCategory};

use self::account_key::{AccountKey, AccountKeyError};
use self::types::{AuthorizationStatus, ChallengeKind, ChallengeStatus, OrderStatus};
use crate::conf::identifier::Identifier;
use crate::conf::issuer::{Issuer, Profile};
use crate::conf::issuer::{CertificateChainMatcher, Issuer, Profile};
use crate::conf::order::CertificateOrder;
use crate::net::http::HttpClient;
use crate::time::Time;

pub mod account_key;
pub mod error;
pub mod headers;
pub mod solvers;
pub mod types;

Expand All @@ -53,7 +54,8 @@ pub enum NewAccountOutput<'a> {
}

pub struct NewCertificateOutput {
pub chain: Bytes,
pub bytes: Bytes,
pub x509: std::vec::Vec<X509>,
pub pkey: PKey<Private>,
}

Expand Down Expand Up @@ -272,7 +274,7 @@ where
if let Some(val) = res
.headers()
.get(http::header::RETRY_AFTER)
.and_then(parse_retry_after)
.and_then(headers::parse_retry_after)
.filter(|x| x > &MAX_SERVER_RETRY_INTERVAL)
{
return Err(RequestError::RateLimited(val));
Expand Down Expand Up @@ -487,11 +489,69 @@ where

let certificate = order
.certificate
.ok_or(NewCertificateError::CertificateUrl)?;
.ok_or(NewCertificateError::MissingCertificate)?;

let chain = self.post(&certificate, b"").await?.into_body();
let res = self.post(&certificate, b"").await?;

Ok(NewCertificateOutput { chain, pkey })
if let Some(ref matcher) = self.issuer.chain {
let (bytes, x509) = self
.find_preferred_chain(&certificate, res, matcher)
.await?;
Ok(NewCertificateOutput { bytes, x509, pkey })
} else {
let bytes = res.into_body();
let x509 =
X509::stack_from_pem(&bytes).map_err(NewCertificateError::InvalidCertificate)?;
if x509.is_empty() {
return Err(NewCertificateError::MissingCertificate);
}

Ok(NewCertificateOutput { bytes, x509, pkey })
}
}

async fn find_preferred_chain(
&self,
base: &Uri,
cert: http::Response<Bytes>,
matcher: &CertificateChainMatcher,
) -> Result<(Bytes, std::vec::Vec<X509>), NewCertificateError> {
let default =
X509::stack_from_pem(cert.body()).map_err(NewCertificateError::InvalidCertificate)?;
Comment on lines +519 to +520

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a properly formatted response, it shouldn't happen (though, could it given some data newer than our OpenSSL version?), but would it make sense to delay the map_err until the time where default becomes the chosen chain (a bit like is_empty)? By delaying that check, it may be possible to allow working around errors where parsing the default certificate fails for some reason, allowing administrators to still specifically configure a preferred chain. That probably requires further extracting the actual search logic within the if though, since it requires treating the Err(_) and Some(stack) if !matcher.test(stack) cases the same.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe this degree of resilience is necessary. Parsing errors are actually unlikely here, because the certificate format haven't changed for a few decades and we are in control of the key algorithm. In addition, the case when the whole response is junk (PEM_R_NO_START_LINE) is already translated to an Ok() with an empty Vec.

What we can encounter is either a memory allocation error, or a corrupted data from the CA, and neither should be recoverable.


if !matcher.test(&default) {
if let Ok(base) = iri_string::types::UriAbsoluteString::try_from(base.to_string()) {
let alternates = cert
.headers()
.get_all(http::header::LINK)
.into_iter()
.filter_map(headers::parse_link)
.flatten()
.filter(|x| x.is_rel("alternate"));

for link in alternates {
let uri = link.target.resolve_against(&base).to_string();
let Ok(uri) = Uri::try_from(uri) else {
continue;
};

let res = self.post(&uri, b"").await?;
let bytes = res.into_body();

let stack = X509::stack_from_pem(&bytes)
.map_err(NewCertificateError::InvalidCertificate)?;
if matcher.test(&stack) {
return Ok((bytes, stack));
}
}
}
}

if default.is_empty() {
return Err(NewCertificateError::MissingCertificate);
}

Ok((cert.into_body(), default))
}

async fn do_authorization(
Expand Down Expand Up @@ -612,7 +672,7 @@ async fn wait_for_retry<B>(
let retry_after = res
.headers()
.get(http::header::RETRY_AFTER)
.and_then(parse_retry_after)
.and_then(headers::parse_retry_after)
.unwrap_or(interval)
.min(MAX_SERVER_RETRY_INTERVAL);

Expand Down Expand Up @@ -642,15 +702,3 @@ where
{
serde_json::from_slice(bytes).map_err(RequestError::ResponseFormat)
}

fn parse_retry_after(val: &http::HeaderValue) -> Option<Duration> {
let val = val.to_str().ok()?;

// Retry-After: <http-date>
if let Ok(time) = Time::parse(val) {
return Some(time - Time::now());
}

// Retry-After: <delay-seconds>
val.parse().map(Duration::from_secs).ok()
}
9 changes: 6 additions & 3 deletions src/acme/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,18 @@ pub enum NewCertificateError {
#[error("unexpected authorization status {0:?}")]
AuthorizationStatus(super::types::AuthorizationStatus),

#[error("no certificate in the validated order")]
CertificateUrl,

#[error("unexpected challenge status {0:?}")]
ChallengeStatus(super::types::ChallengeStatus),

#[error("csr generation failed ({0})")]
Csr(openssl::error::ErrorStack),

#[error("PEM_read_bio_X509() failed: {0}")]
InvalidCertificate(openssl::error::ErrorStack),

#[error("no certificate in the completed order")]
MissingCertificate,

#[error("no supported challenges")]
NoSupportedChallenges,

Expand Down
Loading