ACME changes

ic-bn-lib/Cargo.toml

@@ -127,7 +127,7 @@ zeroize = { workspace = true }
 zstd = { workspace = true }
 
 [dev-dependencies]
-axum-server = { workspace = true }
+axum-server = { workspace = true, features = ["tls-rustls"] }
 criterion = { workspace = true }
 ic-verify-bls-signature = { workspace = true }
 mail-send = { workspace = true }

ic-bn-lib/src/tests/pebble.rs

@@ -28,7 +28,7 @@ use crate::{
     tests::{TEST_CERT_1, TEST_KEY_1},
 };
 
-const VER: &str = "2.8.0";
+const VER: &str = "2.10.1";
 const PEBBLE_KEY: &str = "pebble-key.pem";
 const PEBBLE_CERT: &str = "pebble-cert.pem";
 
@@ -66,43 +66,43 @@ pub async fn download(path: &Path) -> Result<(), Error> {
             "linux": {
                 "x86_64": {
                     "url": format!("https://github.com/letsencrypt/pebble/releases/download/v{VER}/pebble-linux-amd64.tar.gz"),
-                    "sha": "34595d915bbc2fc827affb3f58593034824df57e95353b031c8d5185724485ce",
+                    "sha": "4f2fcb5bca8c85c9cf73ad140fccfc0d2be40bd81ab99879c79b7b8a0b4f70ed",
                 },
                 "aarch64": {
                     "url": format!("https://github.com/letsencrypt/pebble/releases/download/v{VER}/pebble-linux-arm64.tar.gz"),
-                    "sha": "0e70f2537353f61cbf06aa54740bf7f7bb5f963ba00e909f23af5f85bc13fd1a",
+                    "sha": "b53fd072a69eb7692451de4e8b0667e0bdf5cccd7e36fc51b8eaf2fcc135ed9f",
                 }
             },
             "macos": {
                 "x86_64": {
                     "url": format!("https://github.com/letsencrypt/pebble/releases/download/v{VER}/pebble-darwin-amd64.tar.gz"),
-                    "sha": "9b9625651f8ce47706235179503fec149f8f38bce2b2554efe8c0f2a021f877c",
+                    "sha": "e670ff869886022637e077502a62e7f23be693c45a5a6727ebd76da8fdce64dc",
                 },
                 "aarch64": {
                     "url": format!("https://github.com/letsencrypt/pebble/releases/download/v{VER}/pebble-darwin-arm64.tar.gz"),
-                    "sha": "39e07d63dc776521f2ffe0584e5f4f081c984ac02742c882b430891d89f0c866",
+                    "sha": "09a3a4e6ebed71e8d83294a26d361232262f45a7488f5de7bccb5887b395217f",
                 }
             }
         },
         "pebble-challtestsrv": {
             "linux": {
                 "x86_64": {
                     "url": format!("https://github.com/letsencrypt/pebble/releases/download/v{VER}/pebble-challtestsrv-linux-amd64.tar.gz"),
-                    "sha": "a817449d1f05ae58bcb7bf073b4cebe5d31512f859ba4b83951bd825d28d2114",
+                    "sha": "e93a5aa25ecdf3af2f9fbb2de32b0173e64a2eae81002a4ccfe35fa6f4f60b92",
                 },
                 "aarch64": {
                     "url": format!("https://github.com/letsencrypt/pebble/releases/download/v{VER}/pebble-challtestsrv-linux-arm64.tar.gz"),
-                    "sha": "99a276aac8ceac121859b799708218e6dc57d7ca1dc1b8b5b586246b3c4160e6",
+                    "sha": "db8e1a79ccdb2195c489fbe4f40fddb7f30e86f9cd8a07912566ee5025094d6c",
                 }
             },
             "macos": {
-                "aarch64": {
+                "x86_64": {
                     "url": format!("https://github.com/letsencrypt/pebble/releases/download/v{VER}/pebble-challtestsrv-darwin-amd64.tar.gz"),
-                    "sha": "3d1343b1bbe892145fd2da70be36e67b149e482fbff897e109b8053f4f790f40",
+                    "sha": "796bd923f2c595dd7bf15ae693096abfb1df962cb3673c7981ff306daa5c4a52",
                 },
                 "aarch64": {
                     "url": format!("https://github.com/letsencrypt/pebble/releases/download/v{VER}/pebble-challtestsrv-darwin-arm64.tar.gz"),
-                    "sha": "1bc5a6cfa062d9756e98d67825daf67f61dd655bcb6025efca2138fe836c9bbc",
+                    "sha": "59bf917fe39c96e2edca980fc2899f4f04aa1ce5485f28d419d18237b902cf82",
                 }
             }
         }
@@ -233,7 +233,7 @@ impl Dns {
         let mut cmd = Command::new(&opts.path);
         cmd.arg("-management");
         cmd.arg(format!("{}:{}", opts.ip, opts.port_man));
-        cmd.arg("-dns01");
+        cmd.arg("-dnsserver");
         cmd.arg(format!("{}:{}", opts.ip, opts.port_dns));
         // Disable the rest
         cmd.arg("-doh");
@@ -468,7 +468,8 @@ pub mod dns {
             Ok(())
         }
 
-        async fn unset(&self, zone: &str) -> Result<(), Error> {
+        /// pebble-challtestsrv doesn't allow to delete specific TXT record, so we nuke them all
+        async fn unset(&self, zone: &str, _token: &str) -> Result<(), Error> {
             let url = self.url.join("/clear-txt").unwrap();
             let body = json!({
                 "host" : format!("_acme-challenge.{zone}."),
@@ -498,8 +499,8 @@ pub mod dns {
             self.set(zone, &token).await
         }
 
-        async fn delete(&self, zone: &str, _name: &str) -> Result<(), Error> {
-            self.unset(zone).await
+        async fn delete(&self, zone: &str, _name: &str, _record: &Record) -> Result<(), Error> {
+            self.unset(zone, "").await
         }
     }
 
@@ -530,7 +531,7 @@ pub mod dns {
             assert_eq!(r[0].record_type(), RecordType::TXT);
             assert_eq!(r[0].data.to_string(), "bar");
 
-            tm.unset("foo").await.unwrap();
+            tm.unset("foo", "").await.unwrap();
             let r = resolver
                 .resolve(RecordType::TXT, "_acme-challenge.foo")
                 .await;
@@ -548,7 +549,7 @@ pub mod dns {
                 assert_eq!(r[0].record_type(), RecordType::TXT);
                 assert_eq!(r[0].data.to_string(), "deadbeef");
 
-                tm.unset("baz").await.unwrap();
+                tm.unset("baz", "").await.unwrap();
                 let r = resolver
                     .resolve(RecordType::TXT, "_acme-challenge.baz")
                     .await;

ic-bn-lib/src/tls/acme/client.rs

@@ -11,8 +11,10 @@ use instant_acme::{
     HttpClient as AcmeHttpClientTrait, Identifier, NewAccount, NewOrder, Order, OrderStatus,
     RetryPolicy, RevocationRequest,
 };
+use itertools::Itertools;
 use rcgen::{CertificateParams, DistinguishedName, KeyPair};
 use rustls::ClientConfig;
+use tokio::sync::Mutex;
 use tracing::{debug, instrument, warn};
 
 use crate::{
@@ -348,9 +350,14 @@ impl Client {
     }
 
     /// Iterates over authorizations in the order and tries to fulfill them.
-    /// Returns the list of IDs that are later used in the cleanup.
     #[instrument(level = "debug", skip_all)]
-    async fn process_authorizations(&self, order: &mut Order) -> Result<(), Error> {
+    #[allow(clippy::significant_drop_tightening)]
+    async fn process_authorizations(
+        &self,
+        order: &mut Order,
+        challenge_tokens: Arc<Mutex<Vec<(String, String)>>>,
+    ) -> Result<(), Error> {
+        let mut challenge_tokens = challenge_tokens.lock().await;
         let mut authorizations = order.authorizations();
 
         while let Some(authz) = authorizations.next().await {
@@ -361,43 +368,33 @@ impl Client {
                 continue;
             };
 
+            challenge_tokens.push((id.clone(), token.clone()));
             self.process_challenge(id, token, challenge).await?;
         }
 
         Ok(())
     }
 
     /// Cleans up the tokens after issuance using authorization IDs
-    #[instrument(level = "debug", skip_all, fields(ids = %auth_ids.join(", ")))]
-    async fn cleanup(&self, auth_ids: &[String]) -> Result<(), Error> {
-        debug!(
-            "Cleaning up the authorization tokens for ids: {}",
-            auth_ids.join(", ")
-        );
-
-        for id in auth_ids {
-            debug!("Unsetting token for id: '{id}'");
-
-            self.token_manager
-                .unset(id)
-                .await
-                .map_err(Error::UnableToUnsetChallengeToken)?;
-        }
+    #[instrument(level = "debug", skip_all, fields(ids = %challenge_tokens.iter().map(|x| &x.0).join(", ")))]
+    async fn cleanup(&self, challenge_tokens: &[(String, String)]) -> Result<(), Error> {
+        debug!("Cleaning up the authorization tokens");
 
-        Ok(())
-    }
+        let mut errors = vec![];
+        for (id, token) in challenge_tokens {
+            debug!("Unsetting token for '{id}' : {token}");
 
-    async fn get_authorization_ids(&self, order: &mut Order) -> Result<Vec<String>, Error> {
-        let mut auth_ids = vec![];
-        let mut identifiers_stream = order.identifiers();
-        while let Some(id) = identifiers_stream.next().await {
-            let id = id.map_err(Error::UnableToGetAuthorizations)?.to_string();
-            if !auth_ids.contains(&id) {
-                auth_ids.push(id.to_string());
+            if let Err(e) = self.token_manager.unset(id, token).await {
+                warn!("Unable to unset token '{token}' for '{id}': {e:#}");
+                errors.push(e);
             }
         }
 
-        Ok(auth_ids)
+        if errors.is_empty() {
+            Ok(())
+        } else {
+            Err(Error::UnableToUnsetChallengeToken(errors))
+        }
     }
 
     #[instrument(level = "debug", skip_all)]
@@ -417,15 +414,14 @@ impl Client {
         // Prepare the order
         let mut order = self.prepare_order(ids).await?;
 
-        // Get auth ids and clean them up
-        let auth_ids = self.get_authorization_ids(&mut order).await?;
-        self.cleanup(&auth_ids).await?;
-
         debug!(
             "Order obtained (status: {:?}), processing authorizations",
             order.state().status
         );
 
+        let challenge_tokens = Arc::new(Mutex::new(vec![]));
+        let challenge_tokens_clone = challenge_tokens.clone();
+
         // From this point on, challenges may get set up (DNS-01 TXT record /
         // ALPN response), so no matter how issuance finishes we must attempt
         // to clean them up below - otherwise a mid-issuance failure (order
@@ -434,7 +430,8 @@ impl Client {
         // exact same domain set happens to clean it up first.
         let result: Result<AcmeCert, Error> = async move {
             // Process authorizations and fulfill their challenges
-            self.process_authorizations(&mut order).await?;
+            self.process_authorizations(&mut order, challenge_tokens_clone)
+                .await?;
 
             debug!("Authorizations processed, waiting for the order to reach Ready state");
 
@@ -488,9 +485,11 @@ impl Client {
         }
         .await;
 
-        debug!("Cleaning up the authorization tokens");
-        if let Err(e) = self.cleanup(&auth_ids).await {
-            warn!("Unable to clean up ACME challenge tokens for {auth_ids:?}: {e:#}");
+        debug!("Cleaning up the challenge tokens");
+        let challenge_tokens = challenge_tokens.lock().await.drain(..).collect::<Vec<_>>();
+
+        if let Err(e) = self.cleanup(&challenge_tokens).await {
+            warn!("Unable to clean up ACME challenge tokens: {e:#}");
 
             // Only surface the cleanup error if issuance itself succeeded -
             // otherwise the original error is the more important one and