Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move out check credProtectPolicy logic #516

Merged
merged 4 commits into from
Jul 23, 2022
Merged

Move out check credProtectPolicy logic #516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
148410c
Move out check credProtectPolicy logic
hcyang-google Jul 21, 2022
572df0c
Remove extra comma
hcyang-google Jul 21, 2022
56e678a
Merge branch 'develop' into cred_protect
hcyang-google Jul 22, 2022
88b8a32
Rename check_cred_protect
hcyang-google Jul 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 11 additions & 40 deletions src/ctap/credential_id.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ pub fn decrypt_credential_id(
env: &mut impl Env,
credential_id: Vec<u8>,
rp_id_hash: &[u8],
check_cred_protect: bool,
) -> Result<Option<PublicKeyCredentialSource>, Ctap2StatusCode> {
if credential_id.len() < MIN_CREDENTIAL_ID_SIZE {
return Ok(None);
Expand Down Expand Up @@ -240,9 +239,7 @@ pub fn decrypt_credential_id(
return Ok(None);
};

let is_protected = credential_source.cred_protect_policy
== Some(CredentialProtectionPolicy::UserVerificationRequired);
if rp_id_hash != credential_source.rp_id_hash || (check_cred_protect && is_protected) {
if rp_id_hash != credential_source.rp_id_hash {
return Ok(None);
}

Expand Down Expand Up @@ -279,7 +276,7 @@ mod test {
let rp_id_hash = [0x55; 32];
let encrypted_id =
encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None).unwrap();
let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash, false)
let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash)
.unwrap()
.unwrap();

Expand Down Expand Up @@ -313,7 +310,7 @@ mod test {
encrypted_id.extend(&id_hmac);

assert_eq!(
decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash, false),
decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash),
Ok(None)
);
}
Expand All @@ -329,7 +326,7 @@ mod test {
let mut modified_id = encrypted_id.clone();
modified_id[i] ^= 0x01;
assert_eq!(
decrypt_credential_id(&mut env, modified_id, &rp_id_hash, false),
decrypt_credential_id(&mut env, modified_id, &rp_id_hash),
Ok(None)
);
}
Expand All @@ -356,12 +353,7 @@ mod test {

for length in (1..CBOR_CREDENTIAL_ID_SIZE).step_by(16) {
assert_eq!(
decrypt_credential_id(
&mut env,
encrypted_id[..length].to_vec(),
&rp_id_hash,
false
),
decrypt_credential_id(&mut env, encrypted_id[..length].to_vec(), &rp_id_hash),
Ok(None)
);
}
Expand Down Expand Up @@ -408,13 +400,13 @@ mod test {
let rp_id_hash = [0x55; 32];
let encrypted_id =
legacy_encrypt_to_credential_id(&mut env, ecdsa_key, &rp_id_hash).unwrap();
// When checking credProtect for legacy credentials the check will always pass because we didn't persist credProtect
// policy info in it.
let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash, true)
let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash)
.unwrap()
.unwrap();

assert_eq!(private_key, decrypted_source.private_key);
// Legacy credentials didn't persist credProtectPolicy info, so it should be treated as None.
assert!(decrypted_source.cred_protect_policy.is_none());
}

#[test]
Expand All @@ -429,7 +421,7 @@ mod test {
}

#[test]
fn test_check_cred_protect_fail() {
fn test_cred_protect_persisted() {
let mut env = TestEnv::new();
let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);

Expand All @@ -442,34 +434,13 @@ mod test {
)
.unwrap();

assert_eq!(
decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash, true),
Ok(None)
);
}

#[test]
fn test_check_cred_protect_success() {
let mut env = TestEnv::new();
let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);

let rp_id_hash = [0x55; 32];
let encrypted_id = encrypt_to_credential_id(
&mut env,
&private_key,
&rp_id_hash,
Some(CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList),
)
.unwrap();

let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash, true)
let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash)
.unwrap()
.unwrap();

assert_eq!(decrypted_source.private_key, private_key);
assert_eq!(
decrypted_source.cred_protect_policy,
Some(CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList)
Some(CredentialProtectionPolicy::UserVerificationRequired)
);
}
}
7 changes: 3 additions & 4 deletions src/ctap/credential_management.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -872,10 +872,9 @@ mod test {
Ok(ResponseData::AuthenticatorCredentialManagement(None))
);

