diff --git a/src/oauth/pds_write.rs b/src/oauth/pds_write.rs index 9724893..a104271 100644 --- a/src/oauth/pds_write.rs +++ b/src/oauth/pds_write.rs @@ -693,6 +693,77 @@ fn generate_dpop_proof_inner( Ok(format!("{}.{}.{}", header_b64, payload_b64, sig_b64)) } +/// Verify that a DPoP access token belongs to `did` by calling +/// `com.atproto.server.getSession` on the DID's own PDS. +/// +/// This is the trust anchor for session registration: the caller supplies a +/// `did` and an `access_token`, but nothing proves the token was issued for +/// that DID. We resolve the PDS **authoritatively from the DID document** (never +/// from a client-supplied PDS URL, which an attacker could point at a server +/// that lies), then present the token with a DPoP proof signed by the +/// provisioned key the token is bound to. The PDS reports which DID the token +/// actually belongs to; the caller compares it against the claimed `did`. +/// +/// Returns the DID the PDS reports for the token, or an auth error if the token +/// is rejected. +pub async fn verify_access_token_did( + http: &reqwest::Client, + plc_url: &str, + private_jwk: &serde_json::Value, + did: &str, + access_token: &str, +) -> Result { + let pds_url = resolve_pds_from_did(http, plc_url, did).await?; + let target_url = format!( + "{}/xrpc/com.atproto.server.getSession", + pds_url.trim_end_matches('/') + ); + + // The PDS may demand a DPoP nonce on the first attempt; retry once with it. + let mut nonce: Option = None; + for _ in 0..2 { + let proof = generate_dpop_proof( + private_jwk, + "GET", + &target_url, + access_token, + nonce.as_deref(), + )?; + let resp = http + .get(&target_url) + .header("Authorization", format!("DPoP {access_token}")) + .header("DPoP", proof) + .send() + .await + .map_err(|e| AppError::Internal(format!("getSession request failed: {e}")))?; + + if resp.status().is_success() { + let body: serde_json::Value = resp + .json() + .await + .map_err(|e| AppError::Internal(format!("invalid getSession response: {e}")))?; + return body["did"] + .as_str() + .map(|s| s.to_string()) + .ok_or_else(|| AppError::Auth("getSession response missing did".into())); + } + + if nonce.is_none() + && let Some(n) = extract_dpop_nonce(&resp) + { + nonce = Some(n); + continue; + } + + return Err(AppError::Auth(format!( + "access token verification failed ({})", + resp.status() + ))); + } + + Err(AppError::Auth("access token verification failed".into())) +} + /// Resolve a user's PDS URL from their DID document. async fn resolve_pds_from_did( http: &reqwest::Client, diff --git a/src/oauth/routes.rs b/src/oauth/routes.rs index d4580f5..75198c1 100644 --- a/src/oauth/routes.rs +++ b/src/oauth/routes.rs @@ -207,7 +207,7 @@ async fn register_session( .ok_or_else(|| AppError::Internal("TOKEN_ENCRYPTION_KEY not configured".into()))?; // Look up the DPoP key by provision_id - let (dpop_key_id, dpop_client_id, _private_jwk, _thumbprint, pkce_challenge) = + let (dpop_key_id, dpop_client_id, private_jwk, _thumbprint, pkce_challenge) = keys::get_dpop_key( &state.db, state.db_backend, @@ -258,6 +258,31 @@ async fn register_session( return Err(e); } + // Verify the access token actually belongs to the claimed DID. The `did` in + // the request body is client-supplied and untrusted; without this check any + // holder of a provisioned DPoP key could register a session for an arbitrary + // victim DID and be authenticated as them on every DPoP-accepting route. + let verified_did = super::pds_write::verify_access_token_did( + &state.http, + &state.config.plc_url, + &private_jwk, + &body.did, + &body.access_token, + ) + .await?; + + if verified_did != body.did { + tracing::warn!( + client_key = %client_key, + claimed_did = %body.did, + verified_did = %verified_did, + "session registration rejected: access token does not belong to claimed DID" + ); + return Err(AppError::Auth( + "access token does not belong to the claimed DID".into(), + )); + } + // Store the session let session_id = Uuid::new_v4().to_string(); sessions::store_dpop_session( diff --git a/tests/common/app.rs b/tests/common/app.rs index ff9d3d2..3ae5669 100644 --- a/tests/common/app.rs +++ b/tests/common/app.rs @@ -580,6 +580,51 @@ impl TestApp { format!("Bearer {}.{}.{}", header_b64, payload_b64, sig_b64) } + /// Mount mock PLC + PDS responses so that registering a DPoP session for + /// `did` passes access-token verification. + /// + /// The mock PLC (== `plc_url`) serves a DID document for `did` whose + /// `#atproto_pds` service points back at the mock server, and + /// `com.atproto.server.getSession` on the mock server reports `resolved_did` + /// for the presented token. For a legitimate session, pass the same value + /// for both arguments; for a spoofing attempt, make them differ. + pub async fn mock_session_verification(&self, did: &str, resolved_did: &str) { + use wiremock::matchers::{method, path}; + use wiremock::{Mock, ResponseTemplate}; + + // Give each DID its own PDS path so several sessions can be mocked on the + // single shared mock server without their getSession mocks colliding + // (the getSession request itself carries no DID to disambiguate on). + let pds_path = format!("/pds/{did}"); + let pds_url = format!("{}{}", self.mock_server.uri(), pds_path); + + let did_doc = serde_json::json!({ + "id": did, + "service": [{ + "id": "#atproto_pds", + "type": "AtprotoPersonalDataServer", + "serviceEndpoint": pds_url, + }] + }); + + Mock::given(method("GET")) + .and(path(format!("/{did}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(did_doc)) + .mount(&self.mock_server) + .await; + + Mock::given(method("GET")) + .and(path(format!( + "{pds_path}/xrpc/com.atproto.server.getSession" + ))) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(serde_json::json!({ "did": resolved_did })), + ) + .mount(&self.mock_server) + .await; + } + /// Install a fake plugin directly into the registry at the given version. pub async fn install_fake_plugin(&self, id: &str, version: &str) { use happyview::plugin::{LoadedPlugin, PluginInfo, PluginSource}; diff --git a/tests/dev_happyview.rs b/tests/dev_happyview.rs index 3d25216..b5797fe 100644 --- a/tests/dev_happyview.rs +++ b/tests/dev_happyview.rs @@ -56,6 +56,7 @@ async fn setup_dpop_session(app: &common::app::TestApp, user_did: &str) -> (Stri // 2. Register session let access_token = format!("test-access-{}", uuid::Uuid::new_v4()); + app.mock_session_verification(user_did, user_did).await; let session_req = post_json_with_headers( "/oauth/sessions", &json!({ diff --git a/tests/dpop_auth.rs b/tests/dpop_auth.rs index 98aa721..ac0bf65 100644 --- a/tests/dpop_auth.rs +++ b/tests/dpop_auth.rs @@ -215,6 +215,118 @@ async fn test_register_session_requires_atproto_scope() { assert_eq!(resp.status(), StatusCode::BAD_REQUEST); } +#[tokio::test] +#[serial] +async fn test_register_session_rejects_did_not_matching_token() { + common::require_db!(); + let app = common::app::TestApp::new_with_encryption().await; + let (client_key, client_secret, _id) = app.create_api_client("confidential", None).await; + + // The attacker claims to be the victim, but the access token they present + // belongs to the attacker's own DID. getSession on the victim's PDS reports + // the attacker DID, so registration must be rejected. + app.mock_session_verification("did:plc:victim", "did:plc:attacker") + .await; + + let key_req = post_json_with_headers( + "/oauth/dpop-keys", + &json!({}), + vec![ + ("x-client-key", &client_key), + ("x-client-secret", &client_secret), + ], + ); + let key_resp = app.router.clone().oneshot(key_req).await.unwrap(); + let key_body = response_json(key_resp).await; + let provision_id = key_body["provision_id"].as_str().unwrap(); + + let req = post_json_with_headers( + "/oauth/sessions", + &json!({ + "provision_id": provision_id, + "did": "did:plc:victim", + "access_token": "attacker-token", + "scopes": "atproto", + }), + vec![ + ("x-client-key", &client_key), + ("x-client-secret", &client_secret), + ], + ); + + let resp = app.router.clone().oneshot(req).await.unwrap(); + assert_eq!( + resp.status(), + StatusCode::UNAUTHORIZED, + "registering a session for a DID the token does not belong to must be rejected" + ); +} + +#[tokio::test] +#[serial] +async fn test_register_session_rejects_token_pds_refuses() { + use wiremock::matchers::{method, path}; + use wiremock::{Mock, ResponseTemplate}; + + common::require_db!(); + let app = common::app::TestApp::new_with_encryption().await; + let (client_key, client_secret, _id) = app.create_api_client("confidential", None).await; + + // The victim's DID document resolves to their real PDS, which rejects the + // attacker's token outright (401). Registration must fail. + let pds_url = app.mock_server.uri(); + Mock::given(method("GET")) + .and(path("/did:plc:victim2")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "id": "did:plc:victim2", + "service": [{ + "id": "#atproto_pds", + "type": "AtprotoPersonalDataServer", + "serviceEndpoint": pds_url, + }] + }))) + .mount(&app.mock_server) + .await; + Mock::given(method("GET")) + .and(path("/xrpc/com.atproto.server.getSession")) + .respond_with(ResponseTemplate::new(401).set_body_json(json!({ "error": "InvalidToken" }))) + .mount(&app.mock_server) + .await; + + let key_req = post_json_with_headers( + "/oauth/dpop-keys", + &json!({}), + vec![ + ("x-client-key", &client_key), + ("x-client-secret", &client_secret), + ], + ); + let key_resp = app.router.clone().oneshot(key_req).await.unwrap(); + let key_body = response_json(key_resp).await; + let provision_id = key_body["provision_id"].as_str().unwrap(); + + let req = post_json_with_headers( + "/oauth/sessions", + &json!({ + "provision_id": provision_id, + "did": "did:plc:victim2", + "access_token": "attacker-token", + "scopes": "atproto", + }), + vec![ + ("x-client-key", &client_key), + ("x-client-secret", &client_secret), + ], + ); + + let resp = app.router.clone().oneshot(req).await.unwrap(); + assert_eq!( + resp.status(), + StatusCode::UNAUTHORIZED, + "registration must fail when the PDS refuses the presented access token" + ); +} + #[tokio::test] #[serial] async fn test_full_flow_provision_register_delete() { @@ -237,6 +349,8 @@ async fn test_full_flow_provision_register_delete() { let provision_id = key_body["provision_id"].as_str().unwrap(); // 2. Register session + app.mock_session_verification("did:plc:testuser", "did:plc:testuser") + .await; let session_req = post_json_with_headers( "/oauth/sessions", &json!({ @@ -381,6 +495,8 @@ async fn test_xrpc_dpop_auth_accepted() { // 2. Register session let access_token = "test-xrpc-access-token"; + app.mock_session_verification("did:plc:xrpcuser", "did:plc:xrpcuser") + .await; let session_req = post_json_with_headers( "/oauth/sessions", &json!({ @@ -446,6 +562,8 @@ async fn provision_and_register( let provision_id = key_body["provision_id"].as_str().unwrap().to_string(); let dpop_key = key_body["dpop_key"].clone(); + app.mock_session_verification(did, did).await; + let session_req = post_json_with_headers( "/oauth/sessions", &json!({ @@ -601,6 +719,8 @@ async fn test_session_upsert_same_device() { let key_body = response_json(key_resp).await; let provision_id = key_body["provision_id"].as_str().unwrap(); + app.mock_session_verification(did, did).await; + // Register session with token-v1 let reg1 = post_json_with_headers( "/oauth/sessions", @@ -738,6 +858,8 @@ async fn test_public_client_dpop_get_session() { let did = "did:plc:publicuser"; let access_token = "public-client-access-token"; + app.mock_session_verification(did, did).await; + // Register session with PKCE verifier let session_req = post_json_with_headers( "/oauth/sessions", @@ -806,6 +928,8 @@ async fn test_public_client_dpop_delete_session() { let did = "did:plc:publicdelete"; let access_token = "public-delete-token"; + app.mock_session_verification(did, did).await; + let session_req = post_json_with_headers( "/oauth/sessions", &json!({ diff --git a/tests/e2e_delegation.rs b/tests/e2e_delegation.rs index 2326e77..98424c1 100644 --- a/tests/e2e_delegation.rs +++ b/tests/e2e_delegation.rs @@ -64,6 +64,7 @@ async fn setup_dpop_session(app: &common::app::TestApp, user_did: &str) -> (Stri let dpop_key = key_body["dpop_key"].clone(); let access_token = format!("test-access-{}", uuid::Uuid::new_v4()); + app.mock_session_verification(user_did, user_did).await; let session_req = post_json_with_headers( "/oauth/sessions", &json!({ @@ -163,6 +164,7 @@ async fn register_target_session( let provision_id = key_body["provision_id"].as_str().unwrap().to_string(); let access_token = format!("test-target-access-{}", uuid::Uuid::new_v4()); + app.mock_session_verification(target_did, target_did).await; let session_req = post_json_with_headers( "/oauth/sessions", &json!({ @@ -211,6 +213,7 @@ async fn setup_linked_account( let dpop_key = key_body["dpop_key"].clone(); let access_token = format!("test-owner-access-{}", uuid::Uuid::new_v4()); + app.mock_session_verification(owner_did, owner_did).await; let session_req = post_json_with_headers( "/oauth/sessions", &json!({ @@ -269,6 +272,7 @@ async fn setup_session_for_client( let dpop_key = key_body["dpop_key"].clone(); let access_token = format!("test-access-{}", uuid::Uuid::new_v4()); + app.mock_session_verification(user_did, user_did).await; let session_req = post_json_with_headers( "/oauth/sessions", &json!({