Harden ACME account store interfaces

Author: eldryoth

CHANGELOG.md

@@ -39,9 +39,13 @@ behavior when the change improves security or project direction.
   API patch preserves configured contacts and EAB with caller-provided keys.
 - Keep blocking account files and locks off Tokio workers. Async ACME paths use
   nonblocking lifecycle-lock attempts on blocking workers, bounded asynchronous
-  retry, contention diagnostics, and a 10-second lock-wait deadline.
+  retry, contention diagnostics, and a 10-second lock-wait deadline. Public
+  bounded async credential load, store, and removal APIs prevent downstream
+  Tokio callers from falling back to indefinitely blocking lifecycle methods.
 - Verify the vendored `instant-acme` source against published `0.8.5` hashes in
-  CI and release gates after removing the single marked downstream API patch.
+  CI and release gates after removing the single marked downstream API patch;
+  separately pin the exact permitted patch body and use a private unpredictable
+  validation workspace to prevent local symlink clobbering.
 - Validate online ACME directories structurally and require exact advertised ToS
   agreement, with an explicit private-directory override only for omitted terms.
 - Parse every advertised ACME endpoint as a bounded HTTPS URI with a real

crates/fluxheim-acme/src/acme_account_async.rs

@@ -1,16 +1,156 @@
 use super::*;
+use std::sync::Arc;
 
 const ACCOUNT_LOCK_WAIT_TIMEOUT: Duration = Duration::from_secs(10);
 const ACCOUNT_LOCK_RETRY_INTERVAL: Duration = Duration::from_millis(25);
 