let updated_credential =
storage::find_credential(&mut env, "example.com", &[0x1D; 32], false)
.unwrap()
.unwrap();
let updated_credential = storage::find_credential(&mut env, "example.com", &[0x1D; 32])
.unwrap()
.unwrap();
assert_eq!(updated_credential.user_handle, vec![0x01]);
assert_eq!(&updated_credential.user_name.unwrap(), "new_name");
assert_eq!(
Expand Down
3 changes: 1 addition & 2 deletions src/ctap/ctap1.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ impl Ctap1Command {
flags: Ctap1Flags,
ctap_state: &mut CtapState,
) -> Result<Vec<u8>, Ctap1StatusCode> {
let credential_source = decrypt_credential_id(env, key_handle, &application, false)
let credential_source = decrypt_credential_id(env, key_handle, &application)
.map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?;
if let Some(credential_source) = credential_source {
let ecdsa_key = credential_source
Expand Down Expand Up @@ -440,7 +440,6 @@ mod test {
&mut env,
response[67..67 + CBOR_CREDENTIAL_ID_SIZE].to_vec(),
&application,
false
)
.unwrap()
.is_some());
Expand Down
184 changes: 175 additions & 9 deletions src/ctap/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,22 @@ impl CtapState {
Ok(())
}

fn check_cred_protect_for_listed_credential(
&mut self,
credential: &Option<PublicKeyCredentialSource>,
has_uv: bool,
) -> bool {
if let Some(credential) = credential {
has_uv
|| !matches!(
credential.cred_protect_policy,
Some(CredentialProtectionPolicy::UserVerificationRequired),
)
} else {
false
}
}

fn process_make_credential(
&mut self,
env: &mut impl Env,
Expand Down Expand Up @@ -809,9 +825,13 @@ impl CtapState {
let rp_id_hash = Sha256::hash(rp_id.as_bytes());
if let Some(exclude_list) = exclude_list {
for cred_desc in exclude_list {
if storage::find_credential(env, &rp_id, &cred_desc.key_id, !has_uv)?.is_some()
|| decrypt_credential_id(env, cred_desc.key_id, &rp_id_hash, !has_uv)?.is_some()
{
if self.check_cred_protect_for_listed_credential(
&storage::find_credential(env, &rp_id, &cred_desc.key_id)?,
has_uv,
) || self.check_cred_protect_for_listed_credential(
&decrypt_credential_id(env, cred_desc.key_id, &rp_id_hash)?,
has_uv,
) {
// Perform this check, so bad actors can't brute force exclude_list
// without user interaction.
let _ = check_user_presence(env, channel);
Expand Down Expand Up @@ -1078,14 +1098,12 @@ impl CtapState {
has_uv: bool,
) -> Result<Option<PublicKeyCredentialSource>, Ctap2StatusCode> {
for allowed_credential in allow_list {
let credential =
storage::find_credential(env, rp_id, &allowed_credential.key_id, !has_uv)?;
if credential.is_some() {
let credential = storage::find_credential(env, rp_id, &allowed_credential.key_id)?;
if self.check_cred_protect_for_listed_credential(&credential, has_uv) {
return Ok(credential);
}
let credential =
decrypt_credential_id(env, allowed_credential.key_id, rp_id_hash, !has_uv)?;
if credential.is_some() {
let credential = decrypt_credential_id(env, allowed_credential.key_id, rp_id_hash)?;
if self.check_cred_protect_for_listed_credential(&credential, has_uv) {
return Ok(credential);
}
}
Expand Down Expand Up @@ -1802,6 +1820,61 @@ mod test {
assert!(make_credential_response.is_ok());
}

#[test]
fn test_non_resident_process_make_credential_credential_with_cred_protect() {
let mut env = TestEnv::new();
let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));

let test_policy = CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList;
let mut make_credential_params =
create_make_credential_parameters_with_cred_protect_policy(test_policy);
make_credential_params.options.rk = false;
let make_credential_response =
ctap_state.process_make_credential(&mut env, make_credential_params, DUMMY_CHANNEL);
assert!(make_credential_response.is_ok());
let credential_id = match make_credential_response.unwrap() {
ResponseData::AuthenticatorMakeCredential(make_credential_response) => {
let auth_data = make_credential_response.auth_data;
let offset = 37 + storage::aaguid(&mut env).unwrap().len();
assert_eq!(auth_data[offset], 0x00);
assert_eq!(auth_data[offset + 1] as usize, CBOR_CREDENTIAL_ID_SIZE);
auth_data[offset + 2..offset + 2 + CBOR_CREDENTIAL_ID_SIZE].to_vec()
}
_ => panic!("Invalid response type"),
};
let make_credential_params =
create_make_credential_parameters_with_exclude_list(&credential_id);
let make_credential_response =
ctap_state.process_make_credential(&mut env, make_credential_params, DUMMY_CHANNEL);
assert_eq!(
make_credential_response,
Err(Ctap2StatusCode::CTAP2_ERR_CREDENTIAL_EXCLUDED)
);

let test_policy = CredentialProtectionPolicy::UserVerificationRequired;
let mut make_credential_params =
create_make_credential_parameters_with_cred_protect_policy(test_policy);
make_credential_params.options.rk = false;
let make_credential_response =
ctap_state.process_make_credential(&mut env, make_credential_params, DUMMY_CHANNEL);
assert!(make_credential_response.is_ok());
let credential_id = match make_credential_response.unwrap() {
ResponseData::AuthenticatorMakeCredential(make_credential_response) => {
let auth_data = make_credential_response.auth_data;
let offset = 37 + storage::aaguid(&mut env).unwrap().len();
assert_eq!(auth_data[offset], 0x00);
assert_eq!(auth_data[offset + 1] as usize, CBOR_CREDENTIAL_ID_SIZE);
auth_data[offset + 2..offset + 2 + CBOR_CREDENTIAL_ID_SIZE].to_vec()
}
_ => panic!("Invalid response type"),
};
let make_credential_params =
create_make_credential_parameters_with_exclude_list(&credential_id);
let make_credential_response =
ctap_state.process_make_credential(&mut env, make_credential_params, DUMMY_CHANNEL);
assert!(make_credential_response.is_ok());
}

#[test]
fn test_process_make_credential_hmac_secret() {
let mut env = TestEnv::new();
Expand Down Expand Up @@ -2650,6 +2723,99 @@ mod test {
);
}

#[test]
fn test_non_resident_process_get_assertion_with_cred_protect() {
let mut env = TestEnv::new();
let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));

let test_policy = CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList;
let mut make_credential_params =
create_make_credential_parameters_with_cred_protect_policy(test_policy);
make_credential_params.options.rk = false;
let make_credential_response =
ctap_state.process_make_credential(&mut env, make_credential_params, DUMMY_CHANNEL);
assert!(make_credential_response.is_ok());
let credential_id = match make_credential_response.unwrap() {
ResponseData::AuthenticatorMakeCredential(make_credential_response) => {
let auth_data = make_credential_response.auth_data;
let offset = 37 + storage::aaguid(&mut env).unwrap().len();
assert_eq!(auth_data[offset], 0x00);
assert_eq!(auth_data[offset + 1] as usize, CBOR_CREDENTIAL_ID_SIZE);
auth_data[offset + 2..offset + 2 + CBOR_CREDENTIAL_ID_SIZE].to_vec()
}
_ => panic!("Invalid response type"),
};
let cred_desc = PublicKeyCredentialDescriptor {
key_type: PublicKeyCredentialType::PublicKey,
key_id: credential_id,
transports: None,
};
let get_assertion_params = AuthenticatorGetAssertionParameters {
rp_id: String::from("example.com"),
client_data_hash: vec![0xCD],
allow_list: Some(vec![cred_desc]),
extensions: GetAssertionExtensions::default(),
options: GetAssertionOptions {
up: false,
uv: false,
},
pin_uv_auth_param: None,
pin_uv_auth_protocol: None,
};
let get_assertion_response = ctap_state.process_get_assertion(
&mut env,
get_assertion_params,
DUMMY_CHANNEL,
CtapInstant::new(0),
);
assert!(get_assertion_response.is_ok());

let test_policy = CredentialProtectionPolicy::UserVerificationRequired;
let mut make_credential_params =
create_make_credential_parameters_with_cred_protect_policy(test_policy);
make_credential_params.options.rk = false;
let make_credential_response =
ctap_state.process_make_credential(&mut env, make_credential_params, DUMMY_CHANNEL);
assert!(make_credential_response.is_ok());
let credential_id = match make_credential_response.unwrap() {
ResponseData::AuthenticatorMakeCredential(make_credential_response) => {
let auth_data = make_credential_response.auth_data;
let offset = 37 + storage::aaguid(&mut env).unwrap().len();
assert_eq!(auth_data[offset], 0x00);
assert_eq!(auth_data[offset + 1] as usize, CBOR_CREDENTIAL_ID_SIZE);
auth_data[offset + 2..offset + 2 + CBOR_CREDENTIAL_ID_SIZE].to_vec()
}
_ => panic!("Invalid response type"),
};
let cred_desc = PublicKeyCredentialDescriptor {
key_type: PublicKeyCredentialType::PublicKey,
key_id: credential_id,
transports: None,
};
let get_assertion_params = AuthenticatorGetAssertionParameters {
rp_id: String::from("example.com"),
client_data_hash: vec![0xCD],
allow_list: Some(vec![cred_desc]),
extensions: GetAssertionExtensions::default(),
options: GetAssertionOptions {
up: false,
uv: false,
},
pin_uv_auth_param: None,
pin_uv_auth_protocol: None,
};
let get_assertion_response = ctap_state.process_get_assertion(
&mut env,
get_assertion_params,
DUMMY_CHANNEL,
CtapInstant::new(0),
);
assert_eq!(
get_assertion_response,
Err(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS),
);
}

#[test]
fn test_process_get_assertion_with_cred_blob() {
let mut env = TestEnv::new();
Expand Down
Loading