plc: always keep signing key in rotationKeys

Lewis: May this revision serve well! <lu5a@proton.me>

crates/tranquil-api/src/identity/did.rs

@@ -491,16 +491,10 @@ pub async fn get_recommended_did_credentials(
     let rotation_keys = if auth.did.starts_with("did:web:") {
         vec![]
     } else {
-        let server_rotation_key = match &tranquil_config::get().secrets.plc_rotation_key {
-            Some(key) => key.clone(),
-            None => {
-                warn!(
-                    "PLC_ROTATION_KEY not set, falling back to user's signing key for rotation key recommendation"
-                );
-                did_key.clone()
-            }
-        };
-        vec![server_rotation_key]
+        tranquil_pds::plc::rotation_keys_for(
+            tranquil_config::get().secrets.plc_rotation_key.as_deref(),
+            &signing_key,
+        )
     };
     Ok(Json(GetRecommendedDidCredentialsOutput {
         rotation_keys,

crates/tranquil-api/src/identity/plc/sign.rs

@@ -9,7 +9,10 @@ use tranquil_pds::api::ApiError;
 use tranquil_pds::api::error::DbResultExt;
 use tranquil_pds::auth::{Auth, Permissive};
 use tranquil_pds::circuit_breaker::with_circuit_breaker;
-use tranquil_pds::plc::{PlcError, PlcService, create_update_op, sign_operation};
+use tranquil_pds::plc::{
+    PlcError, PlcService, create_update_op, missing_required_rotation_key, sign_operation,
+    signing_key_to_did_key,
+};
 use tranquil_pds::state::AppState;
 
 #[derive(Debug, Deserialize)]
@@ -115,6 +118,18 @@ pub async fn sign_plc_operation(
         }
     })?;
 
+    let signing_did_key = signing_key_to_did_key(&signing_key);
+    if let Some(rotation_keys) = unsigned_op.get("rotationKeys").and_then(Value::as_array) {
+        let rotation_key_strs: Vec<&str> = rotation_keys.iter().filter_map(Value::as_str).collect();
+        if let Some(missing) = missing_required_rotation_key(
+            &rotation_key_strs,
+            &signing_did_key,
+            tranquil_config::get().secrets.plc_rotation_key.as_deref(),
+        ) {
+            return Err(ApiError::InvalidRequest(missing.message().into()));
+        }
+    }
+
     let signed_op = sign_operation(&unsigned_op, &signing_key).map_err(|e| {
         error!("Failed to sign PLC operation: {:?}", e);
         ApiError::InternalError(None)

crates/tranquil-api/src/identity/plc/submit.rs

@@ -67,19 +67,14 @@ pub async fn submit_plc_operation(
     })?;
 
     let user_did_key = signing_key_to_did_key(&signing_key);
-    let server_rotation_key = tranquil_config::get()
-        .secrets
-        .plc_rotation_key
-        .clone()
-        .unwrap_or_else(|| user_did_key.clone());
     if let Some(rotation_keys) = op.get("rotationKeys").and_then(Value::as_array) {
-        let has_server_key = rotation_keys
-            .iter()
-            .any(|k| k.as_str() == Some(&server_rotation_key));
-        if !has_server_key {
-            return Err(ApiError::InvalidRequest(
-                "Rotation keys do not include server's rotation key".into(),
-            ));
+        let rotation_key_strs: Vec<&str> = rotation_keys.iter().filter_map(Value::as_str).collect();
+        if let Some(missing) = tranquil_pds::plc::missing_required_rotation_key(
+            &rotation_key_strs,
+            &user_did_key,
+            tranquil_config::get().secrets.plc_rotation_key.as_deref(),
+        ) {
+            return Err(ApiError::InvalidRequest(missing.message().into()));
         }
     }
     if let Some(services) = op.get("services").and_then(Value::as_object)

crates/tranquil-api/src/identity/provision.rs

@@ -45,15 +45,9 @@ pub async fn submit_plc_genesis(
     let hostname = &tranquil_config::get().server.hostname;
     let pds_endpoint = format!("https://{}", hostname);
 
-    let rotation_key = tranquil_config::get()
-        .secrets
-        .plc_rotation_key
-        .clone()
-        .unwrap_or_else(|| tranquil_pds::plc::signing_key_to_did_key(signing_key));
-
     let genesis_result = tranquil_pds::plc::create_genesis_operation(
         signing_key,
-        &rotation_key,
+        tranquil_config::get().secrets.plc_rotation_key.as_deref(),
         handle,
         &pds_endpoint,
     )

crates/tranquil-api/src/repo/import.rs

@@ -14,6 +14,35 @@ use tranquil_pds::sync::verify::CarVerifier;
 use tranquil_pds::types::Did;
 use tranquil_types::{AtUri, CidLink};
 
+fn map_car_verify_error(e: tranquil_pds::sync::verify::VerifyError) -> ApiError {
+    use tranquil_pds::sync::verify::VerifyError;
+    match e {
+        VerifyError::DidMismatch {
+            commit_did,
+            expected_did,
+        } => ApiError::InvalidRepo(format!(
+            "CAR file is for DID {} but you are authenticated as {}",
+            commit_did, expected_did
+        )),
+        VerifyError::InvalidSignature => ApiError::InvalidRequest(
+            "Repo commit signature does not match the DID document signing key".into(),
+        ),
+        VerifyError::NoSigningKey => {
+            ApiError::InvalidRequest("DID document has no atproto signing key".into())
+        }
+        VerifyError::DidResolutionFailed(msg) => {
+            ApiError::InvalidRequest(format!("Could not resolve DID document: {}", msg))
+        }
+        VerifyError::MstValidationFailed(msg) => {
+            ApiError::InvalidRequest(format!("MST validation failed: {}", msg))
+        }
+        other => {
+            error!("CAR verification failed: {:?}", other);
+            ApiError::InvalidRequest(format!("CAR verification failed: {}", other))
+        }
+    }
+}
+
 pub async fn import_repo(
     State(state): State<AppState>,
     auth: Auth<NotTakendown>,
@@ -88,41 +117,23 @@ pub async fn import_repo(
     let is_migration = user.inbound_migration && user.deactivated_at.is_some();
     if skip_verification {
         warn!("Skipping all CAR verification for repo import (SKIP_IMPORT_VERIFICATION=true)");
-    } else {
+    } else if is_migration {
+        let verified = CarVerifier::new()
+            .verify_car_structure_only(&root, &blocks)
+            .map_err(map_car_verify_error)?;
         debug!(
-            "Verifying CAR file structure for repo import (skipping signature and DID verification)"
+            "CAR structure verified for migration import: rev={}, data_cid={}",
+            verified.rev, verified.data_cid
+        );
+    } else {
+        let verified = CarVerifier::new()
+            .verify_car(did, &root, &blocks)
+            .await
+            .map_err(map_car_verify_error)?;
+        debug!(
+            "CAR signature and structure verified: rev={}, data_cid={}",
+            verified.rev, verified.data_cid
         );
-        let verifier = CarVerifier::new();
-        match verifier.verify_car_structure_only(&root, &blocks) {
-            Ok(verified) => {
-                debug!(
-                    "CAR structure verification successful: rev={}, data_cid={}",
-                    verified.rev, verified.data_cid
-                );
-            }
-            Err(tranquil_pds::sync::verify::VerifyError::DidMismatch {
-                commit_did,
-                expected_did,
-            }) => {
-                return Err(ApiError::InvalidRepo(format!(
-                    "CAR file is for DID {} but you are authenticated as {}",
-                    commit_did, expected_did
-                )));
-            }
-            Err(tranquil_pds::sync::verify::VerifyError::MstValidationFailed(msg)) => {
-                return Err(ApiError::InvalidRequest(format!(
-                    "MST validation failed: {}",
-                    msg
-                )));
-            }
-            Err(e) => {
-                error!("CAR structure verification error: {:?}", e);
-                return Err(ApiError::InvalidRequest(format!(
-                    "CAR verification failed: {}",
-                    e
-                )));
-            }
-        }
     }
     let max_blocks = tranquil_config::get().import.max_blocks as usize;
     let _write_lock = state.repo_write_locks.lock(user_id).await;

crates/tranquil-api/src/server/account_status.rs

@@ -197,18 +197,19 @@ async fn assert_valid_did_document_for_service(
         .await
         .map_err(ApiError::InvalidRequest)?;
 
+        let doc_rotation_keys = doc_data
+            .get("rotationKeys")
+            .and_then(Value::as_array)
+            .map(|arr| arr.iter().filter_map(Value::as_str).collect::<Vec<_>>())
+            .unwrap_or_default();
+
         let server_rotation_key = tranquil_config::get().secrets.plc_rotation_key.clone();
-        if let Some(ref expected_rotation_key) = server_rotation_key {
-            let rotation_keys = doc_data
-                .get("rotationKeys")
-                .and_then(Value::as_array)
-                .map(|arr| arr.iter().filter_map(Value::as_str).collect::<Vec<_>>())
-                .unwrap_or_default();
-            if !rotation_keys.contains(&expected_rotation_key.as_str()) {
-                return Err(ApiError::InvalidRequest(
-                    "Server rotation key not included in PLC DID data".into(),
-                ));
-            }
+        if let Some(ref expected_rotation_key) = server_rotation_key
+            && !doc_rotation_keys.contains(&expected_rotation_key.as_str())
+        {
+            return Err(ApiError::InvalidRequest(
+                "Server rotation key not included in PLC DID data".into(),
+            ));
         }
 
         let doc_signing_key = doc_data
@@ -243,6 +244,16 @@ async fn assert_valid_did_document_for_service(
                     "DID document verification method does not match expected signing key".into(),
                 ));
             }
+
+            if !doc_rotation_keys.contains(&expected_did_key.as_str()) {
+                warn!(
+                    "DID {} rotation keys {:?} omit the PDS-managed signing key {}",
+                    did, doc_rotation_keys, expected_did_key
+                );
+                return Err(ApiError::InvalidRequest(
+                    "PLC rotation keys omit the PDS-managed signing key required to sign operations for this identity".into(),
+                ));
+            }
         }
     } else if let Some(host_and_path) = did.as_str().strip_prefix("did:web:") {
         let client = tranquil_pds::api::proxy_client::did_resolution_client();

crates/tranquil-api/src/server/passkey_account.rs

@@ -224,15 +224,9 @@ pub async fn create_passkey_account(
                     ));
                 }
             } else {
-                let rotation_key = tranquil_config::get()
-                    .secrets
-                    .plc_rotation_key
-                    .clone()
-                    .unwrap_or_else(|| tranquil_pds::plc::signing_key_to_did_key(&secret_key));
-
                 let genesis_result = match tranquil_pds::plc::create_genesis_operation(
                     &secret_key,
-                    &rotation_key,
+                    tranquil_config::get().secrets.plc_rotation_key.as_deref(),
                     &handle,
                     &pds_endpoint,
                 ) {