-pub(crate) async fn load_account_credentials_async(
+/// Loads ACME credentials without blocking a Tokio worker.
+///
+/// Returns an error when the issuer lifecycle lock cannot be acquired within
+/// Fluxheim's bounded account-lock deadline.
+pub async fn load_account_credentials_async(
     storage: &Path,
     issuer_name: &str,
 ) -> Result<Option<instant_acme::AccountCredentials>, AcmeInstantClientError> {
     load_account_credentials_async_with_timeout(storage, issuer_name, ACCOUNT_LOCK_WAIT_TIMEOUT)
         .await
 }
 
+/// Persists ACME credentials without blocking a Tokio worker.
+///
+/// Returns an error when the issuer lifecycle lock cannot be acquired within
+/// Fluxheim's bounded account-lock deadline.
+pub async fn store_account_credentials_async(
+    storage: &Path,
+    issuer_name: &str,
+    credentials: instant_acme::AccountCredentials,
+) -> Result<AcmeAccountCredentialsPath, AcmeInstantClientError> {
+    store_account_credentials_async_with_timeout(
+        storage,
+        issuer_name,
+        credentials,
+        ACCOUNT_LOCK_WAIT_TIMEOUT,
+    )
+    .await
+}
+
+async fn store_account_credentials_async_with_timeout(
+    storage: &Path,
+    issuer_name: &str,
+    credentials: instant_acme::AccountCredentials,
+    wait_timeout: Duration,
+) -> Result<AcmeAccountCredentialsPath, AcmeInstantClientError> {
+    let storage = storage.to_path_buf();
+    let issuer = issuer_name.to_owned();
+    let credentials = Arc::new(credentials);
+    run_account_store_attempt(issuer_name, "credential store", wait_timeout, move || {
+        try_store_account_credentials(&storage, &issuer, credentials.as_ref())
+    })
+    .await
+}
+
+/// Removes ACME credentials without blocking a Tokio worker.
+///
+/// Returns an error when the issuer lifecycle lock cannot be acquired within
+/// Fluxheim's bounded account-lock deadline.
+pub async fn remove_account_credentials_async(
+    storage: &Path,
+    issuer_name: &str,
+) -> Result<bool, AcmeInstantClientError> {
+    remove_account_credentials_async_with_timeout(storage, issuer_name, ACCOUNT_LOCK_WAIT_TIMEOUT)
+        .await
+}
+
+async fn remove_account_credentials_async_with_timeout(
+    storage: &Path,
+    issuer_name: &str,
+    wait_timeout: Duration,
+) -> Result<bool, AcmeInstantClientError> {
+    let storage = storage.to_path_buf();
+    let issuer = issuer_name.to_owned();
+    run_account_store_attempt(issuer_name, "credential removal", wait_timeout, move || {
+        try_remove_account_credentials(&storage, &issuer)
+    })
+    .await
+}
+
+fn try_store_account_credentials(
+    storage: &Path,
+    issuer_name: &str,
+    credentials: &instant_acme::AccountCredentials,
+) -> Result<AccountStoreAttempt<AcmeAccountCredentialsPath>, AcmeAccountStoreError> {
+    let credentials_path = account_credentials_path(storage, issuer_name);
+    let directory = account_directory(&credentials_path)?;
+    ensure_safe_account_directory(directory)?;
+    let Some(_lock) = AcmeMutationLock::try_acquire(directory).map_err(|error| {
+        account_async_store_io_error(&directory.join(".fluxheim-acme.lock"), error)
+    })?
+    else {
+        return Ok(AccountStoreAttempt::Contended);
+    };
+    let pending = directory.join(".credentials.bootstrap.pending");
+    if pending
+        .try_exists()
+        .map_err(|error| account_async_store_io_error(&pending, error))?
+    {
+        return Err(AcmeAccountStoreError::UnsafePath {
+            path: pending,
+            message: "account bootstrap is pending; credentials must be promoted by the bootstrap transaction"
+                .to_owned(),
+        });
+    }
+    store_account_credentials_locked(&credentials_path, directory, credentials)
+        .map(AccountStoreAttempt::Acquired)
+}
+
+fn try_remove_account_credentials(
+    storage: &Path,
+    issuer_name: &str,
+) -> Result<AccountStoreAttempt<bool>, AcmeAccountStoreError> {
+    let credentials_path = account_credentials_path(storage, issuer_name);
+    let directory = account_directory(&credentials_path)?;
+    ensure_safe_account_directory(directory)?;
+    let Some(_lock) = AcmeMutationLock::try_acquire(directory).map_err(|error| {
+        account_async_store_io_error(&directory.join(".fluxheim-acme.lock"), error)
+    })?
+    else {
+        return Ok(AccountStoreAttempt::Contended);
+    };
+    ensure_safe_account_destination(&credentials_path.path)?;
+    let removed = match fs::remove_file(&credentials_path.path) {
+        Ok(()) => {
+            let directory_file = fs::File::open(directory)
+                .map_err(|error| account_async_store_io_error(directory, error))?;
+            directory_file
+                .sync_all()
+                .map_err(|error| account_async_store_io_error(directory, error))?;
+            true
+        }
+        Err(error) if error.kind() == io::ErrorKind::NotFound => false,
+        Err(error) => return Err(account_async_store_io_error(&credentials_path.path, error)),
+    };
+    Ok(AccountStoreAttempt::Acquired(removed))
+}
+
+fn account_directory(
+    credentials_path: &AcmeAccountCredentialsPath,
+) -> Result<&Path, AcmeAccountStoreError> {
+    credentials_path
+        .path
+        .parent()
+        .ok_or_else(|| AcmeAccountStoreError::UnsafePath {
+            path: credentials_path.path.clone(),
+            message: "credentials path has no parent directory".to_owned(),
+        })
+}
+
+fn account_async_store_io_error(path: &Path, error: io::Error) -> AcmeAccountStoreError {
+    AcmeAccountStoreError::Io {
+        path: path.to_path_buf(),
+        error,
+    }
+}
+
 async fn load_account_credentials_async_with_timeout(
     storage: &Path,
     issuer_name: &str,
@@ -169,3 +309,23 @@ pub(crate) async fn load_account_credentials_with_test_timeout(
 ) -> Result<Option<instant_acme::AccountCredentials>, AcmeInstantClientError> {
     load_account_credentials_async_with_timeout(storage, issuer_name, wait_timeout).await
 }
+
+#[cfg(test)]
+pub(crate) async fn store_account_credentials_with_test_timeout(
+    storage: &Path,
+    issuer_name: &str,
+    credentials: instant_acme::AccountCredentials,
+    wait_timeout: Duration,
+) -> Result<AcmeAccountCredentialsPath, AcmeInstantClientError> {
+    store_account_credentials_async_with_timeout(storage, issuer_name, credentials, wait_timeout)
+        .await
+}
+
+#[cfg(test)]
+pub(crate) async fn remove_account_credentials_with_test_timeout(
+    storage: &Path,
+    issuer_name: &str,
+    wait_timeout: Duration,
+) -> Result<bool, AcmeInstantClientError> {
+    remove_account_credentials_async_with_timeout(storage, issuer_name, wait_timeout).await
+}

crates/fluxheim-acme/src/acme_account_store.rs

@@ -49,6 +49,12 @@ pub fn account_credentials_path(storage: &Path, issuer_name: &str) -> AcmeAccoun
     }
 }
 
