Fix private key parsing and add fallback on ACME (#827)

As part of the version updates we evidently lost the ability to parse pem files that have the heading `PRIVATE KEY` and instead need to specifically say `RSA PRIVATE KEY`. Also, if we do fail, we should just fall back on ACME.
jamsocket · Oct 16, 2024 · ef3fa13 · ef3fa13
1 parent 77698c2
commit ef3fa13
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 39 deletions.
diff --git a/plane/plane-tests/tests/cert_manager.rs b/plane/plane-tests/tests/cert_manager.rs
@@ -58,52 +58,72 @@ async fn cert_manager_does_refresh(env: TestEnvironment) {
         .unwrap();
 }
 
-#[plane_test]
+#[plane_test(120)]
 async fn cert_manager_does_refresh_eab(env: TestEnvironment) {
-    let controller = env.controller().await;
+    let certs_dir = env.scratch_dir.join("certs");
 
-    let dns = env.dns(&controller).await;
+    {
+        let controller = env.controller().await;
 
-    let eab_keypair = AcmeEabConfiguration::new(
-        "kid-1".to_string(),
-        "zWNDZM6eQGHWpSRTPal5eIUYFTu7EajVIoguysqZ9wG44nMEtx3MUAsUDkMTQ12W".to_string(),
-    )
-    .unwrap();
+        let dns = env.dns(&controller).await;
 
-    let pebble = env.pebble_with_eab(dns.port, eab_keypair.clone()).await;
-    tracing::info!("Pebble: {}", pebble.directory_url);
+        let eab_keypair = AcmeEabConfiguration::new(
+            "kid-1".to_string(),
+            "zWNDZM6eQGHWpSRTPal5eIUYFTu7EajVIoguysqZ9wG44nMEtx3MUAsUDkMTQ12W".to_string(),
+        )
+        .unwrap();
 
-    let acme_config = AcmeConfig {
-        endpoint: pebble.directory_url.clone(),
-        mailto_email: "test-cert@jamsocket.com".to_string(),
-        accept_insecure_certs_for_testing: true,
-        acme_eab_keypair: Some(eab_keypair),
-    };
+        let pebble = env.pebble_with_eab(dns.port, eab_keypair.clone()).await;
+        tracing::info!("Pebble: {}", pebble.directory_url);
 
-    let certs_dir = env.scratch_dir.join("certs");
-    std::fs::create_dir_all(&certs_dir).unwrap();
+        let acme_config = AcmeConfig {
+            endpoint: pebble.directory_url.clone(),
+            mailto_email: "test-cert@jamsocket.com".to_string(),
+            accept_insecure_certs_for_testing: true,
+            acme_eab_keypair: Some(eab_keypair),
+        };
 
-    let (mut cert_watcher, cert_manager) = watcher_manager_pair(
-        env.cluster.clone(),
-        Some(&certs_dir.join("cert.json")),
-        Some(acme_config.clone()),
-    )
-    .await
-    .unwrap();
+        std::fs::create_dir_all(&certs_dir).unwrap();
 
-    let state = Arc::new(ProxyState::new(None));
+        let (mut cert_watcher, cert_manager) = watcher_manager_pair(
+            env.cluster.clone(),
+            Some(&certs_dir.join("cert.json")),
+            Some(acme_config.clone()),
+        )
+        .await
+        .unwrap();
 
-    let _proxy_connection = ProxyConnection::new(
-        ProxyName::new_random(),
-        controller.client(),
-        env.cluster.clone(),
-        cert_manager,
-        state.clone(),
-    );
-    cert_watcher
-        .wait_for_initial_cert()
-        .with_timeout(60)
+        let state = Arc::new(ProxyState::new(None));
+
+        let _proxy_connection = ProxyConnection::new(
+            ProxyName::new_random(),
+            controller.client(),
+            env.cluster.clone(),
+            cert_manager,
+            state.clone(),
+        );
+        cert_watcher
+            .wait_for_initial_cert()
+            .with_timeout(60)
+            .await
+            .unwrap()
+            .unwrap();
+    }
+
+    {
+        let (mut cert_watcher, _cert_manager) = watcher_manager_pair(
+            env.cluster.clone(),
+            Some(&certs_dir.join("cert.json")),
+            None, // No ACME config; force load from disk.
+        )
         .await
-        .unwrap()
         .unwrap();
+
+        cert_watcher
+            .wait_for_initial_cert()
+            .with_timeout(60)
+            .await
+            .unwrap()
+            .unwrap();
+    }
 }
diff --git a/plane/src/proxy/cert_manager.rs b/plane/src/proxy/cert_manager.rs
@@ -138,7 +138,17 @@ impl CertManager {
     ) -> Result<Self> {
         let initial_cert = if let Some(cert_path) = cert_path {
             if cert_path.exists() {
-                Some(CertificatePair::load(cert_path)?)
+                match CertificatePair::load(cert_path) {
+                    Ok(cert) => Some(cert),
+                    Err(err) => {
+                        tracing::error!(
+                            ?err,
+                            ?cert_path,
+                            "Error loading certificate; obtaining via ACME instead."
+                        );
+                        None
+                    }
+                }
             } else {
                 None
             }

diff --git a/plane/src/proxy/cert_pair.rs b/plane/src/proxy/cert_pair.rs
@@ -124,7 +124,7 @@ impl CertificatePair {
                 .as_slice(),
         );
 
-        let key = pem::encode(&Pem::new("PRIVATE KEY", self.private_key_der.clone()));
+        let key = pem::encode(&Pem::new("RSA PRIVATE KEY", self.private_key_der.clone()));
 
         let cert_pair = SerializedCertificatePair { cert, key };