From ad7310de3f81ae11f4f88456c588a0b728538d9c Mon Sep 17 00:00:00 2001 From: Aleksei Bavshin Date: Fri, 25 Jul 2025 12:07:00 -0700 Subject: [PATCH 1/4] ACME: forbid specifying files in "acme_certificate key=...". The ACME protocol assumes short-term certificates to ensure a regular revalidation of ownership and to limit consequences of key compromise. Allowing to request certificate renewals with the same key goes agains the second goal and should not be supported without a good reason. This was an oversight inherited from the proof-of-concept code and was not supposed to be committed here. --- README.md | 11 +++-------- src/conf.rs | 1 + 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index aec8f94..b7ecc7f 100644 --- a/README.md +++ b/README.md @@ -218,7 +218,7 @@ challenge data for all the configured certificate issuers. ### acme_certificate -**Syntax:** acme_certificate `issuer` [`identifier` ...] [ `key` = `alg[:size]` | `file` ] +**Syntax:** acme_certificate `issuer` [`identifier` ...] [ `key` = `alg[:size]` ] **Default:** - @@ -234,17 +234,12 @@ regular expressions and wildcards are not supported. [server_name]: https://nginx.org/en/docs/http/ngx_http_core_module.html#server_name -The `key` parameter sets the type of generated private key or a -path to an existing file. Supported key algorithms and sizes: +The `key` parameter sets the type of a generated private key. Supported key +algorithms and sizes: `ecdsa:256` (default), `ecdsa:384`, `ecdsa:521`, `rsa:2048` .. `rsa:4096`. -> Since 1.27.2, the `key` parameter supports the additional schemes implemented in the -> [ssl_certificate_key](https://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_certificate_key) -> directive: `data:` , `engine:` and more recently `store:` , -> with a caveat that password-protected keys are not supported. - ## Embedded Variables The `ngx_http_acme_module` module defines following embedded diff --git a/src/conf.rs b/src/conf.rs index 19deab4..4a308ca 100644 --- a/src/conf.rs +++ b/src/conf.rs @@ -284,6 +284,7 @@ extern "C" fn cmd_add_certificate( for value in &args[2..] { if let Some(key) = value.strip_prefix(b"key=") { order.key = match PrivateKey::try_from(key) { + Ok(PrivateKey::File(_)) => return c"invalid \"key\" value".as_ptr().cast_mut(), Ok(val) => val, Err(err) => return cf.error(args[0], &err), }; From 437d026dbfbbf8ce88bcb9f5f398cf53c72cd5b2 Mon Sep 17 00:00:00 2001 From: Aleksei Bavshin Date: Wed, 23 Jul 2025 15:03:34 -0700 Subject: [PATCH 2/4] ACME: define and allocate shared data structures. --- src/conf.rs | 5 ++ src/conf/issuer.rs | 11 ++- src/conf/shared_zone.rs | 9 ++- src/lib.rs | 2 + src/state.rs | 140 ++++++++++++++++++++++++++++++++++++ src/state/certificate.rs | 150 +++++++++++++++++++++++++++++++++++++++ src/state/issuer.rs | 31 ++++++++ src/time.rs | 89 +++++++++++++++++++++++ src/variables.rs | 63 ++++++++++++++-- 9 files changed, 490 insertions(+), 10 deletions(-) create mode 100644 src/state.rs create mode 100644 src/state/certificate.rs create mode 100644 src/state/issuer.rs create mode 100644 src/time.rs diff --git a/src/conf.rs b/src/conf.rs index 4a308ca..45e888d 100644 --- a/src/conf.rs +++ b/src/conf.rs @@ -16,6 +16,7 @@ use self::issuer::Issuer; use self::order::CertificateOrder; use self::pkey::PrivateKey; use self::shared_zone::{SharedZone, ACME_ZONE_NAME, ACME_ZONE_SIZE}; +use crate::state::AcmeSharedData; pub mod ext; pub mod identifier; @@ -31,6 +32,7 @@ const NGX_CONF_DUPLICATE: *mut c_char = c"is duplicate".as_ptr().cast_mut(); #[derive(Debug, Default)] pub struct AcmeMainConfig { pub issuers: Vec, + pub data: Option<&'static AcmeSharedData>, pub shm_zone: shared_zone::SharedZone, } @@ -500,7 +502,10 @@ impl AcmeMainConfig { self.shm_zone = SharedZone::Configured(ACME_ZONE_NAME, ACME_ZONE_SIZE); } + let amcfp = ptr::from_mut(self).cast(); let shm_zone = self.shm_zone.request(cf)?; + shm_zone.init = Some(crate::state::ngx_acme_shared_zone_init); + shm_zone.data = amcfp; shm_zone.noreuse = 1; Ok(()) diff --git a/src/conf/issuer.rs b/src/conf/issuer.rs index 8b06c5d..02a4ca1 100644 --- a/src/conf/issuer.rs +++ b/src/conf/issuer.rs @@ -13,6 +13,7 @@ use ngx::collections::{RbTreeMap, Vec}; use ngx::core::{Pool, Status}; use ngx::http::{HttpModuleLocationConf, NgxHttpCoreModule}; use ngx::ngx_log_debug; +use ngx::sync::RwLock; use openssl::pkey::{PKey, Private}; use thiserror::Error; @@ -20,6 +21,8 @@ use super::order::CertificateOrder; use super::pkey::PrivateKey; use super::ssl::NgxSsl; use super::AcmeMainConfig; +use crate::state::certificate::CertificateContext; +use crate::state::issuer::IssuerContext; const ACCOUNT_KEY_FILE: &str = "account.key"; const NGX_ACME_DEFAULT_RESOLVER_TIMEOUT: ngx_msec_t = 30000; @@ -41,8 +44,9 @@ pub struct Issuer { // Generated fields // ngx_ssl_t stores a pointer to itself in SSL_CTX ex_data. pub ssl: Box, - pub orders: RbTreeMap, (), Pool>, + pub orders: RbTreeMap, CertificateContext, Pool>, pub pkey: Option>, + pub data: Option<&'static RwLock>, } #[derive(Debug, Error)] @@ -83,6 +87,7 @@ impl Issuer { ssl, pkey: None, orders: RbTreeMap::try_new_in(alloc)?, + data: None, }) } @@ -158,7 +163,9 @@ impl Issuer { self.name ); - if self.orders.try_insert(order.clone(), ()).is_err() { + let cert = CertificateContext::Empty; + + if self.orders.try_insert(order.clone(), cert).is_err() { return Err(Status::NGX_ERROR); } } else { diff --git a/src/conf/shared_zone.rs b/src/conf/shared_zone.rs index f935e9c..ccb6012 100644 --- a/src/conf/shared_zone.rs +++ b/src/conf/shared_zone.rs @@ -2,7 +2,7 @@ use core::ffi::c_void; use core::ptr::{self, NonNull}; use nginx_sys::{ngx_conf_t, ngx_int_t, ngx_shm_zone_t, ngx_str_t, NGX_ERROR}; -use ngx::core::Status; +use ngx::core::{SlabPool, Status}; use ngx::http::HttpModule; use ngx::log::ngx_cycle_log; use ngx::{ngx_log_debug, ngx_string}; @@ -32,6 +32,13 @@ pub enum SharedZoneError { } impl SharedZone { + pub fn allocator(&self) -> Option { + match self { + Self::Ready(zone) => unsafe { SlabPool::from_shm_zone(zone.as_ref()) }, + _ => None, + } + } + pub fn is_configured(&self) -> bool { !matches!(self, Self::Unset) } diff --git a/src/lib.rs b/src/lib.rs index 30ee931..3b6d33d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -14,6 +14,8 @@ use crate::conf::{AcmeMainConfig, AcmeServerConfig, NGX_HTTP_ACME_COMMANDS}; use crate::variables::NGX_HTTP_ACME_VARS; mod conf; +mod state; +mod time; mod variables; #[derive(Debug)] diff --git a/src/state.rs b/src/state.rs new file mode 100644 index 0000000..bff2c84 --- /dev/null +++ b/src/state.rs @@ -0,0 +1,140 @@ +//! Shared runtime state of the module. +use core::ffi::c_void; +use core::ptr; + +use nginx_sys::{ngx_int_t, ngx_shm_zone_t, NGX_LOG_EMERG}; +use ngx::allocator::{AllocError, Allocator, Box, TryCloneIn}; +use ngx::collections::Queue; +use ngx::core::{SlabPool, Status}; +use ngx::log::ngx_cycle_log; +use ngx::sync::RwLock; +use ngx::{ngx_log_debug, ngx_log_error}; + +use crate::conf::shared_zone::SharedZone; +use crate::conf::AcmeMainConfig; + +pub use self::certificate::CertificateContext; +pub use self::issuer::IssuerContext; + +pub mod certificate; +pub mod issuer; + +#[derive(Debug)] +pub struct AcmeSharedData +where + A: Allocator + Clone, +{ + pub issuers: Queue, A>, +} + +impl AcmeSharedData +where + A: Allocator + Clone, +{ + pub fn try_new_in(alloc: A) -> Result { + Ok(Self { + issuers: Queue::try_new_in(alloc)?, + }) + } +} + +pub extern "C" fn ngx_acme_shared_zone_init( + shm_zone: *mut ngx_shm_zone_t, + data: *mut c_void, +) -> ngx_int_t { + // SAFETY: shm_zone is always valid in this callback + let shm_zone = unsafe { &mut *shm_zone }; + let log = ngx_cycle_log().as_ptr(); + + ngx_log_debug!( + log, + "acme: init shared zone \"{}:{}\"", + shm_zone.shm.name, + shm_zone.shm.size, + ); + + let oamcf = unsafe { data.cast::().as_ref() }; + let amcf = unsafe { shm_zone.data.cast::().as_mut().unwrap() }; + let zone = SharedZone::Ready(shm_zone.into()); + + let mut alloc = zone.allocator().expect("shared zone allocator"); + + // Our shared zone is `noreuse`, meaning that we get an empty zone every time unless we are + // running on Windows. + + let Ok(mut data) = + AcmeSharedData::try_new_in(alloc.clone()).and_then(|x| Box::try_new_in(x, alloc.clone())) + else { + ngx_log_error!(NGX_LOG_EMERG, log, "cannot allocate acme shared data"); + return Status::NGX_ERROR.into(); + }; + + for issuer in &mut amcf.issuers[..] { + // Create new shared data. + let Ok(ctx) = IssuerContext::try_new_in(issuer, alloc.clone()) else { + ngx_log_error!( + NGX_LOG_EMERG, + log, + "cannot allocate acme issuer \"{}\"", + issuer.name, + ); + return Status::NGX_ERROR.into(); + }; + + // Copy data from the previous cycle. + if let Some(oissuer) = oamcf.and_then(|x| x.issuer(&issuer.name)) { + ngx_log_debug!(log, "acme: copy old data for issuer \"{}\"", issuer.name); + + for (order, ctx) in issuer.orders.iter_mut() { + // Should not fail as we just allocated all the certificate contexts. + let CertificateContext::Shared(ctx) = ctx else { + continue; + }; + + let Some(CertificateContext::Shared(octx)) = oissuer.orders.get(order) else { + continue; + }; + + // The old shared zone is going away as soon as we're done, so we have to copy the + // data to the new slab pool. + let Ok(cloned) = octx.read().try_clone_in(alloc.clone()) else { + return Status::NGX_ERROR.into(); + }; + + *ctx.write() = cloned; + } + } + + if let Ok(ctx) = data.issuers.push_back(RwLock::new(ctx)) { + // SAFETY: we ensured that the chosen data structure will not move the IssuerContext, + // thus the pointer will remain valid beyond this scope. + // + // The assigned lifetime is a bit misleading though; shared zone will be unmapped + // while the main config is still present, right before calling the cycle pool cleanup. + // A proper ownership-tracking pointer could attempt to unref the data from the config + // destructor _after_ the zone is unmapped and thus trip on an invalid address. + // + // Of all the ways to handle that, we are picking the most obviously unsafe to make + // sure this detail is not missed while reading. + issuer.data = Some(unsafe { &*ptr::from_ref(ctx) }); + } else { + ngx_log_error!( + NGX_LOG_EMERG, + log, + "cannot allocate acme issuer \"{}\"", + issuer.name, + ); + return Status::NGX_ERROR.into(); + } + } + + // Will be freed when the zone is unmapped. + let data = Box::leak(data); + + alloc.as_mut().data = ptr::from_mut(data).cast(); + + amcf.data = Some(data); + amcf.shm_zone = zone; + + Status::NGX_OK.into() +} diff --git a/src/state/certificate.rs b/src/state/certificate.rs new file mode 100644 index 0000000..3955c7f --- /dev/null +++ b/src/state/certificate.rs @@ -0,0 +1,150 @@ +use ngx::allocator::{AllocError, Allocator, TryCloneIn}; +use ngx::collections::Vec; +use ngx::core::SlabPool; +use ngx::sync::RwLock; + +use crate::time::{jitter, Time, TimeRange}; + +pub type SharedCertificateContext = RwLock>; + +#[derive(Debug, Default)] +pub enum CertificateContext { + #[default] + Empty, + // Ready to use certificate in shared memory. + Shared(&'static SharedCertificateContext), +} + +impl CertificateContext { + pub fn as_ref(&self) -> Option<&'static SharedCertificateContext> { + if let CertificateContext::Shared(data) = self { + Some(data) + } else { + None + } + } +} + +#[derive(Debug, Default, PartialEq, Eq)] +pub enum CertificateState { + #[default] + Pending, + Ready, +} + +#[derive(Debug)] +pub struct CertificateContextInner +where + A: Allocator + Clone, +{ + pub state: CertificateState, + pub chain: Vec, + pub pkey: Vec, + pub valid: TimeRange, + pub next: Time, +} + +impl TryCloneIn for CertificateContextInner +where + OA: Allocator + Clone, +{ + type Target = CertificateContextInner; + + fn try_clone_in(&self, alloc: A) -> Result, AllocError> { + let mut chain = Vec::new_in(alloc.clone()); + chain + .try_reserve_exact(self.chain.len()) + .map_err(|_| AllocError)?; + chain.extend(self.chain.iter()); + + let mut pkey = Vec::new_in(alloc); + pkey.try_reserve_exact(self.pkey.len()) + .map_err(|_| AllocError)?; + pkey.extend(self.pkey.iter()); + + Ok(Self::Target { + state: CertificateState::Ready, + chain, + pkey, + valid: self.valid.clone(), + next: self.next, + }) + } +} + +impl CertificateContextInner +where + A: Allocator + Clone, +{ + pub fn new_in(alloc: A) -> Self { + Self { + state: CertificateState::Pending, + chain: Vec::new_in(alloc.clone()), + pkey: Vec::new_in(alloc.clone()), + valid: Default::default(), + next: Default::default(), + } + } + + pub fn set(&mut self, chain: &[u8], pkey: &[u8], valid: TimeRange) -> Result { + const PREFIX: &[u8] = b"data:"; + + // reallocate the storage only if the current capacity is insufficient + + fn needs_realloc(buf: &Vec, new_size: usize) -> bool { + buf.capacity() < PREFIX.len() + new_size + } + + if needs_realloc(&self.chain, chain.len()) || needs_realloc(&self.pkey, pkey.len()) { + let alloc = self.chain.allocator(); + + let mut new_chain: Vec = Vec::new_in(alloc.clone()); + new_chain + .try_reserve_exact(PREFIX.len() + chain.len()) + .map_err(|_| AllocError)?; + + let mut new_pkey: Vec = Vec::new_in(alloc.clone()); + new_pkey + .try_reserve_exact(PREFIX.len() + pkey.len()) + .map_err(|_| AllocError)?; + + self.chain = new_chain; + self.pkey = new_pkey; + } + + // update the stored data in-place + + self.chain.clear(); + self.chain.extend(PREFIX); + self.chain.extend(chain); + + self.pkey.clear(); + self.pkey.extend(PREFIX); + self.pkey.extend(pkey); + + // Schedule the next update at around 2/3 of the cert lifetime, + // as recommended in Let's Encrypt integration guide + self.next = valid.start + jitter(valid.duration() * 2 / 3, 2); + self.valid = valid; + + self.state = CertificateState::Ready; + + Ok(self.next) + } + + pub fn chain(&self) -> Option<&[u8]> { + if matches!(self.state, CertificateState::Ready) { + return Some(&self.chain); + } + + None + } + + pub fn pkey(&self) -> Option<&[u8]> { + if matches!(self.state, CertificateState::Ready) { + return Some(&self.pkey); + } + + None + } +} diff --git a/src/state/issuer.rs b/src/state/issuer.rs new file mode 100644 index 0000000..eb45a41 --- /dev/null +++ b/src/state/issuer.rs @@ -0,0 +1,31 @@ +use core::ptr; + +use ngx::allocator::AllocError; +use ngx::collections::Queue; +use ngx::core::SlabPool; +use ngx::sync::RwLock; + +use crate::conf::issuer::Issuer; + +use super::certificate::{CertificateContext, CertificateContextInner, SharedCertificateContext}; + +#[derive(Debug)] +pub struct IssuerContext { + // Using Queue here to ensure address stability. + #[allow(unused)] + pub certificates: Queue, +} + +impl IssuerContext { + pub fn try_new_in(issuer: &mut Issuer, alloc: SlabPool) -> Result { + let mut certificates = Queue::try_new_in(alloc.clone())?; + + for (_, value) in issuer.orders.iter_mut() { + let ctx = CertificateContextInner::new_in(alloc.clone()); + let ctx = certificates.push_back(RwLock::new(ctx))?; + *value = CertificateContext::Shared(unsafe { &*ptr::from_ref(ctx) }); + } + + Ok(IssuerContext { certificates }) + } +} diff --git a/src/time.rs b/src/time.rs new file mode 100644 index 0000000..757cc19 --- /dev/null +++ b/src/time.rs @@ -0,0 +1,89 @@ +use core::ops; +use core::time::Duration; + +use nginx_sys::{ngx_random, ngx_time, time_t}; +use thiserror::Error; + +#[derive(Debug, Error)] +#[error("invalid time")] +pub struct InvalidTime; + +/// Unix timestamp value in seconds. +/// +/// We could take a more complete implementation, like `::time::UtcDateTime`, +/// but it wolud be noticeably larger with unnecessary for this scenario precision. +#[derive(Clone, Copy, Debug, Default, Hash, PartialEq, Eq, PartialOrd, Ord)] +#[repr(transparent)] +pub struct Time(time_t); + +impl Time { + // time_t can be signed, but is not supposed to be negative + pub const MIN: Self = Self(0); + + pub fn now() -> Self { + Self(ngx_time()) + } +} + +/// This type represents an open-ended interval of time measured in seconds. +#[derive(Clone, Debug, Default)] +pub struct TimeRange { + pub start: Time, + pub end: Time, +} + +impl TimeRange { + /// Returns duration between the start and the end of the interval. + #[inline] + pub fn duration(&self) -> Duration { + self.end - self.start + } +} + +/// Randomizes the duration within the specified percentage, with a 1s accuracy. +pub fn jitter(value: Duration, pct: u8) -> Duration { + let var = value * (pct as u32) / 100; + + let var_secs = var.as_secs(); + if var_secs == 0 { + return value; + } + + let diff = Duration::from_secs(ngx_random() as u64 % (var_secs * 2)); + + value + diff - var +} + +/* A reasonable set of arithmetic operations: + * time + duration = time + * time - duration = time + * time - time = duration + * time + time = ??? + */ + +impl ops::Add for Time { + type Output = Self; + + fn add(self, rhs: Duration) -> Self::Output { + Self(self.0.saturating_add(rhs.as_secs() as _)) + } +} + +impl ops::Sub for Time { + type Output = Self; + + fn sub(self, rhs: Duration) -> Self::Output { + // time_t is not supposed to be negative + Self(self.0 - rhs.as_secs() as time_t).max(Self::MIN) + } +} + +impl ops::Sub for Time { + type Output = Duration; + + fn sub(self, rhs: Self) -> Self::Output { + // duration cannot be negative + let diff = (self.0 - rhs.0).max(0) as u64; + Duration::from_secs(diff) + } +} diff --git a/src/variables.rs b/src/variables.rs index 8469a6b..96ceea7 100644 --- a/src/variables.rs +++ b/src/variables.rs @@ -1,8 +1,12 @@ +use nginx_sys::{ + ngx_http_request_t, ngx_http_variable_t, ngx_int_t, ngx_str_t, ngx_variable_value_t, +}; use ngx::core::Status; -use ngx::ffi::{ngx_http_request_t, ngx_http_variable_t, ngx_int_t, ngx_variable_value_t}; use ngx::http::{HttpModuleMainConf, HttpModuleServerConf}; use ngx::ngx_string; +use crate::conf::{AcmeMainConfig, AcmeServerConfig}; +use crate::state::certificate::SharedCertificateContext; use crate::HttpAcmeModule; pub(crate) static mut NGX_HTTP_ACME_VARS: [ngx_http_variable_t; 2] = [ @@ -32,10 +36,28 @@ extern "C" fn acme_var_certificate( let r = unsafe { &mut *r }; let v = unsafe { &mut *v }; - let _amcf = HttpAcmeModule::main_conf(r).expect("acme main conf"); - let _ascf = HttpAcmeModule::server_conf(r).expect("acme server conf"); + let amcf = HttpAcmeModule::main_conf(r).expect("acme main conf"); + let ascf = HttpAcmeModule::server_conf(r).expect("acme server conf"); + + let Some(cert_data) = lookup_certificate_data(amcf, ascf) else { + (*v).set_not_found(1); + return Status::NGX_OK.into(); + }; + + let Some(bytes) = cert_data + .read() + .chain() + .and_then(|x| unsafe { ngx_str_t::from_bytes(r.pool, x) }) + else { + return Status::NGX_ERROR.into(); + }; + + v.set_valid(1); + v.set_no_cacheable(0); + v.set_not_found(0); + v.set_len(bytes.len as u32 - 1); + v.data = bytes.data; - (*v).set_not_found(1); Status::NGX_OK.into() } @@ -47,9 +69,36 @@ unsafe extern "C" fn acme_var_certificate_key( let r = unsafe { &mut *r }; let v = unsafe { &mut *v }; - let _amcf = HttpAcmeModule::main_conf(r).expect("acme config"); - let _ascf = HttpAcmeModule::server_conf(r).expect("acme server conf"); + let amcf = HttpAcmeModule::main_conf(r).expect("acme config"); + let ascf = HttpAcmeModule::server_conf(r).expect("acme server conf"); + + let Some(cert_data) = lookup_certificate_data(amcf, ascf) else { + (*v).set_not_found(1); + return Status::NGX_OK.into(); + }; + + let Some(bytes) = cert_data + .read() + .pkey() + .and_then(|x| unsafe { ngx_str_t::from_bytes(r.pool, x) }) + else { + return Status::NGX_ERROR.into(); + }; + + v.set_valid(1); + v.set_no_cacheable(0); + v.set_not_found(0); + v.set_len(bytes.len as u32 - 1); + v.data = bytes.data; - (*v).set_not_found(1); Status::NGX_OK.into() } + +fn lookup_certificate_data<'a>( + amcf: &'a AcmeMainConfig, + ascf: &AcmeServerConfig, +) -> Option<&'a SharedCertificateContext> { + let order = ascf.order.as_ref()?; + let issuer = amcf.issuer(&ascf.issuer)?; + issuer.orders.get(order)?.as_ref() +} From 2a435bb671a9b98d51dbdd53cf0c7dd6b392c1fa Mon Sep 17 00:00:00 2001 From: Aleksei Bavshin Date: Fri, 25 Jul 2025 12:43:23 -0700 Subject: [PATCH 3/4] ACME: restore previously issued certs from "state_path". Improves time to readiness after a full restart of the server. --- Cargo.lock | 1 + Cargo.toml | 1 + build.rs | 31 ++++++++++++++ src/conf/issuer.rs | 93 +++++++++++++++++++++++++++++++++++++++- src/conf/ssl.rs | 25 +++++++++++ src/state/certificate.rs | 4 +- src/state/issuer.rs | 9 +++- src/time.rs | 75 ++++++++++++++++++++++++++++++++ 8 files changed, 234 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 119c59d..5400ce2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -285,6 +285,7 @@ version = "0.1.0" dependencies = [ "foreign-types", "http", + "libc", "nginx-sys", "ngx", "openssl", diff --git a/Cargo.toml b/Cargo.toml index 18912be..f022b03 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,7 @@ crate-type = ["cdylib"] [dependencies] http = "1.3.1" +libc = "0.2.174" openssl = { version = "0.10.73", features = ["bindgen"] } openssl-foreign-types = { package = "foreign-types", version = "0.3" } openssl-sys = { version = "0.9.109", features = ["bindgen"] } diff --git a/build.rs b/build.rs index c44ad9e..7ea22fc 100644 --- a/build.rs +++ b/build.rs @@ -8,6 +8,7 @@ use std::env; /// [1]: https://github.com/rust-lang/cargo/issues/3544 fn main() { detect_nginx_features(); + detect_libssl_features(); // Generate required compiler flags if cfg!(target_os = "macos") { @@ -57,3 +58,33 @@ fn detect_nginx_features() { } } } + +/// Detects libssl implementation and version. +fn detect_libssl_features() { + // OpenSSL + let openssl_features = ["awslc", "boringssl", "libressl", "openssl", "openssl111"]; + let openssl_version = env::var("DEP_OPENSSL_VERSION_NUMBER").unwrap_or_default(); + let openssl_version = u64::from_str_radix(&openssl_version, 16).unwrap_or(0); + + println!( + "cargo::rustc-check-cfg=cfg(openssl, values(\"{}\"))", + openssl_features.join("\",\"") + ); + + #[allow(clippy::unusual_byte_groupings)] + let openssl = if env::var("DEP_OPENSSL_AWSLC").is_ok() { + "awslc" + } else if env::var("DEP_OPENSSL_BORINGSSL").is_ok() { + "boringssl" + } else if env::var("DEP_OPENSSL_LIBRESSL").is_ok() { + "libressl" + } else { + if openssl_version >= 0x01_01_01_00_0 { + println!("cargo::rustc-cfg=openssl=\"openssl111\""); + } + + "openssl" + }; + + println!("cargo::rustc-cfg=openssl=\"{openssl}\""); +} diff --git a/src/conf/issuer.rs b/src/conf/issuer.rs index 02a4ca1..5f24063 100644 --- a/src/conf/issuer.rs +++ b/src/conf/issuer.rs @@ -17,12 +17,14 @@ use ngx::sync::RwLock; use openssl::pkey::{PKey, Private}; use thiserror::Error; +use super::ext::NgxConfExt; use super::order::CertificateOrder; use super::pkey::PrivateKey; use super::ssl::NgxSsl; use super::AcmeMainConfig; -use crate::state::certificate::CertificateContext; +use crate::state::certificate::{CertificateContext, CertificateContextInner}; use crate::state::issuer::IssuerContext; +use crate::time::{Time, TimeRange}; const ACCOUNT_KEY_FILE: &str = "account.key"; const NGX_ACME_DEFAULT_RESOLVER_TIMEOUT: ngx_msec_t = 30000; @@ -163,7 +165,32 @@ impl Issuer { self.name ); - let cert = CertificateContext::Empty; + let mut cert = CertificateContext::Empty; + + if let Some(state_dir) = unsafe { StateDir::from_ptr(self.state_path) } { + match state_dir.load_certificate(cf, order) { + Ok(x) => { + ngx_log_debug!( + cf.log, + "acme: found cached certificate {}/{}, next update in {:?}", + self.name, + order.cache_key(), + (x.next - Time::now()), + ); + cert = CertificateContext::Local(x); + } + Err(CachedCertificateError::NotFound) => (), + Err(err) => { + ngx_log_debug!( + cf.log, + "acme: cannot load certificate {}/{} from state path: {}", + self.name, + order.cache_key(), + err + ); + } + } + } if self.orders.try_insert(order.clone(), cert).is_err() { return Err(Status::NGX_ERROR); @@ -239,6 +266,20 @@ impl Issuer { } } +#[derive(Debug, thiserror::Error)] +enum CachedCertificateError { + #[error(transparent)] + Alloc(#[from] AllocError), + #[error("X509_check_private_key() failed: {0}")] + Mismatch(openssl::error::ErrorStack), + #[error("file not found")] + NotFound, + #[error(transparent)] + Ssl(#[from] openssl::error::ErrorStack), + #[error("failed to load file: {0}")] + CertificateFetch(#[from] super::ssl::CertificateFetchError), +} + /// The StateDir helper encapsulates operations with a persistent state in the state directory. #[repr(transparent)] struct StateDir(ngx_path_t); @@ -264,4 +305,52 @@ impl StateDir { pub fn write(&self, path: &std::path::Path, data: &[u8]) -> Result<(), std::io::Error> { std::fs::write(path, data) } + + pub fn load_certificate( + &self, + cf: &mut ngx_conf_t, + order: &CertificateOrder, + ) -> Result, CachedCertificateError> { + use openssl_foreign_types::ForeignType; + #[cfg(ngx_ssl_cache)] + use openssl_foreign_types::ForeignTypeRef; + + let name = order.cache_key(); + + let cert = std::format!("{}/{}.crt", self.0.name, name); + if matches!(std::fs::exists(&cert), Ok(false)) { + return Err(CachedCertificateError::NotFound); + } + + let key = std::format!("{}/{}.key", self.0.name, name); + if matches!(std::fs::exists(&key), Ok(false)) { + return Err(CachedCertificateError::NotFound); + } + + let stack = super::ssl::conf_read_certificate(cf, &cert)?; + #[allow(clippy::get_first)] // ^ can return Stack or Vec, depending on the NGINX version + let cert = stack + .get(0) + .ok_or(super::ssl::CertificateFetchError::Fetch(c"no certificates"))?; + let pkey = super::ssl::conf_read_private_key(cf, &key)?; + + if unsafe { openssl_sys::X509_check_private_key(cert.as_ptr(), pkey.as_ptr()) } != 1 { + return Err(CachedCertificateError::Mismatch( + openssl::error::ErrorStack::get(), + )); + } + + let valid = TimeRange::from_x509(cert).unwrap_or_default(); + let temp_alloc = unsafe { Pool::from_ngx_pool(cf.temp_pool) }; + + let mut chain: Vec = Vec::new_in(temp_alloc.clone()); + for x509 in stack.into_iter() { + chain.extend(x509.to_pem()?.into_iter()); + } + + let mut cert = CertificateContextInner::new_in(cf.pool()); + cert.set(&chain, &pkey.private_key_to_pem_pkcs8()?, valid)?; + + Ok(cert) + } } diff --git a/src/conf/ssl.rs b/src/conf/ssl.rs index adb913d..532125a 100644 --- a/src/conf/ssl.rs +++ b/src/conf/ssl.rs @@ -9,6 +9,7 @@ use nginx_sys::{ use ngx::allocator::AllocError; use ngx::core::Status; use openssl::pkey::{PKey, Private}; +use openssl::x509::X509; use openssl_sys::SSL_CTX_set_default_verify_paths; use thiserror::Error; @@ -21,6 +22,30 @@ pub enum CertificateFetchError { #[error("{0:?} {1}")] Ssl(&'static CStr, openssl::error::ErrorStack), } + +#[cfg(ngx_ssl_cache)] +pub fn conf_read_certificate( + cf: &mut ngx_conf_t, + name: &str, +) -> Result, CertificateFetchError> { + conf_ssl_cache_fetch(cf, nginx_sys::NGX_SSL_CACHE_CERT as _, name) +} + +#[cfg(not(ngx_ssl_cache))] +pub fn conf_read_certificate( + _cf: &mut ngx_conf_t, + name: &str, +) -> Result, CertificateFetchError> { + let Ok(buf) = std::fs::read_to_string(name) else { + return Err(CertificateFetchError::Fetch(c"cannot load certificate")); + }; + + match X509::stack_from_pem(buf.as_bytes()) { + Ok(x) => Ok(x), + Err(err) => Err(CertificateFetchError::Ssl(c"cannot load key", err)), + } +} + #[cfg(ngx_ssl_cache)] pub fn conf_read_private_key( cf: &mut ngx_conf_t, diff --git a/src/state/certificate.rs b/src/state/certificate.rs index 3955c7f..3cbbafc 100644 --- a/src/state/certificate.rs +++ b/src/state/certificate.rs @@ -1,6 +1,6 @@ use ngx::allocator::{AllocError, Allocator, TryCloneIn}; use ngx::collections::Vec; -use ngx::core::SlabPool; +use ngx::core::{Pool, SlabPool}; use ngx::sync::RwLock; use crate::time::{jitter, Time, TimeRange}; @@ -11,6 +11,8 @@ pub type SharedCertificateContext = RwLock>; pub enum CertificateContext { #[default] Empty, + // Previously issued certificate, restored from the state directory. + Local(CertificateContextInner), // Ready to use certificate in shared memory. Shared(&'static SharedCertificateContext), } diff --git a/src/state/issuer.rs b/src/state/issuer.rs index eb45a41..fe370e0 100644 --- a/src/state/issuer.rs +++ b/src/state/issuer.rs @@ -1,6 +1,6 @@ use core::ptr; -use ngx::allocator::AllocError; +use ngx::allocator::{AllocError, TryCloneIn}; use ngx::collections::Queue; use ngx::core::SlabPool; use ngx::sync::RwLock; @@ -21,7 +21,12 @@ impl IssuerContext { let mut certificates = Queue::try_new_in(alloc.clone())?; for (_, value) in issuer.orders.iter_mut() { - let ctx = CertificateContextInner::new_in(alloc.clone()); + let ctx = if let CertificateContext::Local(value) = value { + value.try_clone_in(alloc.clone())? + } else { + CertificateContextInner::new_in(alloc.clone()) + }; + let ctx = certificates.push_back(RwLock::new(ctx))?; *value = CertificateContext::Shared(unsafe { &*ptr::from_ref(ctx) }); } diff --git a/src/time.rs b/src/time.rs index 757cc19..c33d238 100644 --- a/src/time.rs +++ b/src/time.rs @@ -2,6 +2,9 @@ use core::ops; use core::time::Duration; use nginx_sys::{ngx_random, ngx_time, time_t}; +use openssl::asn1::Asn1TimeRef; +use openssl::x509::X509Ref; +use openssl_foreign_types::ForeignTypeRef; use thiserror::Error; #[derive(Debug, Error)] @@ -16,6 +19,66 @@ pub struct InvalidTime; #[repr(transparent)] pub struct Time(time_t); +impl TryFrom<&Asn1TimeRef> for Time { + type Error = InvalidTime; + + #[cfg(openssl = "openssl111")] + fn try_from(asn1time: &Asn1TimeRef) -> Result { + let val = unsafe { + let mut tm: libc::tm = core::mem::zeroed(); + if openssl_sys::ASN1_TIME_to_tm(asn1time.as_ptr(), &mut tm) != 1 { + return Err(InvalidTime); + } + libc::timegm(&mut tm) as _ + }; + + Ok(Time(val)) + } + + #[cfg(any(openssl = "awslc", openssl = "boringssl"))] + fn try_from(asn1time: &Asn1TimeRef) -> Result { + let mut val: time_t = 0; + if unsafe { openssl_sys::ASN1_TIME_to_time_t(asn1time.as_ptr(), &mut val) } != 1 { + return Err(InvalidTime); + } + Ok(Time(val)) + } + + #[cfg(not(any(openssl = "openssl111", openssl = "awslc", openssl = "boringssl")))] + fn try_from(asn1time: &Asn1TimeRef) -> Result { + pub const NGX_INVALID_TIME: time_t = nginx_sys::NGX_ERROR as _; + + use openssl_sys::{ + ASN1_TIME_print, BIO_free, BIO_get_mem_data, BIO_new, BIO_s_mem, BIO_write, + }; + + let val = unsafe { + let bio = BIO_new(BIO_s_mem()); + if bio.is_null() { + openssl::error::ErrorStack::get(); // clear errors + return Err(InvalidTime); + } + + let mut value: *mut core::ffi::c_char = core::ptr::null_mut(); + /* fake weekday prepended to match C asctime() format */ + let prefix = c"Tue "; + BIO_write(bio, prefix.as_ptr().cast(), prefix.count_bytes() as _); + ASN1_TIME_print(bio, asn1time.as_ptr()); + let len = BIO_get_mem_data(bio, &mut value); + let val = ngx_parse_http_time(value.cast(), len as _); + + BIO_free(bio); + val + }; + + if val == NGX_INVALID_TIME { + return Err(InvalidTime); + } + + Ok(Time(val)) + } +} + impl Time { // time_t can be signed, but is not supposed to be negative pub const MIN: Self = Self(0); @@ -33,6 +96,18 @@ pub struct TimeRange { } impl TimeRange { + pub fn new(start: Time, end: Time) -> Self { + // ensure that end >= start + let end = end.max(start); + Self { start, end } + } + + pub fn from_x509(x509: &X509Ref) -> Option { + let start = Time::try_from(x509.not_before()).ok()?; + let end = Time::try_from(x509.not_after()).ok()?; + Some(Self::new(start, end)) + } + /// Returns duration between the start and the end of the interval. #[inline] pub fn duration(&self) -> Duration { From cde9ae61b85495aa14c90ca98b578b6c1a50b237 Mon Sep 17 00:00:00 2001 From: Aleksei Bavshin Date: Fri, 25 Jul 2025 12:55:13 -0700 Subject: [PATCH 4/4] ACME: zeroize buffers with private keys after use. We trust the SSL library to securely clear the EVP_PKEY objects, but all the places where we may store a PEM data should be cleared by us. --- Cargo.lock | 7 +++++++ Cargo.toml | 1 + src/conf/issuer.rs | 6 ++++-- src/conf/ssl.rs | 2 +- src/state/certificate.rs | 16 ++++++++++++++++ 5 files changed, 29 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5400ce2..d06bd8e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -292,6 +292,7 @@ dependencies = [ "openssl-sys", "siphasher", "thiserror", + "zeroize", ] [[package]] @@ -715,3 +716,9 @@ checksum = "fe5c30ade05e61656247b2e334a031dfd0cc466fadef865bdcdea8d537951bf1" dependencies = [ "winapi", ] + +[[package]] +name = "zeroize" +version = "1.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ced3678a2879b30306d323f4542626697a464a97c0a07c9aebf7ebca65cd4dde" diff --git a/Cargo.toml b/Cargo.toml index f022b03..152a706 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ openssl-foreign-types = { package = "foreign-types", version = "0.3" } openssl-sys = { version = "0.9.109", features = ["bindgen"] } siphasher = { version = "1.0.1", default-features = false } thiserror = { version = "2.0.12", default-features = false } +zeroize = "1.8.1" [dependencies.nginx-sys] git = "https://github.com/nginx/ngx-rust" diff --git a/src/conf/issuer.rs b/src/conf/issuer.rs index 5f24063..bb8f2f8 100644 --- a/src/conf/issuer.rs +++ b/src/conf/issuer.rs @@ -16,6 +16,7 @@ use ngx::ngx_log_debug; use ngx::sync::RwLock; use openssl::pkey::{PKey, Private}; use thiserror::Error; +use zeroize::Zeroizing; use super::ext::NgxConfExt; use super::order::CertificateOrder; @@ -256,7 +257,7 @@ impl Issuer { } } - if let Ok(buf) = pkey.private_key_to_pem_pkcs8() { + if let Ok(buf) = pkey.private_key_to_pem_pkcs8().map(Zeroizing::new) { // Ignore write errors. let _ = state_dir.write(&path, &buf); } @@ -349,7 +350,8 @@ impl StateDir { } let mut cert = CertificateContextInner::new_in(cf.pool()); - cert.set(&chain, &pkey.private_key_to_pem_pkcs8()?, valid)?; + let pkey = Zeroizing::new(pkey.private_key_to_pem_pkcs8()?); + cert.set(&chain, &pkey, valid)?; Ok(cert) } diff --git a/src/conf/ssl.rs b/src/conf/ssl.rs index 532125a..1f49d3e 100644 --- a/src/conf/ssl.rs +++ b/src/conf/ssl.rs @@ -59,7 +59,7 @@ pub fn conf_read_private_key( _cf: &mut ngx_conf_t, name: &str, ) -> Result, CertificateFetchError> { - let Ok(buf) = std::fs::read_to_string(name) else { + let Ok(buf) = std::fs::read_to_string(name).map(zeroize::Zeroizing::new) else { return Err(CertificateFetchError::Fetch(c"cannot load key")); }; diff --git a/src/state/certificate.rs b/src/state/certificate.rs index 3cbbafc..1740167 100644 --- a/src/state/certificate.rs +++ b/src/state/certificate.rs @@ -2,6 +2,7 @@ use ngx::allocator::{AllocError, Allocator, TryCloneIn}; use ngx::collections::Vec; use ngx::core::{Pool, SlabPool}; use ngx::sync::RwLock; +use zeroize::Zeroize; use crate::time::{jitter, Time, TimeRange}; @@ -110,6 +111,10 @@ where .try_reserve_exact(PREFIX.len() + pkey.len()) .map_err(|_| AllocError)?; + // Zeroize is not implemented for allocator-api2 types. + self.chain.as_mut_slice().zeroize(); + self.pkey.as_mut_slice().zeroize(); + self.chain = new_chain; self.pkey = new_pkey; } @@ -150,3 +155,14 @@ where None } } + +impl Drop for CertificateContextInner +where + A: Allocator + Clone, +{ + fn drop(&mut self) { + // Zeroize is not implemented for allocator-api2 types. + self.chain.as_mut_slice().zeroize(); + self.pkey.as_mut_slice().zeroize(); + } +}