Skip to content

Commit 537aca1

Browse files
fix(webauthn): bound and validate getNextAssertion iteration (#290)
The multi-assertion loop trusted the device-reported credential count with no limit, so a misbehaving device could drive unbounded iteration. The loop is now bounded and each returned assertion is checked for consistency with the request. This hardens the client against a hostile authenticator.
1 parent e050904 commit 537aca1

2 files changed

Lines changed: 103 additions & 3 deletions

File tree

libwebauthn/src/ops/webauthn/large_blob.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,13 @@ pub(crate) async fn delete_authenticator_large_blob<C: Ctap2 + ?Sized>(
697697
mod tests {
698698
use super::*;
699699

700+
fn rp_id_hash(rp_id: &str) -> Vec<u8> {
701+
use sha2::{Digest, Sha256};
702+
let mut hasher = Sha256::default();
703+
hasher.update(rp_id.as_bytes());
704+
hasher.finalize().to_vec()
705+
}
706+
700707
fn raw_entry(bytes: &[u8]) -> RawArrayEntry {
701708
RawArrayEntry {
702709
raw: bytes.to_vec(),
@@ -955,6 +962,7 @@ mod tests {
955962

956963
let credential_id = b"cred-id".to_vec();
957964
let mut auth_data = vec![0u8; 37];
965+
auth_data[..32].copy_from_slice(&rp_id_hash("example.com"));
958966
auth_data[32] = 0x01; // USER_PRESENT flag
959967
let mut cred_id_map = BTreeMap::new();
960968
cred_id_map.insert(Value::Text("type".into()), Value::Text("public-key".into()));
@@ -1091,6 +1099,7 @@ mod tests {
10911099

10921100
let assertion_cbor = |cred: &[u8], lbk: &[u8; 32], count: Option<i128>| {
10931101
let mut auth_data = vec![0u8; 37];
1102+
auth_data[..32].copy_from_slice(&rp_id_hash("example.com"));
10941103
auth_data[32] = 0x01; // USER_PRESENT
10951104
let mut cred_map = BTreeMap::new();
10961105
cred_map.insert(Value::Text("type".into()), Value::Text("public-key".into()));

libwebauthn/src/webauthn.rs

Lines changed: 94 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -402,12 +402,33 @@ async fn get_assertion_fido2<C: Channel>(
402402
op.timeout
403403
)
404404
}?;
405-
let count = response.credentials_count.unwrap_or(1);
405+
let expected_rp_id_hash = {
406+
use sha2::{Digest, Sha256};
407+
let mut hasher = Sha256::default();
408+
hasher.update(op.relying_party_id.as_bytes());
409+
hasher.finalize()
410+
};
411+
let validate_rp_id_hash = |resp: &Ctap2GetAssertionResponse| -> Result<(), Error> {
412+
if resp.authenticator_data.rp_id_hash.as_slice() != expected_rp_id_hash.as_slice() {
413+
warn!("getAssertion rpIdHash does not match the requested RP ID");
414+
return Err(Error::Platform(PlatformError::InvalidDeviceResponse));
415+
}
416+
Ok(())
417+
};
418+
419+
validate_rp_id_hash(&response)?;
420+
// Cap iteration so a hostile numberOfCredentials cannot force an unbounded loop.
421+
let max_count = get_info_response
422+
.max_credential_count_in_list()
423+
.unwrap_or(255);
424+
let count = (response.credentials_count.unwrap_or(1) as usize).min(max_count);
406425
let mut ctap_responses = vec![response];
407426
for i in 1..count {
408427
debug!({ i }, "Fetching additional credential");
409428
// GetNextAssertion doesn't use PinUVAuthToken, so we don't need to check uv_auth_used here
410-
ctap_responses.push(channel.ctap2_get_next_assertion(op.timeout).await?);
429+
let next = channel.ctap2_get_next_assertion(op.timeout).await?;
430+
validate_rp_id_hash(&next)?;
431+
ctap_responses.push(next);
411432
}
412433

413434
// largeBlob extension (WebAuthn L3 §10.1.5):
@@ -846,18 +867,36 @@ mod tests {
846867
}
847868
}
848869

849-
fn get_assertion_response() -> CborResponse {
870+
fn rp_id_hash(rp_id: &str) -> Vec<u8> {
871+
use sha2::{Digest, Sha256};
872+
let mut hasher = Sha256::default();
873+
hasher.update(rp_id.as_bytes());
874+
hasher.finalize().to_vec()
875+
}
876+
877+
fn get_assertion_response_with(
878+
rp_hash: &[u8],
879+
credentials_count: Option<u32>,
880+
) -> CborResponse {
850881
let mut auth_data = vec![0u8; 37];
882+
auth_data[..32].copy_from_slice(rp_hash);
851883
auth_data[32] = AuthenticatorDataFlags::USER_PRESENT.bits();
852884
let mut map: BTreeMap<u64, Value> = BTreeMap::new();
853885
map.insert(0x02, Value::Bytes(auth_data));
854886
map.insert(0x03, Value::Bytes(vec![0xAAu8; 64]));
887+
if let Some(count) = credentials_count {
888+
map.insert(0x05, Value::Integer(count as i128));
889+
}
855890
CborResponse {
856891
status_code: CtapError::Ok,
857892
data: Some(cbor::to_vec(&map).unwrap()),
858893
}
859894
}
860895

896+
fn get_assertion_response() -> CborResponse {
897+
get_assertion_response_with(&rp_id_hash("example.com"), None)
898+
}
899+
861900
fn assertion_request(
862901
allow: Vec<Ctap2PublicKeyCredentialDescriptor>,
863902
) -> GetAssertionRequest {
@@ -979,5 +1018,57 @@ mod tests {
9791018
let request = ctap2_get_assertion(vec![descriptor(&[0u8; 16])]);
9801019
assert_eq!(enforce_get_assertion_limits(&request, &info), Ok(()));
9811020
}
1021+
1022+
#[tokio::test]
1023+
async fn get_next_assertion_iteration_is_bounded() {
1024+
let info = Ctap2GetInfoResponse {
1025+
max_credential_count: Some(2),
1026+
..Default::default()
1027+
};
1028+
let op = assertion_request(vec![]);
1029+
let expected_request =
1030+
Ctap2GetAssertionRequest::from_webauthn_request(&op, &info).unwrap();
1031+
let expected_cbor: CborRequest = (&expected_request).try_into().unwrap();
1032+
1033+
let hash = rp_id_hash("example.com");
1034+
let mut channel = NoPreflightChannel::new();
1035+
let get_info_request = CborRequest::new(Ctap2CommandCode::AuthenticatorGetInfo);
1036+
channel.push_command_pair(get_info_request.clone(), get_info_response(&info));
1037+
channel.push_command_pair(get_info_request, get_info_response(&info));
1038+
channel.push_command_pair(
1039+
expected_cbor,
1040+
get_assertion_response_with(&hash, Some(u32::MAX)),
1041+
);
1042+
// Only one getNextAssertion is queued, so unbounded iteration would panic.
1043+
let next_request = CborRequest::new(Ctap2CommandCode::AuthenticatorGetNextAssertion);
1044+
channel.push_command_pair(next_request, get_assertion_response_with(&hash, None));
1045+
1046+
let result = get_assertion_fido2(&mut channel, &op).await;
1047+
assert!(result.is_ok(), "bounded iteration failed: {result:?}");
1048+
}
1049+
1050+
#[tokio::test]
1051+
async fn mismatched_rp_id_hash_is_rejected() {
1052+
let info = Ctap2GetInfoResponse::default();
1053+
let op = assertion_request(vec![]);
1054+
let expected_request =
1055+
Ctap2GetAssertionRequest::from_webauthn_request(&op, &info).unwrap();
1056+
let expected_cbor: CborRequest = (&expected_request).try_into().unwrap();
1057+
1058+
let mut channel = NoPreflightChannel::new();
1059+
let get_info_request = CborRequest::new(Ctap2CommandCode::AuthenticatorGetInfo);
1060+
channel.push_command_pair(get_info_request.clone(), get_info_response(&info));
1061+
channel.push_command_pair(get_info_request, get_info_response(&info));
1062+
channel.push_command_pair(
1063+
expected_cbor,
1064+
get_assertion_response_with(&[0xFFu8; 32], None),
1065+
);
1066+
1067+
let result = get_assertion_fido2(&mut channel, &op).await;
1068+
assert_eq!(
1069+
result.err(),
1070+
Some(Error::Platform(PlatformError::InvalidDeviceResponse))
1071+
);
1072+
}
9821073
}
9831074
}

0 commit comments

Comments
 (0)