From 77d1b63284b8518b3e6b0685e3abe9acbec17f74 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Fri, 6 Nov 2020 17:30:59 +0100 Subject: [PATCH 01/28] adds a UP check where 2.1 is asking for it --- src/ctap/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 442d942..3ec7c6a 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -342,6 +342,7 @@ where if let Some(auth_param) = &pin_uv_auth_param { // This case was added in FIDO 2.1. if auth_param.is_empty() { + let _ = (self.check_user_presence)(cid); if self.persistent_store.pin_hash()?.is_none() { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_NOT_SET); } else { @@ -545,6 +546,7 @@ where if let Some(auth_param) = &pin_uv_auth_param { // This case was added in FIDO 2.1. if auth_param.is_empty() { + let _ = (self.check_user_presence)(cid); if self.persistent_store.pin_hash()?.is_none() { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_NOT_SET); } else { From 51681e49100887dc4b6d46ac86db8426d0df73ef Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Fri, 13 Nov 2020 06:51:53 +0100 Subject: [PATCH 02/28] changes operation touch behaviour --- src/ctap/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index b3bbd64..06e3a57 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -345,7 +345,7 @@ where if let Some(auth_param) = &pin_uv_auth_param { // This case was added in FIDO 2.1. if auth_param.is_empty() { - let _ = (self.check_user_presence)(cid); + (self.check_user_presence)(cid)?; if self.persistent_store.pin_hash()?.is_none() { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_NOT_SET); } else { @@ -549,7 +549,7 @@ where if let Some(auth_param) = &pin_uv_auth_param { // This case was added in FIDO 2.1. if auth_param.is_empty() { - let _ = (self.check_user_presence)(cid); + (self.check_user_presence)(cid)?; if self.persistent_store.pin_hash()?.is_none() { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_NOT_SET); } else { From 315016f55265621cc7448c31817b41e66b694e49 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Fri, 20 Nov 2020 02:50:47 +0100 Subject: [PATCH 03/28] unwraps credentials in the exclude list --- src/ctap/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 1a98ce5..66ef234 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -392,12 +392,16 @@ where let has_extension_output = use_hmac_extension || cred_protect_policy.is_some(); let rp_id = rp.rp_id; + let rp_id_hash = Sha256::hash(rp_id.as_bytes()); if let Some(exclude_list) = exclude_list { for cred_desc in exclude_list { if self .persistent_store .find_credential(&rp_id, &cred_desc.key_id, pin_uv_auth_param.is_none())? .is_some() + || self + .decrypt_credential_source(cred_desc.key_id, &rp_id_hash)? + .is_some() { // Perform this check, so bad actors can't brute force exclude_list // without user interaction. @@ -446,7 +450,6 @@ where let sk = crypto::ecdsa::SecKey::gensk(self.rng); let pk = sk.genpk(); - let rp_id_hash = Sha256::hash(rp_id.as_bytes()); let credential_id = if options.rk { let random_id = self.rng.gen_uniform_u8x32().to_vec(); let credential_source = PublicKeyCredentialSource { From e1b419c104903cded5310f9313e5191675aa3092 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Fri, 20 Nov 2020 05:49:11 +0100 Subject: [PATCH 04/28] changes sync response and tests it --- src/ctap/hid/mod.rs | 117 ++++++++++++++++++++++++++++------------ src/ctap/hid/receive.rs | 28 ++++++++++ 2 files changed, 112 insertions(+), 33 deletions(-) diff --git a/src/ctap/hid/mod.rs b/src/ctap/hid/mod.rs index 7489f6e..3a96699 100644 --- a/src/ctap/hid/mod.rs +++ b/src/ctap/hid/mod.rs @@ -227,44 +227,35 @@ impl CtapHid { } // CTAP specification (version 20190130) section 8.1.9.1.3 CtapHid::COMMAND_INIT => { - if cid == CtapHid::CHANNEL_BROADCAST { - if message.payload.len() != 8 { - return CtapHid::error_message(cid, CtapHid::ERR_INVALID_LEN); - } + if message.payload.len() != 8 { + return CtapHid::error_message(cid, CtapHid::ERR_INVALID_LEN); + } + let new_cid = if cid == CtapHid::CHANNEL_BROADCAST { // TODO: Prevent allocating 2^32 channels. self.allocated_cids += 1; - let allocated_cid = (self.allocated_cids as u32).to_ne_bytes(); - - let mut payload = vec![0; 17]; - payload[..8].copy_from_slice(&message.payload); - payload[8..12].copy_from_slice(&allocated_cid); - payload[12] = CtapHid::PROTOCOL_VERSION; - payload[13] = CtapHid::DEVICE_VERSION_MAJOR; - payload[14] = CtapHid::DEVICE_VERSION_MINOR; - payload[15] = CtapHid::DEVICE_VERSION_BUILD; - payload[16] = CtapHid::CAPABILITIES; - - // This unwrap is safe because the payload length is 17 <= 7609 bytes. - CtapHid::split_message(Message { - cid, - cmd: CtapHid::COMMAND_INIT, - payload, - }) - .unwrap() + (self.allocated_cids as u32).to_ne_bytes() } else { // Sync the channel and discard the current transaction. - // TODO: The specification (version 20190130) wording isn't clear about - // the payload format in this case. - // - // This unwrap is safe because the payload length is 0 <= 7609 bytes. - CtapHid::split_message(Message { - cid, - cmd: CtapHid::COMMAND_INIT, - payload: vec![], - }) - .unwrap() - } + cid + }; + + let mut payload = vec![0; 17]; + payload[..8].copy_from_slice(&message.payload); + payload[8..12].copy_from_slice(&new_cid); + payload[12] = CtapHid::PROTOCOL_VERSION; + payload[13] = CtapHid::DEVICE_VERSION_MAJOR; + payload[14] = CtapHid::DEVICE_VERSION_MINOR; + payload[15] = CtapHid::DEVICE_VERSION_BUILD; + payload[16] = CtapHid::CAPABILITIES; + + // This unwrap is safe because the payload length is 17 <= 7609 bytes. + CtapHid::split_message(Message { + cid, + cmd: CtapHid::COMMAND_INIT, + payload, + }) + .unwrap() } // CTAP specification (version 20190130) section 8.1.9.1.4 CtapHid::COMMAND_PING => { @@ -568,6 +559,66 @@ mod test { ); } + #[test] + fn test_command_init_for_sync() { + let mut rng = ThreadRng256 {}; + let user_immediately_present = |_| Ok(()); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_hid = CtapHid::new(); + let cid = cid_from_init(&mut ctap_hid, &mut ctap_state); + + // Ping packet with a length longer than one packet. + let mut packet1 = [0x51; 64]; + packet1[..4].copy_from_slice(&cid); + packet1[4..7].copy_from_slice(&[0x81, 0x02, 0x00]); + // Init packet on the same channel. + let mut packet2 = [0x00; 64]; + packet2[..4].copy_from_slice(&cid); + packet2[4..15].copy_from_slice(&[ + 0x86, 0x00, 0x08, 0x12, 0x34, 0x56, 0x78, 0x9A, 0xBC, 0xDE, 0xF0, + ]); + let mut result = Vec::new(); + let mut assembler_reply = MessageAssembler::new(); + for pkt_request in &[packet1, packet2] { + for pkt_reply in + ctap_hid.process_hid_packet(&pkt_request, DUMMY_CLOCK_VALUE, &mut ctap_state) + { + if let Some(message) = assembler_reply + .parse_packet(&pkt_reply, DUMMY_TIMESTAMP) + .unwrap() + { + result.push(message); + } + } + } + assert_eq!( + result, + vec![Message { + cid, + cmd: CtapHid::COMMAND_INIT, + payload: vec![ + 0x12, // Nonce + 0x34, + 0x56, + 0x78, + 0x9A, + 0xBC, + 0xDE, + 0xF0, + cid[0], // Allocated CID + cid[1], + cid[2], + cid[3], + 0x02, // Protocol version + 0x00, // Device version + 0x00, + 0x00, + CtapHid::CAPABILITIES + ] + }] + ); + } + #[test] fn test_command_ping() { let mut rng = ThreadRng256 {}; diff --git a/src/ctap/hid/receive.rs b/src/ctap/hid/receive.rs index fef51a4..b522837 100644 --- a/src/ctap/hid/receive.rs +++ b/src/ctap/hid/receive.rs @@ -586,5 +586,33 @@ mod test { ); } + #[test] + fn test_init_sync() { + let mut assembler = MessageAssembler::new(); + // Ping packet with a length longer than one packet. + assert_eq!( + assembler.parse_packet( + &byte_extend(&[0x12, 0x34, 0x56, 0x78, 0x81, 0x02, 0x00], 0x51), + DUMMY_TIMESTAMP + ), + Ok(None) + ); + // Init packet on the same channel. + assert_eq!( + assembler.parse_packet( + &zero_extend(&[ + 0x12, 0x34, 0x56, 0x78, 0x86, 0x00, 0x08, 0x12, 0x34, 0x56, 0x78, 0x9A, 0xBC, + 0xDE, 0xF0 + ]), + DUMMY_TIMESTAMP + ), + Ok(Some(Message { + cid: [0x12, 0x34, 0x56, 0x78], + cmd: 0x06, + payload: vec![0x12, 0x34, 0x56, 0x78, 0x9A, 0xBC, 0xDE, 0xF0] + })) + ); + } + // TODO: more tests } From 9a29795ca65870508267b45d5744878357a97c62 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Fri, 20 Nov 2020 06:03:52 +0100 Subject: [PATCH 05/28] changes priority of error codes --- src/ctap/hid/mod.rs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/ctap/hid/mod.rs b/src/ctap/hid/mod.rs index 7489f6e..e975f33 100644 --- a/src/ctap/hid/mod.rs +++ b/src/ctap/hid/mod.rs @@ -307,7 +307,9 @@ impl CtapHid { HidPacketIterator::none() } Err((cid, error)) => { - if !self.is_allocated_channel(cid) { + if !self.is_allocated_channel(cid) + && error != receive::Error::UnexpectedContinuation + { CtapHid::error_message(cid, CtapHid::ERR_INVALID_CHANNEL) } else { match error { @@ -523,6 +525,27 @@ mod test { } } + #[test] + fn test_spurious_continuation_packet() { + let mut rng = ThreadRng256 {}; + let user_immediately_present = |_| Ok(()); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_hid = CtapHid::new(); + + let mut packet = [0x00; 64]; + packet[0..7].copy_from_slice(&[0xC1, 0xC1, 0xC1, 0xC1, 0x00, 0x51, 0x51]); + let mut assembler_reply = MessageAssembler::new(); + for pkt_reply in ctap_hid.process_hid_packet(&packet, DUMMY_CLOCK_VALUE, &mut ctap_state) { + // Continuation packets are silently ignored. + assert_eq!( + assembler_reply + .parse_packet(&pkt_reply, DUMMY_TIMESTAMP) + .unwrap(), + None + ); + } + } + #[test] fn test_command_init() { let mut rng = ThreadRng256 {}; From dab0077b87451fa448f72f1dadd1d2c42c142ca6 Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Fri, 20 Nov 2020 11:58:39 +0100 Subject: [PATCH 06/28] Fix broken crypto_test workflow. The use of `::set-env` command in workflows is not being depreacted. Moving to the new way of setting environment variables. --- .github/workflows/crypto_test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/crypto_test.yml b/.github/workflows/crypto_test.yml index 1740280..50fdf88 100644 --- a/.github/workflows/crypto_test.yml +++ b/.github/workflows/crypto_test.yml @@ -27,7 +27,7 @@ jobs: - name: Set up OpenSK run: ./setup.sh - - run: echo "::set-env name=RUSTFLAGS::-C target-feature=+aes" + - run: echo "RUSTFLAGS=-C target-feature=+aes" >> $GITHUB_ENV - name: Unit testing of crypto library (release mode) uses: actions-rs/cargo@v1 From 5bf73cb8fdcd4bcb33381992c8608fd77df7dce6 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Fri, 20 Nov 2020 03:26:26 +0100 Subject: [PATCH 07/28] fail on UP=true in make --- src/ctap/data_formats.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index dacafe5..45ebf9f 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -361,10 +361,8 @@ impl TryFrom for MakeCredentialOptions { Some(options_entry) => extract_bool(options_entry)?, None => false, }; - if let Some(options_entry) = up { - if !extract_bool(options_entry)? { - return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_OPTION); - } + if up.is_some() { + return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_OPTION); } let uv = match uv { Some(options_entry) => extract_bool(options_entry)?, From 9bb1aad45d526b29bb5f6477c56b5136a880cff2 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Mon, 23 Nov 2020 12:02:51 +0100 Subject: [PATCH 08/28] wraps HMAC secret into credentials --- src/ctap/ctap1.rs | 50 ++++++---- src/ctap/mod.rs | 229 ++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 236 insertions(+), 43 deletions(-) diff --git a/src/ctap/ctap1.rs b/src/ctap/ctap1.rs index 84c6fb0..a5a7921 100644 --- a/src/ctap/ctap1.rs +++ b/src/ctap/ctap1.rs @@ -288,7 +288,7 @@ impl Ctap1Command { let sk = crypto::ecdsa::SecKey::gensk(ctap_state.rng); let pk = sk.genpk(); let key_handle = ctap_state - .encrypt_key_handle(sk, &application) + .encrypt_key_handle(sk, &application, None) .map_err(|_| Ctap1StatusCode::SW_VENDOR_KEY_HANDLE_TOO_LONG)?; if key_handle.len() > 0xFF { // This is just being defensive with unreachable code. @@ -373,7 +373,7 @@ impl Ctap1Command { #[cfg(test)] mod test { - use super::super::{ENCRYPTED_CREDENTIAL_ID_SIZE, USE_SIGNATURE_COUNTER}; + use super::super::{CREDENTIAL_ID_BASE_SIZE, USE_SIGNATURE_COUNTER}; use super::*; use crypto::rng256::ThreadRng256; use crypto::Hash256; @@ -413,12 +413,12 @@ mod test { 0x00, 0x00, 0x00, - 65 + ENCRYPTED_CREDENTIAL_ID_SIZE as u8, + 65 + CREDENTIAL_ID_BASE_SIZE as u8, ]; let challenge = [0x0C; 32]; message.extend(&challenge); message.extend(application); - message.push(ENCRYPTED_CREDENTIAL_ID_SIZE as u8); + message.push(CREDENTIAL_ID_BASE_SIZE as u8); message.extend(key_handle); message } @@ -437,15 +437,15 @@ mod test { Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE).unwrap(); assert_eq!(response[0], Ctap1Command::LEGACY_BYTE); - assert_eq!(response[66], ENCRYPTED_CREDENTIAL_ID_SIZE as u8); + assert_eq!(response[66], CREDENTIAL_ID_BASE_SIZE as u8); assert!(ctap_state .decrypt_credential_source( - response[67..67 + ENCRYPTED_CREDENTIAL_ID_SIZE].to_vec(), + response[67..67 + CREDENTIAL_ID_BASE_SIZE].to_vec(), &application ) .unwrap() .is_some()); - const CERT_START: usize = 67 + ENCRYPTED_CREDENTIAL_ID_SIZE; + const CERT_START: usize = 67 + CREDENTIAL_ID_BASE_SIZE; assert_eq!( &response[CERT_START..CERT_START + ATTESTATION_CERTIFICATE.len()], &ATTESTATION_CERTIFICATE[..] @@ -494,7 +494,9 @@ mod test { let rp_id = "example.com"; let application = crypto::sha256::Sha256::hash(rp_id.as_bytes()); - let key_handle = ctap_state.encrypt_key_handle(sk, &application).unwrap(); + let key_handle = ctap_state + .encrypt_key_handle(sk, &application, None) + .unwrap(); let message = create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle); let response = Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE); @@ -510,7 +512,9 @@ mod test { let rp_id = "example.com"; let application = crypto::sha256::Sha256::hash(rp_id.as_bytes()); - let key_handle = ctap_state.encrypt_key_handle(sk, &application).unwrap(); + let key_handle = ctap_state + .encrypt_key_handle(sk, &application, None) + .unwrap(); let application = [0x55; 32]; let message = create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle); @@ -527,7 +531,9 @@ mod test { let rp_id = "example.com"; let application = crypto::sha256::Sha256::hash(rp_id.as_bytes()); - let key_handle = ctap_state.encrypt_key_handle(sk, &application).unwrap(); + let key_handle = ctap_state + .encrypt_key_handle(sk, &application, None) + .unwrap(); let mut message = create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle); @@ -551,7 +557,9 @@ mod test { let rp_id = "example.com"; let application = crypto::sha256::Sha256::hash(rp_id.as_bytes()); - let key_handle = ctap_state.encrypt_key_handle(sk, &application).unwrap(); + let key_handle = ctap_state + .encrypt_key_handle(sk, &application, None) + .unwrap(); let mut message = create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle); message[0] = 0xEE; @@ -569,7 +577,9 @@ mod test { let rp_id = "example.com"; let application = crypto::sha256::Sha256::hash(rp_id.as_bytes()); - let key_handle = ctap_state.encrypt_key_handle(sk, &application).unwrap(); + let key_handle = ctap_state + .encrypt_key_handle(sk, &application, None) + .unwrap(); let mut message = create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle); message[1] = 0xEE; @@ -587,7 +597,9 @@ mod test { let rp_id = "example.com"; let application = crypto::sha256::Sha256::hash(rp_id.as_bytes()); - let key_handle = ctap_state.encrypt_key_handle(sk, &application).unwrap(); + let key_handle = ctap_state + .encrypt_key_handle(sk, &application, None) + .unwrap(); let mut message = create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle); message[2] = 0xEE; @@ -605,7 +617,9 @@ mod test { let rp_id = "example.com"; let application = crypto::sha256::Sha256::hash(rp_id.as_bytes()); - let key_handle = ctap_state.encrypt_key_handle(sk, &application).unwrap(); + let key_handle = ctap_state + .encrypt_key_handle(sk, &application, None) + .unwrap(); let message = create_authenticate_message(&application, Ctap1Flags::EnforceUpAndSign, &key_handle); @@ -630,7 +644,9 @@ mod test { let rp_id = "example.com"; let application = crypto::sha256::Sha256::hash(rp_id.as_bytes()); - let key_handle = ctap_state.encrypt_key_handle(sk, &application).unwrap(); + let key_handle = ctap_state + .encrypt_key_handle(sk, &application, None) + .unwrap(); let message = create_authenticate_message( &application, Ctap1Flags::DontEnforceUpAndSign, @@ -650,7 +666,7 @@ mod test { #[test] fn test_process_authenticate_bad_key_handle() { let application = [0x0A; 32]; - let key_handle = vec![0x00; ENCRYPTED_CREDENTIAL_ID_SIZE]; + let key_handle = vec![0x00; CREDENTIAL_ID_BASE_SIZE]; let message = create_authenticate_message(&application, Ctap1Flags::EnforceUpAndSign, &key_handle); @@ -667,7 +683,7 @@ mod test { #[test] fn test_process_authenticate_without_up() { let application = [0x0A; 32]; - let key_handle = vec![0x00; ENCRYPTED_CREDENTIAL_ID_SIZE]; + let key_handle = vec![0x00; CREDENTIAL_ID_BASE_SIZE]; let message = create_authenticate_message(&application, Ctap1Flags::EnforceUpAndSign, &key_handle); diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 66ef234..2e095ed 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -83,8 +83,9 @@ const USE_SIGNATURE_COUNTER: bool = true; // - 16 byte initialization vector for AES-256, // - 32 byte ECDSA private key for the credential, // - 32 byte relying party ID hashed with SHA256, +// - (optional) 32 byte for HMAC-secret, // - 32 byte HMAC-SHA256 over everything else. -pub const ENCRYPTED_CREDENTIAL_ID_SIZE: usize = 112; +pub const CREDENTIAL_ID_BASE_SIZE: usize = 112; // Set this bit when checking user presence. const UP_FLAG: u8 = 0x01; // Set this bit when checking user verification. @@ -195,6 +196,7 @@ where &mut self, private_key: crypto::ecdsa::SecKey, application: &[u8; 32], + cred_random: Option<&[u8; 32]>, ) -> Result, Ctap2StatusCode> { let master_keys = self.persistent_store.master_keys()?; let aes_enc_key = crypto::aes256::EncryptionKey::new(&master_keys.encryption); @@ -203,14 +205,19 @@ where let mut iv = [0; 16]; iv.copy_from_slice(&self.rng.gen_uniform_u8x32()[..16]); - let mut blocks = [[0u8; 16]; 4]; + let block_len = if cred_random.is_some() { 6 } else { 4 }; + let mut blocks = vec![[0u8; 16]; block_len]; blocks[0].copy_from_slice(&sk_bytes[..16]); blocks[1].copy_from_slice(&sk_bytes[16..]); blocks[2].copy_from_slice(&application[..16]); blocks[3].copy_from_slice(&application[16..]); + if let Some(cred_random) = cred_random { + blocks[4].copy_from_slice(&cred_random[..16]); + blocks[5].copy_from_slice(&cred_random[16..]); + } cbc_encrypt(&aes_enc_key, iv, &mut blocks); - let mut encrypted_id = Vec::with_capacity(ENCRYPTED_CREDENTIAL_ID_SIZE); + let mut encrypted_id = Vec::with_capacity(16 * (block_len + 3)); encrypted_id.extend(&iv); for b in &blocks { encrypted_id.extend(b); @@ -228,11 +235,15 @@ where credential_id: Vec, rp_id_hash: &[u8], ) -> Result, Ctap2StatusCode> { - if credential_id.len() != ENCRYPTED_CREDENTIAL_ID_SIZE { + let has_cred_random = if credential_id.len() == CREDENTIAL_ID_BASE_SIZE { + false + } else if credential_id.len() == CREDENTIAL_ID_BASE_SIZE + 32 { + true + } else { return Ok(None); - } + }; let master_keys = self.persistent_store.master_keys()?; - let payload_size = ENCRYPTED_CREDENTIAL_ID_SIZE - 32; + let payload_size = credential_id.len() - 32; if !verify_hmac_256::( &master_keys.hmac, &credential_id[..payload_size], @@ -244,8 +255,9 @@ where let aes_dec_key = crypto::aes256::DecryptionKey::new(&aes_enc_key); let mut iv = [0; 16]; iv.copy_from_slice(&credential_id[..16]); - let mut blocks = [[0u8; 16]; 4]; - for i in 0..4 { + let block_len = if has_cred_random { 6 } else { 4 }; + let mut blocks = vec![[0u8; 16]; block_len]; + for i in 0..block_len { blocks[i].copy_from_slice(&credential_id[16 * (i + 1)..16 * (i + 2)]); } @@ -256,6 +268,14 @@ where decrypted_sk[16..].clone_from_slice(&blocks[1]); decrypted_rp_id_hash[..16].clone_from_slice(&blocks[2]); decrypted_rp_id_hash[16..].clone_from_slice(&blocks[3]); + let cred_random = if has_cred_random { + let mut decrypted_cred_random = [0; 32]; + decrypted_cred_random[..16].clone_from_slice(&blocks[4]); + decrypted_cred_random[16..].clone_from_slice(&blocks[5]); + Some(decrypted_cred_random.to_vec()) + } else { + None + }; if rp_id_hash != decrypted_rp_id_hash { return Ok(None); @@ -269,7 +289,7 @@ where rp_id: String::from(""), user_handle: vec![], other_ui: None, - cred_random: None, + cred_random, cred_protect_policy: None, })) } @@ -380,11 +400,7 @@ where }; let cred_random = if use_hmac_extension { - if !options.rk { - // The extension is actually supported, but we need resident keys. - return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_EXTENSION); - } - Some(self.rng.gen_uniform_u8x32().to_vec()) + Some(self.rng.gen_uniform_u8x32()) } else { None }; @@ -463,13 +479,13 @@ where other_ui: user .user_display_name .map(|s| truncate_to_char_boundary(&s, 64).to_string()), - cred_random, + cred_random: cred_random.map(|c| c.to_vec()), cred_protect_policy, }; self.persistent_store.store_credential(credential_source)?; random_id } else { - self.encrypt_key_handle(sk.clone(), &rp_id_hash)? + self.encrypt_key_handle(sk.clone(), &rp_id_hash, cred_random.as_ref())? }; let mut auth_data = self.generate_auth_data(&rp_id_hash, flags)?; @@ -728,10 +744,9 @@ where ]), #[cfg(feature = "with_ctap2_1")] max_credential_count_in_list: MAX_CREDENTIAL_COUNT_IN_LIST.map(|c| c as u64), - // You can use ENCRYPTED_CREDENTIAL_ID_SIZE here, but if your - // browser passes that value, it might be used to fingerprint. + // #TODO(106) update with version 2.1 of HMAC-secret #[cfg(feature = "with_ctap2_1")] - max_credential_id_length: None, + max_credential_id_length: Some(CREDENTIAL_ID_BASE_SIZE as u64 + 32), #[cfg(feature = "with_ctap2_1")] transports: Some(vec![AuthenticatorTransport::Usb]), #[cfg(feature = "with_ctap2_1")] @@ -829,7 +844,7 @@ mod test { let info_reponse = ctap_state.process_command(&[0x04], DUMMY_CHANNEL_ID); #[cfg(feature = "with_ctap2_1")] - let mut expected_response = vec![0x00, 0xA9, 0x01]; + let mut expected_response = vec![0x00, 0xAA, 0x01]; #[cfg(not(feature = "with_ctap2_1"))] let mut expected_response = vec![0x00, 0xA6, 0x01]; // The difference here is a longer array of supported versions. @@ -864,9 +879,9 @@ mod test { #[cfg(feature = "with_ctap2_1")] expected_response.extend( [ - 0x09, 0x81, 0x63, 0x75, 0x73, 0x62, 0x0A, 0x81, 0xA2, 0x63, 0x61, 0x6C, 0x67, 0x26, - 0x64, 0x74, 0x79, 0x70, 0x65, 0x6A, 0x70, 0x75, 0x62, 0x6C, 0x69, 0x63, 0x2D, 0x6B, - 0x65, 0x79, 0x0D, 0x04, + 0x08, 0x18, 0x90, 0x09, 0x81, 0x63, 0x75, 0x73, 0x62, 0x0A, 0x81, 0xA2, 0x63, 0x61, + 0x6C, 0x67, 0x26, 0x64, 0x74, 0x79, 0x70, 0x65, 0x6A, 0x70, 0x75, 0x62, 0x6C, 0x69, + 0x63, 0x2D, 0x6B, 0x65, 0x79, 0x0D, 0x04, ] .iter(), ); @@ -993,7 +1008,7 @@ mod test { 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0x41, 0x00, 0x00, 0x00, 0x00, ]; expected_auth_data.extend(&ctap_state.persistent_store.aaguid().unwrap()); - expected_auth_data.extend(&[0x00, ENCRYPTED_CREDENTIAL_ID_SIZE as u8]); + expected_auth_data.extend(&[0x00, CREDENTIAL_ID_BASE_SIZE as u8]); assert_eq!( auth_data[0..expected_auth_data.len()], expected_auth_data[..] @@ -1114,6 +1129,57 @@ mod test { let user_immediately_present = |_| Ok(()); let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let extensions = Some(MakeCredentialExtensions { + hmac_secret: true, + cred_protect: None, + }); + let mut make_credential_params = create_minimal_make_credential_parameters(); + make_credential_params.options.rk = false; + make_credential_params.extensions = extensions; + let make_credential_response = + ctap_state.process_make_credential(make_credential_params, DUMMY_CHANNEL_ID); + + match make_credential_response.unwrap() { + ResponseData::AuthenticatorMakeCredential(make_credential_response) => { + let AuthenticatorMakeCredentialResponse { + fmt, + auth_data, + att_stmt, + } = make_credential_response; + // The expected response is split to only assert the non-random parts. + assert_eq!(fmt, "packed"); + let mut expected_auth_data = vec![ + 0xA3, 0x79, 0xA6, 0xF6, 0xEE, 0xAF, 0xB9, 0xA5, 0x5E, 0x37, 0x8C, 0x11, 0x80, + 0x34, 0xE2, 0x75, 0x1E, 0x68, 0x2F, 0xAB, 0x9F, 0x2D, 0x30, 0xAB, 0x13, 0xD2, + 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0xC1, 0x00, 0x00, 0x00, 0x00, + ]; + expected_auth_data.extend(&ctap_state.persistent_store.aaguid().unwrap()); + let credential_size = CREDENTIAL_ID_BASE_SIZE + 32; + expected_auth_data.extend(&[0x00, credential_size as u8]); + assert_eq!( + auth_data[0..expected_auth_data.len()], + expected_auth_data[..] + ); + let expected_extension_cbor = vec![ + 0xA1, 0x6B, 0x68, 0x6D, 0x61, 0x63, 0x2D, 0x73, 0x65, 0x63, 0x72, 0x65, 0x74, + 0xF5, + ]; + assert_eq!( + auth_data[auth_data.len() - expected_extension_cbor.len()..auth_data.len()], + expected_extension_cbor[..] + ); + assert_eq!(att_stmt.alg, SignatureAlgorithm::ES256 as i64); + } + _ => panic!("Invalid response type"), + } + } + + #[test] + fn test_process_make_credential_hmac_secret_resident_key() { + let mut rng = ThreadRng256 {}; + let user_immediately_present = |_| Ok(()); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let extensions = Some(MakeCredentialExtensions { hmac_secret: true, cred_protect: None, @@ -1220,6 +1286,71 @@ mod test { } } + #[test] + fn test_process_get_assertion_hmac_secret() { + let mut rng = ThreadRng256 {}; + let sk = crypto::ecdh::SecKey::gensk(&mut rng); + let user_immediately_present = |_| Ok(()); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + + let make_extensions = Some(MakeCredentialExtensions { + hmac_secret: true, + cred_protect: None, + }); + let mut make_credential_params = create_minimal_make_credential_parameters(); + make_credential_params.options.rk = false; + make_credential_params.extensions = make_extensions; + let make_credential_response = + ctap_state.process_make_credential(make_credential_params, DUMMY_CHANNEL_ID); + 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 + ctap_state.persistent_store.aaguid().unwrap().len(); + let credential_size = CREDENTIAL_ID_BASE_SIZE + 32; + assert_eq!(auth_data[offset], 0x00); + assert_eq!(auth_data[offset + 1] as usize, credential_size); + auth_data[offset + 2..offset + 2 + credential_size].to_vec() + } + _ => panic!("Invalid response type"), + }; + + let pk = sk.genpk(); + let hmac_secret_input = GetAssertionHmacSecretInput { + key_agreement: CoseKey::from(pk), + salt_enc: vec![0x02; 32], + salt_auth: vec![0x03; 16], + }; + let get_extensions = Some(GetAssertionExtensions { + hmac_secret: Some(hmac_secret_input), + }); + + 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: get_extensions, + 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(get_assertion_params, DUMMY_CHANNEL_ID); + + assert_eq!( + get_assertion_response, + Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_EXTENSION) + ); + } + #[test] fn test_residential_process_get_assertion_hmac_secret() { let mut rng = ThreadRng256 {}; @@ -1435,7 +1566,7 @@ mod test { // We are not testing the correctness of our SHA256 here, only if it is checked. let rp_id_hash = [0x55; 32]; let encrypted_id = ctap_state - .encrypt_key_handle(private_key.clone(), &rp_id_hash) + .encrypt_key_handle(private_key.clone(), &rp_id_hash, None) .unwrap(); let decrypted_source = ctap_state .decrypt_credential_source(encrypted_id, &rp_id_hash) @@ -1445,6 +1576,29 @@ mod test { assert_eq!(private_key, decrypted_source.private_key); } + #[test] + fn test_encrypt_decrypt_credential_with_cred_random() { + let mut rng = ThreadRng256 {}; + let user_immediately_present = |_| Ok(()); + let private_key = crypto::ecdsa::SecKey::gensk(&mut rng); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + + // Usually, the relying party ID or its hash is provided by the client. + // We are not testing the correctness of our SHA256 here, only if it is checked. + let rp_id_hash = [0x55; 32]; + let cred_random = [0xC9; 32]; + let encrypted_id = ctap_state + .encrypt_key_handle(private_key.clone(), &rp_id_hash, Some(&cred_random)) + .unwrap(); + let decrypted_source = ctap_state + .decrypt_credential_source(encrypted_id, &rp_id_hash) + .unwrap() + .unwrap(); + + assert_eq!(private_key, decrypted_source.private_key); + assert_eq!(Some(cred_random.to_vec()), decrypted_source.cred_random); + } + #[test] fn test_encrypt_decrypt_bad_hmac() { let mut rng = ThreadRng256 {}; @@ -1455,7 +1609,30 @@ mod test { // Same as above. let rp_id_hash = [0x55; 32]; let encrypted_id = ctap_state - .encrypt_key_handle(private_key, &rp_id_hash) + .encrypt_key_handle(private_key, &rp_id_hash, None) + .unwrap(); + for i in 0..encrypted_id.len() { + let mut modified_id = encrypted_id.clone(); + modified_id[i] ^= 0x01; + assert!(ctap_state + .decrypt_credential_source(modified_id, &rp_id_hash) + .unwrap() + .is_none()); + } + } + + #[test] + fn test_encrypt_decrypt_bad_hmac_with_cred_random() { + let mut rng = ThreadRng256 {}; + let user_immediately_present = |_| Ok(()); + let private_key = crypto::ecdsa::SecKey::gensk(&mut rng); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + + // Same as above. + let rp_id_hash = [0x55; 32]; + let cred_random = [0xC9; 32]; + let encrypted_id = ctap_state + .encrypt_key_handle(private_key, &rp_id_hash, Some(&cred_random)) .unwrap(); for i in 0..encrypted_id.len() { let mut modified_id = encrypted_id.clone(); From a099ddbabde0c208b821c5417d5ed16e86560bd9 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Mon, 23 Nov 2020 14:34:38 +0100 Subject: [PATCH 09/28] introduce max credential size for readability --- src/ctap/mod.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 2e095ed..dd9cabe 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -86,6 +86,7 @@ const USE_SIGNATURE_COUNTER: bool = true; // - (optional) 32 byte for HMAC-secret, // - 32 byte HMAC-SHA256 over everything else. pub const CREDENTIAL_ID_BASE_SIZE: usize = 112; +pub const CREDENTIAL_ID_MAX_SIZE: usize = CREDENTIAL_ID_BASE_SIZE + 32; // Set this bit when checking user presence. const UP_FLAG: u8 = 0x01; // Set this bit when checking user verification. @@ -235,12 +236,10 @@ where credential_id: Vec, rp_id_hash: &[u8], ) -> Result, Ctap2StatusCode> { - let has_cred_random = if credential_id.len() == CREDENTIAL_ID_BASE_SIZE { - false - } else if credential_id.len() == CREDENTIAL_ID_BASE_SIZE + 32 { - true - } else { - return Ok(None); + let has_cred_random = match credential_id.len() { + CREDENTIAL_ID_BASE_SIZE => false, + CREDENTIAL_ID_MAX_SIZE => true, + _ => return Ok(None), }; let master_keys = self.persistent_store.master_keys()?; let payload_size = credential_id.len() - 32; @@ -1154,8 +1153,7 @@ mod test { 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0xC1, 0x00, 0x00, 0x00, 0x00, ]; expected_auth_data.extend(&ctap_state.persistent_store.aaguid().unwrap()); - let credential_size = CREDENTIAL_ID_BASE_SIZE + 32; - expected_auth_data.extend(&[0x00, credential_size as u8]); + expected_auth_data.extend(&[0x00, CREDENTIAL_ID_MAX_SIZE as u8]); assert_eq!( auth_data[0..expected_auth_data.len()], expected_auth_data[..] @@ -1307,10 +1305,9 @@ mod test { ResponseData::AuthenticatorMakeCredential(make_credential_response) => { let auth_data = make_credential_response.auth_data; let offset = 37 + ctap_state.persistent_store.aaguid().unwrap().len(); - let credential_size = CREDENTIAL_ID_BASE_SIZE + 32; assert_eq!(auth_data[offset], 0x00); - assert_eq!(auth_data[offset + 1] as usize, credential_size); - auth_data[offset + 2..offset + 2 + credential_size].to_vec() + assert_eq!(auth_data[offset + 1] as usize, CREDENTIAL_ID_MAX_SIZE); + auth_data[offset + 2..offset + 2 + CREDENTIAL_ID_MAX_SIZE].to_vec() } _ => panic!("Invalid response type"), }; From 174c292f2f700cbfc85dec8da6c0412180e50fff Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Mon, 23 Nov 2020 19:16:48 +0100 Subject: [PATCH 10/28] Adding metadata file used for certification. --- metadata/metadata.json | 47 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 metadata/metadata.json diff --git a/metadata/metadata.json b/metadata/metadata.json new file mode 100644 index 0000000..bb81d0c --- /dev/null +++ b/metadata/metadata.json @@ -0,0 +1,47 @@ +{ + "assertionScheme" : "FIDOV2", + "keyProtection" : 1, + "attestationRootCertificates" : [ + ], + "aaguid" : "664d9f67-84a2-412a-9ff7-b4f7d8ee6d05", + "publicKeyAlgAndEncoding" : 260, + "protocolFamily" : "fido2", + "upv" : [ + { + "major" : 1, + "minor" : 0 + } + ], + "icon" : "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAEAAAABACAYAAACqaXHeAAAAAXNSR0IArs4c6QAAAERlWElmTU0AKgAAAAgAAYdpAAQAAAABAAAAGgAAAAAAA6ABAAMAAAABAAEAAKACAAQAAAABAAAAQKADAAQAAAABAAAAQAAAAABGUUKwAAAIQ0lEQVR4Ae1aCVSUVRT+kGVYBBQFBYzYFJFNLdPQVksz85QnszRNbaPNzDI0OaIH27VUUnOpzAqXMJNIszKTUEQWRXBnExRiUYEUBATs3lfzJw3LDP/MMOfMPI/M++97///uve+++9797jO7TgVGXLoYsexCdJMCTBZg5BowLQEjNwCYLMBkAUauAdMSMHIDgEVnKqC8/AKOZh2Do6MDAgMGwMbaWu/s6FUBTU1NyMnNQ8bRTPqfheI/SySBzc3N4devLwaGBGFgcBBcXJylNl1WzHQdDVbX1CDr2HEcJYEz6be6ukYteVxdewtFsEL6+vqgSxfduCudKaCgsBCbt27Dmexc8MzLKba2tggOCkDYszNgZmYm51Mq7+pGrTRMcXEJTp3Oli08c1xDVpR8KBW6gC50pgAVVRsoQWcKcHd3w4jht6N7924GKvo/bGl1F+C1fu78eWH+TdebcOeIUEyfOhkHk1OwJXY7OcBqg1OG1hRwICkZ38fF48LFS82EdHLqjkmPT8DihRF4b8nH4L3fkIrsJcCO6cuvYrD+i40qwrOgly5VYNWn65GUfAjhb7wGKysrQ5Jffji8a/ev2PfH/naF2rY9jma/HA+PG9tuX312kLUErly5grj4H9XmN3b7Dix4Kxz33n2H2u+czs5B9Mo1sLS01MlhSJYC0g5noL7+WjNh+NAydsxoMnVL/ETWcamiQmrPzy9AZWUV2C+oW/hY7KTDnUSWDygoKFSRY/pTk0kBo3D/yHvwyovPq7SXlpWr0Noi/PZ7gvAtDg4ObXXrcJssBdTV16sM7O7mJtFaDmhUE1HFxX/SqfGM9J6ykpySim82bRWPHjf1UZK1+itLAT1aMOWkg4ckBhMSVZ2ju5ur1M47yO5f9iAy6l18sHQ59tJsK0vigYNYu36DdPz18vJUNmn1V5YP4Bg+fufuZgz5+nhLzzY2NlKdKwED+qOJhN7xw04h2PETJ0V4rOz0VcwWnDh1WgQ8qWmHlWTxHBIcKD1rsyJLARy/e3t5Ii//rODJx9sLgwYGS/zdessgxGz+Fo2NjWL/f2LiBPxICtuzd5/U5/+VtPQj/yfB368fujk6qtC1QZC1BJiBZ5+eBtt/Z/qxRx9pxpODvT2G3z4UFhYWCHtuBi5fvgx2apqWUaNGavqK2v21ggcUFJ4Th6FpUyapDHzh4kXU1taK7W/l6nWoratT6dMWwfNmDyxa8FZbXWS1aUUB7XGQkZmF5dGr2+um0s7gx8KIufD0vFmlTVsE2UtAHUaCAwMI1vrPOarzDvcZN3aMToXnMfSiAMbzXnj+GXTrpr4jGzwoBOMffoh51GnRiwJYgh5OTpj35utqefOgwAGE/z2tdfyvJU3qxQfcOHAZHYU/Wb2WgJOiG8lSfXjoMMx4agrtHOYSTZcVvSuAham/dg2bt8Ti94RESTYbG2tMfXISQofdJtH0UekUBSgFY+g89rs4uLn1xrgHx8DevquySW+/naoAvUnZxkB6c4Jt8NCpTSYFdKr6DWDwDltAQ0Mjjh0/ifQjGWBsUFflfFERODTOyzsrDVFRUYnsnFzpuZ6AmRMnT3UIcu9QOMwBzocfrSDBq2FHGGBlVRVeCnuGQuEQiSltVDZs/AaHUtLg4XGTSLj08/XFrJkvIjX9MIGxu7BqxVKBKzAkn5uXT3HDPI2H7ZACNm2OFZcZoiLnw5ouNTDau/7zjVi29H1crb2KSpohOzs7nKVtjpnmCxDKwtgBzyBjCV272lGIfAWlZWXo5eKCMzk56EOQWq9eLigimCwh8QDmz52Dfn19UFpahrkRC8nqTig/JX7j4nciM+s4IubNaTZOs05tPGisAAY3+FbH1MmPC+H526PvH4mdu36mVHi2SITE0CHHxbkneJn8RRjA4kUR4ij8+YavxZLp2cNJoMVRkRHIzc8X0FcfyiU2NV0nwYso/J0vhOFLEympaXB3dxVKWfdpNCyIVkLK4JKSli4s4dWXw9BRzFBjH8D5PVbCjYENAx8c8FRV/SUY4z8L5ofjnagFQpB9dOLjmU88kIRIokdRmsy1d2/8smev6N/Q0IDXX3uF6Cy4o1jP/E1GlY9kZOLV2eGIXrUGZWQpyosSdYQrfEam70hocf/+ftK4mlY0VoBC8c89ntra/4ANFoATowprhRifESCFQgGeQR8vTzLxchQSaMLx/ScEikRELhYmXkaZIjP6x4UF5sLoEjs1LgyvLXl/MebMnolGsqa3310ilg+38Zh33TEC1+lfzL/IMdM1LRovAYXCSpgbz8ywoUPEeMp16evtTevxWDMeKigRwibPCuHZmzXzBVhZWgnGrSjbc/KUKhzOH2BInBMrbEn+NMPeXl4Ie3mWBKJyAubJSRPFzZGPlq9ECF2lGXLL4GZjq/OgsQL4oxMnjMey6FVY95k5nJ17CJCT/YDyLgDf6NhEfoADHN6ewt+YJYANPuszzs+MJlHK/B5KkXUxa9kI/f38sGXrd1i6LBpBgQG07eUJ6/D29kT64QwpVOa2kffeJRK0PAFKHtQRnvuYL6KibmdlP0548OUl9sx8BuAs0AOj7xPNnC3KpT2bEWEOeR98YJTYHi1pWQy5dTBKSkpxlvoM8PcjwHSYgMl5yfAdIC41NVfhRRAYO7XQ0KGEJ9aJJcROddqUyXDuyc61ATa2Ngjw7y/eYdSYcUcubjfkHQShnT9aD4YS/tiP7TviseLjD9oZ2jCaW7Y/GbzZkzPz8NBNGksGW62+qnULaHUkA23QugUYqJytsmVSQKuqMZIGkwUYyUS3KqbJAlpVjZE0mCzASCa6VTH/Bnoy/0KF7w+OAAAAAElFTkSuQmCC", + "matcherProtection" : 1, + "supportedExtensions" : [ + { + "id" : "hmac-secret", + "fail_if_unknown" : false + }, + { + "id" : "credProtect", + "fail_if_unknown" : false + } + ], + "cryptoStrength" : 128, + "description" : "OpenSK authenticator", + "authenticatorVersion" : 1, + "isSecondFactorOnly" : false, + "userVerificationDetails" : [ + [ + { + "userVerification" : 1 + }, + { + "userVerification" : 4 + } + ] + ], + "attachmentHint" : 6, + "attestationTypes" : [ + 15880 + ], + "authenticationAlgorithm" : 1, + "tcDisplay" : 0 +} From 90f2d4a24955544fe426eb79a201f18b20253aa3 Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Mon, 23 Nov 2020 20:33:01 +0100 Subject: [PATCH 11/28] Fix indentation --- metadata/metadata.json | 55 +++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/metadata/metadata.json b/metadata/metadata.json index bb81d0c..eedeed9 100644 --- a/metadata/metadata.json +++ b/metadata/metadata.json @@ -1,47 +1,46 @@ { - "assertionScheme" : "FIDOV2", - "keyProtection" : 1, - "attestationRootCertificates" : [ - ], - "aaguid" : "664d9f67-84a2-412a-9ff7-b4f7d8ee6d05", - "publicKeyAlgAndEncoding" : 260, - "protocolFamily" : "fido2", - "upv" : [ + "assertionScheme": "FIDOV2", + "keyProtection": 1, + "attestationRootCertificates": [], + "aaguid": "664d9f67-84a2-412a-9ff7-b4f7d8ee6d05", + "publicKeyAlgAndEncoding": 260, + "protocolFamily": "fido2", + "upv": [ { - "major" : 1, - "minor" : 0 + "major": 1, + "minor": 0 } ], - "icon" : "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAEAAAABACAYAAACqaXHeAAAAAXNSR0IArs4c6QAAAERlWElmTU0AKgAAAAgAAYdpAAQAAAABAAAAGgAAAAAAA6ABAAMAAAABAAEAAKACAAQAAAABAAAAQKADAAQAAAABAAAAQAAAAABGUUKwAAAIQ0lEQVR4Ae1aCVSUVRT+kGVYBBQFBYzYFJFNLdPQVksz85QnszRNbaPNzDI0OaIH27VUUnOpzAqXMJNIszKTUEQWRXBnExRiUYEUBATs3lfzJw3LDP/MMOfMPI/M++97///uve+++9797jO7TgVGXLoYsexCdJMCTBZg5BowLQEjNwCYLMBkAUauAdMSMHIDgEVnKqC8/AKOZh2Do6MDAgMGwMbaWu/s6FUBTU1NyMnNQ8bRTPqfheI/SySBzc3N4devLwaGBGFgcBBcXJylNl1WzHQdDVbX1CDr2HEcJYEz6be6ukYteVxdewtFsEL6+vqgSxfduCudKaCgsBCbt27Dmexc8MzLKba2tggOCkDYszNgZmYm51Mq7+pGrTRMcXEJTp3Oli08c1xDVpR8KBW6gC50pgAVVRsoQWcKcHd3w4jht6N7924GKvo/bGl1F+C1fu78eWH+TdebcOeIUEyfOhkHk1OwJXY7OcBqg1OG1hRwICkZ38fF48LFS82EdHLqjkmPT8DihRF4b8nH4L3fkIrsJcCO6cuvYrD+i40qwrOgly5VYNWn65GUfAjhb7wGKysrQ5Jffji8a/ev2PfH/naF2rY9jma/HA+PG9tuX312kLUErly5grj4H9XmN3b7Dix4Kxz33n2H2u+czs5B9Mo1sLS01MlhSJYC0g5noL7+WjNh+NAydsxoMnVL/ETWcamiQmrPzy9AZWUV2C+oW/hY7KTDnUSWDygoKFSRY/pTk0kBo3D/yHvwyovPq7SXlpWr0Noi/PZ7gvAtDg4ObXXrcJssBdTV16sM7O7mJtFaDmhUE1HFxX/SqfGM9J6ykpySim82bRWPHjf1UZK1+itLAT1aMOWkg4ckBhMSVZ2ju5ur1M47yO5f9iAy6l18sHQ59tJsK0vigYNYu36DdPz18vJUNmn1V5YP4Bg+fufuZgz5+nhLzzY2NlKdKwED+qOJhN7xw04h2PETJ0V4rOz0VcwWnDh1WgQ8qWmHlWTxHBIcKD1rsyJLARy/e3t5Ii//rODJx9sLgwYGS/zdessgxGz+Fo2NjWL/f2LiBPxICtuzd5/U5/+VtPQj/yfB368fujk6qtC1QZC1BJiBZ5+eBtt/Z/qxRx9pxpODvT2G3z4UFhYWCHtuBi5fvgx2apqWUaNGavqK2v21ggcUFJ4Th6FpUyapDHzh4kXU1taK7W/l6nWoratT6dMWwfNmDyxa8FZbXWS1aUUB7XGQkZmF5dGr2+um0s7gx8KIufD0vFmlTVsE2UtAHUaCAwMI1vrPOarzDvcZN3aMToXnMfSiAMbzXnj+GXTrpr4jGzwoBOMffoh51GnRiwJYgh5OTpj35utqefOgwAGE/z2tdfyvJU3qxQfcOHAZHYU/Wb2WgJOiG8lSfXjoMMx4agrtHOYSTZcVvSuAham/dg2bt8Ti94RESTYbG2tMfXISQofdJtH0UekUBSgFY+g89rs4uLn1xrgHx8DevquySW+/naoAvUnZxkB6c4Jt8NCpTSYFdKr6DWDwDltAQ0Mjjh0/ifQjGWBsUFflfFERODTOyzsrDVFRUYnsnFzpuZ6AmRMnT3UIcu9QOMwBzocfrSDBq2FHGGBlVRVeCnuGQuEQiSltVDZs/AaHUtLg4XGTSLj08/XFrJkvIjX9MIGxu7BqxVKBKzAkn5uXT3HDPI2H7ZACNm2OFZcZoiLnw5ouNTDau/7zjVi29H1crb2KSpohOzs7nKVtjpnmCxDKwtgBzyBjCV272lGIfAWlZWXo5eKCMzk56EOQWq9eLigimCwh8QDmz52Dfn19UFpahrkRC8nqTig/JX7j4nciM+s4IubNaTZOs05tPGisAAY3+FbH1MmPC+H526PvH4mdu36mVHi2SITE0CHHxbkneJn8RRjA4kUR4ij8+YavxZLp2cNJoMVRkRHIzc8X0FcfyiU2NV0nwYso/J0vhOFLEympaXB3dxVKWfdpNCyIVkLK4JKSli4s4dWXw9BRzFBjH8D5PVbCjYENAx8c8FRV/SUY4z8L5ofjnagFQpB9dOLjmU88kIRIokdRmsy1d2/8smev6N/Q0IDXX3uF6Cy4o1jP/E1GlY9kZOLV2eGIXrUGZWQpyosSdYQrfEam70hocf/+ftK4mlY0VoBC8c89ntra/4ANFoATowprhRifESCFQgGeQR8vTzLxchQSaMLx/ScEikRELhYmXkaZIjP6x4UF5sLoEjs1LgyvLXl/MebMnolGsqa3310ilg+38Zh33TEC1+lfzL/IMdM1LRovAYXCSpgbz8ywoUPEeMp16evtTevxWDMeKigRwibPCuHZmzXzBVhZWgnGrSjbc/KUKhzOH2BInBMrbEn+NMPeXl4Ie3mWBKJyAubJSRPFzZGPlq9ECF2lGXLL4GZjq/OgsQL4oxMnjMey6FVY95k5nJ17CJCT/YDyLgDf6NhEfoADHN6ewt+YJYANPuszzs+MJlHK/B5KkXUxa9kI/f38sGXrd1i6LBpBgQG07eUJ6/D29kT64QwpVOa2kffeJRK0PAFKHtQRnvuYL6KibmdlP0548OUl9sx8BuAs0AOj7xPNnC3KpT2bEWEOeR98YJTYHi1pWQy5dTBKSkpxlvoM8PcjwHSYgMl5yfAdIC41NVfhRRAYO7XQ0KGEJ9aJJcROddqUyXDuyc61ATa2Ngjw7y/eYdSYcUcubjfkHQShnT9aD4YS/tiP7TviseLjD9oZ2jCaW7Y/GbzZkzPz8NBNGksGW62+qnULaHUkA23QugUYqJytsmVSQKuqMZIGkwUYyUS3KqbJAlpVjZE0mCzASCa6VTH/Bnoy/0KF7w+OAAAAAElFTkSuQmCC", - "matcherProtection" : 1, - "supportedExtensions" : [ + "icon": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAEAAAABACAYAAACqaXHeAAAAAXNSR0IArs4c6QAAAERlWElmTU0AKgAAAAgAAYdpAAQAAAABAAAAGgAAAAAAA6ABAAMAAAABAAEAAKACAAQAAAABAAAAQKADAAQAAAABAAAAQAAAAABGUUKwAAAIQ0lEQVR4Ae1aCVSUVRT+kGVYBBQFBYzYFJFNLdPQVksz85QnszRNbaPNzDI0OaIH27VUUnOpzAqXMJNIszKTUEQWRXBnExRiUYEUBATs3lfzJw3LDP/MMOfMPI/M++97///uve+++9797jO7TgVGXLoYsexCdJMCTBZg5BowLQEjNwCYLMBkAUauAdMSMHIDgEVnKqC8/AKOZh2Do6MDAgMGwMbaWu/s6FUBTU1NyMnNQ8bRTPqfheI/SySBzc3N4devLwaGBGFgcBBcXJylNl1WzHQdDVbX1CDr2HEcJYEz6be6ukYteVxdewtFsEL6+vqgSxfduCudKaCgsBCbt27Dmexc8MzLKba2tggOCkDYszNgZmYm51Mq7+pGrTRMcXEJTp3Oli08c1xDVpR8KBW6gC50pgAVVRsoQWcKcHd3w4jht6N7924GKvo/bGl1F+C1fu78eWH+TdebcOeIUEyfOhkHk1OwJXY7OcBqg1OG1hRwICkZ38fF48LFS82EdHLqjkmPT8DihRF4b8nH4L3fkIrsJcCO6cuvYrD+i40qwrOgly5VYNWn65GUfAjhb7wGKysrQ5Jffji8a/ev2PfH/naF2rY9jma/HA+PG9tuX312kLUErly5grj4H9XmN3b7Dix4Kxz33n2H2u+czs5B9Mo1sLS01MlhSJYC0g5noL7+WjNh+NAydsxoMnVL/ETWcamiQmrPzy9AZWUV2C+oW/hY7KTDnUSWDygoKFSRY/pTk0kBo3D/yHvwyovPq7SXlpWr0Noi/PZ7gvAtDg4ObXXrcJssBdTV16sM7O7mJtFaDmhUE1HFxX/SqfGM9J6ykpySim82bRWPHjf1UZK1+itLAT1aMOWkg4ckBhMSVZ2ju5ur1M47yO5f9iAy6l18sHQ59tJsK0vigYNYu36DdPz18vJUNmn1V5YP4Bg+fufuZgz5+nhLzzY2NlKdKwED+qOJhN7xw04h2PETJ0V4rOz0VcwWnDh1WgQ8qWmHlWTxHBIcKD1rsyJLARy/e3t5Ii//rODJx9sLgwYGS/zdessgxGz+Fo2NjWL/f2LiBPxICtuzd5/U5/+VtPQj/yfB368fujk6qtC1QZC1BJiBZ5+eBtt/Z/qxRx9pxpODvT2G3z4UFhYWCHtuBi5fvgx2apqWUaNGavqK2v21ggcUFJ4Th6FpUyapDHzh4kXU1taK7W/l6nWoratT6dMWwfNmDyxa8FZbXWS1aUUB7XGQkZmF5dGr2+um0s7gx8KIufD0vFmlTVsE2UtAHUaCAwMI1vrPOarzDvcZN3aMToXnMfSiAMbzXnj+GXTrpr4jGzwoBOMffoh51GnRiwJYgh5OTpj35utqefOgwAGE/z2tdfyvJU3qxQfcOHAZHYU/Wb2WgJOiG8lSfXjoMMx4agrtHOYSTZcVvSuAham/dg2bt8Ti94RESTYbG2tMfXISQofdJtH0UekUBSgFY+g89rs4uLn1xrgHx8DevquySW+/naoAvUnZxkB6c4Jt8NCpTSYFdKr6DWDwDltAQ0Mjjh0/ifQjGWBsUFflfFERODTOyzsrDVFRUYnsnFzpuZ6AmRMnT3UIcu9QOMwBzocfrSDBq2FHGGBlVRVeCnuGQuEQiSltVDZs/AaHUtLg4XGTSLj08/XFrJkvIjX9MIGxu7BqxVKBKzAkn5uXT3HDPI2H7ZACNm2OFZcZoiLnw5ouNTDau/7zjVi29H1crb2KSpohOzs7nKVtjpnmCxDKwtgBzyBjCV272lGIfAWlZWXo5eKCMzk56EOQWq9eLigimCwh8QDmz52Dfn19UFpahrkRC8nqTig/JX7j4nciM+s4IubNaTZOs05tPGisAAY3+FbH1MmPC+H526PvH4mdu36mVHi2SITE0CHHxbkneJn8RRjA4kUR4ij8+YavxZLp2cNJoMVRkRHIzc8X0FcfyiU2NV0nwYso/J0vhOFLEympaXB3dxVKWfdpNCyIVkLK4JKSli4s4dWXw9BRzFBjH8D5PVbCjYENAx8c8FRV/SUY4z8L5ofjnagFQpB9dOLjmU88kIRIokdRmsy1d2/8smev6N/Q0IDXX3uF6Cy4o1jP/E1GlY9kZOLV2eGIXrUGZWQpyosSdYQrfEam70hocf/+ftK4mlY0VoBC8c89ntra/4ANFoATowprhRifESCFQgGeQR8vTzLxchQSaMLx/ScEikRELhYmXkaZIjP6x4UF5sLoEjs1LgyvLXl/MebMnolGsqa3310ilg+38Zh33TEC1+lfzL/IMdM1LRovAYXCSpgbz8ywoUPEeMp16evtTevxWDMeKigRwibPCuHZmzXzBVhZWgnGrSjbc/KUKhzOH2BInBMrbEn+NMPeXl4Ie3mWBKJyAubJSRPFzZGPlq9ECF2lGXLL4GZjq/OgsQL4oxMnjMey6FVY95k5nJ17CJCT/YDyLgDf6NhEfoADHN6ewt+YJYANPuszzs+MJlHK/B5KkXUxa9kI/f38sGXrd1i6LBpBgQG07eUJ6/D29kT64QwpVOa2kffeJRK0PAFKHtQRnvuYL6KibmdlP0548OUl9sx8BuAs0AOj7xPNnC3KpT2bEWEOeR98YJTYHi1pWQy5dTBKSkpxlvoM8PcjwHSYgMl5yfAdIC41NVfhRRAYO7XQ0KGEJ9aJJcROddqUyXDuyc61ATa2Ngjw7y/eYdSYcUcubjfkHQShnT9aD4YS/tiP7TviseLjD9oZ2jCaW7Y/GbzZkzPz8NBNGksGW62+qnULaHUkA23QugUYqJytsmVSQKuqMZIGkwUYyUS3KqbJAlpVjZE0mCzASCa6VTH/Bnoy/0KF7w+OAAAAAElFTkSuQmCC", + "matcherProtection": 1, + "supportedExtensions": [ { - "id" : "hmac-secret", - "fail_if_unknown" : false + "id": "hmac-secret", + "fail_if_unknown": false }, { - "id" : "credProtect", - "fail_if_unknown" : false + "id": "credProtect", + "fail_if_unknown": false } ], - "cryptoStrength" : 128, - "description" : "OpenSK authenticator", - "authenticatorVersion" : 1, - "isSecondFactorOnly" : false, - "userVerificationDetails" : [ + "cryptoStrength": 128, + "description": "OpenSK authenticator", + "authenticatorVersion": 1, + "isSecondFactorOnly": false, + "userVerificationDetails": [ [ { - "userVerification" : 1 + "userVerification": 1 }, { - "userVerification" : 4 + "userVerification": 4 } ] ], - "attachmentHint" : 6, - "attestationTypes" : [ + "attachmentHint": 6, + "attestationTypes": [ 15880 ], - "authenticationAlgorithm" : 1, - "tcDisplay" : 0 + "authenticationAlgorithm": 1, + "tcDisplay": 0 } From 29ee45de6cf809e1a78250476d914a5e34308163 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Tue, 24 Nov 2020 11:29:14 +0100 Subject: [PATCH 12/28] Do not crash in the driver for store errors We prefer to return those errors to the fuzzer which can then decide whether they are expected or not (e.g. when starting from a dirty storage, the store is expected to have errors). --- libraries/persistent_store/src/driver.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/libraries/persistent_store/src/driver.rs b/libraries/persistent_store/src/driver.rs index 15001cb..e529274 100644 --- a/libraries/persistent_store/src/driver.rs +++ b/libraries/persistent_store/src/driver.rs @@ -181,6 +181,12 @@ pub enum StoreInvariant { }, } +impl From for StoreInvariant { + fn from(error: StoreError) -> StoreInvariant { + StoreInvariant::StoreError(error) + } +} + impl StoreDriver { /// Provides read-only access to the storage. pub fn storage(&self) -> &BufferStorage { @@ -249,6 +255,10 @@ impl StoreDriverOff { } /// Powers on the store without interruption. + /// + /// # Panics + /// + /// Panics if the store cannot be powered on. pub fn power_on(self) -> Result { Ok(self .partial_power_on(StoreInterruption::none()) @@ -506,8 +516,8 @@ impl StoreDriverOn { /// Checks that the store and model are in sync. fn check_model(&self) -> Result<(), StoreInvariant> { let mut model_content = self.model.content().clone(); - for handle in self.store.iter().unwrap() { - let handle = handle.unwrap(); + for handle in self.store.iter()? { + let handle = handle?; let model_value = match model_content.remove(&handle.get_key()) { None => { return Err(StoreInvariant::OnlyInStore { @@ -516,7 +526,7 @@ impl StoreDriverOn { } Some(x) => x, }; - let store_value = handle.get_value(&self.store).unwrap().into_boxed_slice(); + let store_value = handle.get_value(&self.store)?.into_boxed_slice(); if store_value != model_value { return Err(StoreInvariant::DifferentValue { key: handle.get_key(), @@ -528,7 +538,7 @@ impl StoreDriverOn { if let Some(&key) = model_content.keys().next() { return Err(StoreInvariant::OnlyInModel { key }); } - let store_capacity = self.store.capacity().unwrap().remaining(); + let store_capacity = self.store.capacity()?.remaining(); let model_capacity = self.model.capacity().remaining(); if store_capacity != model_capacity { return Err(StoreInvariant::DifferentCapacity { @@ -544,8 +554,8 @@ impl StoreDriverOn { let format = self.model.format(); let storage = self.store.storage(); let num_words = format.page_size() / format.word_size(); - let head = self.store.head().unwrap(); - let tail = self.store.tail().unwrap(); + let head = self.store.head()?; + let tail = self.store.tail()?; for page in 0..format.num_pages() { // Check the erase cycle of the page. let store_erase = head.cycle(format) + (page < head.page(format)) as Nat; From 0b2ea7d98be2dabe7925ed8c98504631d261b09e Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Tue, 24 Nov 2020 15:14:03 +0100 Subject: [PATCH 13/28] makes HMAC secret output reproducible --- src/ctap/pin_protocol_v1.rs | 118 ++++++++++++++++++++++++++---------- 1 file changed, 85 insertions(+), 33 deletions(-) diff --git a/src/ctap/pin_protocol_v1.rs b/src/ctap/pin_protocol_v1.rs index c03b81b..564ca6d 100644 --- a/src/ctap/pin_protocol_v1.rs +++ b/src/ctap/pin_protocol_v1.rs @@ -19,7 +19,6 @@ use super::status_code::Ctap2StatusCode; use super::storage::PersistentStore; #[cfg(feature = "with_ctap2_1")] use alloc::string::String; -#[cfg(feature = "with_ctap2_1")] use alloc::vec; use alloc::vec::Vec; use arrayref::array_ref; @@ -74,10 +73,9 @@ fn encrypt_hmac_secret_output( let mut cred_random_secret = [0u8; 32]; cred_random_secret.copy_from_slice(cred_random); - // Initialization of 4 blocks in any case makes this function more readable. - let mut blocks = [[0u8; 16]; 4]; // With the if clause restriction above, block_len can only be 2 or 4. let block_len = salt_enc.len() / 16; + let mut blocks = vec![[0u8; 16]; block_len]; for i in 0..block_len { blocks[i].copy_from_slice(&salt_enc[16 * i..16 * (i + 1)]); } @@ -85,8 +83,8 @@ fn encrypt_hmac_secret_output( let mut decrypted_salt1 = [0u8; 32]; decrypted_salt1[..16].copy_from_slice(&blocks[0]); - let output1 = hmac_256::(&cred_random_secret, &decrypted_salt1[..]); decrypted_salt1[16..].copy_from_slice(&blocks[1]); + let output1 = hmac_256::(&cred_random_secret, &decrypted_salt1[..]); for i in 0..2 { blocks[i].copy_from_slice(&output1[16 * i..16 * (i + 1)]); } @@ -638,36 +636,52 @@ impl PinProtocolV1 { #[cfg(test)] mod test { use super::*; - use arrayref::array_refs; use crypto::rng256::ThreadRng256; // Stores a PIN hash corresponding to the dummy PIN "1234". fn set_standard_pin(persistent_store: &mut PersistentStore) { let mut pin = [0u8; 64]; - pin[0] = 0x31; - pin[1] = 0x32; - pin[2] = 0x33; - pin[3] = 0x34; + pin[..4].copy_from_slice(b"1234"); let mut pin_hash = [0u8; 16]; pin_hash.copy_from_slice(&Sha256::hash(&pin[..])[..16]); persistent_store.set_pin_hash(&pin_hash).unwrap(); } + // Encrypts the message with a zero IV and key derived from shared_secret. + fn encrypt_message(shared_secret: &[u8; 32], message: &[u8]) -> Vec { + assert!(message.len() % 16 == 0); + let block_len = message.len() / 16; + let mut blocks = vec![[0u8; 16]; block_len]; + for i in 0..block_len { + blocks[i][..].copy_from_slice(&message[i * 16..(i + 1) * 16]); + } + let aes_enc_key = crypto::aes256::EncryptionKey::new(shared_secret); + let iv = [0u8; 16]; + cbc_encrypt(&aes_enc_key, iv, &mut blocks); + blocks.iter().flatten().cloned().collect::>() + } + + // Decrypts the message with a zero IV and key derived from shared_secret. + fn decrypt_message(shared_secret: &[u8; 32], message: &[u8]) -> Vec { + assert!(message.len() % 16 == 0); + let block_len = message.len() / 16; + let mut blocks = vec![[0u8; 16]; block_len]; + for i in 0..block_len { + blocks[i][..].copy_from_slice(&message[i * 16..(i + 1) * 16]); + } + let aes_enc_key = crypto::aes256::EncryptionKey::new(shared_secret); + let aes_dec_key = crypto::aes256::DecryptionKey::new(&aes_enc_key); + let iv = [0u8; 16]; + cbc_decrypt(&aes_dec_key, iv, &mut blocks); + blocks.iter().flatten().cloned().collect::>() + } + // Fails on PINs bigger than 64 bytes. fn encrypt_pin(shared_secret: &[u8; 32], pin: Vec) -> Vec { assert!(pin.len() <= 64); let mut padded_pin = [0u8; 64]; padded_pin[..pin.len()].copy_from_slice(&pin[..]); - let aes_enc_key = crypto::aes256::EncryptionKey::new(shared_secret); - let mut blocks = [[0u8; 16]; 4]; - let (b0, b1, b2, b3) = array_refs!(&padded_pin, 16, 16, 16, 16); - blocks[0][..].copy_from_slice(b0); - blocks[1][..].copy_from_slice(b1); - blocks[2][..].copy_from_slice(b2); - blocks[3][..].copy_from_slice(b3); - let iv = [0u8; 16]; - cbc_encrypt(&aes_enc_key, iv, &mut blocks); - blocks.iter().flatten().cloned().collect::>() + encrypt_message(shared_secret, &padded_pin) } // Encrypts the dummy PIN "1234". @@ -677,22 +691,10 @@ mod test { // Encrypts the PIN hash corresponding to the dummy PIN "1234". fn encrypt_standard_pin_hash(shared_secret: &[u8; 32]) -> Vec { - let aes_enc_key = crypto::aes256::EncryptionKey::new(shared_secret); let mut pin = [0u8; 64]; - pin[0] = 0x31; - pin[1] = 0x32; - pin[2] = 0x33; - pin[3] = 0x34; + pin[..4].copy_from_slice(b"1234"); let pin_hash = Sha256::hash(&pin); - - let mut blocks = [[0u8; 16]; 1]; - blocks[0].copy_from_slice(&pin_hash[..16]); - let iv = [0u8; 16]; - cbc_encrypt(&aes_enc_key, iv, &mut blocks); - - let mut encrypted_pin_hash = Vec::with_capacity(16); - encrypted_pin_hash.extend(&blocks[0]); - encrypted_pin_hash + encrypt_message(shared_secret, &pin_hash[..16]) } #[test] @@ -1184,6 +1186,56 @@ mod test { output, Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_EXTENSION) ); + + let mut salt_enc = [0x00; 32]; + let cred_random = [0xC9; 32]; + + // Test values to check for reproducibility. + let salt1 = [0x01; 32]; + let salt2 = [0x02; 32]; + let expected_output1 = hmac_256::(&cred_random, &salt1); + let expected_output2 = hmac_256::(&cred_random, &salt2); + + let salt_enc1 = encrypt_message(&shared_secret, &salt1); + salt_enc.copy_from_slice(salt_enc1.as_slice()); + let output = encrypt_hmac_secret_output(&shared_secret, &salt_enc, &cred_random).unwrap(); + let output_dec = decrypt_message(&shared_secret, &output); + assert_eq!(&output_dec, &expected_output1); + + let salt_enc2 = &encrypt_message(&shared_secret, &salt2); + salt_enc.copy_from_slice(salt_enc2.as_slice()); + let output = encrypt_hmac_secret_output(&shared_secret, &salt_enc, &cred_random).unwrap(); + let output_dec = decrypt_message(&shared_secret, &output); + assert_eq!(&output_dec, &expected_output2); + + let mut salt_enc = [0x00; 64]; + let mut salt12 = [0x00; 64]; + salt12[..32].copy_from_slice(&salt1); + salt12[32..].copy_from_slice(&salt2); + let salt_enc12 = encrypt_message(&shared_secret, &salt12); + salt_enc.copy_from_slice(salt_enc12.as_slice()); + let output = encrypt_hmac_secret_output(&shared_secret, &salt_enc, &cred_random).unwrap(); + let output_dec = decrypt_message(&shared_secret, &output); + assert_eq!(&output_dec[..32], &expected_output1); + assert_eq!(&output_dec[32..], &expected_output2); + + let mut salt_enc = [0x00; 64]; + let mut salt02 = [0x00; 64]; + salt02[32..].copy_from_slice(&salt2); + let salt_enc02 = encrypt_message(&shared_secret, &salt02); + salt_enc.copy_from_slice(salt_enc02.as_slice()); + let output = encrypt_hmac_secret_output(&shared_secret, &salt_enc, &cred_random).unwrap(); + let output_dec = decrypt_message(&shared_secret, &output); + assert_eq!(&output_dec[32..], &expected_output2); + + let mut salt_enc = [0x00; 64]; + let mut salt10 = [0x00; 64]; + salt10[..32].copy_from_slice(&salt1); + let salt_enc10 = encrypt_message(&shared_secret, &salt10); + salt_enc.copy_from_slice(salt_enc10.as_slice()); + let output = encrypt_hmac_secret_output(&shared_secret, &salt_enc, &cred_random).unwrap(); + let output_dec = decrypt_message(&shared_secret, &output); + assert_eq!(&output_dec[..32], &expected_output1); } #[cfg(feature = "with_ctap2_1")] From 65f4f2de25b813ccf07871c0e57d8c84db8fd3f9 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Tue, 24 Nov 2020 18:11:18 +0100 Subject: [PATCH 14/28] moves shared precheck into helper function --- src/ctap/mod.rs | 69 +++++++++++++++++++++---------------------------- 1 file changed, 29 insertions(+), 40 deletions(-) diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index caea0a0..13fc951 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -344,6 +344,33 @@ where } } + fn pin_uv_auth_precheck( + &mut self, + pin_uv_auth_param: &Option>, + pin_uv_auth_protocol: Option, + cid: ChannelID, + ) -> Result<(), Ctap2StatusCode> { + if let Some(auth_param) = &pin_uv_auth_param { + // This case was added in FIDO 2.1. + if auth_param.is_empty() { + (self.check_user_presence)(cid)?; + if self.persistent_store.pin_hash()?.is_none() { + return Err(Ctap2StatusCode::CTAP2_ERR_PIN_NOT_SET); + } else { + return Err(Ctap2StatusCode::CTAP2_ERR_PIN_INVALID); + } + } + + match pin_uv_auth_protocol { + Some(CtapState::::PIN_PROTOCOL_VERSION) => Ok(()), + Some(_) => Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID), + None => Err(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER), + } + } else { + Ok(()) + } + } + fn process_make_credential( &mut self, make_credential_params: AuthenticatorMakeCredentialParameters, @@ -361,26 +388,7 @@ where pin_uv_auth_protocol, } = make_credential_params; - if let Some(auth_param) = &pin_uv_auth_param { - // This case was added in FIDO 2.1. - if auth_param.is_empty() { - (self.check_user_presence)(cid)?; - if self.persistent_store.pin_hash()?.is_none() { - return Err(Ctap2StatusCode::CTAP2_ERR_PIN_NOT_SET); - } else { - return Err(Ctap2StatusCode::CTAP2_ERR_PIN_INVALID); - } - } - - match pin_uv_auth_protocol { - Some(protocol) => { - if protocol != CtapState::::PIN_PROTOCOL_VERSION { - return Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID); - } - } - None => return Err(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER), - } - } + self.pin_uv_auth_precheck(&pin_uv_auth_param, pin_uv_auth_protocol, cid)?; if !pub_key_cred_params.contains(&ES256_CRED_PARAM) { return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM); @@ -564,26 +572,7 @@ where pin_uv_auth_protocol, } = get_assertion_params; - if let Some(auth_param) = &pin_uv_auth_param { - // This case was added in FIDO 2.1. - if auth_param.is_empty() { - (self.check_user_presence)(cid)?; - if self.persistent_store.pin_hash()?.is_none() { - return Err(Ctap2StatusCode::CTAP2_ERR_PIN_NOT_SET); - } else { - return Err(Ctap2StatusCode::CTAP2_ERR_PIN_INVALID); - } - } - - match pin_uv_auth_protocol { - Some(protocol) => { - if protocol != CtapState::::PIN_PROTOCOL_VERSION { - return Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID); - } - } - None => return Err(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER), - } - } + self.pin_uv_auth_precheck(&pin_uv_auth_param, pin_uv_auth_protocol, cid)?; let hmac_secret_input = extensions.map(|e| e.hmac_secret).flatten(); if hmac_secret_input.is_some() && !options.up { From 3dbfae972fa5721e12c48deea2c538136d8e5824 Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Wed, 25 Nov 2020 17:12:03 +0100 Subject: [PATCH 15/28] Always insert attestation material in the store --- src/ctap/key_material.rs | 7 +++-- src/ctap/storage.rs | 63 ++++++++++++++++------------------------ 2 files changed, 30 insertions(+), 40 deletions(-) diff --git a/src/ctap/key_material.rs b/src/ctap/key_material.rs index 1958040..f8a833f 100644 --- a/src/ctap/key_material.rs +++ b/src/ctap/key_material.rs @@ -12,10 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -pub const AAGUID: &[u8; 16] = include_bytes!(concat!(env!("OUT_DIR"), "/opensk_aaguid.bin")); +pub const ATTESTATION_PRIVATE_KEY_LENGTH: usize = 32; +pub const AAGUID_LENGTH: usize = 16; + +pub const AAGUID: &[u8; AAGUID_LENGTH] = include_bytes!(concat!(env!("OUT_DIR"), "/opensk_aaguid.bin")); pub const ATTESTATION_CERTIFICATE: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/opensk_cert.bin")); -pub const ATTESTATION_PRIVATE_KEY: &[u8; 32] = +pub const ATTESTATION_PRIVATE_KEY: &[u8; ATTESTATION_PRIVATE_KEY_LENGTH] = include_bytes!(concat!(env!("OUT_DIR"), "/opensk_pkey.bin")); diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 127dc23..5793e6c 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -15,9 +15,9 @@ #[cfg(feature = "with_ctap2_1")] use crate::ctap::data_formats::{extract_array, extract_text_string}; use crate::ctap::data_formats::{CredentialProtectionPolicy, PublicKeyCredentialSource}; +use crate::ctap::key_material; use crate::ctap::pin_protocol_v1::PIN_AUTH_LENGTH; use crate::ctap::status_code::Ctap2StatusCode; -use crate::ctap::{key_material, USE_BATCH_ATTESTATION}; use crate::embedded_flash::{self, StoreConfig, StoreEntry, StoreError}; use alloc::string::String; #[cfg(any(test, feature = "ram_storage", feature = "with_ctap2_1"))] @@ -76,8 +76,6 @@ const MIN_PIN_LENGTH_RP_IDS: usize = 9; const NUM_TAGS: usize = 10; const MAX_PIN_RETRIES: u8 = 8; -const ATTESTATION_PRIVATE_KEY_LENGTH: usize = 32; -const AAGUID_LENGTH: usize = 16; #[cfg(feature = "with_ctap2_1")] const DEFAULT_MIN_PIN_LENGTH: u8 = 4; // TODO(kaczmarczyck) use this for the minPinLength extension @@ -231,17 +229,16 @@ impl PersistentStore { }) .unwrap(); } - // The following 3 entries are meant to be written by vendor-specific commands. - if USE_BATCH_ATTESTATION { - if self.store.find_one(&Key::AttestationPrivateKey).is_none() { - self.set_attestation_private_key(key_material::ATTESTATION_PRIVATE_KEY) - .unwrap(); - } - if self.store.find_one(&Key::AttestationCertificate).is_none() { - self.set_attestation_certificate(key_material::ATTESTATION_CERTIFICATE) - .unwrap(); - } + // The following 2 entries are meant to be written by vendor-specific commands. + if self.store.find_one(&Key::AttestationPrivateKey).is_none() { + self.set_attestation_private_key(key_material::ATTESTATION_PRIVATE_KEY) + .unwrap(); } + if self.store.find_one(&Key::AttestationCertificate).is_none() { + self.set_attestation_certificate(key_material::ATTESTATION_CERTIFICATE) + .unwrap(); + } + if self.store.find_one(&Key::Aaguid).is_none() { self.set_aaguid(key_material::AAGUID).unwrap(); } @@ -525,20 +522,24 @@ impl PersistentStore { pub fn attestation_private_key( &self, - ) -> Result, Ctap2StatusCode> { + ) -> Result, Ctap2StatusCode> { let data = match self.store.find_one(&Key::AttestationPrivateKey) { None => return Ok(None), Some((_, entry)) => entry.data, }; - if data.len() != ATTESTATION_PRIVATE_KEY_LENGTH { + if data.len() != key_material::ATTESTATION_PRIVATE_KEY_LENGTH { return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR); } - Ok(Some(array_ref!(data, 0, ATTESTATION_PRIVATE_KEY_LENGTH))) + Ok(Some(array_ref!( + data, + 0, + key_material::ATTESTATION_PRIVATE_KEY_LENGTH + ))) } pub fn set_attestation_private_key( &mut self, - attestation_private_key: &[u8; ATTESTATION_PRIVATE_KEY_LENGTH], + attestation_private_key: &[u8; key_material::ATTESTATION_PRIVATE_KEY_LENGTH], ) -> Result<(), Ctap2StatusCode> { let entry = StoreEntry { tag: ATTESTATION_PRIVATE_KEY, @@ -576,19 +577,22 @@ impl PersistentStore { Ok(()) } - pub fn aaguid(&self) -> Result<[u8; AAGUID_LENGTH], Ctap2StatusCode> { + pub fn aaguid(&self) -> Result<[u8; key_material::AAGUID_LENGTH], Ctap2StatusCode> { let (_, entry) = self .store .find_one(&Key::Aaguid) .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?; let data = entry.data; - if data.len() != AAGUID_LENGTH { + if data.len() != key_material::AAGUID_LENGTH { return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR); } - Ok(*array_ref![data, 0, AAGUID_LENGTH]) + Ok(*array_ref![data, 0, key_material::AAGUID_LENGTH]) } - pub fn set_aaguid(&mut self, aaguid: &[u8; AAGUID_LENGTH]) -> Result<(), Ctap2StatusCode> { + pub fn set_aaguid( + &mut self, + aaguid: &[u8; key_material::AAGUID_LENGTH], + ) -> Result<(), Ctap2StatusCode> { let entry = StoreEntry { tag: AAGUID, data: aaguid, @@ -996,23 +1000,6 @@ mod test { let mut rng = ThreadRng256 {}; let mut persistent_store = PersistentStore::new(&mut rng); - // Make sure the attestation are absent. There is no batch attestation in tests. - assert!(persistent_store - .attestation_private_key() - .unwrap() - .is_none()); - assert!(persistent_store - .attestation_certificate() - .unwrap() - .is_none()); - - // Make sure the persistent keys are initialized. - persistent_store - .set_attestation_private_key(key_material::ATTESTATION_PRIVATE_KEY) - .unwrap(); - persistent_store - .set_attestation_certificate(key_material::ATTESTATION_CERTIFICATE) - .unwrap(); assert_eq!(&persistent_store.aaguid().unwrap(), key_material::AAGUID); // The persistent keys stay initialized and preserve their value after a reset. From 026b4a66ac1d983c09d1bf9385adf52ad94411ba Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Wed, 25 Nov 2020 17:26:08 +0100 Subject: [PATCH 16/28] Fix CTAP2 batch attestation --- src/ctap/mod.rs | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 13fc951..e92afb7 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -522,25 +522,27 @@ where let mut signature_data = auth_data.clone(); signature_data.extend(client_data_hash); - // We currently use the presence of the attestation private key in the persistent storage to - // decide whether batch attestation is needed. - let (signature, x5c) = match self.persistent_store.attestation_private_key()? { - Some(attestation_private_key) => { - let attestation_key = - crypto::ecdsa::SecKey::from_bytes(attestation_private_key).unwrap(); - let attestation_certificate = self - .persistent_store - .attestation_certificate()? - .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?; - ( - attestation_key.sign_rfc6979::(&signature_data), - Some(vec![attestation_certificate]), - ) - } - None => ( + + let (signature, x5c) = if USE_BATCH_ATTESTATION { + let attestation_private_key = self + .persistent_store + .attestation_private_key()? + .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?; + let attestation_key = + crypto::ecdsa::SecKey::from_bytes(attestation_private_key).unwrap(); + let attestation_certificate = self + .persistent_store + .attestation_certificate()? + .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?; + ( + attestation_key.sign_rfc6979::(&signature_data), + Some(vec![attestation_certificate]), + ) + } else { + ( sk.sign_rfc6979::(&signature_data), None, - ), + ) }; let attestation_statement = PackedAttestationStatement { alg: SignatureAlgorithm::ES256 as i64, From 41f7cc7b141db809d146d522be142635447b8d52 Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Wed, 25 Nov 2020 17:31:05 +0100 Subject: [PATCH 17/28] CTAP1/U2F accesses attestation material through the store. --- src/ctap/ctap1.rs | 42 +++++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/src/ctap/ctap1.rs b/src/ctap/ctap1.rs index a5a7921..9eb1186 100644 --- a/src/ctap/ctap1.rs +++ b/src/ctap/ctap1.rs @@ -13,7 +13,6 @@ // limitations under the License. use super::hid::ChannelID; -use super::key_material::{ATTESTATION_CERTIFICATE, ATTESTATION_PRIVATE_KEY}; use super::status_code::Ctap2StatusCode; use super::CtapState; use alloc::vec::Vec; @@ -295,14 +294,22 @@ impl Ctap1Command { return Err(Ctap1StatusCode::SW_VENDOR_KEY_HANDLE_TOO_LONG); } - let mut response = - Vec::with_capacity(105 + key_handle.len() + ATTESTATION_CERTIFICATE.len()); + let certificate = match ctap_state.persistent_store.attestation_certificate() { + Ok(Some(value)) => value, + _ => return Err(Ctap1StatusCode::SW_INS_NOT_SUPPORTED), + }; + let private_key = match ctap_state.persistent_store.attestation_private_key() { + Ok(Some(value)) => value, + _ => return Err(Ctap1StatusCode::SW_INS_NOT_SUPPORTED), + }; + + let mut response = Vec::with_capacity(105 + key_handle.len() + certificate.len()); response.push(Ctap1Command::LEGACY_BYTE); let user_pk = pk.to_uncompressed(); response.extend_from_slice(&user_pk); response.push(key_handle.len() as u8); response.extend(key_handle.clone()); - response.extend_from_slice(&ATTESTATION_CERTIFICATE); + response.extend_from_slice(&certificate); // The first byte is reserved. let mut signature_data = Vec::with_capacity(66 + key_handle.len()); @@ -312,7 +319,7 @@ impl Ctap1Command { signature_data.extend(key_handle); signature_data.extend_from_slice(&user_pk); - let attestation_key = crypto::ecdsa::SecKey::from_bytes(ATTESTATION_PRIVATE_KEY).unwrap(); + let attestation_key = crypto::ecdsa::SecKey::from_bytes(private_key).unwrap(); let signature = attestation_key.sign_rfc6979::(&signature_data); response.extend(signature.to_asn1_der()); @@ -373,7 +380,7 @@ impl Ctap1Command { #[cfg(test)] mod test { - use super::super::{CREDENTIAL_ID_BASE_SIZE, USE_SIGNATURE_COUNTER}; + use super::super::{key_material, CREDENTIAL_ID_BASE_SIZE, USE_SIGNATURE_COUNTER}; use super::*; use crypto::rng256::ThreadRng256; use crypto::Hash256; @@ -434,8 +441,25 @@ mod test { ctap_state.u2f_up_state.consume_up(START_CLOCK_VALUE); ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE); let response = - Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE).unwrap(); + Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE); + // Certificate and private key are missing + assert_eq!(response, Err(Ctap1StatusCode::SW_INS_NOT_SUPPORTED)); + let fake_key = [0x41u8; key_material::ATTESTATION_PRIVATE_KEY_LENGTH]; + assert!(ctap_state.persistent_store.set_attestation_private_key(&fake_key).is_ok()); + ctap_state.u2f_up_state.consume_up(START_CLOCK_VALUE); + ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE); + let response = + Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE); + // Certificate is still missing + assert_eq!(response, Err(Ctap1StatusCode::SW_INS_NOT_SUPPORTED)); + + let fake_cert = [0x99u8; 100]; // Arbitrary length + assert!(ctap_state.persistent_store.set_attestation_certificate(&fake_cert[..]).is_ok()); + ctap_state.u2f_up_state.consume_up(START_CLOCK_VALUE); + ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE); + let response = + Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE).unwrap(); assert_eq!(response[0], Ctap1Command::LEGACY_BYTE); assert_eq!(response[66], CREDENTIAL_ID_BASE_SIZE as u8); assert!(ctap_state @@ -447,8 +471,8 @@ mod test { .is_some()); const CERT_START: usize = 67 + CREDENTIAL_ID_BASE_SIZE; assert_eq!( - &response[CERT_START..CERT_START + ATTESTATION_CERTIFICATE.len()], - &ATTESTATION_CERTIFICATE[..] + &response[CERT_START..CERT_START + fake_cert.len()], + &fake_cert[..] ); } From f47e1e2a8663e60a3c9b61db246cebfeb193d8f5 Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Wed, 25 Nov 2020 17:44:19 +0100 Subject: [PATCH 18/28] Ensure store behaves as expected in prod --- src/ctap/storage.rs | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 5793e6c..da8828c 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -229,6 +229,18 @@ impl PersistentStore { }) .unwrap(); } + // TODO(jmichel): remove this when vendor command is in place + #[cfg(not(any(test, feature = "ram_storage")))] + self.load_attestation_from_firmware(); + + if self.store.find_one(&Key::Aaguid).is_none() { + self.set_aaguid(key_material::AAGUID).unwrap(); + } + } + + // TODO(jmichel): remove this function when vendor command is in place. + #[cfg(not(any(test, feature = "ram_storage")))] + fn load_attestation_from_firmware(&mut self) { // The following 2 entries are meant to be written by vendor-specific commands. if self.store.find_one(&Key::AttestationPrivateKey).is_none() { self.set_attestation_private_key(key_material::ATTESTATION_PRIVATE_KEY) @@ -238,10 +250,6 @@ impl PersistentStore { self.set_attestation_certificate(key_material::ATTESTATION_CERTIFICATE) .unwrap(); } - - if self.store.find_one(&Key::Aaguid).is_none() { - self.set_aaguid(key_material::AAGUID).unwrap(); - } } pub fn find_credential( @@ -1000,6 +1008,23 @@ mod test { let mut rng = ThreadRng256 {}; let mut persistent_store = PersistentStore::new(&mut rng); + // Make sure the attestation are absent. There is no batch attestation in tests. + assert!(persistent_store + .attestation_private_key() + .unwrap() + .is_none()); + assert!(persistent_store + .attestation_certificate() + .unwrap() + .is_none()); + + // Make sure the persistent keys are initialized. + persistent_store + .set_attestation_private_key(key_material::ATTESTATION_PRIVATE_KEY) + .unwrap(); + persistent_store + .set_attestation_certificate(key_material::ATTESTATION_CERTIFICATE) + .unwrap(); assert_eq!(&persistent_store.aaguid().unwrap(), key_material::AAGUID); // The persistent keys stay initialized and preserve their value after a reset. From f2b3ca4029227e532eb88972c766fd8dc99aba5d Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Wed, 25 Nov 2020 17:44:52 +0100 Subject: [PATCH 19/28] Make private key sensitive and ensure attestation is OTP --- src/ctap/storage.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index da8828c..8e396b6 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -552,11 +552,11 @@ impl PersistentStore { let entry = StoreEntry { tag: ATTESTATION_PRIVATE_KEY, data: attestation_private_key, - sensitive: false, + sensitive: true, }; match self.store.find_one(&Key::AttestationPrivateKey) { None => self.store.insert(entry)?, - Some((index, _)) => self.store.replace(index, entry)?, + _ => return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR), } Ok(()) } @@ -580,7 +580,7 @@ impl PersistentStore { }; match self.store.find_one(&Key::AttestationCertificate) { None => self.store.insert(entry)?, - Some((index, _)) => self.store.replace(index, entry)?, + _ => return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR), } Ok(()) } From d491492554e5e86a40d1d71b524fec569144217f Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Wed, 25 Nov 2020 17:48:47 +0100 Subject: [PATCH 20/28] Format --- src/ctap/ctap1.rs | 16 ++++++++++------ src/ctap/key_material.rs | 3 ++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/ctap/ctap1.rs b/src/ctap/ctap1.rs index 9eb1186..0fa4745 100644 --- a/src/ctap/ctap1.rs +++ b/src/ctap/ctap1.rs @@ -440,22 +440,26 @@ mod test { let message = create_register_message(&application); ctap_state.u2f_up_state.consume_up(START_CLOCK_VALUE); ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE); - let response = - Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE); + let response = Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE); // Certificate and private key are missing assert_eq!(response, Err(Ctap1StatusCode::SW_INS_NOT_SUPPORTED)); let fake_key = [0x41u8; key_material::ATTESTATION_PRIVATE_KEY_LENGTH]; - assert!(ctap_state.persistent_store.set_attestation_private_key(&fake_key).is_ok()); + assert!(ctap_state + .persistent_store + .set_attestation_private_key(&fake_key) + .is_ok()); ctap_state.u2f_up_state.consume_up(START_CLOCK_VALUE); ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE); - let response = - Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE); + let response = Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE); // Certificate is still missing assert_eq!(response, Err(Ctap1StatusCode::SW_INS_NOT_SUPPORTED)); let fake_cert = [0x99u8; 100]; // Arbitrary length - assert!(ctap_state.persistent_store.set_attestation_certificate(&fake_cert[..]).is_ok()); + assert!(ctap_state + .persistent_store + .set_attestation_certificate(&fake_cert[..]) + .is_ok()); ctap_state.u2f_up_state.consume_up(START_CLOCK_VALUE); ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE); let response = diff --git a/src/ctap/key_material.rs b/src/ctap/key_material.rs index f8a833f..2563798 100644 --- a/src/ctap/key_material.rs +++ b/src/ctap/key_material.rs @@ -15,7 +15,8 @@ pub const ATTESTATION_PRIVATE_KEY_LENGTH: usize = 32; pub const AAGUID_LENGTH: usize = 16; -pub const AAGUID: &[u8; AAGUID_LENGTH] = include_bytes!(concat!(env!("OUT_DIR"), "/opensk_aaguid.bin")); +pub const AAGUID: &[u8; AAGUID_LENGTH] = + include_bytes!(concat!(env!("OUT_DIR"), "/opensk_aaguid.bin")); pub const ATTESTATION_CERTIFICATE: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/opensk_cert.bin")); From 3ae59ce1ecfdfaa7a2e84f86d0c8050d2b1e08e5 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Thu, 17 Sep 2020 09:23:17 +0200 Subject: [PATCH 21/28] GetNextAssertion command minimal implementation This still lacks order of credentials and timeouts. --- src/ctap/command.rs | 2 +- src/ctap/data_formats.rs | 6 +- src/ctap/mod.rs | 168 +++++++++++++++++++++++++-------------- 3 files changed, 112 insertions(+), 64 deletions(-) diff --git a/src/ctap/command.rs b/src/ctap/command.rs index 5c58234..1b4b96a 100644 --- a/src/ctap/command.rs +++ b/src/ctap/command.rs @@ -57,8 +57,8 @@ impl Command { const AUTHENTICATOR_GET_INFO: u8 = 0x04; const AUTHENTICATOR_CLIENT_PIN: u8 = 0x06; const AUTHENTICATOR_RESET: u8 = 0x07; - // TODO(kaczmarczyck) use or remove those constants const AUTHENTICATOR_GET_NEXT_ASSERTION: u8 = 0x08; + // TODO(kaczmarczyck) use or remove those constants const AUTHENTICATOR_BIO_ENROLLMENT: u8 = 0x09; const AUTHENTICATOR_CREDENTIAL_MANAGEMENT: u8 = 0xA0; const AUTHENTICATOR_SELECTION: u8 = 0xB0; diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index 45ebf9f..35d5bcf 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -308,7 +308,8 @@ impl TryFrom for GetAssertionExtensions { } } -#[cfg_attr(any(test, feature = "debug_ctap"), derive(Clone, Debug, PartialEq))] +#[derive(Clone)] +#[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))] pub struct GetAssertionHmacSecretInput { pub key_agreement: CoseKey, pub salt_enc: Vec, @@ -603,7 +604,8 @@ impl PublicKeyCredentialSource { // TODO(kaczmarczyck) we could decide to split this data type up // It depends on the algorithm though, I think. // So before creating a mess, this is my workaround. -#[cfg_attr(any(test, feature = "debug_ctap"), derive(Clone, Debug, PartialEq))] +#[derive(Clone)] +#[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))] pub struct CoseKey(pub BTreeMap); // This is the algorithm specifier that is supposed to be used in a COSE key diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 13fc951..f037655 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -33,9 +33,9 @@ use self::command::{ #[cfg(feature = "with_ctap2_1")] use self::data_formats::AuthenticatorTransport; use self::data_formats::{ - CredentialProtectionPolicy, PackedAttestationStatement, PublicKeyCredentialDescriptor, - PublicKeyCredentialParameter, PublicKeyCredentialSource, PublicKeyCredentialType, - PublicKeyCredentialUserEntity, SignatureAlgorithm, + CredentialProtectionPolicy, GetAssertionHmacSecretInput, PackedAttestationStatement, + PublicKeyCredentialDescriptor, PublicKeyCredentialParameter, PublicKeyCredentialSource, + PublicKeyCredentialType, PublicKeyCredentialUserEntity, SignatureAlgorithm, }; use self::hid::ChannelID; #[cfg(feature = "with_ctap2_1")] @@ -133,6 +133,14 @@ fn truncate_to_char_boundary(s: &str, mut max: usize) -> &str { } } +#[derive(Clone)] +struct AssertionInput { + client_data_hash: Vec, + auth_data: Vec, + uv: bool, + hmac_secret_input: Option, +} + // This struct currently holds all state, not only the persistent memory. The persistent members are // in the persistent store field. pub struct CtapState<'a, R: Rng256, CheckUserPresence: Fn(ChannelID) -> Result<(), Ctap2StatusCode>> @@ -147,6 +155,8 @@ pub struct CtapState<'a, R: Rng256, CheckUserPresence: Fn(ChannelID) -> Result<( accepts_reset: bool, #[cfg(feature = "with_ctap1")] pub u2f_up_state: U2fUserPresenceState, + next_credentials: Vec, + next_assertion_input: Option, } impl<'a, R, CheckUserPresence> CtapState<'a, R, CheckUserPresence> @@ -173,6 +183,8 @@ where U2F_UP_PROMPT_TIMEOUT, Duration::from_ms(TOUCH_TIMEOUT_MS), ), + next_credentials: vec![], + next_assertion_input: None, } } @@ -314,13 +326,13 @@ where Command::AuthenticatorGetAssertion(params) => { self.process_get_assertion(params, cid) } + Command::AuthenticatorGetNextAssertion => self.process_get_next_assertion(), Command::AuthenticatorGetInfo => self.process_get_info(), Command::AuthenticatorClientPin(params) => self.process_client_pin(params), Command::AuthenticatorReset => self.process_reset(cid), #[cfg(feature = "with_ctap2_1")] Command::AuthenticatorSelection => self.process_selection(cid), - // TODO(kaczmarczyck) implement GetNextAssertion and FIDO 2.1 commands - _ => self.process_unknown_command(), + // TODO(kaczmarczyck) implement FIDO 2.1 commands }; #[cfg(feature = "debug_ctap")] writeln!(&mut Console::new(), "Sending response: {:#?}", response).unwrap(); @@ -557,6 +569,63 @@ where )) } + fn assertion_response( + &self, + credential: &PublicKeyCredentialSource, + assertion_input: AssertionInput, + ) -> Result { + let AssertionInput { + client_data_hash, + mut auth_data, + uv, + hmac_secret_input, + } = assertion_input; + + // Process extensions. + if let Some(hmac_secret_input) = hmac_secret_input { + let encrypted_output = self + .pin_protocol_v1 + .process_hmac_secret(hmac_secret_input, &credential.cred_random)?; + let extensions_output = cbor_map! { + "hmac-secret" => encrypted_output, + }; + if !cbor::write(extensions_output, &mut auth_data) { + return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_RESPONSE_CANNOT_WRITE_CBOR); + } + } + + let mut signature_data = auth_data.clone(); + signature_data.extend(client_data_hash); + let signature = credential + .private_key + .sign_rfc6979::(&signature_data); + + let cred_desc = PublicKeyCredentialDescriptor { + key_type: PublicKeyCredentialType::PublicKey, + key_id: credential.credential_id.clone(), + transports: None, // You can set USB as a hint here. + }; + let user = if uv && !credential.user_handle.is_empty() { + Some(PublicKeyCredentialUserEntity { + user_id: credential.user_handle.clone(), + user_name: None, + user_display_name: credential.other_ui.clone(), + user_icon: None, + }) + } else { + None + }; + Ok(ResponseData::AuthenticatorGetAssertion( + AuthenticatorGetAssertionResponse { + credential: Some(cred_desc), + auth_data, + signature: signature.to_asn1_der(), + user, + number_of_credentials: None, + }, + )) + } + fn process_get_assertion( &mut self, get_assertion_params: AuthenticatorGetAssertionParameters, @@ -643,17 +712,19 @@ where } found_credentials } else { - // TODO(kaczmarczyck) use GetNextAssertion self.persistent_store.filter_credential(&rp_id, !has_uv)? }; - let credential = if let Some(credential) = credentials.first() { - credential - } else { - decrypted_credential - .as_ref() - .ok_or(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS)? - }; + let credential = + if let Some((credential, remaining_credentials)) = credentials.split_first() { + // TODO(kaczmarczyck) correct credential order + self.next_credentials = remaining_credentials.to_vec(); + credential + } else { + decrypted_credential + .as_ref() + .ok_or(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS)? + }; if options.up { (self.check_user_presence)(cid)?; @@ -661,50 +732,30 @@ where self.increment_global_signature_counter()?; - let mut auth_data = self.generate_auth_data(&rp_id_hash, flags)?; - // Process extensions. - if let Some(hmac_secret_input) = hmac_secret_input { - let encrypted_output = self - .pin_protocol_v1 - .process_hmac_secret(hmac_secret_input, &credential.cred_random)?; - let extensions_output = cbor_map! { - "hmac-secret" => encrypted_output, - }; - if !cbor::write(extensions_output, &mut auth_data) { - return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_RESPONSE_CANNOT_WRITE_CBOR); - } + let assertion_input = AssertionInput { + client_data_hash, + auth_data: self.generate_auth_data(&rp_id_hash, flags)?, + uv: flags & UV_FLAG != 0, + hmac_secret_input, + }; + if !self.next_credentials.is_empty() { + self.next_assertion_input = Some(assertion_input.clone()); } + self.assertion_response(credential, assertion_input) + } - let mut signature_data = auth_data.clone(); - signature_data.extend(client_data_hash); - let signature = credential - .private_key - .sign_rfc6979::(&signature_data); - - let cred_desc = PublicKeyCredentialDescriptor { - key_type: PublicKeyCredentialType::PublicKey, - key_id: credential.credential_id.clone(), - transports: None, // You can set USB as a hint here. - }; - let user = if (flags & UV_FLAG != 0) && !credential.user_handle.is_empty() { - Some(PublicKeyCredentialUserEntity { - user_id: credential.user_handle.clone(), - user_name: None, - user_display_name: credential.other_ui.clone(), - user_icon: None, - }) + fn process_get_next_assertion(&mut self) -> Result { + // TODO(kaczmarczyck) introduce timer + let assertion_input = self + .next_assertion_input + .clone() + .ok_or(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED)?; + if let Some(credential) = self.next_credentials.pop() { + self.assertion_response(&credential, assertion_input) } else { - None - }; - Ok(ResponseData::AuthenticatorGetAssertion( - AuthenticatorGetAssertionResponse { - credential: Some(cred_desc), - auth_data, - signature: signature.to_asn1_der(), - user, - number_of_credentials: None, - }, - )) + self.next_assertion_input = None; + Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED) + } } fn process_get_info(&self) -> Result { @@ -786,10 +837,6 @@ where Ok(ResponseData::AuthenticatorSelection) } - fn process_unknown_command(&self) -> Result { - Err(Ctap2StatusCode::CTAP1_ERR_INVALID_COMMAND) - } - pub fn generate_auth_data( &self, rp_id_hash: &[u8], @@ -813,9 +860,8 @@ where #[cfg(test)] mod test { use super::data_formats::{ - CoseKey, GetAssertionExtensions, GetAssertionHmacSecretInput, GetAssertionOptions, - MakeCredentialExtensions, MakeCredentialOptions, PublicKeyCredentialRpEntity, - PublicKeyCredentialUserEntity, + CoseKey, GetAssertionExtensions, GetAssertionOptions, MakeCredentialExtensions, + MakeCredentialOptions, PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity, }; use super::*; use crypto::rng256::ThreadRng256; From af4eef808519b10055a92cce3aa8a97960df3d2a Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Tue, 17 Nov 2020 16:57:03 +0100 Subject: [PATCH 22/28] adds credential ordering --- src/ctap/data_formats.rs | 7 +++++++ src/ctap/mod.rs | 11 +++++++++-- src/ctap/storage.rs | 25 +++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index 35d5bcf..fa747bc 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -500,6 +500,7 @@ pub struct PublicKeyCredentialSource { pub other_ui: Option, pub cred_random: Option>, pub cred_protect_policy: Option, + pub creation_order: u64, } // We serialize credentials for the persistent storage using CBOR maps. Each field of a credential @@ -512,6 +513,7 @@ enum PublicKeyCredentialSourceField { OtherUi = 4, CredRandom = 5, CredProtectPolicy = 6, + CreationOrder = 7, // When a field is removed, its tag should be reserved and not used for new fields. We document // those reserved tags below. // Reserved tags: none. @@ -535,6 +537,7 @@ impl From for cbor::Value { PublicKeyCredentialSourceField::OtherUi => credential.other_ui, PublicKeyCredentialSourceField::CredRandom => credential.cred_random, PublicKeyCredentialSourceField::CredProtectPolicy => credential.cred_protect_policy, + PublicKeyCredentialSourceField::CreationOrder => credential.creation_order, } } } @@ -552,6 +555,7 @@ impl TryFrom for PublicKeyCredentialSource { PublicKeyCredentialSourceField::OtherUi => other_ui, PublicKeyCredentialSourceField::CredRandom => cred_random, PublicKeyCredentialSourceField::CredProtectPolicy => cred_protect_policy, + PublicKeyCredentialSourceField::CreationOrder => creation_order, } = extract_map(cbor_value)?; } @@ -569,6 +573,7 @@ impl TryFrom for PublicKeyCredentialSource { let cred_protect_policy = cred_protect_policy .map(CredentialProtectionPolicy::try_from) .transpose()?; + let creation_order = creation_order.map(extract_unsigned).unwrap_or(Ok(0))?; // We don't return whether there were unknown fields in the CBOR value. This means that // deserialization is not injective. In particular deserialization is only an inverse of // serialization at a given version of OpenSK. This is not a problem because: @@ -588,6 +593,7 @@ impl TryFrom for PublicKeyCredentialSource { other_ui, cred_random, cred_protect_policy, + creation_order, }) } } @@ -1357,6 +1363,7 @@ mod test { other_ui: None, cred_random: None, cred_protect_policy: None, + creation_order: 0, }; assert_eq!( diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index f037655..331ccc1 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -155,6 +155,7 @@ pub struct CtapState<'a, R: Rng256, CheckUserPresence: Fn(ChannelID) -> Result<( accepts_reset: bool, #[cfg(feature = "with_ctap1")] pub u2f_up_state: U2fUserPresenceState, + // Sorted by ascending order of creation, so the last element is the most recent one. next_credentials: Vec, next_assertion_input: Option, } @@ -302,6 +303,7 @@ where other_ui: None, cred_random, cred_protect_policy: None, + creation_order: 0, })) } @@ -424,7 +426,6 @@ where } else { None }; - // TODO(kaczmarczyck) unsolicited output for default credProtect level let has_extension_output = use_hmac_extension || cred_protect_policy.is_some(); let rp_id = rp.rp_id; @@ -501,6 +502,7 @@ where .map(|s| truncate_to_char_boundary(&s, 64).to_string()), cred_random: cred_random.map(|c| c.to_vec()), cred_protect_policy, + creation_order: self.persistent_store.new_creation_order()?, }; self.persistent_store.store_credential(credential_source)?; random_id @@ -717,8 +719,9 @@ where let credential = if let Some((credential, remaining_credentials)) = credentials.split_first() { - // TODO(kaczmarczyck) correct credential order self.next_credentials = remaining_credentials.to_vec(); + self.next_credentials + .sort_unstable_by_key(|c| c.creation_order); credential } else { decrypted_credential @@ -1091,6 +1094,7 @@ mod test { other_ui: None, cred_random: None, cred_protect_policy: None, + creation_order: 0, }; assert!(ctap_state .persistent_store @@ -1457,6 +1461,7 @@ mod test { cred_protect_policy: Some( CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList, ), + creation_order: 0, }; assert!(ctap_state .persistent_store @@ -1507,6 +1512,7 @@ mod test { other_ui: None, cred_random: None, cred_protect_policy: Some(CredentialProtectionPolicy::UserVerificationRequired), + creation_order: 0, }; assert!(ctap_state .persistent_store @@ -1550,6 +1556,7 @@ mod test { other_ui: None, cred_random: None, cred_protect_policy: None, + creation_order: 0, }; assert!(ctap_state .persistent_store diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 127dc23..6e4506a 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -337,6 +337,26 @@ impl PersistentStore { .count()) } + pub fn new_creation_order(&self) -> Result { + Ok(self + .store + .find_all(&Key::Credential { + rp_id: None, + credential_id: None, + user_handle: None, + }) + .filter_map(|(_, entry)| { + debug_assert_eq!(entry.tag, TAG_CREDENTIAL); + let credential = deserialize_credential(entry.data); + debug_assert!(credential.is_some()); + credential + }) + .map(|c| c.creation_order) + .max() + .unwrap_or(0) + + 1) + } + pub fn global_signature_counter(&self) -> Result { Ok(self .store @@ -690,6 +710,7 @@ mod test { other_ui: None, cred_random: None, cred_protect_policy: None, + creation_order: 0, } } @@ -853,6 +874,7 @@ mod test { cred_protect_policy: Some( CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList, ), + creation_order: 0, }; assert!(persistent_store.store_credential(credential).is_ok()); @@ -894,6 +916,7 @@ mod test { other_ui: None, cred_random: None, cred_protect_policy: None, + creation_order: 0, }; assert_eq!(found_credential, Some(expected_credential)); } @@ -913,6 +936,7 @@ mod test { other_ui: None, cred_random: None, cred_protect_policy: Some(CredentialProtectionPolicy::UserVerificationRequired), + creation_order: 0, }; assert!(persistent_store.store_credential(credential).is_ok()); @@ -1090,6 +1114,7 @@ mod test { other_ui: None, cred_random: None, cred_protect_policy: None, + creation_order: 0, }; let serialized = serialize_credential(credential.clone()).unwrap(); let reconstructed = deserialize_credential(&serialized).unwrap(); From 5ff381678251473c0417b77f4b9a657d95d870a4 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Mon, 23 Nov 2020 17:33:20 +0100 Subject: [PATCH 23/28] sets the correct user and number of credentials --- src/ctap/mod.rs | 209 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 162 insertions(+), 47 deletions(-) diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 331ccc1..1f91f4f 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -137,7 +137,6 @@ fn truncate_to_char_boundary(s: &str, mut max: usize) -> &str { struct AssertionInput { client_data_hash: Vec, auth_data: Vec, - uv: bool, hmac_secret_input: Option, } @@ -571,15 +570,17 @@ where )) } + // Processes the input of a get_assertion operation for a given credential + // and returns the correct Get(Next)Assertion response. fn assertion_response( &self, credential: &PublicKeyCredentialSource, assertion_input: AssertionInput, + number_of_credentials: Option, ) -> Result { let AssertionInput { client_data_hash, mut auth_data, - uv, hmac_secret_input, } = assertion_input; @@ -607,7 +608,7 @@ where key_id: credential.credential_id.clone(), transports: None, // You can set USB as a hint here. }; - let user = if uv && !credential.user_handle.is_empty() { + let user = if !credential.user_handle.is_empty() { Some(PublicKeyCredentialUserEntity { user_id: credential.user_handle.clone(), user_name: None, @@ -623,11 +624,37 @@ where auth_data, signature: signature.to_asn1_der(), user, - number_of_credentials: None, + number_of_credentials: number_of_credentials.map(|n| n as u64), }, )) } + // Returns the first applicable credential from the allow list. + fn get_any_credential_from_allow_list( + &mut self, + allow_list: Vec, + rp_id: &str, + rp_id_hash: &[u8], + has_uv: bool, + ) -> Result, Ctap2StatusCode> { + for allowed_credential in allow_list { + let credential = self.persistent_store.find_credential( + rp_id, + &allowed_credential.key_id, + !has_uv, + )?; + if credential.is_some() { + return Ok(credential); + } + let credential = + self.decrypt_credential_source(allowed_credential.key_id, &rp_id_hash)?; + if credential.is_some() { + return Ok(credential); + } + } + Ok(None) + } + fn process_get_assertion( &mut self, get_assertion_params: AuthenticatorGetAssertionParameters, @@ -690,44 +717,29 @@ where } let rp_id_hash = Sha256::hash(rp_id.as_bytes()); - let mut decrypted_credential = None; - let credentials = if let Some(allow_list) = allow_list { - let mut found_credentials = vec![]; - for allowed_credential in allow_list { - match self.persistent_store.find_credential( - &rp_id, - &allowed_credential.key_id, - !has_uv, - )? { - Some(credential) => { - found_credentials.push(credential); - } - None => { - if decrypted_credential.is_none() { - decrypted_credential = self.decrypt_credential_source( - allowed_credential.key_id, - &rp_id_hash, - )?; - } - } - } + let mut credentials = if let Some(allow_list) = allow_list { + if let Some(credential) = + self.get_any_credential_from_allow_list(allow_list, &rp_id, &rp_id_hash, has_uv)? + { + vec![credential] + } else { + vec![] } - found_credentials } else { self.persistent_store.filter_credential(&rp_id, !has_uv)? }; + // Remove user identifiable information without uv. + if !has_uv { + for credential in &mut credentials { + credential.other_ui = None; + } + } + credentials.sort_unstable_by_key(|c| c.creation_order); - let credential = - if let Some((credential, remaining_credentials)) = credentials.split_first() { - self.next_credentials = remaining_credentials.to_vec(); - self.next_credentials - .sort_unstable_by_key(|c| c.creation_order); - credential - } else { - decrypted_credential - .as_ref() - .ok_or(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS)? - }; + let (credential, remaining_credentials) = credentials + .split_last() + .ok_or(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS)?; + self.next_credentials = remaining_credentials.to_vec(); if options.up { (self.check_user_presence)(cid)?; @@ -738,13 +750,16 @@ where let assertion_input = AssertionInput { client_data_hash, auth_data: self.generate_auth_data(&rp_id_hash, flags)?, - uv: flags & UV_FLAG != 0, hmac_secret_input, }; - if !self.next_credentials.is_empty() { + let number_of_credentials = if self.next_credentials.is_empty() { + self.next_assertion_input = None; + None + } else { self.next_assertion_input = Some(assertion_input.clone()); - } - self.assertion_response(credential, assertion_input) + Some(self.next_credentials.len() + 1) + }; + self.assertion_response(credential, assertion_input, number_of_credentials) } fn process_get_next_assertion(&mut self) -> Result { @@ -753,12 +768,14 @@ where .next_assertion_input .clone() .ok_or(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED)?; - if let Some(credential) = self.next_credentials.pop() { - self.assertion_response(&credential, assertion_input) - } else { + let credential = self + .next_credentials + .pop() + .ok_or(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED)?; + if self.next_credentials.is_empty() { self.next_assertion_input = None; - Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED) - } + }; + self.assertion_response(&credential, assertion_input, None) } fn process_get_info(&self) -> Result { @@ -1318,7 +1335,13 @@ mod test { 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0x00, 0x00, 0x00, 0x00, 0x01, ]; assert_eq!(auth_data, expected_auth_data); - assert!(user.is_none()); + let expected_user = PublicKeyCredentialUserEntity { + user_id: vec![0xFA, 0xB1, 0xA2], + user_name: None, + user_display_name: None, + user_icon: None, + }; + assert_eq!(user, Some(expected_user)); assert!(number_of_credentials.is_none()); } _ => panic!("Invalid response type"), @@ -1539,6 +1562,98 @@ mod test { ); } + #[test] + fn test_process_get_next_assertion() { + let mut rng = ThreadRng256 {}; + let user_immediately_present = |_| Ok(()); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + + let mut make_credential_params = create_minimal_make_credential_parameters(); + make_credential_params.user.user_id = vec![0x01]; + assert!(ctap_state + .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID) + .is_ok()); + let mut make_credential_params = create_minimal_make_credential_parameters(); + make_credential_params.user.user_id = vec![0x02]; + assert!(ctap_state + .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID) + .is_ok()); + + let get_assertion_params = AuthenticatorGetAssertionParameters { + rp_id: String::from("example.com"), + client_data_hash: vec![0xCD], + allow_list: None, + extensions: None, + 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(get_assertion_params, DUMMY_CHANNEL_ID); + + match get_assertion_response.unwrap() { + ResponseData::AuthenticatorGetAssertion(get_assertion_response) => { + let AuthenticatorGetAssertionResponse { + auth_data, + user, + number_of_credentials, + .. + } = get_assertion_response; + let expected_auth_data = vec![ + 0xA3, 0x79, 0xA6, 0xF6, 0xEE, 0xAF, 0xB9, 0xA5, 0x5E, 0x37, 0x8C, 0x11, 0x80, + 0x34, 0xE2, 0x75, 0x1E, 0x68, 0x2F, 0xAB, 0x9F, 0x2D, 0x30, 0xAB, 0x13, 0xD2, + 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0x00, 0x00, 0x00, 0x00, 0x01, + ]; + assert_eq!(auth_data, expected_auth_data); + let expected_user = PublicKeyCredentialUserEntity { + user_id: vec![0x02], + user_name: None, + user_display_name: None, + user_icon: None, + }; + assert_eq!(user, Some(expected_user)); + assert_eq!(number_of_credentials, Some(2)); + } + _ => panic!("Invalid response type"), + } + + let get_assertion_response = ctap_state.process_get_next_assertion(); + match get_assertion_response.unwrap() { + ResponseData::AuthenticatorGetAssertion(get_assertion_response) => { + let AuthenticatorGetAssertionResponse { + auth_data, + user, + number_of_credentials, + .. + } = get_assertion_response; + let expected_auth_data = vec![ + 0xA3, 0x79, 0xA6, 0xF6, 0xEE, 0xAF, 0xB9, 0xA5, 0x5E, 0x37, 0x8C, 0x11, 0x80, + 0x34, 0xE2, 0x75, 0x1E, 0x68, 0x2F, 0xAB, 0x9F, 0x2D, 0x30, 0xAB, 0x13, 0xD2, + 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0x00, 0x00, 0x00, 0x00, 0x01, + ]; + assert_eq!(auth_data, expected_auth_data); + let expected_user = PublicKeyCredentialUserEntity { + user_id: vec![0x01], + user_name: None, + user_display_name: None, + user_icon: None, + }; + assert_eq!(user, Some(expected_user)); + assert!(number_of_credentials.is_none()); + } + _ => panic!("Invalid response type"), + } + + let get_assertion_response = ctap_state.process_get_next_assertion(); + assert_eq!( + get_assertion_response, + Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED) + ); + } + #[test] fn test_process_reset() { let mut rng = ThreadRng256 {}; From ffe19e152b4bc334a188f7e35fcaf1bf51853ca2 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Tue, 24 Nov 2020 17:17:18 +0100 Subject: [PATCH 24/28] moves UP check in GetAssertion before NO_CREDENTIALS --- src/ctap/mod.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 1f91f4f..98058ce 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -736,15 +736,17 @@ where } credentials.sort_unstable_by_key(|c| c.creation_order); + // This check comes before CTAP2_ERR_NO_CREDENTIALS in CTAP 2.0. + // For CTAP 2.1, it was moved to a later protocol step. + if options.up { + (self.check_user_presence)(cid)?; + } + let (credential, remaining_credentials) = credentials .split_last() .ok_or(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS)?; self.next_credentials = remaining_credentials.to_vec(); - if options.up { - (self.check_user_presence)(cid)?; - } - self.increment_global_signature_counter()?; let assertion_input = AssertionInput { From ed59ebac0dd3dc5f44057e11c3b8a106d20a46e7 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Thu, 26 Nov 2020 14:40:02 +0100 Subject: [PATCH 25/28] command timeout for GetNextAssertion --- fuzz/fuzz_helper/src/lib.rs | 4 +- src/ctap/ctap1.rs | 26 +- src/ctap/hid/mod.rs | 11 +- src/ctap/mod.rs | 490 ++++++++++++++++++++++++------------ src/ctap/storage.rs | 15 ++ src/main.rs | 11 +- 6 files changed, 373 insertions(+), 184 deletions(-) diff --git a/fuzz/fuzz_helper/src/lib.rs b/fuzz/fuzz_helper/src/lib.rs index a6dc1a7..1553f65 100644 --- a/fuzz/fuzz_helper/src/lib.rs +++ b/fuzz/fuzz_helper/src/lib.rs @@ -147,7 +147,7 @@ fn process_message( pub fn process_ctap_any_type(data: &[u8]) { // Initialize ctap state and hid and get the allocated cid. let mut rng = ThreadRng256 {}; - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let mut ctap_hid = CtapHid::new(); let cid = initialize(&mut ctap_state, &mut ctap_hid); // Wrap input as message with the allocated cid. @@ -165,7 +165,7 @@ pub fn process_ctap_specific_type(data: &[u8], input_type: InputType) { } // Initialize ctap state and hid and get the allocated cid. let mut rng = ThreadRng256 {}; - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let mut ctap_hid = CtapHid::new(); let cid = initialize(&mut ctap_state, &mut ctap_hid); // Wrap input as message with allocated cid and command type. diff --git a/src/ctap/ctap1.rs b/src/ctap/ctap1.rs index a5a7921..f299926 100644 --- a/src/ctap/ctap1.rs +++ b/src/ctap/ctap1.rs @@ -427,7 +427,7 @@ mod test { fn test_process_register() { let mut rng = ThreadRng256 {}; let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); - let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence); + let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence, START_CLOCK_VALUE); let application = [0x0A; 32]; let message = create_register_message(&application); @@ -456,7 +456,7 @@ mod test { fn test_process_register_bad_message() { let mut rng = ThreadRng256 {}; let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); - let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence); + let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence, START_CLOCK_VALUE); let application = [0x0A; 32]; let message = create_register_message(&application); @@ -476,7 +476,7 @@ mod test { let mut rng = ThreadRng256 {}; let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); - let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence); + let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence, START_CLOCK_VALUE); ctap_state.u2f_up_state.consume_up(START_CLOCK_VALUE); ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE); @@ -490,7 +490,7 @@ mod test { let mut rng = ThreadRng256 {}; let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); let sk = crypto::ecdsa::SecKey::gensk(&mut rng); - let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence); + let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence, START_CLOCK_VALUE); let rp_id = "example.com"; let application = crypto::sha256::Sha256::hash(rp_id.as_bytes()); @@ -508,7 +508,7 @@ mod test { let mut rng = ThreadRng256 {}; let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); let sk = crypto::ecdsa::SecKey::gensk(&mut rng); - let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence); + let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence, START_CLOCK_VALUE); let rp_id = "example.com"; let application = crypto::sha256::Sha256::hash(rp_id.as_bytes()); @@ -527,7 +527,7 @@ mod test { let mut rng = ThreadRng256 {}; let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); let sk = crypto::ecdsa::SecKey::gensk(&mut rng); - let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence); + let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence, START_CLOCK_VALUE); let rp_id = "example.com"; let application = crypto::sha256::Sha256::hash(rp_id.as_bytes()); @@ -553,7 +553,7 @@ mod test { let mut rng = ThreadRng256 {}; let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); let sk = crypto::ecdsa::SecKey::gensk(&mut rng); - let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence); + let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence, START_CLOCK_VALUE); let rp_id = "example.com"; let application = crypto::sha256::Sha256::hash(rp_id.as_bytes()); @@ -573,7 +573,7 @@ mod test { let mut rng = ThreadRng256 {}; let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); let sk = crypto::ecdsa::SecKey::gensk(&mut rng); - let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence); + let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence, START_CLOCK_VALUE); let rp_id = "example.com"; let application = crypto::sha256::Sha256::hash(rp_id.as_bytes()); @@ -593,7 +593,7 @@ mod test { let mut rng = ThreadRng256 {}; let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); let sk = crypto::ecdsa::SecKey::gensk(&mut rng); - let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence); + let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence, START_CLOCK_VALUE); let rp_id = "example.com"; let application = crypto::sha256::Sha256::hash(rp_id.as_bytes()); @@ -613,7 +613,7 @@ mod test { let mut rng = ThreadRng256 {}; let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); let sk = crypto::ecdsa::SecKey::gensk(&mut rng); - let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence); + let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence, START_CLOCK_VALUE); let rp_id = "example.com"; let application = crypto::sha256::Sha256::hash(rp_id.as_bytes()); @@ -640,7 +640,7 @@ mod test { let mut rng = ThreadRng256 {}; let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); let sk = crypto::ecdsa::SecKey::gensk(&mut rng); - let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence); + let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence, START_CLOCK_VALUE); let rp_id = "example.com"; let application = crypto::sha256::Sha256::hash(rp_id.as_bytes()); @@ -672,7 +672,7 @@ mod test { let mut rng = ThreadRng256 {}; let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); - let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence); + let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence, START_CLOCK_VALUE); ctap_state.u2f_up_state.consume_up(START_CLOCK_VALUE); ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE); @@ -689,7 +689,7 @@ mod test { let mut rng = ThreadRng256 {}; let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); - let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence); + let mut ctap_state = CtapState::new(&mut rng, dummy_user_presence, START_CLOCK_VALUE); ctap_state.u2f_up_state.consume_up(START_CLOCK_VALUE); ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE); diff --git a/src/ctap/hid/mod.rs b/src/ctap/hid/mod.rs index a6a18f7..3874792 100644 --- a/src/ctap/hid/mod.rs +++ b/src/ctap/hid/mod.rs @@ -200,7 +200,8 @@ impl CtapHid { // Each transaction is atomic, so we process the command directly here and // don't handle any other packet in the meantime. // TODO: Send keep-alive packets in the meantime. - let response = ctap_state.process_command(&message.payload, cid); + let response = + ctap_state.process_command(&message.payload, cid, clock_value); if let Some(iterator) = CtapHid::split_message(Message { cid, cmd: CtapHid::COMMAND_CBOR, @@ -520,7 +521,7 @@ mod test { fn test_spurious_continuation_packet() { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let mut ctap_hid = CtapHid::new(); let mut packet = [0x00; 64]; @@ -541,7 +542,7 @@ mod test { fn test_command_init() { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let mut ctap_hid = CtapHid::new(); let reply = process_messages( @@ -586,7 +587,7 @@ mod test { fn test_command_init_for_sync() { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let mut ctap_hid = CtapHid::new(); let cid = cid_from_init(&mut ctap_hid, &mut ctap_state); @@ -646,7 +647,7 @@ mod test { fn test_command_ping() { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let mut ctap_hid = CtapHid::new(); let cid = cid_from_init(&mut ctap_hid, &mut ctap_state); diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 98058ce..4675350 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -47,6 +47,7 @@ use self::response::{ }; use self::status_code::Ctap2StatusCode; use self::storage::PersistentStore; +use self::timed_permission::TimedPermission; #[cfg(feature = "with_ctap1")] use self::timed_permission::U2fUserPresenceState; use alloc::collections::BTreeMap; @@ -65,7 +66,7 @@ use crypto::sha256::Sha256; use crypto::Hash256; #[cfg(feature = "debug_ctap")] use libtock_drivers::console::Console; -use libtock_drivers::timer::{Duration, Timestamp}; +use libtock_drivers::timer::{ClockValue, Duration}; // This flag enables or disables basic attestation for FIDO2. U2F is unaffected by // this setting. The basic attestation uses the signing key from key_material.rs @@ -99,7 +100,8 @@ const ED_FLAG: u8 = 0x80; pub const TOUCH_TIMEOUT_MS: isize = 30000; #[cfg(feature = "with_ctap1")] const U2F_UP_PROMPT_TIMEOUT: Duration = Duration::from_ms(10000); -const RESET_TIMEOUT_MS: isize = 10000; +const RESET_TIMEOUT_DURATION: Duration = Duration::from_ms(10000); +const STATEFUL_COMMAND_TIMEOUT_DURATION: Duration = Duration::from_ms(30000); pub const FIDO2_VERSION_STRING: &str = "FIDO_2_0"; #[cfg(feature = "with_ctap1")] @@ -140,6 +142,17 @@ struct AssertionInput { hmac_secret_input: Option, } +struct AssertionState { + assertion_input: AssertionInput, + // Sorted by ascending order of creation, so the last element is the most recent one. + next_credentials: Vec, +} + +enum StatefulCommand { + Reset, + GetAssertion(AssertionState), +} + // This struct currently holds all state, not only the persistent memory. The persistent members are // in the persistent store field. pub struct CtapState<'a, R: Rng256, CheckUserPresence: Fn(ChannelID) -> Result<(), Ctap2StatusCode>> @@ -150,13 +163,11 @@ pub struct CtapState<'a, R: Rng256, CheckUserPresence: Fn(ChannelID) -> Result<( check_user_presence: CheckUserPresence, persistent_store: PersistentStore, pin_protocol_v1: PinProtocolV1, - // This variable will be irreversibly set to false RESET_TIMEOUT_MS milliseconds after boot. - accepts_reset: bool, #[cfg(feature = "with_ctap1")] pub u2f_up_state: U2fUserPresenceState, - // Sorted by ascending order of creation, so the last element is the most recent one. - next_credentials: Vec, - next_assertion_input: Option, + // The state initializes to Reset and its timeout, and never goes back to Reset. + stateful_command_permission: TimedPermission, + stateful_command_type: Option, } impl<'a, R, CheckUserPresence> CtapState<'a, R, CheckUserPresence> @@ -169,6 +180,7 @@ where pub fn new( rng: &'a mut R, check_user_presence: CheckUserPresence, + now: ClockValue, ) -> CtapState<'a, R, CheckUserPresence> { let persistent_store = PersistentStore::new(rng); let pin_protocol_v1 = PinProtocolV1::new(rng); @@ -177,20 +189,26 @@ where check_user_presence, persistent_store, pin_protocol_v1, - accepts_reset: true, #[cfg(feature = "with_ctap1")] u2f_up_state: U2fUserPresenceState::new( U2F_UP_PROMPT_TIMEOUT, Duration::from_ms(TOUCH_TIMEOUT_MS), ), - next_credentials: vec![], - next_assertion_input: None, + stateful_command_permission: TimedPermission::granted(now, RESET_TIMEOUT_DURATION), + stateful_command_type: Some(StatefulCommand::Reset), } } - pub fn check_disable_reset(&mut self, timestamp: Timestamp) { - if timestamp - Timestamp::::from_ms(0) > Duration::from_ms(RESET_TIMEOUT_MS) { - self.accepts_reset = false; + pub fn update_command_permission(&mut self, now: ClockValue) { + self.stateful_command_permission = self.stateful_command_permission.check_expiration(now); + } + + fn check_command_permission(&mut self, now: ClockValue) -> Result<(), Ctap2StatusCode> { + self.stateful_command_permission = self.stateful_command_permission.check_expiration(now); + if self.stateful_command_permission.is_granted(now) { + Ok(()) + } else { + Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED) } } @@ -306,7 +324,12 @@ where })) } - pub fn process_command(&mut self, command_cbor: &[u8], cid: ChannelID) -> Vec { + pub fn process_command( + &mut self, + command_cbor: &[u8], + cid: ChannelID, + now: ClockValue, + ) -> Vec { let cmd = Command::deserialize(command_cbor); #[cfg(feature = "debug_ctap")] writeln!(&mut Console::new(), "Received command: {:#?}", cmd).unwrap(); @@ -320,17 +343,32 @@ where Duration::from_ms(TOUCH_TIMEOUT_MS), ); } + match (&command, &self.stateful_command_type) { + ( + Command::AuthenticatorGetNextAssertion, + Some(StatefulCommand::GetAssertion(_)), + ) => (), + (Command::AuthenticatorReset, Some(StatefulCommand::Reset)) => (), + // GetInfo does not reset stateful commands. + (Command::AuthenticatorGetInfo, _) => (), + // AuthenticatorSelection does not reset stateful commands. + #[cfg(feature = "with_ctap2_1")] + (Command::AuthenticatorSelection, _) => (), + (_, _) => { + self.stateful_command_type = None; + } + } let response = match command { Command::AuthenticatorMakeCredential(params) => { self.process_make_credential(params, cid) } Command::AuthenticatorGetAssertion(params) => { - self.process_get_assertion(params, cid) + self.process_get_assertion(params, cid, now) } - Command::AuthenticatorGetNextAssertion => self.process_get_next_assertion(), + Command::AuthenticatorGetNextAssertion => self.process_get_next_assertion(now), Command::AuthenticatorGetInfo => self.process_get_info(), Command::AuthenticatorClientPin(params) => self.process_client_pin(params), - Command::AuthenticatorReset => self.process_reset(cid), + Command::AuthenticatorReset => self.process_reset(cid, now), #[cfg(feature = "with_ctap2_1")] Command::AuthenticatorSelection => self.process_selection(cid), // TODO(kaczmarczyck) implement FIDO 2.1 commands @@ -659,6 +697,7 @@ where &mut self, get_assertion_params: AuthenticatorGetAssertionParameters, cid: ChannelID, + now: ClockValue, ) -> Result { let AuthenticatorGetAssertionParameters { rp_id, @@ -745,7 +784,6 @@ where let (credential, remaining_credentials) = credentials .split_last() .ok_or(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS)?; - self.next_credentials = remaining_credentials.to_vec(); self.increment_global_signature_counter()?; @@ -754,29 +792,37 @@ where auth_data: self.generate_auth_data(&rp_id_hash, flags)?, hmac_secret_input, }; - let number_of_credentials = if self.next_credentials.is_empty() { - self.next_assertion_input = None; + let number_of_credentials = if remaining_credentials.is_empty() { None } else { - self.next_assertion_input = Some(assertion_input.clone()); - Some(self.next_credentials.len() + 1) + self.stateful_command_permission = + TimedPermission::granted(now, STATEFUL_COMMAND_TIMEOUT_DURATION); + self.stateful_command_type = Some(StatefulCommand::GetAssertion(AssertionState { + assertion_input: assertion_input.clone(), + next_credentials: remaining_credentials.to_vec(), + })); + Some(remaining_credentials.len() + 1) }; self.assertion_response(credential, assertion_input, number_of_credentials) } - fn process_get_next_assertion(&mut self) -> Result { - // TODO(kaczmarczyck) introduce timer - let assertion_input = self - .next_assertion_input - .clone() - .ok_or(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED)?; - let credential = self - .next_credentials - .pop() - .ok_or(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED)?; - if self.next_credentials.is_empty() { - self.next_assertion_input = None; - }; + fn process_get_next_assertion( + &mut self, + now: ClockValue, + ) -> Result { + self.check_command_permission(now)?; + let (assertion_input, credential) = + if let Some(StatefulCommand::GetAssertion(assertion_state)) = + &mut self.stateful_command_type + { + let credential = assertion_state + .next_credentials + .pop() + .ok_or(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED)?; + (assertion_state.assertion_input.clone(), credential) + } else { + return Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED); + }; self.assertion_response(&credential, assertion_input, None) } @@ -834,10 +880,17 @@ where ) } - fn process_reset(&mut self, cid: ChannelID) -> Result { + fn process_reset( + &mut self, + cid: ChannelID, + now: ClockValue, + ) -> Result { // Resets are only possible in the first 10 seconds after booting. - if !self.accepts_reset { - return Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED); + // TODO(kaczmarczyck) 2.1 allows Reset after Reset and 15 seconds? + self.check_command_permission(now)?; + match &self.stateful_command_type { + Some(StatefulCommand::Reset) => (), + _ => return Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED), } (self.check_user_presence)(cid)?; @@ -886,8 +939,11 @@ mod test { MakeCredentialOptions, PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity, }; use super::*; + use cbor::cbor_array; use crypto::rng256::ThreadRng256; + const CLOCK_FREQUENCY_HZ: usize = 32768; + const DUMMY_CLOCK_VALUE: ClockValue = ClockValue::new(0, CLOCK_FREQUENCY_HZ); // The keep-alive logic in the processing of some commands needs a channel ID to send // keep-alive packets to. // In tests where we define a dummy user-presence check that immediately returns, the channel @@ -898,8 +954,8 @@ mod test { fn test_get_info() { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); - let info_reponse = ctap_state.process_command(&[0x04], DUMMY_CHANNEL_ID); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); + let info_reponse = ctap_state.process_command(&[0x04], DUMMY_CHANNEL_ID, DUMMY_CLOCK_VALUE); #[cfg(feature = "with_ctap2_1")] let mut expected_response = vec![0x00, 0xAA, 0x01]; @@ -955,7 +1011,7 @@ mod test { rp_icon: None, }; let user = PublicKeyCredentialUserEntity { - user_id: vec![0xFA, 0xB1, 0xA2], + user_id: vec![0x1D], user_name: None, user_display_name: None, user_icon: None, @@ -1008,7 +1064,7 @@ mod test { fn test_residential_process_make_credential() { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let make_credential_params = create_minimal_make_credential_parameters(); let make_credential_response = @@ -1044,7 +1100,7 @@ mod test { fn test_non_residential_process_make_credential() { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let mut make_credential_params = create_minimal_make_credential_parameters(); make_credential_params.options.rk = false; @@ -1081,7 +1137,7 @@ mod test { fn test_process_make_credential_unsupported_algorithm() { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let mut make_credential_params = create_minimal_make_credential_parameters(); make_credential_params.pub_key_cred_params = vec![]; @@ -1099,7 +1155,7 @@ mod test { let mut rng = ThreadRng256 {}; let excluded_private_key = crypto::ecdsa::SecKey::gensk(&mut rng); let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let excluded_credential_id = vec![0x01, 0x23, 0x45, 0x67]; let make_credential_params = @@ -1132,7 +1188,7 @@ mod test { fn test_process_make_credential_credential_with_cred_protect() { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let test_policy = CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList; let make_credential_params = @@ -1186,7 +1242,7 @@ mod test { fn test_process_make_credential_hmac_secret() { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let extensions = Some(MakeCredentialExtensions { hmac_secret: true, @@ -1236,7 +1292,7 @@ mod test { fn test_process_make_credential_hmac_secret_resident_key() { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let extensions = Some(MakeCredentialExtensions { hmac_secret: true, @@ -1285,7 +1341,8 @@ mod test { fn test_process_make_credential_cancelled() { let mut rng = ThreadRng256 {}; let user_presence_always_cancel = |_| Err(Ctap2StatusCode::CTAP2_ERR_KEEPALIVE_CANCEL); - let mut ctap_state = CtapState::new(&mut rng, user_presence_always_cancel); + let mut ctap_state = + CtapState::new(&mut rng, user_presence_always_cancel, DUMMY_CLOCK_VALUE); let make_credential_params = create_minimal_make_credential_parameters(); let make_credential_response = @@ -1297,11 +1354,43 @@ mod test { ); } + fn check_assertion_response( + response: Result, + expected_user_id: Vec, + expected_number_of_credentials: Option, + ) { + match response.unwrap() { + ResponseData::AuthenticatorGetAssertion(get_assertion_response) => { + let AuthenticatorGetAssertionResponse { + auth_data, + user, + number_of_credentials, + .. + } = get_assertion_response; + let expected_auth_data = vec![ + 0xA3, 0x79, 0xA6, 0xF6, 0xEE, 0xAF, 0xB9, 0xA5, 0x5E, 0x37, 0x8C, 0x11, 0x80, + 0x34, 0xE2, 0x75, 0x1E, 0x68, 0x2F, 0xAB, 0x9F, 0x2D, 0x30, 0xAB, 0x13, 0xD2, + 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0x00, 0x00, 0x00, 0x00, 0x01, + ]; + assert_eq!(auth_data, expected_auth_data); + let expected_user = PublicKeyCredentialUserEntity { + user_id: expected_user_id, + user_name: None, + user_display_name: None, + user_icon: None, + }; + assert_eq!(user, Some(expected_user)); + assert_eq!(number_of_credentials, expected_number_of_credentials); + } + _ => panic!("Invalid response type"), + } + } + #[test] fn test_residential_process_get_assertion() { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let make_credential_params = create_minimal_make_credential_parameters(); assert!(ctap_state @@ -1320,34 +1409,12 @@ mod test { pin_uv_auth_param: None, pin_uv_auth_protocol: None, }; - let get_assertion_response = - ctap_state.process_get_assertion(get_assertion_params, DUMMY_CHANNEL_ID); - - match get_assertion_response.unwrap() { - ResponseData::AuthenticatorGetAssertion(get_assertion_response) => { - let AuthenticatorGetAssertionResponse { - auth_data, - user, - number_of_credentials, - .. - } = get_assertion_response; - let expected_auth_data = vec![ - 0xA3, 0x79, 0xA6, 0xF6, 0xEE, 0xAF, 0xB9, 0xA5, 0x5E, 0x37, 0x8C, 0x11, 0x80, - 0x34, 0xE2, 0x75, 0x1E, 0x68, 0x2F, 0xAB, 0x9F, 0x2D, 0x30, 0xAB, 0x13, 0xD2, - 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0x00, 0x00, 0x00, 0x00, 0x01, - ]; - assert_eq!(auth_data, expected_auth_data); - let expected_user = PublicKeyCredentialUserEntity { - user_id: vec![0xFA, 0xB1, 0xA2], - user_name: None, - user_display_name: None, - user_icon: None, - }; - assert_eq!(user, Some(expected_user)); - assert!(number_of_credentials.is_none()); - } - _ => panic!("Invalid response type"), - } + let get_assertion_response = ctap_state.process_get_assertion( + get_assertion_params, + DUMMY_CHANNEL_ID, + DUMMY_CLOCK_VALUE, + ); + check_assertion_response(get_assertion_response, vec![0x1D], None); } #[test] @@ -1355,7 +1422,7 @@ mod test { let mut rng = ThreadRng256 {}; let sk = crypto::ecdh::SecKey::gensk(&mut rng); let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let make_extensions = Some(MakeCredentialExtensions { hmac_secret: true, @@ -1405,8 +1472,11 @@ mod test { pin_uv_auth_param: None, pin_uv_auth_protocol: None, }; - let get_assertion_response = - ctap_state.process_get_assertion(get_assertion_params, DUMMY_CHANNEL_ID); + let get_assertion_response = ctap_state.process_get_assertion( + get_assertion_params, + DUMMY_CHANNEL_ID, + DUMMY_CLOCK_VALUE, + ); assert_eq!( get_assertion_response, @@ -1419,7 +1489,7 @@ mod test { let mut rng = ThreadRng256 {}; let sk = crypto::ecdh::SecKey::gensk(&mut rng); let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let make_extensions = Some(MakeCredentialExtensions { hmac_secret: true, @@ -1453,8 +1523,11 @@ mod test { pin_uv_auth_param: None, pin_uv_auth_protocol: None, }; - let get_assertion_response = - ctap_state.process_get_assertion(get_assertion_params, DUMMY_CHANNEL_ID); + let get_assertion_response = ctap_state.process_get_assertion( + get_assertion_params, + DUMMY_CHANNEL_ID, + DUMMY_CLOCK_VALUE, + ); assert_eq!( get_assertion_response, @@ -1468,7 +1541,7 @@ mod test { let private_key = crypto::ecdsa::SecKey::gensk(&mut rng); let credential_id = rng.gen_uniform_u8x32().to_vec(); let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let cred_desc = PublicKeyCredentialDescriptor { key_type: PublicKeyCredentialType::PublicKey, @@ -1480,7 +1553,7 @@ mod test { credential_id: credential_id.clone(), private_key: private_key.clone(), rp_id: String::from("example.com"), - user_handle: vec![0x00], + user_handle: vec![0x1D], other_ui: None, cred_random: None, cred_protect_policy: Some( @@ -1505,8 +1578,11 @@ mod test { pin_uv_auth_param: None, pin_uv_auth_protocol: None, }; - let get_assertion_response = - ctap_state.process_get_assertion(get_assertion_params, DUMMY_CHANNEL_ID); + let get_assertion_response = ctap_state.process_get_assertion( + get_assertion_params, + DUMMY_CHANNEL_ID, + DUMMY_CLOCK_VALUE, + ); assert_eq!( get_assertion_response, Err(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS), @@ -1524,16 +1600,19 @@ mod test { pin_uv_auth_param: None, pin_uv_auth_protocol: None, }; - let get_assertion_response = - ctap_state.process_get_assertion(get_assertion_params, DUMMY_CHANNEL_ID); - assert!(get_assertion_response.is_ok()); + let get_assertion_response = ctap_state.process_get_assertion( + get_assertion_params, + DUMMY_CHANNEL_ID, + DUMMY_CLOCK_VALUE, + ); + check_assertion_response(get_assertion_response, vec![0x1D], None); let credential = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id, private_key, rp_id: String::from("example.com"), - user_handle: vec![0x00], + user_handle: vec![0x1D], other_ui: None, cred_random: None, cred_protect_policy: Some(CredentialProtectionPolicy::UserVerificationRequired), @@ -1556,8 +1635,11 @@ mod test { pin_uv_auth_param: None, pin_uv_auth_protocol: None, }; - let get_assertion_response = - ctap_state.process_get_assertion(get_assertion_params, DUMMY_CHANNEL_ID); + let get_assertion_response = ctap_state.process_get_assertion( + get_assertion_params, + DUMMY_CHANNEL_ID, + DUMMY_CLOCK_VALUE, + ); assert_eq!( get_assertion_response, Err(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS), @@ -1565,10 +1647,10 @@ mod test { } #[test] - fn test_process_get_next_assertion() { + fn test_process_get_next_assertion_two_credentials() { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let mut make_credential_params = create_minimal_make_credential_parameters(); make_credential_params.user.user_id = vec![0x01]; @@ -1593,63 +1675,135 @@ mod test { pin_uv_auth_param: None, pin_uv_auth_protocol: None, }; - let get_assertion_response = - ctap_state.process_get_assertion(get_assertion_params, DUMMY_CHANNEL_ID); + let get_assertion_response = ctap_state.process_get_assertion( + get_assertion_params, + DUMMY_CHANNEL_ID, + DUMMY_CLOCK_VALUE, + ); + check_assertion_response(get_assertion_response, vec![0x02], Some(2)); - match get_assertion_response.unwrap() { - ResponseData::AuthenticatorGetAssertion(get_assertion_response) => { - let AuthenticatorGetAssertionResponse { - auth_data, - user, - number_of_credentials, - .. - } = get_assertion_response; - let expected_auth_data = vec![ - 0xA3, 0x79, 0xA6, 0xF6, 0xEE, 0xAF, 0xB9, 0xA5, 0x5E, 0x37, 0x8C, 0x11, 0x80, - 0x34, 0xE2, 0x75, 0x1E, 0x68, 0x2F, 0xAB, 0x9F, 0x2D, 0x30, 0xAB, 0x13, 0xD2, - 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0x00, 0x00, 0x00, 0x00, 0x01, - ]; - assert_eq!(auth_data, expected_auth_data); - let expected_user = PublicKeyCredentialUserEntity { - user_id: vec![0x02], - user_name: None, - user_display_name: None, - user_icon: None, - }; - assert_eq!(user, Some(expected_user)); - assert_eq!(number_of_credentials, Some(2)); - } - _ => panic!("Invalid response type"), - } + let get_assertion_response = ctap_state.process_get_next_assertion(DUMMY_CLOCK_VALUE); + check_assertion_response(get_assertion_response, vec![0x01], None); - let get_assertion_response = ctap_state.process_get_next_assertion(); - match get_assertion_response.unwrap() { - ResponseData::AuthenticatorGetAssertion(get_assertion_response) => { - let AuthenticatorGetAssertionResponse { - auth_data, - user, - number_of_credentials, - .. - } = get_assertion_response; - let expected_auth_data = vec![ - 0xA3, 0x79, 0xA6, 0xF6, 0xEE, 0xAF, 0xB9, 0xA5, 0x5E, 0x37, 0x8C, 0x11, 0x80, - 0x34, 0xE2, 0x75, 0x1E, 0x68, 0x2F, 0xAB, 0x9F, 0x2D, 0x30, 0xAB, 0x13, 0xD2, - 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0x00, 0x00, 0x00, 0x00, 0x01, - ]; - assert_eq!(auth_data, expected_auth_data); - let expected_user = PublicKeyCredentialUserEntity { - user_id: vec![0x01], - user_name: None, - user_display_name: None, - user_icon: None, - }; - assert_eq!(user, Some(expected_user)); - assert!(number_of_credentials.is_none()); - } - _ => panic!("Invalid response type"), - } + let get_assertion_response = ctap_state.process_get_next_assertion(DUMMY_CLOCK_VALUE); + assert_eq!( + get_assertion_response, + Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED) + ); + } - let get_assertion_response = ctap_state.process_get_next_assertion(); + #[test] + fn test_process_get_next_assertion_three_credentials() { + let mut rng = ThreadRng256 {}; + let user_immediately_present = |_| Ok(()); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); + + let mut make_credential_params = create_minimal_make_credential_parameters(); + make_credential_params.user.user_id = vec![0x01]; + assert!(ctap_state + .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID) + .is_ok()); + let mut make_credential_params = create_minimal_make_credential_parameters(); + make_credential_params.user.user_id = vec![0x02]; + assert!(ctap_state + .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID) + .is_ok()); + let mut make_credential_params = create_minimal_make_credential_parameters(); + make_credential_params.user.user_id = vec![0x03]; + assert!(ctap_state + .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID) + .is_ok()); + + let get_assertion_params = AuthenticatorGetAssertionParameters { + rp_id: String::from("example.com"), + client_data_hash: vec![0xCD], + allow_list: None, + extensions: None, + 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( + get_assertion_params, + DUMMY_CHANNEL_ID, + DUMMY_CLOCK_VALUE, + ); + check_assertion_response(get_assertion_response, vec![0x03], Some(3)); + + let get_assertion_response = ctap_state.process_get_next_assertion(DUMMY_CLOCK_VALUE); + check_assertion_response(get_assertion_response, vec![0x02], None); + + let get_assertion_response = ctap_state.process_get_next_assertion(DUMMY_CLOCK_VALUE); + check_assertion_response(get_assertion_response, vec![0x01], None); + + let get_assertion_response = ctap_state.process_get_next_assertion(DUMMY_CLOCK_VALUE); + assert_eq!( + get_assertion_response, + Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED) + ); + } + + #[test] + fn test_process_get_next_assertion_not_allowed() { + let mut rng = ThreadRng256 {}; + let user_immediately_present = |_| Ok(()); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); + + let get_assertion_response = ctap_state.process_get_next_assertion(DUMMY_CLOCK_VALUE); + assert_eq!( + get_assertion_response, + Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED) + ); + + let mut make_credential_params = create_minimal_make_credential_parameters(); + make_credential_params.user.user_id = vec![0x01]; + assert!(ctap_state + .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID) + .is_ok()); + let mut make_credential_params = create_minimal_make_credential_parameters(); + make_credential_params.user.user_id = vec![0x02]; + assert!(ctap_state + .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID) + .is_ok()); + + let get_assertion_params = AuthenticatorGetAssertionParameters { + rp_id: String::from("example.com"), + client_data_hash: vec![0xCD], + allow_list: None, + extensions: None, + 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( + get_assertion_params, + DUMMY_CHANNEL_ID, + DUMMY_CLOCK_VALUE, + ); + assert!(get_assertion_response.is_ok()); + + // This is a MakeCredential command. + let mut command_cbor = vec![0x01]; + let cbor_value = cbor_map! { + 1 => vec![0xCD; 16], + 2 => cbor_map! { + "id" => "example.com", + }, + 3 => cbor_map! { + "id" => vec![0x1D, 0x1D, 0x1D, 0x1D], + }, + 4 => cbor_array![ES256_CRED_PARAM], + }; + assert!(cbor::write(cbor_value, &mut command_cbor)); + ctap_state.process_command(&command_cbor, DUMMY_CHANNEL_ID, DUMMY_CLOCK_VALUE); + + let get_assertion_response = ctap_state.process_get_next_assertion(DUMMY_CLOCK_VALUE); assert_eq!( get_assertion_response, Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED) @@ -1661,7 +1815,7 @@ mod test { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); let private_key = crypto::ecdsa::SecKey::gensk(&mut rng); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let credential_id = vec![0x01, 0x23, 0x45, 0x67]; let credential_source = PublicKeyCredentialSource { @@ -1681,7 +1835,8 @@ mod test { .is_ok()); assert!(ctap_state.persistent_store.count_credentials().unwrap() > 0); - let reset_reponse = ctap_state.process_command(&[0x07], DUMMY_CHANNEL_ID); + let reset_reponse = + ctap_state.process_command(&[0x07], DUMMY_CHANNEL_ID, DUMMY_CLOCK_VALUE); let expected_response = vec![0x00]; assert_eq!(reset_reponse, expected_response); assert!(ctap_state.persistent_store.count_credentials().unwrap() == 0); @@ -1691,9 +1846,10 @@ mod test { fn test_process_reset_cancelled() { let mut rng = ThreadRng256 {}; let user_presence_always_cancel = |_| Err(Ctap2StatusCode::CTAP2_ERR_KEEPALIVE_CANCEL); - let mut ctap_state = CtapState::new(&mut rng, user_presence_always_cancel); + let mut ctap_state = + CtapState::new(&mut rng, user_presence_always_cancel, DUMMY_CLOCK_VALUE); - let reset_reponse = ctap_state.process_reset(DUMMY_CHANNEL_ID); + let reset_reponse = ctap_state.process_reset(DUMMY_CHANNEL_ID, DUMMY_CLOCK_VALUE); assert_eq!( reset_reponse, @@ -1701,14 +1857,28 @@ mod test { ); } + #[test] + fn test_process_reset_not_first() { + let mut rng = ThreadRng256 {}; + let user_immediately_present = |_| Ok(()); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); + + // This is a GetNextAssertion command. + ctap_state.process_command(&[0x08], DUMMY_CHANNEL_ID, DUMMY_CLOCK_VALUE); + + let reset_reponse = ctap_state.process_reset(DUMMY_CHANNEL_ID, DUMMY_CLOCK_VALUE); + assert_eq!(reset_reponse, Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED)); + } + #[test] fn test_process_unknown_command() { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); // This command does not exist. - let reset_reponse = ctap_state.process_command(&[0xDF], DUMMY_CHANNEL_ID); + let reset_reponse = + ctap_state.process_command(&[0xDF], DUMMY_CHANNEL_ID, DUMMY_CLOCK_VALUE); let expected_response = vec![Ctap2StatusCode::CTAP1_ERR_INVALID_COMMAND as u8]; assert_eq!(reset_reponse, expected_response); } @@ -1718,7 +1888,7 @@ mod test { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); let private_key = crypto::ecdsa::SecKey::gensk(&mut rng); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); // Usually, the relying party ID or its hash is provided by the client. // We are not testing the correctness of our SHA256 here, only if it is checked. @@ -1739,7 +1909,7 @@ mod test { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); let private_key = crypto::ecdsa::SecKey::gensk(&mut rng); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); // Usually, the relying party ID or its hash is provided by the client. // We are not testing the correctness of our SHA256 here, only if it is checked. @@ -1762,7 +1932,7 @@ mod test { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); let private_key = crypto::ecdsa::SecKey::gensk(&mut rng); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); // Same as above. let rp_id_hash = [0x55; 32]; @@ -1784,7 +1954,7 @@ mod test { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); let private_key = crypto::ecdsa::SecKey::gensk(&mut rng); - let mut ctap_state = CtapState::new(&mut rng, user_immediately_present); + let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); // Same as above. let rp_id_hash = [0x55; 32]; diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 6e4506a..aae6add 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -746,6 +746,21 @@ mod test { assert!(persistent_store.count_credentials().unwrap() > 0); } + #[test] + fn test_credential_order() { + let mut rng = ThreadRng256 {}; + let mut persistent_store = PersistentStore::new(&mut rng); + let credential_source = create_credential_source(&mut rng, "example.com", vec![]); + let current_latest_creation = credential_source.creation_order; + assert!(persistent_store.store_credential(credential_source).is_ok()); + let mut credential_source = create_credential_source(&mut rng, "example.com", vec![]); + credential_source.creation_order = persistent_store.new_creation_order().unwrap(); + assert!(credential_source.creation_order > current_latest_creation); + let current_latest_creation = credential_source.creation_order; + assert!(persistent_store.store_credential(credential_source).is_ok()); + assert!(persistent_store.new_creation_order().unwrap() > current_latest_creation); + } + #[test] #[allow(clippy::assertions_on_constants)] fn test_fill_store() { diff --git a/src/main.rs b/src/main.rs index 855325e..51c8305 100644 --- a/src/main.rs +++ b/src/main.rs @@ -37,9 +37,11 @@ use libtock_drivers::console::Console; use libtock_drivers::led; use libtock_drivers::result::{FlexUnwrap, TockError}; use libtock_drivers::timer; +use libtock_drivers::timer::Duration; #[cfg(feature = "debug_ctap")] use libtock_drivers::timer::Timer; -use libtock_drivers::timer::{Duration, Timestamp}; +#[cfg(feature = "debug_ctap")] +use libtock_drivers::timer::Timestamp; use libtock_drivers::usb_ctap_hid; const KEEPALIVE_DELAY_MS: isize = 100; @@ -57,12 +59,13 @@ fn main() { panic!("Cannot setup USB driver"); } + let boot_time = timer.get_current_clock().flex_unwrap(); let mut rng = TockRng256 {}; - let mut ctap_state = CtapState::new(&mut rng, check_user_presence); + let mut ctap_state = CtapState::new(&mut rng, check_user_presence, boot_time); let mut ctap_hid = CtapHid::new(); let mut led_counter = 0; - let mut last_led_increment = timer.get_current_clock().flex_unwrap(); + let mut last_led_increment = boot_time; // Main loop. If CTAP1 is used, we register button presses for U2F while receiving and waiting. // The way TockOS and apps currently interact, callbacks need a yield syscall to execute, @@ -115,7 +118,7 @@ fn main() { // These calls are making sure that even for long inactivity, wrapping clock values // never randomly wink or grant user presence for U2F. - ctap_state.check_disable_reset(Timestamp::::from_clock_value(now)); + ctap_state.update_command_permission(now); ctap_hid.wink_permission = ctap_hid.wink_permission.check_expiration(now); if has_packet { From 3aef7e8b19d6a522af175f73de1b239bae3bd682 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Thu, 26 Nov 2020 15:56:59 +0100 Subject: [PATCH 26/28] reuse update_command_permission --- src/ctap/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 4675350..f2043d0 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -204,7 +204,7 @@ where } fn check_command_permission(&mut self, now: ClockValue) -> Result<(), Ctap2StatusCode> { - self.stateful_command_permission = self.stateful_command_permission.check_expiration(now); + self.update_command_permission(now); if self.stateful_command_permission.is_granted(now) { Ok(()) } else { From 3d1d8279847ef0dd8407b0a74e6c963a2eef4ce0 Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Thu, 26 Nov 2020 16:29:14 +0100 Subject: [PATCH 27/28] Address PR comments --- src/ctap/ctap1.rs | 28 +++++++++++++++++----------- src/ctap/storage.rs | 8 ++++---- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/ctap/ctap1.rs b/src/ctap/ctap1.rs index 0fa4745..41df364 100644 --- a/src/ctap/ctap1.rs +++ b/src/ctap/ctap1.rs @@ -35,6 +35,8 @@ pub enum Ctap1StatusCode { SW_WRONG_LENGTH = 0x6700, SW_CLA_NOT_SUPPORTED = 0x6E00, SW_INS_NOT_SUPPORTED = 0x6D00, + SW_MEMERR = 0x6501, + SW_COMMAND_ABORTED = 0x6F00, SW_VENDOR_KEY_HANDLE_TOO_LONG = 0xF000, } @@ -49,6 +51,8 @@ impl TryFrom for Ctap1StatusCode { 0x6700 => Ok(Ctap1StatusCode::SW_WRONG_LENGTH), 0x6E00 => Ok(Ctap1StatusCode::SW_CLA_NOT_SUPPORTED), 0x6D00 => Ok(Ctap1StatusCode::SW_INS_NOT_SUPPORTED), + 0x6501 => Ok(Ctap1StatusCode::SW_MEMERR), + 0x6F00 => Ok(Ctap1StatusCode::SW_COMMAND_ABORTED), 0xF000 => Ok(Ctap1StatusCode::SW_VENDOR_KEY_HANDLE_TOO_LONG), _ => Err(()), } @@ -288,20 +292,22 @@ impl Ctap1Command { let pk = sk.genpk(); let key_handle = ctap_state .encrypt_key_handle(sk, &application, None) - .map_err(|_| Ctap1StatusCode::SW_VENDOR_KEY_HANDLE_TOO_LONG)?; + .map_err(|_| Ctap1StatusCode::SW_COMMAND_ABORTED)?; if key_handle.len() > 0xFF { // This is just being defensive with unreachable code. return Err(Ctap1StatusCode::SW_VENDOR_KEY_HANDLE_TOO_LONG); } - let certificate = match ctap_state.persistent_store.attestation_certificate() { - Ok(Some(value)) => value, - _ => return Err(Ctap1StatusCode::SW_INS_NOT_SUPPORTED), - }; - let private_key = match ctap_state.persistent_store.attestation_private_key() { - Ok(Some(value)) => value, - _ => return Err(Ctap1StatusCode::SW_INS_NOT_SUPPORTED), - }; + let certificate = ctap_state + .persistent_store + .attestation_certificate() + .map_err(|_| Ctap1StatusCode::SW_MEMERR)? + .ok_or(Ctap1StatusCode::SW_COMMAND_ABORTED)?; + let private_key = ctap_state + .persistent_store + .attestation_private_key() + .map_err(|_| Ctap1StatusCode::SW_MEMERR)? + .ok_or(Ctap1StatusCode::SW_COMMAND_ABORTED)?; let mut response = Vec::with_capacity(105 + key_handle.len() + certificate.len()); response.push(Ctap1Command::LEGACY_BYTE); @@ -442,7 +448,7 @@ mod test { ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE); let response = Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE); // Certificate and private key are missing - assert_eq!(response, Err(Ctap1StatusCode::SW_INS_NOT_SUPPORTED)); + assert_eq!(response, Err(Ctap1StatusCode::SW_COMMAND_ABORTED)); let fake_key = [0x41u8; key_material::ATTESTATION_PRIVATE_KEY_LENGTH]; assert!(ctap_state @@ -453,7 +459,7 @@ mod test { ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE); let response = Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE); // Certificate is still missing - assert_eq!(response, Err(Ctap1StatusCode::SW_INS_NOT_SUPPORTED)); + assert_eq!(response, Err(Ctap1StatusCode::SW_COMMAND_ABORTED)); let fake_cert = [0x99u8; 100]; // Arbitrary length assert!(ctap_state diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 8e396b6..5ec6511 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -230,8 +230,8 @@ impl PersistentStore { .unwrap(); } // TODO(jmichel): remove this when vendor command is in place - #[cfg(not(any(test, feature = "ram_storage")))] - self.load_attestation_from_firmware(); + #[cfg(not(test))] + self.load_attestation_data_from_firmware(); if self.store.find_one(&Key::Aaguid).is_none() { self.set_aaguid(key_material::AAGUID).unwrap(); @@ -239,8 +239,8 @@ impl PersistentStore { } // TODO(jmichel): remove this function when vendor command is in place. - #[cfg(not(any(test, feature = "ram_storage")))] - fn load_attestation_from_firmware(&mut self) { + #[cfg(not(test))] + fn load_attestation_data_from_firmware(&mut self) { // The following 2 entries are meant to be written by vendor-specific commands. if self.store.find_one(&Key::AttestationPrivateKey).is_none() { self.set_attestation_private_key(key_material::ATTESTATION_PRIVATE_KEY) From 1571f58cd3e51849626cb3681bb15ccef10155b8 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Thu, 26 Nov 2020 19:21:41 +0100 Subject: [PATCH 28/28] wrapping_add in storage and more moving --- src/ctap/mod.rs | 27 ++++++++++++++------------- src/ctap/storage.rs | 2 +- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 9626da4..5fdfa9e 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -614,7 +614,7 @@ where // and returns the correct Get(Next)Assertion response. fn assertion_response( &self, - credential: &PublicKeyCredentialSource, + credential: PublicKeyCredentialSource, assertion_input: AssertionInput, number_of_credentials: Option, ) -> Result { @@ -645,14 +645,14 @@ where let cred_desc = PublicKeyCredentialDescriptor { key_type: PublicKeyCredentialType::PublicKey, - key_id: credential.credential_id.clone(), + key_id: credential.credential_id, transports: None, // You can set USB as a hint here. }; let user = if !credential.user_handle.is_empty() { Some(PublicKeyCredentialUserEntity { - user_id: credential.user_handle.clone(), + user_id: credential.user_handle, user_name: None, - user_display_name: credential.other_ui.clone(), + user_display_name: credential.other_ui, user_icon: None, }) } else { @@ -758,7 +758,7 @@ where } let rp_id_hash = Sha256::hash(rp_id.as_bytes()); - let mut credentials = if let Some(allow_list) = allow_list { + let mut applicable_credentials = if let Some(allow_list) = allow_list { if let Some(credential) = self.get_any_credential_from_allow_list(allow_list, &rp_id, &rp_id_hash, has_uv)? { @@ -771,11 +771,11 @@ where }; // Remove user identifiable information without uv. if !has_uv { - for credential in &mut credentials { + for credential in &mut applicable_credentials { credential.other_ui = None; } } - credentials.sort_unstable_by_key(|c| c.creation_order); + applicable_credentials.sort_unstable_by_key(|c| c.creation_order); // This check comes before CTAP2_ERR_NO_CREDENTIALS in CTAP 2.0. // For CTAP 2.1, it was moved to a later protocol step. @@ -783,8 +783,8 @@ where (self.check_user_presence)(cid)?; } - let (credential, remaining_credentials) = credentials - .split_last() + let credential = applicable_credentials + .pop() .ok_or(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS)?; self.increment_global_signature_counter()?; @@ -794,16 +794,17 @@ where auth_data: self.generate_auth_data(&rp_id_hash, flags)?, hmac_secret_input, }; - let number_of_credentials = if remaining_credentials.is_empty() { + let number_of_credentials = if applicable_credentials.is_empty() { None } else { + let number_of_credentials = Some(applicable_credentials.len() + 1); self.stateful_command_permission = TimedPermission::granted(now, STATEFUL_COMMAND_TIMEOUT_DURATION); self.stateful_command_type = Some(StatefulCommand::GetAssertion(AssertionState { assertion_input: assertion_input.clone(), - next_credentials: remaining_credentials.to_vec(), + next_credentials: applicable_credentials, })); - Some(remaining_credentials.len() + 1) + number_of_credentials }; self.assertion_response(credential, assertion_input, number_of_credentials) } @@ -825,7 +826,7 @@ where } else { return Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED); }; - self.assertion_response(&credential, assertion_input, None) + self.assertion_response(credential, assertion_input, None) } fn process_get_info(&self) -> Result { diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index fd91cce..7952d75 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -359,7 +359,7 @@ impl PersistentStore { .map(|c| c.creation_order) .max() .unwrap_or(0) - + 1) + .wrapping_add(1)) } pub fn global_signature_counter(&self) -> Result {