+/// Loads persisted ACME account credentials.
+///
+/// # Blocking
+///
+/// Waits indefinitely for the issuer lifecycle lock. Async code should use
+/// `load_account_credentials_async` instead.
 pub fn load_account_credentials(
     storage: &Path,
     issuer_name: &str,
@@ -276,6 +282,12 @@ impl AccountDeactivationTransaction {
     }
 }
 
+/// Persists ACME account credentials atomically.
+///
+/// # Blocking
+///
+/// Waits indefinitely for the issuer lifecycle lock. Async code should use
+/// `store_account_credentials_async` instead.
 pub fn store_account_credentials(
     storage: &Path,
     issuer_name: &str,
@@ -307,7 +319,7 @@ pub fn store_account_credentials(
     store_account_credentials_locked(&credentials_path, directory, credentials)
 }
 
-fn store_account_credentials_locked(
+pub(crate) fn store_account_credentials_locked(
     credentials_path: &AcmeAccountCredentialsPath,
     directory: &Path,
     credentials: &instant_acme::AccountCredentials,
@@ -345,6 +357,12 @@ fn store_account_credentials_locked(
     Ok(credentials_path.clone())
 }
 
+/// Removes persisted ACME account credentials.
+///
+/// # Blocking
+///
+/// Waits indefinitely for the issuer lifecycle lock. Async code should use
+/// `remove_account_credentials_async` instead.
 pub fn remove_account_credentials(
     storage: &Path,
     issuer_name: &str,
@@ -372,7 +390,7 @@ pub fn remove_account_credentials(
     }
 }
 
-fn ensure_safe_account_directory(directory: &Path) -> Result<(), AcmeAccountStoreError> {
+pub(crate) fn ensure_safe_account_directory(directory: &Path) -> Result<(), AcmeAccountStoreError> {
     reject_existing_symlink_in_account_path(directory)?;
     fs::create_dir_all(directory).map_err(|error| account_store_io_error(directory, error))?;
     reject_existing_symlink_in_account_path(directory)?;
@@ -388,7 +406,7 @@ fn ensure_safe_account_directory(directory: &Path) -> Result<(), AcmeAccountStor
     Ok(())
 }
 
-fn ensure_safe_account_destination(path: &Path) -> Result<(), AcmeAccountStoreError> {
+pub(crate) fn ensure_safe_account_destination(path: &Path) -> Result<(), AcmeAccountStoreError> {
     match fs::symlink_metadata(path) {
         Ok(metadata) if metadata.file_type().is_symlink() || !metadata.is_file() => {
             Err(AcmeAccountStoreError::UnsafePath {

crates/fluxheim-acme/src/acme_tests.rs

@@ -41,6 +41,9 @@ mod plan;
 mod renewal;
 #[path = "acme_tests_storage.rs"]
 mod storage;
+#[cfg(feature = "acme-client")]
+#[path = "acme_tests_storage_async.rs"]
+mod storage_async;
 
 fn acme_config_with_vhosts(vhosts: Vec<VhostConfig>) -> Config {
     Config {

crates/fluxheim-acme/src/acme_tests_account_bootstrap.rs

@@ -142,6 +142,53 @@ fn async_account_lock_contention_has_a_bounded_deadline() {
     drop(pending);
 }
 
+#[test]
+fn async_account_mutations_remain_responsive_and_time_out_under_contention() {
+    let storage = fluxheim_common::test_support::unique_temp_path("acme-account-async-mutations");
+    let pending =
+        match begin_account_bootstrap(&storage, "issuer", "https://acme.example.test/directory")
+            .unwrap()
+        {
+            AccountBootstrap::Pending(pending) => pending,
+            AccountBootstrap::Existing(_) => panic!("expected a pending account bootstrap"),
+        };
+    let runtime = tokio::runtime::Builder::new_current_thread()
+        .enable_all()
+        .build()
+        .unwrap();
+    let heartbeat = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false));
+    let started = std::time::Instant::now();
+
+    runtime.block_on(async {
+        let store = crate::acme_account_async::store_account_credentials_with_test_timeout(
+            &storage,
+            "issuer",
+            test_account_credentials(),
+            std::time::Duration::from_millis(50),
+        );
+        let remove = crate::acme_account_async::remove_account_credentials_with_test_timeout(
+            &storage,
+            "issuer",
+            std::time::Duration::from_millis(50),
+        );
+        let heartbeat_task = {
+            let heartbeat = heartbeat.clone();
+            async move {
+                tokio::time::sleep(std::time::Duration::from_millis(20)).await;
+                heartbeat.store(true, std::sync::atomic::Ordering::Release);
+            }
+        };
+        let (store_result, remove_result, ()) =
+            futures::future::join3(store, remove, heartbeat_task).await;
+        assert!(store_result.unwrap_err().to_string().contains("timed out"));
+        assert!(remove_result.unwrap_err().to_string().contains("timed out"));
+    });
+
+    assert!(heartbeat.load(std::sync::atomic::Ordering::Acquire));
+    assert!(started.elapsed() < std::time::Duration::from_secs(1));
+    drop(pending);
+}
+
 #[cfg(feature = "acme-client")]
 #[test]
 fn account_bootstrap_promotion_is_durable_and_cleans_pending_key() {

crates/fluxheim-acme/src/acme_tests_storage_async.rs

@@ -0,0 +1,38 @@
+use super::*;
+
+#[test]
+fn account_credentials_store_load_and_remove_round_trip() {
+    let storage = fluxheim_common::test_support::unique_temp_path("acme-account-async-roundtrip");
+    let runtime = tokio::runtime::Builder::new_current_thread()
+        .enable_all()
+        .build()
+        .unwrap();
+
+    runtime.block_on(async {
+        let stored = crate::store_account_credentials_async(
+            &storage,
+            "letsencrypt",
+            test_account_credentials(),
+        )
+        .await
+        .unwrap();
+        assert!(stored.path.is_file());
+        assert!(
+            crate::load_account_credentials_async(&storage, "letsencrypt")
+                .await
+                .unwrap()
+                .is_some()
+        );
+        assert!(
+            crate::remove_account_credentials_async(&storage, "letsencrypt")
+                .await
+                .unwrap()
+        );
+        assert!(
+            crate::load_account_credentials_async(&storage, "letsencrypt")
+                .await
+                .unwrap()
+                .is_none()
+        );
+    });
+}

crates/fluxheim-acme/src/lib.rs

@@ -261,9 +261,16 @@ mod acme_tls_alpn;
 #[path = "acme_transaction.rs"]
 mod acme_transaction;
 #[cfg(feature = "acme-client")]
+pub use acme_account_async::{
+    load_account_credentials_async, remove_account_credentials_async,
+    store_account_credentials_async,
+};
+#[cfg(feature = "acme-client")]
 use acme_account_store::{
     AccountBootstrap, AccountDeactivationTransaction, AccountStoreAttempt, PendingAccountBootstrap,
-    try_begin_account_bootstrap, try_begin_account_deactivation, try_load_account_credentials,
+    ensure_safe_account_destination, ensure_safe_account_directory,
+    store_account_credentials_locked, try_begin_account_bootstrap, try_begin_account_deactivation,
+    try_load_account_credentials,
 };
 pub use acme_account_store::{
     account_credentials_path, load_account_credentials, remove_account_credentials,