An easy-to-host PDS on the ATProtocol, iPhone and MacOS. Maintain control of your keys and data, always.
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

fix(relay): auth middleware round 3 — RFC compliance and test gaps

RFC compliance:
- Reject requests with multiple DPoP headers (RFC 9449 §11.1): a
header-prepending proxy could inject a forged proof as the first value
- Case-insensitive Bearer scheme matching (RFC 7235 §2.1): use
eq_ignore_ascii_case so "bearer"/"BEARER" are accepted

Test gaps:
- Fix dpop_header_present_but_access_token_has_no_cnf: was using
"dummy.dpop.value" which failed at base64 decode; now uses a valid
DPoP proof so the "access token missing DPoP binding" branch is
actually exercised
- Add dpop_cnf_present_without_jkt_returns_401 covering the cnf:{}
guard added in round 2
- Add dpop_iat_at_i64_min_returns_401: the specific value motivating
the i128 widening fix; confirms no panic in debug or bypass in release
- Add multiple_dpop_headers_returns_401 covering the new §11.1 guard

authored by

Malpercio and committed by
Tangled
2c693a63 8f5863d3

+110 -6
+110 -6
crates/relay/src/auth/mod.rs
··· 112 112 let token_str = extract_bearer_token(&parts.headers)?; 113 113 114 114 // 2. Detect the DPoP header before decoding the access token. 115 + // RFC 9449 §11.1: reject if multiple DPoP headers are present — a 116 + // header-prepending proxy could inject a forged proof as the first value. 117 + if parts.headers.get_all("DPoP").iter().count() > 1 { 118 + return Err(ApiError::new( 119 + ErrorCode::InvalidToken, 120 + "multiple DPoP headers are not permitted", 121 + )); 122 + } 115 123 let dpop_value = parts 116 124 .headers 117 125 .get("DPoP") ··· 203 211 ) 204 212 })?; 205 213 206 - auth_value.strip_prefix("Bearer ").ok_or_else(|| { 207 - ApiError::new( 214 + // RFC 7235 §2.1: auth scheme names are case-insensitive ("bearer", "BEARER", etc.). 215 + const BEARER_LEN: usize = 7; // "Bearer ".len() — scheme name + single SP 216 + if !auth_value 217 + .get(..BEARER_LEN) 218 + .map_or(false, |s| s.eq_ignore_ascii_case("Bearer ")) 219 + { 220 + return Err(ApiError::new( 208 221 ErrorCode::AuthenticationRequired, 209 222 "Authorization header must use Bearer scheme", 210 - ) 211 - }) 223 + )); 224 + } 225 + Ok(&auth_value[BEARER_LEN..]) 212 226 } 213 227 214 228 /// Decode and verify the HS256 access/refresh JWT issued by this server. ··· 755 769 756 770 #[tokio::test] 757 771 async fn dpop_header_present_but_access_token_has_no_cnf_returns_401() { 772 + // Access token has no cnf claim at all. DPoP header is present with a valid 773 + // proof — must be rejected at the "access token missing DPoP binding" guard, 774 + // not earlier (the old test used "dummy.dpop.value" which failed at base64 775 + // decode and never reached the binding check). 758 776 let state = test_state().await; 759 - // Access token has no cnf claim. 777 + let dpop_key = SigningKey::random(&mut OsRng); 760 778 let token = mint_token("did:plc:user", "com.atproto.access", 3600, &state.jwt_secret, None); 779 + let dpop_proof = make_dpop_proof( 780 + &dpop_key, 781 + "GET", 782 + &format!("{}/protected", state.config.public_url), 783 + ); 761 784 let req = Request::builder() 762 785 .uri("/protected") 763 786 .header("Authorization", format!("Bearer {token}")) 764 - .header("DPoP", "dummy.dpop.value") 787 + .header("DPoP", dpop_proof) 765 788 .body(Body::empty()) 766 789 .unwrap(); 767 790 let resp = protected_app(state).oneshot(req).await.unwrap(); 791 + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); 792 + let json = json_body(resp).await; 793 + assert_eq!(json["error"]["code"], "INVALID_TOKEN"); 794 + } 795 + 796 + #[tokio::test] 797 + async fn dpop_cnf_present_without_jkt_returns_401() { 798 + // Token with cnf:{} — cnf present but no jkt field. Must be rejected before 799 + // any DPoP proof is considered (guard added in round 2). 800 + let state = test_state().await; 801 + let token = mint_token( 802 + "did:plc:user", 803 + "com.atproto.access", 804 + 3600, 805 + &state.jwt_secret, 806 + Some(serde_json::json!({})), // cnf present but empty — no jkt 807 + ); 808 + let resp = get_protected(protected_app(state), Some(&token)).await; 768 809 assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); 769 810 let json = json_body(resp).await; 770 811 assert_eq!(json["error"]["code"], "INVALID_TOKEN"); ··· 1033 1074 .uri("/protected") 1034 1075 .header("Authorization", format!("Bearer {token}")) 1035 1076 .header("DPoP", dpop_proof) 1077 + .body(Body::empty()) 1078 + .unwrap(); 1079 + let resp = protected_app(state).oneshot(req).await.unwrap(); 1080 + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); 1081 + let json = json_body(resp).await; 1082 + assert_eq!(json["error"]["code"], "INVALID_TOKEN"); 1083 + } 1084 + 1085 + #[tokio::test] 1086 + async fn dpop_iat_at_i64_min_returns_401() { 1087 + // i64::MIN is the specific iat value that motivated the i128 widening fix: 1088 + // `now - i64::MIN` overflows in debug (panic → DoS) and wraps in release 1089 + // (magnitude check bypass). The widened arithmetic must reject it as stale. 1090 + let state = test_state().await; 1091 + let dpop_key = SigningKey::random(&mut OsRng); 1092 + let thumbprint = dpop_key_thumbprint(&dpop_key); 1093 + let token = mint_token( 1094 + "did:plc:alice", 1095 + "com.atproto.access", 1096 + 3600, 1097 + &state.jwt_secret, 1098 + Some(serde_json::json!({ "jkt": thumbprint })), 1099 + ); 1100 + let dpop_proof = make_dpop_proof_with_iat( 1101 + &dpop_key, 1102 + "GET", 1103 + &format!("{}/protected", state.config.public_url), 1104 + i64::MIN, 1105 + ); 1106 + let req = Request::builder() 1107 + .uri("/protected") 1108 + .header("Authorization", format!("Bearer {token}")) 1109 + .header("DPoP", dpop_proof) 1110 + .body(Body::empty()) 1111 + .unwrap(); 1112 + let resp = protected_app(state).oneshot(req).await.unwrap(); 1113 + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); 1114 + let json = json_body(resp).await; 1115 + assert_eq!(json["error"]["code"], "INVALID_TOKEN"); 1116 + } 1117 + 1118 + #[tokio::test] 1119 + async fn multiple_dpop_headers_returns_401() { 1120 + // RFC 9449 §11.1: multiple DPoP headers must be rejected. 1121 + // A header-prepending proxy could inject a forged proof as the first value. 1122 + let state = test_state().await; 1123 + let dpop_key = SigningKey::random(&mut OsRng); 1124 + let thumbprint = dpop_key_thumbprint(&dpop_key); 1125 + let token = mint_token( 1126 + "did:plc:alice", 1127 + "com.atproto.access", 1128 + 3600, 1129 + &state.jwt_secret, 1130 + Some(serde_json::json!({ "jkt": thumbprint })), 1131 + ); 1132 + let htu = format!("{}/protected", state.config.public_url); 1133 + let proof1 = make_dpop_proof(&dpop_key, "GET", &htu); 1134 + let proof2 = make_dpop_proof(&dpop_key, "GET", &htu); 1135 + let req = Request::builder() 1136 + .uri("/protected") 1137 + .header("Authorization", format!("Bearer {token}")) 1138 + .header("DPoP", proof1) 1139 + .header("DPoP", proof2) 1036 1140 .body(Body::empty()) 1037 1141 .unwrap(); 1038 1142 let resp = protected_app(state).oneshot(req).await.unwrap();