Fail closed on pending ACME account state

Author: eldryoth

CHANGELOG.md

@@ -27,6 +27,8 @@ behavior when the change improves security or project direction.
   account state fails closed, while revocation quarantines the active pair before
   the remote operation, preserves ambiguous outcomes for operator resolution,
   and requests live reload after success even when scheduled renewal is disabled.
+  Ordinary synchronous and async credential store/removal APIs now reject both
+  bootstrap and ambiguous deactivation journals while holding the lifecycle lock.
 - Journal revocation quarantine phases durably so pre-remote crashes restore the
   complete pair, ambiguous remote outcomes stay fail-closed, and confirmed
   quarantine survives crashes while permitting replacement issuance.

crates/fluxheim-acme/src/acme_account_async.rs

@@ -88,17 +88,7 @@ fn try_store_account_credentials(
     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(),
-        });
-    }
+    reject_pending_account_mutation(directory)?;
     store_account_credentials_locked(&credentials_path, directory, credentials)
         .map(AccountStoreAttempt::Acquired)
 }
@@ -116,6 +106,7 @@ fn try_remove_account_credentials(
     else {
         return Ok(AccountStoreAttempt::Contended);
     };
+    reject_pending_account_mutation(directory)?;
     ensure_safe_account_destination(&credentials_path.path)?;
     let removed = match fs::remove_file(&credentials_path.path) {
         Ok(()) => {

crates/fluxheim-acme/src/acme_account_state.rs

@@ -0,0 +1,80 @@
+use std::fs;
+use std::io;
+use std::path::Path;
+
+use super::AcmeAccountStoreError;
+
+pub(crate) const ACCOUNT_BOOTSTRAP_PENDING_FILE: &str = ".credentials.bootstrap.pending";
+pub(crate) const ACCOUNT_DEACTIVATION_PENDING_FILE: &str = ".credentials.deactivation.pending";
+
+pub(crate) fn reject_pending_account_mutation(
+    directory: &Path,
+) -> Result<(), AcmeAccountStoreError> {
+    reject_pending_account_bootstrap(directory)?;
+    reject_pending_account_deactivation(directory)
+}
+
+pub(crate) fn reject_pending_account_bootstrap(
+    directory: &Path,
+) -> Result<(), AcmeAccountStoreError> {
+    reject_pending_account_state(
+        directory,
+        ACCOUNT_BOOTSTRAP_PENDING_FILE,
+        "account bootstrap is pending; bootstrap recovery must complete",
+    )
+}
+
+pub(crate) fn reject_pending_account_deactivation(
+    directory: &Path,
+) -> Result<(), AcmeAccountStoreError> {
+    reject_pending_account_state(
+        directory,
+        ACCOUNT_DEACTIVATION_PENDING_FILE,
+        "account deactivation is in an ambiguous pending state; operator recovery is required",
+    )
+}
+
+fn reject_pending_account_state(
+    directory: &Path,
+    name: &str,
+    message: &str,
+) -> Result<(), AcmeAccountStoreError> {
+    let path = directory.join(name);
+    match fs::symlink_metadata(&path) {
+        Ok(_) => Err(AcmeAccountStoreError::UnsafePath {
+            path,
+            message: message.to_owned(),
+        }),
+        Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(()),
+        Err(error) => Err(AcmeAccountStoreError::Io { path, error }),
+    }
+}
+
+#[cfg(all(test, unix))]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn pending_mutation_guard_rejects_dangling_symlink() {
+        let storage = fluxheim_common::test_support::unique_temp_path("acme-account-state-link");
+        let credentials = crate::account_credentials_path(&storage, "issuer");
+        let directory = credentials.path.parent().unwrap().to_path_buf();
+        fs::create_dir_all(&directory).unwrap();
+        std::os::unix::fs::symlink(
+            directory.join("missing-target"),
+            directory.join(ACCOUNT_BOOTSTRAP_PENDING_FILE),
+        )
+        .unwrap();
+
+        let error = reject_pending_account_mutation(&directory).unwrap_err();
+        assert!(matches!(error, AcmeAccountStoreError::UnsafePath { .. }));
+        let load_error = match crate::load_account_credentials(&storage, "issuer") {
+            Ok(_) => panic!("expected dangling pending state to fail closed"),
+            Err(error) => error,
+        };
+        assert!(matches!(
+            load_error,
+            AcmeAccountStoreError::UnsafePath { .. }
+        ));
+    }
+}

crates/fluxheim-acme/src/acme_account_store.rs

@@ -19,12 +19,12 @@ pub(crate) use bootstrap::{
 use super::UNIX_O_NOFOLLOW;
 use super::{
     AcmeAccountCredentialsPath, AcmeAccountStoreError, AcmeMutationLock,
-    MAX_ACCOUNT_CREDENTIALS_BYTES, managed_certificate_segment, unique_transaction_id,
+    MAX_ACCOUNT_CREDENTIALS_BYTES, managed_certificate_segment, reject_pending_account_bootstrap,
+    reject_pending_account_deactivation, reject_pending_account_mutation, unique_transaction_id,
 };
 
 const ACME_ACCOUNT_DIR: &str = "accounts";
 const ACME_ACCOUNT_CREDENTIALS_FILE: &str = "credentials.json";
-const ACME_ACCOUNT_DEACTIVATION_FILE: &str = ".credentials.deactivation.pending";
 
 #[cfg(feature = "acme-client")]
 pub(crate) enum AccountStoreAttempt<T> {
@@ -120,29 +120,9 @@ fn load_account_credentials_locked(
     directory: &Path,
     allow_bootstrap_recovery: bool,
 ) -> Result<Option<instant_acme::AccountCredentials>, AcmeAccountStoreError> {
-    let pending = directory.join(ACME_ACCOUNT_DEACTIVATION_FILE);
-    if pending
-        .try_exists()
-        .map_err(|error| account_store_io_error(&pending, error))?
-    {
-        return Err(AcmeAccountStoreError::UnsafePath {
-            path: pending,
-            message: "account deactivation is in an ambiguous pending state; operator recovery is required"
-                .to_owned(),
-        });
-    }
-    let bootstrap_pending = directory.join(".credentials.bootstrap.pending");
-    if !allow_bootstrap_recovery
-        && bootstrap_pending
-            .try_exists()
-            .map_err(|error| account_store_io_error(&bootstrap_pending, error))?
-    {
-        return Err(AcmeAccountStoreError::UnsafePath {
-            path: bootstrap_pending,
-            message:
-                "account bootstrap is pending; recovery must complete before credentials are used"
-                    .to_owned(),
-        });
+    reject_pending_account_deactivation(directory)?;
+    if !allow_bootstrap_recovery {
+        reject_pending_account_bootstrap(directory)?;
     }
     let metadata = match fs::symlink_metadata(path) {
         Ok(metadata) => metadata,
@@ -243,7 +223,7 @@ fn begin_account_deactivation_locked(
     lock: AcmeMutationLock,
 ) -> Result<AccountDeactivationTransaction, AcmeAccountStoreError> {
     ensure_safe_account_destination(&active)?;
-    let pending = directory.join(ACME_ACCOUNT_DEACTIVATION_FILE);
+    let pending = directory.join(super::acme_account_state::ACCOUNT_DEACTIVATION_PENDING_FILE);
     match fs::symlink_metadata(&pending) {
         Ok(_) => {
             return Err(AcmeAccountStoreError::UnsafePath {
@@ -305,17 +285,7 @@ pub fn store_account_credentials(
     ensure_safe_account_directory(directory)?;
     let _mutation_lock = AcmeMutationLock::acquire(directory)
         .map_err(|error| account_store_io_error(&directory.join(".fluxheim-acme.lock"), error))?;
-    let bootstrap_pending = directory.join(".credentials.bootstrap.pending");
-    if bootstrap_pending
-        .try_exists()
-        .map_err(|error| account_store_io_error(&bootstrap_pending, error))?
-    {
-        return Err(AcmeAccountStoreError::UnsafePath {
-            path: bootstrap_pending,
-            message: "account bootstrap is pending; credentials must be promoted by the bootstrap transaction"
-                .to_owned(),
-        });
-    }
+    reject_pending_account_mutation(directory)?;
     store_account_credentials_locked(&credentials_path, directory, credentials)
 }
 
@@ -379,6 +349,7 @@ pub fn remove_account_credentials(
     ensure_safe_account_directory(directory)?;
     let _mutation_lock = AcmeMutationLock::acquire(directory)
         .map_err(|error| account_store_io_error(&directory.join(".fluxheim-acme.lock"), error))?;
+    reject_pending_account_mutation(directory)?;
     ensure_safe_account_destination(&credentials_path.path)?;
     match fs::remove_file(&credentials_path.path) {
         Ok(()) => {

crates/fluxheim-acme/src/acme_tests_storage.rs

@@ -80,12 +80,20 @@ fn account_deactivation_quarantine_is_fail_closed_and_recoverable() {
 
     let transaction = crate::begin_account_deactivation(&storage, "letsencrypt").unwrap();
     transaction.abandon();
+    let active = account_credentials_path(&storage, "letsencrypt").path;
+    assert!(!active.exists());
+    let store_error =
+        store_account_credentials(&storage, "letsencrypt", &test_account_credentials())
+            .unwrap_err();
+    assert!(store_error.to_string().contains("ambiguous pending state"));
+    let remove_error = remove_account_credentials(&storage, "letsencrypt").unwrap_err();
+    assert!(remove_error.to_string().contains("ambiguous pending state"));
+    assert!(!active.exists());
     let pending = match load_account_credentials(&storage, "letsencrypt") {
         Ok(_) => panic!("expected pending deactivation to fail closed"),
         Err(error) => error,
     };
     assert!(pending.to_string().contains("ambiguous pending state"));
-    let active = account_credentials_path(&storage, "letsencrypt").path;
     std::fs::rename(
         active
             .parent()

crates/fluxheim-acme/src/acme_tests_storage_async.rs

@@ -36,3 +36,34 @@ fn account_credentials_store_load_and_remove_round_trip() {
         );
     });
 }
+
+#[test]
+fn account_mutations_reject_ambiguous_deactivation_state() {
+    let storage = fluxheim_common::test_support::unique_temp_path("acme-account-async-pending");
+    store_account_credentials(&storage, "letsencrypt", &test_account_credentials()).unwrap();
+    crate::begin_account_deactivation(&storage, "letsencrypt")
+        .unwrap()
+        .abandon();
+    let active = account_credentials_path(&storage, "letsencrypt").path;
+    let runtime = tokio::runtime::Builder::new_current_thread()
+        .enable_all()
+        .build()
+        .unwrap();
+
+    runtime.block_on(async {
+        let store_error = crate::store_account_credentials_async(
+            &storage,
+            "letsencrypt",
+            test_account_credentials(),
+        )
+        .await
+        .unwrap_err();
+        assert!(store_error.to_string().contains("ambiguous pending state"));
+        let remove_error = crate::remove_account_credentials_async(&storage, "letsencrypt")
+            .await
+            .unwrap_err();
+        assert!(remove_error.to_string().contains("ambiguous pending state"));
+    });
+
+    assert!(!active.exists());
+}

crates/fluxheim-acme/src/lib.rs

@@ -221,6 +221,8 @@ impl fmt::Debug for AcmeExternalAccountBindingSecrets {
 #[cfg(feature = "acme-client")]
 #[path = "acme_account_async.rs"]
 mod acme_account_async;
+#[path = "acme_account_state.rs"]
+mod acme_account_state;
 #[path = "acme_account_store.rs"]
 mod acme_account_store;
 #[cfg(feature = "acme-client")]
@@ -265,6 +267,10 @@ pub use acme_account_async::{
     load_account_credentials_async, remove_account_credentials_async,
     store_account_credentials_async,
 };
+use acme_account_state::{
+    reject_pending_account_bootstrap, reject_pending_account_deactivation,
+    reject_pending_account_mutation,
+};
 #[cfg(feature = "acme-client")]
 use acme_account_store::{
     AccountBootstrap, AccountDeactivationTransaction, AccountStoreAttempt, PendingAccountBootstrap,

docs/certificate-renewal.md

@@ -187,7 +187,9 @@ Crate consumers running inside Tokio should use
 `remove_account_credentials_async`. Their synchronous counterparts are
 intended for synchronous startup and maintenance code and may wait indefinitely
 on the advisory lifecycle lock; each is marked with a Rustdoc `# Blocking`
-contract.
+contract. Both API families reject ordinary credential stores and removals while
+a bootstrap or ambiguous deactivation journal exists, so recovery state cannot
+be hidden by publishing a second active credential file.
 
 Pending account-key buffers use `sanitization::SecretVec`. The pending file is
 overwritten, synced, truncated, and unlinked after durable