From 27d81935a520ad8d9b8e4c770063997472914ffc Mon Sep 17 00:00:00 2001 From: zealsham Date: Sun, 29 Mar 2026 14:11:39 +0100 Subject: [PATCH] OHTTP keys should be rotated This pr addresses #445. It implements OHTTP-key rotation to payjoin-mailroom Mailroom operators can now decide the time interval for keys to be rotated. Also if a key has expired, a 422 error is returned to clients. Clients can handle they key-rotation via the cach-control header returned by the directory. set rotation grace to 7 days --- Cargo-minimal.lock | 1 + Cargo-recent.lock | 1 + payjoin-mailroom/Cargo.toml | 1 + payjoin-mailroom/src/config.rs | 28 ++ payjoin-mailroom/src/directory.rs | 450 +++++++++++++++++++++++++++-- payjoin-mailroom/src/key_config.rs | 141 +++++++-- payjoin-mailroom/src/lib.rs | 182 ++++++++++-- payjoin-test-utils/src/lib.rs | 4 + 8 files changed, 746 insertions(+), 62 deletions(-) diff --git a/Cargo-minimal.lock b/Cargo-minimal.lock index 8d188f60c..0ccd8a4e3 100644 --- a/Cargo-minimal.lock +++ b/Cargo-minimal.lock @@ -2879,6 +2879,7 @@ dependencies = [ "hex-conservative 0.1.2", "http", "http-body-util", + "httpdate", "hyper", "hyper-rustls", "hyper-util", diff --git a/Cargo-recent.lock b/Cargo-recent.lock index b6ca1be0a..821fe9e0d 100644 --- a/Cargo-recent.lock +++ b/Cargo-recent.lock @@ -2844,6 +2844,7 @@ dependencies = [ "hex-conservative 0.1.2", "http", "http-body-util", + "httpdate", "hyper", "hyper-rustls", "hyper-util", diff --git a/payjoin-mailroom/Cargo.toml b/payjoin-mailroom/Cargo.toml index 3393b9efe..325e00b0b 100644 --- a/payjoin-mailroom/Cargo.toml +++ b/payjoin-mailroom/Cargo.toml @@ -40,6 +40,7 @@ futures = "0.3" hex = { package = "hex-conservative", version = "0.1.1" } http = "1.3.1" http-body-util = "0.1.3" +httpdate = "1.0.3" hyper = { version = "1.6.0", features = ["http1", "server"] } hyper-rustls = { version = "0.27.7", default-features = false, features = [ "webpki-roots", diff --git a/payjoin-mailroom/src/config.rs b/payjoin-mailroom/src/config.rs index 77debbf70..27ae15129 100644 --- a/payjoin-mailroom/src/config.rs +++ b/payjoin-mailroom/src/config.rs @@ -12,6 +12,8 @@ pub struct Config { pub storage_dir: PathBuf, #[serde(deserialize_with = "deserialize_duration_secs")] pub timeout: Duration, + #[serde(deserialize_with = "deserialize_key_epoch_duration_secs")] + pub ohttp_keys_max_age: Option, #[serde(deserialize_with = "deserialize_duration_secs")] pub mailbox_ttl: Duration, pub v1: Option, @@ -87,6 +89,7 @@ impl Default for Config { listener: "[::]:8080".parse().expect("valid default listener address"), storage_dir: PathBuf::from("./data"), timeout: Duration::from_secs(30), + ohttp_keys_max_age: None, //Some(Duration::from_secs(30)), mailbox_ttl: Duration::from_secs(60 * 60 * 24 * 7), // 1 week v1: None, #[cfg(feature = "telemetry")] @@ -107,17 +110,42 @@ where Ok(Duration::from_secs(secs)) } +fn deserialize_key_epoch_duration_secs<'de, D>( + deserializer: D, +) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + let secs: Option = Option::deserialize(deserializer)?; + let minimum = crate::directory::ROTATION_GRACE.saturating_mul(2); + match secs { + None => Ok(None), + Some(s) => { + let duration = Duration::from_secs(s); + if duration <= minimum { + return Err(::custom(format!( + "ohttp_keys_max_age must be greater than {} seconds when set", + minimum.as_secs() + ))); + } + Ok(Some(duration)) + } + } +} + impl Config { pub fn new( listener: ListenerAddress, storage_dir: PathBuf, timeout: Duration, + ohttp_keys_max_age: Option, v1: Option, ) -> Self { Self { listener, storage_dir, timeout, + ohttp_keys_max_age, mailbox_ttl: Duration::from_secs(60 * 60 * 24 * 7), // 1 week v1, #[cfg(feature = "telemetry")] diff --git a/payjoin-mailroom/src/directory.rs b/payjoin-mailroom/src/directory.rs index 066bf8430..e39ac564d 100644 --- a/payjoin-mailroom/src/directory.rs +++ b/payjoin-mailroom/src/directory.rs @@ -1,14 +1,17 @@ +use std::path::PathBuf; use std::pin::Pin; use std::str::FromStr; use std::sync::Arc; use std::task::{Context, Poll}; +use std::time::{Duration, Instant, SystemTime}; use anyhow::Result; use axum::body::{Body, Bytes}; -use axum::http::header::{HeaderValue, ACCESS_CONTROL_ALLOW_ORIGIN, CONTENT_TYPE}; +use axum::http::header::{HeaderValue, ACCESS_CONTROL_ALLOW_ORIGIN, CACHE_CONTROL, CONTENT_TYPE}; use axum::http::{Method, Request, Response, StatusCode, Uri}; use http_body_util::BodyExt; use payjoin::directory::{ShortId, ShortIdError, ENCAPSULATED_MESSAGE_BYTES}; +use tokio::sync::RwLock; use tracing::{debug, error, trace, warn}; use crate::db::{Db, Error as DbError, SendableError}; @@ -28,6 +31,132 @@ const V1_VERSION_UNSUPPORTED_RES_JSON: &str = pub type BoxError = Box; +// Two-slot OHTTP key set supporting rotation overlap. +// +// Key IDs alternate between 0 and 1. Both slots are always populated. +// The current key is served to new clients; both slots are accepted +// for decapsulation so that clients with a cached previous key still +// work during the grace window after a switch. +#[derive(Debug)] +struct KeySlot { + server: Box>, +} + +impl KeySlot { + fn new(server: ohttp::Server) -> Self { Self { server: Box::new(RwLock::new(server)) } } + + async fn decapsulate( + &self, + ohttp_body: &[u8], + ) -> std::result::Result<(Vec, ohttp::ServerResponse), ohttp::Error> { + self.server.read().await.decapsulate(ohttp_body) + } + + async fn encode(&self) -> std::result::Result, ohttp::Error> { + self.server.read().await.config().encode() + } + + async fn overwrite(&self, server: ohttp::Server) { *self.server.write().await = server; } +} + +#[derive(Debug)] +struct ActiveKey { + key_id: u8, + valid_until: Instant, + rekey_at: Instant, + activated_at: SystemTime, +} + +#[derive(Debug)] +pub struct KeyRotatingServer { + keys: [KeySlot; 2], + current: RwLock, +} + +impl KeyRotatingServer { + fn new( + slot0: ohttp::Server, + slot1: ohttp::Server, + current_key_id: u8, + activated_at: SystemTime, + valid_until: Instant, + rekey_at: Instant, + ) -> Self { + assert!(current_key_id <= 1, "key_id must be 0 or 1"); + Self { + keys: [KeySlot::new(slot0), KeySlot::new(slot1)], + current: RwLock::new(ActiveKey { + key_id: current_key_id, + valid_until, + rekey_at, + activated_at, + }), + } + } + + async fn current_key_id(&self) -> u8 { self.current.read().await.key_id } + + pub async fn valid_until(&self) -> Instant { self.current.read().await.valid_until } + + /// Returns the current active key id, the instant at which the standby + /// key needs to be refreshed (`rekey_at`), and the instant at which the + /// active key stops being advertised (`valid_until`). + pub async fn key_timeouts(&self) -> (u8, Instant, Instant) { + let c = self.current.read().await; + (c.key_id, c.rekey_at, c.valid_until) + } + + // Look up the server matching the key_id in an OHTTP message and + // decapsulate. The first byte of an OHTTP encapsulated request is the + // key identifier (RFC 9458 Section 4.3). + async fn decapsulate( + &self, + ohttp_body: &[u8], + ) -> std::result::Result<(Vec, ohttp::ServerResponse), ohttp::Error> { + let key_id = ohttp_body.first().copied().ok_or(ohttp::Error::Truncated)? as usize; + self.keys.get(key_id).ok_or(ohttp::Error::KeyId)?.decapsulate(ohttp_body).await + } + + // Encode the current key's config for serving to clients. + async fn encode_current( + &self, + ) -> std::result::Result<(Vec, Duration, SystemTime), ohttp::Error> { + let current = self.current.read().await; + let valid_for = current.valid_until.saturating_duration_since(Instant::now()); + let encoded = self.keys[current.key_id as usize].encode().await?; + Ok((encoded, valid_for, current.activated_at)) + } + + // Flip which key is advertised to new clients and stamp the new expiry. + // + // `valid_until`, `rekey_at`, and `activated_at` are supplied by the caller + // so that the values served to clients match the timestamps the rotation + // loop wrote to disk (file mtime). This keeps the view consistent across + // restarts and across any latency between touching the key file and + // calling `switch`: on restart the server re-derives the same fields from + // the file mtimes, and we want that to agree with what live clients have + // already observed. + async fn switch(&self, activated_at: SystemTime, valid_until: Instant, rekey_at: Instant) { + let mut current = self.current.write().await; + current.key_id = 1 - current.key_id; + current.valid_until = valid_until; + current.rekey_at = rekey_at; + current.activated_at = activated_at; + } + + // Record that the standby slot has been refreshed and the next rekey is + // now `interval` away. + async fn set_rekey_at(&self, rekey_at: Instant) { + self.current.write().await.rekey_at = rekey_at; + } + + // Replace a slot with fresh key material. + async fn overwrite(&self, key_id: u8, server: ohttp::Server) { + assert!(key_id <= 1, "key_id must be 0 or 1"); + self.keys[key_id as usize].overwrite(server).await; + } +} + /// Opaque blocklist of Bitcoin addresses stored as script pubkeys. /// /// Addresses are converted to `ScriptBuf` at parse time so that @@ -91,7 +220,8 @@ fn parse_address_lines(text: &str) -> std::collections::HashSet { db: D, - ohttp: ohttp::Server, + ohttp: Arc, + ohttp_keys_max_age: Option, sentinel_tag: SentinelTag, v1: Option, } @@ -117,10 +247,18 @@ where } impl Service { - pub fn new(db: D, ohttp: ohttp::Server, sentinel_tag: SentinelTag, v1: Option) -> Self { - Self { db, ohttp, sentinel_tag, v1 } + pub fn new( + db: D, + ohttp: Arc, + ohttp_keys_max_age: Option, + sentinel_tag: SentinelTag, + v1: Option, + ) -> Self { + Self { db, ohttp, ohttp_keys_max_age, sentinel_tag, v1 } } + pub fn ohttp_key_set(&self) -> &Arc { &self.ohttp } + async fn serve_request(&self, req: Request) -> Result> where B: axum::body::HttpBody + Send + 'static, @@ -200,10 +338,10 @@ impl Service { .map_err(|e| HandlerError::BadRequest(anyhow::anyhow!(e.into())))? .to_bytes(); - // Decapsulate OHTTP request let (bhttp_req, res_ctx) = self .ohttp .decapsulate(&ohttp_body) + .await .map_err(|e| HandlerError::OhttpKeyRejection(e.into()))?; let mut cursor = std::io::Cursor::new(bhttp_req); let req = bhttp::Message::read_bhttp(&mut cursor) @@ -378,13 +516,30 @@ impl Service { } async fn get_ohttp_keys(&self) -> Result, HandlerError> { - let ohttp_keys = self + let (ohttp_keys, valid_for, activated_at) = self .ohttp - .config() - .encode() + .encode_current() + .await .map_err(|e| HandlerError::InternalServerError(e.into()))?; let mut res = Response::new(full(ohttp_keys)); res.headers_mut().insert(CONTENT_TYPE, HeaderValue::from_static("application/ohttp-keys")); + + // Last-Modified: when this key became the active key + res.headers_mut().insert( + axum::http::header::LAST_MODIFIED, + HeaderValue::from_str(&httpdate::fmt_http_date(activated_at)).expect("valid http-date"), + ); + + if self.ohttp_keys_max_age.is_some() { + res.headers_mut().insert( + CACHE_CONTROL, + HeaderValue::from_str(&format!( + "public, s-maxage={}, immutable", + valid_for.saturating_add(ACTIVATION_GRACE_PERIOD).as_secs() + )) + .expect("valid header value"), + ); + } Ok(res) } @@ -411,6 +566,232 @@ impl Service { Ok(res) } } +// Grace period after a switch during which the old key is still accepted +// for decapsulation but no longer advertised to new clients. +// +pub(crate) const ROTATION_GRACE: Duration = Duration::from_secs(7 * 24 * 60 * 60); + +// This time frame allows for minor client/server clock skew. +// It also prevent clients with cache and those without cache from +// different using different keys over a long time frame. +pub(crate) const ACTIVATION_GRACE_PERIOD: Duration = Duration::from_secs(15); + +// === OHTTP key rotation === +// Returns a Arc that contains the current and standby keys. +// If an interval is set, it will start a rotation loop. +// The rotation loop will refresh the standby key when the active key expires and +// also handles key switching. + +pub async fn start_ohttp_keys( + keys_dir: PathBuf, + interval: Option, +) -> anyhow::Result> { + let keyset = init_ohttp_keyset(&keys_dir, interval).await?; + + if let Some(interval) = interval { + assert!( + interval > ROTATION_GRACE.saturating_mul(2), + "ohttp_keys_max_age must be greater than 2 * ROTATION_GRACE" + ); + let keyset_bg = keyset.clone(); + tokio::spawn(async move { run_rotation_loop(keyset_bg, keys_dir, interval).await }); + } + + Ok(keyset) +} + +async fn run_rotation_loop(keyset: Arc, keys_dir: PathBuf, interval: Duration) { + let (current_key_id, mut rekey_at, mut valid_until) = keyset.key_timeouts().await; + + // Step 2: if the standby was loaded already past its rekey deadline + // (e.g., the server was down long enough for one key but not both to + // expire), refresh it before entering the main loop. After this the + // invariant `valid_until < rekey_at` holds. + if rekey_at <= valid_until { + tracing::info!( + "Standby key_id {} past rekey deadline ({:?}); refreshing immediately", + 1 - current_key_id, + rekey_at, + ); + tokio::time::sleep_until(rekey_at.into()).await; + rekey_at = refresh_standby(&keyset, &keys_dir, 1 - current_key_id, interval).await; + } + + // Step 3: main loop. Invariant on entry to each iteration: + // `valid_until < rekey_at`. + loop { + assert!(valid_until < rekey_at, "rotation invariant violated"); + tracing::info!("Next switch at {valid_until:?}, next rekey at {rekey_at:?}"); + + tokio::time::sleep_until(valid_until.into()).await; + let (new_valid_until, new_rekey_at) = + switch_active(&keyset, &keys_dir, valid_until, interval).await; + valid_until = new_valid_until; + rekey_at = new_rekey_at; + + tokio::time::sleep_until(rekey_at.into()).await; + let old_active_id = 1 - keyset.current_key_id().await; + rekey_at = refresh_standby(&keyset, &keys_dir, old_active_id, interval).await; + } +} + +/// Phase 1: rebuild the keyset from disk so the rotation loop can resume. +/// +/// Pure recovery primitive: handles "first start" (no files), "warm restart" +/// (both keys valid), "partial expiry" (skip the expired one) and "long +/// downtime" (both expired → regenerate both, restart at key_id=0). +pub(crate) async fn init_ohttp_keyset( + ohttp_keys_dir: &std::path::Path, + interval: Option, +) -> anyhow::Result> { + tokio::fs::create_dir_all(ohttp_keys_dir).await?; + + // Ensure both key files exist, generating any that are missing. + for key_id in [0u8, 1] { + let path = crate::key_config::key_path_for_id(ohttp_keys_dir, key_id); + if !path.exists() { + let config = crate::key_config::gen_ohttp_server_config_with_id(key_id)?; + crate::key_config::persist_key_config(&config, ohttp_keys_dir).await?; + tracing::info!("Generated missing OHTTP key_id {key_id}"); + } + } + + // Read both keys with their mtimes. + let sys_now = SystemTime::now(); + let mut candidates: Vec<(crate::key_config::ServerKeyConfig, Duration, SystemTime)> = + Vec::with_capacity(2); + + for key_id in [0u8, 1] { + let path = crate::key_config::key_path_for_id(ohttp_keys_dir, key_id); + let mtime = std::fs::metadata(&path)?.modified()?; + let age = sys_now.duration_since(mtime).expect("mtime is in the future"); + let config = crate::key_config::read_server_config_for_id(ohttp_keys_dir, key_id)?; + candidates.push((config, age, mtime)); + } + + // Oldest mtime (largest age) first — the active key is always the older one. + // if it isn't expired + candidates.sort_by_key(|(_, age, _)| std::cmp::Reverse(*age)); + + let now = Instant::now(); + // Choose the active key and read off the standby's age. Per the lifecycle + // if the chosen standby happens to be past + // its rekey deadline, the rotation loop's preamble handles it. + let (current_key_id, current_age, activated_at, standby_age) = if let Some(ivl) = interval { + match candidates.iter().find(|(_, age, _)| *age < ivl) { + Some((cfg, age, mtime)) => { + let active_id = cfg.key_id(); + let standby_age = candidates + .iter() + .find(|(c, _, _)| c.key_id() != active_id) + .map(|(_, a, _)| *a) + .expect("two candidates"); + (active_id, *age, *mtime, standby_age) + } + None => { + // Both expired — regenerate both and start fresh with key_id=0. + candidates.clear(); + let regen_at = SystemTime::now(); + for key_id in [0u8, 1] { + let config = + crate::key_config::regenerate_key_config(ohttp_keys_dir, key_id).await?; + candidates.push((config, Duration::ZERO, regen_at)); + tracing::info!("Regenerated expired OHTTP key_id {key_id}"); + } + (0u8, Duration::ZERO, regen_at, Duration::ZERO) + } + } + } else { + // No interval — oldest key is always current, never expires. + (candidates[0].0.key_id(), candidates[0].1, candidates[0].2, candidates[1].1) + }; + + tracing::info!("Active OHTTP key_id={current_key_id}, age={current_age:?}"); + + let mut slots: [Option; 2] = [None, None]; + for (cfg, _, _) in candidates { + let key_id = cfg.key_id(); + slots[key_id as usize] = Some(cfg.into_server()); + } + + let slot0 = slots[0].take().expect("slot 0 missing after init"); + let slot1 = slots[1].take().expect("slot 1 missing after init"); + let (valid_until, rekey_at) = match interval { + Some(ivl) => { + // Active stops being advertised one ROTATION_GRACE before it + // would be overwritten. Standby is overwritten at its + // activated_at + ivl, which may already be in the past for an + // expired standby — the loop preamble handles that. + let valid_until = now + ivl.saturating_sub(current_age).saturating_sub(ROTATION_GRACE); + let rekey_at = now + ivl.saturating_sub(standby_age); + (valid_until, rekey_at) + } + None => { + let far = Duration::from_secs(60 * 60 * 24 * 365 * 10); + (now + far, now + far) + } + }; + + let keyset = + KeyRotatingServer::new(slot0, slot1, current_key_id, activated_at, valid_until, rekey_at); + Ok(Arc::new(keyset)) +} + +/// Switch the in-memory active slot to what was previously the standby. +/// Touches the new active key file so its mtime matches what live clients +/// +/// Returns `(new_valid_until, new_rekey_at)`: +/// * `new_valid_until` is when the just-activated key will stop being +/// advertised (`now + interval - ROTATION_GRACE`). +/// * `new_rekey_at` is when the previous active (now standby) needs to be +/// overwritten with fresh material. That is its previous +/// `activated_at + interval`, which equals `prev_valid_until + ROTATION_GRACE`. +async fn switch_active( + keyset: &KeyRotatingServer, + keys_dir: &std::path::Path, + prev_valid_until: Instant, + interval: Duration, +) -> (Instant, Instant) { + let old_key_id = keyset.current_key_id().await; + let new_key_id = 1 - old_key_id; + + let activated_at = SystemTime::now(); + let now = Instant::now(); + let valid_until = now + interval.saturating_sub(ROTATION_GRACE); + // The old active is now standby. Its file mtime is unchanged from when it + // was promoted, so its `activated_at + interval` equals + // `prev_valid_until + ROTATION_GRACE`. Using that (instead of + // `now + ROTATION_GRACE`) keeps the schedule anchored to disk state so + // that a restart at any point in the cycle picks up the same deadlines. + let rekey_at = prev_valid_until + ROTATION_GRACE; + + if let Err(e) = crate::key_config::set_key_mtime(keys_dir, new_key_id, activated_at).await { + tracing::warn!("Failed to change mtime for key_id {new_key_id}: {e}"); + } + + keyset.switch(activated_at, valid_until, rekey_at).await; + tracing::info!("Switched OHTTP serving: From key_id {old_key_id} -> TO {new_key_id}"); + + (valid_until, rekey_at) +} + +/// Overwrite the standby slot with fresh key material and return the next +/// `rekey_at` deadline (`now + interval`). +async fn refresh_standby( + keyset: &KeyRotatingServer, + keys_dir: &std::path::Path, + standby_key_id: u8, + interval: Duration, +) -> Instant { + let config = crate::key_config::regenerate_key_config(keys_dir, standby_key_id) + .await + .expect("OHTTP key regeneration must not fail"); + keyset.overwrite(standby_key_id, config.into_server()).await; + let rekey_at = Instant::now() + interval; + keyset.set_rekey_at(rekey_at).await; + tracing::info!("Refreshed standby OHTTP key_id {standby_key_id}"); + rekey_at +} fn handle_peek( result: Result>, DbError>, @@ -485,8 +866,8 @@ impl HandlerError { } HandlerError::OhttpKeyRejection(e) => { const OHTTP_KEY_REJECTION_RES_JSON: &str = r#"{"type":"https://iana.org/assignments/http-problem-types#ohttp-key", "title": "key identifier unknown"}"#; - warn!("Bad request: Key configuration rejected: {}", e); - *res.status_mut() = StatusCode::BAD_REQUEST; + warn!("Key configuration rejected: {}", e); + *res.status_mut() = StatusCode::UNPROCESSABLE_ENTITY; res.headers_mut() .insert(CONTENT_TYPE, HeaderValue::from_static("application/problem+json")); *res.body_mut() = full(OHTTP_KEY_REJECTION_RES_JSON); @@ -598,9 +979,19 @@ mod tests { ) .await .expect("db init"); - let ohttp: ohttp::Server = - crate::key_config::gen_ohttp_server_config().expect("ohttp config").into(); - Service::new(db, ohttp, SentinelTag::new([0u8; 32]), v1) + let c0 = crate::key_config::gen_ohttp_server_config_with_id(0).expect("ohttp config"); + let c1 = crate::key_config::gen_ohttp_server_config_with_id(1).expect("ohttp config"); + // valid_until = now + a generous test interval so nothing rotates during tests + let valid_until = Instant::now() + Duration::from_secs(3600); + let keyset = Arc::new(KeyRotatingServer::new( + c0.into_server(), + c1.into_server(), + 0, + SystemTime::now(), + valid_until, + valid_until + Duration::from_secs(3600), + )); + Service::new(db, keyset, None, SentinelTag::new([0u8; 32]), v1) } /// A valid ShortId encoded as bech32 for use in URL paths. @@ -838,9 +1229,18 @@ mod tests { .await .expect("db init"); let db = MetricsDb::new(db, metrics); - let ohttp: ohttp::Server = - crate::key_config::gen_ohttp_server_config().expect("ohttp config").into(); - let svc = Service::new(db, ohttp, SentinelTag::new([0u8; 32]), None); + let c0 = crate::key_config::gen_ohttp_server_config_with_id(0).expect("ohttp config"); + let c1 = crate::key_config::gen_ohttp_server_config_with_id(1).expect("ohttp config"); + let valid_until = Instant::now() + Duration::from_secs(3600); + let keyset = Arc::new(KeyRotatingServer::new( + c0.into_server(), + c1.into_server(), + 0, + SystemTime::now(), + valid_until, + valid_until + Duration::from_secs(3600), + )); + let svc = Service::new(db, keyset, None, SentinelTag::new([0u8; 32]), None); let id = valid_short_id_path(); let res = svc @@ -861,7 +1261,7 @@ mod tests { use opentelemetry::KeyValue; use opentelemetry_sdk::metrics::data::{AggregatedMetrics, MetricData}; - // This checks that counter value is 1 as post_mailbox was called once + // This checks that counter value is 1 as post_mailbox was called once // Also confirms the v2 label is recorded match db_metric.data() { AggregatedMetrics::U64(MetricData::Sum(sum)) => { @@ -901,9 +1301,19 @@ mod tests { .await .expect("db init"); let db = MetricsDb::new(db, metrics); - let ohttp: ohttp::Server = - crate::key_config::gen_ohttp_server_config().expect("ohttp config").into(); - let svc = Service::new(db, ohttp, SentinelTag::new([0u8; 32]), None); + let c0 = crate::key_config::gen_ohttp_server_config_with_id(0).expect("ohttp config"); + let c1 = crate::key_config::gen_ohttp_server_config_with_id(1).expect("ohttp config"); + let valid_until = Instant::now() + Duration::from_secs(3600); + let activated_at = SystemTime::now(); + let keyset = Arc::new(KeyRotatingServer::new( + c0.into_server(), + c1.into_server(), + 0, + activated_at, + valid_until, + valid_until + Duration::from_secs(3600), + )); + let svc = Service::new(db, keyset, None, SentinelTag::new([0u8; 32]), None); let id = valid_short_id_path(); let res = svc diff --git a/payjoin-mailroom/src/key_config.rs b/payjoin-mailroom/src/key_config.rs index 0326eebc9..ec77ebe7d 100644 --- a/payjoin-mailroom/src/key_config.rs +++ b/payjoin-mailroom/src/key_config.rs @@ -8,7 +8,7 @@ use ohttp::hpke::{Aead, Kdf, Kem}; use ohttp::SymmetricSuite; use tracing::info; -const KEY_ID: u8 = 1; +const DEFAULT_KEY_ID: u8 = 0; const KEM: Kem = Kem::K256Sha256; const SYMMETRIC: &[SymmetricSuite] = &[SymmetricSuite::new(Kdf::HkdfSha256, Aead::ChaCha20Poly1305)]; @@ -20,72 +20,163 @@ const SYMMETRIC: &[SymmetricSuite] = /// server is used to run the directory server. #[derive(Debug, Clone)] pub struct ServerKeyConfig { + key_id: u8, ikm: [u8; 32], server: ohttp::Server, } +impl ServerKeyConfig { + pub fn key_id(&self) -> u8 { self.key_id } + + pub fn into_server(self) -> ohttp::Server { self.server } +} + impl From for ohttp::Server { fn from(value: ServerKeyConfig) -> Self { value.server } } -/// Generate a new OHTTP server key configuration +/// Generate a new OHTTP server key configuration with the default key ID. pub fn gen_ohttp_server_config() -> Result { + gen_ohttp_server_config_with_id(DEFAULT_KEY_ID) +} + +/// Generate a new OHTTP server key configuration with a specific key ID. +pub fn gen_ohttp_server_config_with_id(key_id: u8) -> Result { let ikm = bitcoin::key::rand::random::<[u8; 32]>(); - let config = ohttp::KeyConfig::new(KEY_ID, KEM, Vec::from(SYMMETRIC))?; - Ok(ServerKeyConfig { ikm, server: ohttp::Server::new(config)? }) + let config = ohttp::KeyConfig::new(key_id, KEM, Vec::from(SYMMETRIC))?; + Ok(ServerKeyConfig { key_id, ikm, server: ohttp::Server::new(config)? }) } -/// Persist an OHTTP Key Configuration to the default path -pub fn persist_new_key_config(ohttp_config: ServerKeyConfig, dir: &Path) -> Result { - use std::fs::OpenOptions; - use std::io::Write; +/// Persist an OHTTP Key Configuration to the directory, named by its key_id. +pub async fn persist_key_config(ohttp_config: &ServerKeyConfig, dir: &Path) -> Result { + use tokio::fs::OpenOptions; + use tokio::io::AsyncWriteExt; - let key_path = key_path(dir); + let key_path = key_path_for_id(dir, ohttp_config.key_id); let mut file = OpenOptions::new() .write(true) .create_new(true) .open(&key_path) - .map_err(|e| anyhow!("Failed to create new OHTTP key file: {}", e))?; + .await + .map_err(|e| anyhow!("Failed to create OHTTP key file: {}", e))?; file.write_all(&ohttp_config.ikm) + .await .map_err(|e| anyhow!("Failed to write OHTTP keys to file: {}", e))?; - info!("Saved OHTTP Key Configuration to {}", key_path.display()); + file.flush().await.map_err(|e| anyhow!("Failed to flush OHTTP key file: {}", e))?; + info!( + "Saved OHTTP Key Configuration (key_id={}) to {}", + ohttp_config.key_id, + key_path.display() + ); Ok(key_path) } -/// Read the configured server from the default path -/// May panic if key exists but is the unexpected format. -pub fn read_server_config(dir: &Path) -> Result { - let key_path = key_path(dir); +/// Set the modification time of the key file for `key_id` in `dir`. +/// +/// The mtime is how restarts re-derive `activated_at`/`valid_until` for the +/// active key, so callers stamp it at the moment a key becomes the served +pub async fn set_key_mtime( + dir: &Path, + key_id: u8, + mtime: std::time::SystemTime, +) -> std::io::Result<()> { + let path = key_path_for_id(dir, key_id); + let times = fs::FileTimes::new().set_modified(mtime); + let std_file = tokio::fs::File::open(&path).await?.into_std().await; + tokio::task::spawn_blocking(move || std_file.set_times(times)) + .await + .map_err(std::io::Error::other)? +} + +/// Remove any existing key file for `key_id` in `dir`, generate a fresh +/// `ServerKeyConfig` for that id, persist it, and return the config. +pub async fn regenerate_key_config(dir: &Path, key_id: u8) -> Result { + let _ = tokio::fs::remove_file(key_path_for_id(dir, key_id)).await; + let config = gen_ohttp_server_config_with_id(key_id)?; + persist_key_config(&config, dir).await?; + Ok(config) +} + +/// Read a single server config for a specific key_id from the directory. +/// May panic if key exists but is the unexpected format. +pub fn read_server_config_for_id(dir: &Path, key_id: u8) -> Result { + let key_path = key_path_for_id(dir, key_id); let ikm: [u8; 32] = fs::read(&key_path) .map_err(|e| anyhow!("Failed to read OHTTP key file: {}", e))? .try_into() .expect("Key wrong size: expected 32 bytes"); - let server_config = ohttp::KeyConfig::derive(KEY_ID, KEM, SYMMETRIC.to_vec(), &ikm) + let server_config = ohttp::KeyConfig::derive(key_id, KEM, SYMMETRIC.to_vec(), &ikm) .expect("Failed to derive OHTTP keys from file"); - info!("Loaded existing OHTTP Key Configuration from {}", key_path.display()); - Ok(ServerKeyConfig { ikm, server: ohttp::Server::new(server_config)? }) + info!("Loaded OHTTP Key Configuration (key_id={key_id}) from {}", key_path.display()); + Ok(ServerKeyConfig { key_id, ikm, server: ohttp::Server::new(server_config)? }) +} + +/// Read the legacy single-key config (key_id=0). +pub fn read_server_config(dir: &Path) -> Result { + read_server_config_for_id(dir, DEFAULT_KEY_ID) } -/// Get the path to the key configuration file -/// For now, default to [KEY_ID].ikm. -/// In the future this might be able to save multiple keys named by KeyId. -fn key_path(dir: &Path) -> PathBuf { dir.join(format!("{KEY_ID}.ikm")) } +pub(crate) fn key_path_for_id(dir: &Path, key_id: u8) -> PathBuf { + dir.join(format!("{key_id}.ikm")) +} #[cfg(test)] mod tests { use super::*; - #[test] - fn round_trip_server_config() { + #[tokio::test] + async fn round_trip_server_config() { let temp_dir = tempfile::tempdir().expect("Failed to create temp dir"); let ohttp_config = gen_ohttp_server_config().expect("Failed to generate server config"); - let _path = persist_new_key_config(ohttp_config.clone(), temp_dir.path()) + let _path = persist_key_config(&ohttp_config, temp_dir.path()) + .await .expect("Failed to persist server config"); let ohttp_config_again = read_server_config(temp_dir.path()).expect("Failed to read server config"); assert_eq!(ohttp_config.ikm, ohttp_config_again.ikm); + assert_eq!(ohttp_config.key_id, ohttp_config_again.key_id); + } + + #[tokio::test] + async fn round_trip_with_custom_key_id() { + let temp_dir = tempfile::tempdir().expect("Failed to create temp dir"); + let config = gen_ohttp_server_config_with_id(42).expect("gen config"); + assert_eq!(config.key_id(), 42); + persist_key_config(&config, temp_dir.path()).await.expect("persist"); + let loaded = read_server_config_for_id(temp_dir.path(), 42).expect("read"); + assert_eq!(config.ikm, loaded.ikm); + assert_eq!(loaded.key_id(), 42); + } + + #[tokio::test] + async fn read_both_configs() { + let temp_dir = tempfile::tempdir().expect("Failed to create temp dir"); + let c0 = gen_ohttp_server_config_with_id(0).expect("gen"); + let c1 = gen_ohttp_server_config_with_id(1).expect("gen"); + persist_key_config(&c0, temp_dir.path()).await.expect("persist"); + persist_key_config(&c1, temp_dir.path()).await.expect("persist"); + + let loaded0 = read_server_config_for_id(temp_dir.path(), 0).expect("read 0"); + let loaded1 = read_server_config_for_id(temp_dir.path(), 1).expect("read 1"); + assert_eq!(c0.ikm, loaded0.ikm); + assert_eq!(c1.ikm, loaded1.ikm); + } + + fn parse_key_id_from_path(path: &Path) -> Option { + let name = path.file_name()?.to_str()?; + let stem = name.strip_suffix(".ikm")?; + stem.parse::().ok() + } + + #[test] + fn parse_key_id_from_filename() { + assert_eq!(parse_key_id_from_path(Path::new("1.ikm")), Some(1)); + assert_eq!(parse_key_id_from_path(Path::new("255.ikm")), Some(255)); + assert_eq!(parse_key_id_from_path(Path::new("foo.ikm")), None); + assert_eq!(parse_key_id_from_path(Path::new("1.txt")), None); + assert_eq!(parse_key_id_from_path(Path::new("256.ikm")), None); } } diff --git a/payjoin-mailroom/src/lib.rs b/payjoin-mailroom/src/lib.rs index ccd075f7b..fc7c13a67 100644 --- a/payjoin-mailroom/src/lib.rs +++ b/payjoin-mailroom/src/lib.rs @@ -235,7 +235,11 @@ async fn init_directory( let db = crate::db::MetricsDb::new(crate::db::DbServiceAdapter::new(files_db), metrics.clone()); let ohttp_keys_dir = config.storage_dir.join("ohttp-keys"); - let ohttp_config = init_ohttp_config(&ohttp_keys_dir)?; + if let Some(max_age) = config.ohttp_keys_max_age { + info!("OHTTP key rotation enabled: interval={}s", max_age.as_secs()); + } + let keyset = + crate::directory::start_ohttp_keys(ohttp_keys_dir, config.ohttp_keys_max_age).await?; let v1 = if config.v1.is_some() { #[cfg(feature = "access-control")] @@ -246,7 +250,10 @@ async fn init_directory( } else { None }; - Ok(crate::directory::Service::new(db, ohttp_config.into(), sentinel_tag, v1)) + let service = + crate::directory::Service::new(db, keyset, config.ohttp_keys_max_age, sentinel_tag, v1); + + Ok(service) } #[cfg(feature = "access-control")] @@ -343,20 +350,6 @@ async fn load_address_cache( } } -fn init_ohttp_config( - ohttp_keys_dir: &std::path::Path, -) -> anyhow::Result { - std::fs::create_dir_all(ohttp_keys_dir)?; - match crate::key_config::read_server_config(ohttp_keys_dir) { - Ok(config) => Ok(config), - Err(_) => { - let config = crate::key_config::gen_ohttp_server_config()?; - crate::key_config::persist_new_key_config(config.clone(), ohttp_keys_dir)?; - Ok(config) - } - } -} - fn build_app(services: Services) -> Router { let metrics = services.metrics.clone(); @@ -426,7 +419,7 @@ fn is_relay_request(req: &axum::extract::Request) -> bool { #[cfg(test)] mod tests { use std::sync::Arc; - use std::time::Duration; + use std::time::{Duration, SystemTime}; use axum_server::tls_rustls::RustlsConfig; use opentelemetry_sdk::metrics::{InMemoryMetricExporter, PeriodicReader, SdkMeterProvider}; @@ -436,8 +429,25 @@ mod tests { use tempfile::tempdir; use super::*; + use crate::directory::init_ohttp_keyset; use crate::metrics::{ACTIVE_CONNECTIONS, HTTP_REQUESTS, TOTAL_CONNECTIONS}; + /// Helper to set the mtime of a key file to a specific SystemTime. + fn set_mtime(path: &std::path::Path, time: SystemTime) { + let file = std::fs::File::open(path).expect("open file"); + let times = std::fs::FileTimes::new().set_modified(time); + file.set_times(times).expect("set mtime"); + } + + /// Helper to create both key files in a directory. + async fn create_both_keys(dir: &std::path::Path) { + for key_id in [0u8, 1] { + let config = + crate::key_config::gen_ohttp_server_config_with_id(key_id).expect("gen config"); + crate::key_config::persist_key_config(&config, dir).await.expect("persist"); + } + } + async fn start_service( cert_der: Vec, key_der: Vec, @@ -448,6 +458,7 @@ mod tests { tempdir.path().to_path_buf(), Duration::from_secs(2), None, + None, ); let mut root_store = RootCertStore::empty(); @@ -544,6 +555,7 @@ mod tests { tempdir.path().to_path_buf(), Duration::from_secs(2), None, + None, ); let sentinel_tag = generate_sentinel_tag(); @@ -593,6 +605,7 @@ mod tests { tempdir.path().to_path_buf(), Duration::from_secs(2), None, + None, ); let sentinel_tag = generate_sentinel_tag(); @@ -648,4 +661,139 @@ mod tests { "actual short ID value must not appear in metrics" ); } + + const DAY: Duration = Duration::from_secs(24 * 3600); + + fn set_age(dir: &std::path::Path, key_id: u8, age: Duration) { + set_mtime(&crate::key_config::key_path_for_id(dir, key_id), SystemTime::now() - age); + } + + fn key_bytes(dir: &std::path::Path, key_id: u8) -> Vec { + std::fs::read(crate::key_config::key_path_for_id(dir, key_id)).expect("read key") + } + + #[track_caller] + fn assert_near(actual: Duration, expected: Duration) { + let tolerance = Duration::from_secs(60); + let diff = + if actual > expected { actual - expected } else { expected.saturating_sub(actual) }; + assert!(diff < tolerance, "expected ~{expected:?}, got {actual:?}"); + } + + #[tokio::test] + async fn both_keys_expired_regenerates_and_uses_key_id_0() { + let dir = tempdir().expect("tempdir"); + let interval = 30 * DAY; + let grace = crate::directory::ROTATION_GRACE; + + create_both_keys(dir.path()).await; + set_age(dir.path(), 0, 35 * DAY); + set_age(dir.path(), 1, 35 * DAY); + + let keyset = init_ohttp_keyset(dir.path(), Some(interval)).await.expect("init keyset"); + let (id, rekey_at, valid_until) = keyset.key_timeouts().await; + let now = std::time::Instant::now(); + + assert_eq!(id, 0); + assert_near(valid_until.saturating_duration_since(now), interval - grace); + assert_near(rekey_at.saturating_duration_since(now), interval); + assert!(valid_until < rekey_at); + } + + #[tokio::test] + async fn one_valid_one_expired_uses_valid_active_and_marks_standby_overdue() { + let dir = tempdir().expect("tempdir"); + let interval = 30 * DAY; + let grace = crate::directory::ROTATION_GRACE; + + create_both_keys(dir.path()).await; + set_age(dir.path(), 0, 10 * DAY); + set_age(dir.path(), 1, 35 * DAY); + + let keyset = init_ohttp_keyset(dir.path(), Some(interval)).await.expect("init keyset"); + let (id, rekey_at, valid_until) = keyset.key_timeouts().await; + let now = std::time::Instant::now(); + + assert_eq!(id, 0); + assert_near(valid_until.saturating_duration_since(now), interval - 10 * DAY - grace); + assert!(rekey_at <= now, "rekey_at should be saturated to init time"); + assert!(rekey_at <= valid_until, "preamble should be triggered"); + } + + #[tokio::test] + async fn two_valid_keys_uses_older_mtime() { + let dir = tempdir().expect("tempdir"); + let interval = 30 * DAY; + let grace = crate::directory::ROTATION_GRACE; + + create_both_keys(dir.path()).await; + set_age(dir.path(), 0, 20 * DAY); + set_age(dir.path(), 1, 5 * DAY); + + let keyset = init_ohttp_keyset(dir.path(), Some(interval)).await.expect("init keyset"); + let (id, rekey_at, valid_until) = keyset.key_timeouts().await; + let now = std::time::Instant::now(); + + assert_eq!(id, 0); + assert_near(valid_until.saturating_duration_since(now), interval - 20 * DAY - grace); + assert_near(rekey_at.saturating_duration_since(now), interval - 5 * DAY); + assert!(valid_until < rekey_at); + } + + #[tokio::test] + async fn no_interval_never_expires() { + let dir = tempdir().expect("tempdir"); + create_both_keys(dir.path()).await; + set_age(dir.path(), 0, 5 * DAY); + set_age(dir.path(), 1, DAY); + + let keyset = init_ohttp_keyset(dir.path(), None).await.expect("init keyset"); + let (id, rekey_at, valid_until) = keyset.key_timeouts().await; + let now = std::time::Instant::now(); + + assert_eq!(id, 0); + assert!(valid_until.saturating_duration_since(now) > 365 * DAY); + assert!(rekey_at.saturating_duration_since(now) > 365 * DAY); + } + + #[tokio::test] + async fn missing_keys_are_generated_on_init() { + let dir = tempdir().expect("tempdir"); + let interval = 30 * DAY; + let grace = crate::directory::ROTATION_GRACE; + + let keyset = init_ohttp_keyset(dir.path(), Some(interval)).await.expect("init keyset"); + let (id, rekey_at, valid_until) = keyset.key_timeouts().await; + let now = std::time::Instant::now(); + + assert_eq!(id, 0); + assert_near(valid_until.saturating_duration_since(now), interval - grace); + assert_near(rekey_at.saturating_duration_since(now), interval); + } + + #[tokio::test] + async fn start_ohttp_keys_refreshes_expired_standby() { + let dir = tempdir().expect("tempdir"); + let interval = 30 * DAY; + + create_both_keys(dir.path()).await; + set_age(dir.path(), 0, 10 * DAY); + set_age(dir.path(), 1, 35 * DAY); + + let stale_standby = key_bytes(dir.path(), 1); + let active_before = key_bytes(dir.path(), 0); + let _keyset = crate::directory::start_ohttp_keys(dir.path().to_path_buf(), Some(interval)) + .await + .expect("start ohttp keys"); + + let deadline = std::time::Instant::now() + Duration::from_secs(2); + while key_bytes(dir.path(), 1) == stale_standby { + assert!( + std::time::Instant::now() < deadline, + "expired standby was not refreshed by preamble" + ); + tokio::time::sleep(Duration::from_millis(20)).await; + } + assert_eq!(key_bytes(dir.path(), 0), active_before, "active key must not be touched"); + } } diff --git a/payjoin-test-utils/src/lib.rs b/payjoin-test-utils/src/lib.rs index 48f37c1bf..38682367b 100644 --- a/payjoin-test-utils/src/lib.rs +++ b/payjoin-test-utils/src/lib.rs @@ -120,6 +120,8 @@ pub async fn init_directory( "[::]:0".parse().expect("valid listener address"), tempdir.path().to_path_buf(), Duration::from_secs(2), + // Must be > 2 * ROTATION_GRACE (= 14 days); use 30 days. + Some(Duration::from_secs(30 * 24 * 60 * 60)), Some(payjoin_mailroom::config::V1Config::default()), ); @@ -150,6 +152,8 @@ pub async fn init_ohttp_relay( "[::]:0".parse().expect("valid listener address"), tempdir.path().to_path_buf(), Duration::from_secs(2), + // Must be > 2 * ROTATION_GRACE (= 14 days); use 30 days. + Some(Duration::from_secs(30 * 24 * 60 * 60)), None, );