From ae9c2d5b99d6b53e6fcdf19ea282de19fde62ae8 Mon Sep 17 00:00:00 2001 From: bunnie Date: Tue, 7 Jun 2022 17:36:28 +0800 Subject: [PATCH] add authentication request UI; fix tests again. trying to debug why PIN authentication doesn't work. looks like Cosekey conversion is fine, the data is arriving OK...but the dh key exchange is not generated the expected shared secret. --- apps/vault/libraries/crypto/Cargo.toml | 2 ++ apps/vault/locales/i18n.json | 6 +++++ apps/vault/src/ctap/apdu.rs | 2 +- apps/vault/src/ctap/ctap1.rs | 30 ++++++++++++------------- apps/vault/src/ctap/data_formats.rs | 2 +- apps/vault/src/ctap/mod.rs | 13 ++++++++--- apps/vault/src/ctap/pin_protocol_v1.rs | 8 +++++++ apps/vault/src/ctap/storage.rs | 5 ++++- apps/vault/src/ctap/timed_permission.rs | 11 ++++----- apps/vault/src/ux.rs | 7 +++--- 10 files changed, 56 insertions(+), 30 deletions(-) diff --git a/apps/vault/libraries/crypto/Cargo.toml b/apps/vault/libraries/crypto/Cargo.toml index c2cf500f6..015308092 100644 --- a/apps/vault/libraries/crypto/Cargo.toml +++ b/apps/vault/libraries/crypto/Cargo.toml @@ -21,6 +21,8 @@ cbc = "0.1.2" aes = {path="../../../../services/aes"} sha2 = {path="../../../../services/engine-sha512"} +log = "0.4.14" # xous debugging + [target.'cfg(any(windows,unix))'.dependencies] rand = "0.8.5" rand_chacha = "0.3.1" diff --git a/apps/vault/locales/i18n.json b/apps/vault/locales/i18n.json index 822d994d3..d88e37d53 100644 --- a/apps/vault/locales/i18n.json +++ b/apps/vault/locales/i18n.json @@ -11,6 +11,12 @@ "zh": "FIDO 提出凭证请求\n按任意键批准", "en-tts": "FIDO Make Credential Request. Press any key to approve." }, + "vault.fido.get_assertion": { + "en": "FIDO Assertion Request.\nPress any key to consent to this transaction.", + "ja": "FIDOアサーションリクエスト。\nこのトランザクションに同意するには、任意のキーを押してください。", + "zh": "FIDO 断言请求。\n按任意键同意此事务。", + "en-tts": "FIDO Assertion Request.\nPress any key to consent to this transaction." + }, "vault.fido.exclude_list": { "en": "FIDO server is excluding existing credentials.\nPress any key to acknowledge.", "ja": "FIDOサーバーは既存のクレデンシャルを除外しています。\n任意のキーを押して確認します。", diff --git a/apps/vault/src/ctap/apdu.rs b/apps/vault/src/ctap/apdu.rs index 55cf2a858..31d8ff4c6 100644 --- a/apps/vault/src/ctap/apdu.rs +++ b/apps/vault/src/ctap/apdu.rs @@ -19,7 +19,7 @@ use crate::ctap::array_ref; const APDU_HEADER_LEN: usize = 4; -#[cfg_attr(test, derive(Clone, Debug))] +#[cfg_attr(test, derive(Clone))] #[allow(non_camel_case_types, dead_code)] #[derive(PartialEq, Debug)] pub enum ApduStatusCode { diff --git a/apps/vault/src/ctap/ctap1.rs b/apps/vault/src/ctap/ctap1.rs index 29be1434d..f81d3b8a2 100644 --- a/apps/vault/src/ctap/ctap1.rs +++ b/apps/vault/src/ctap/ctap1.rs @@ -373,7 +373,7 @@ mod test { const CLOCK_FREQUENCY_HZ: usize = 32768; const START_CLOCK_VALUE: ClockValue = ClockValue::new(0, CLOCK_FREQUENCY_HZ); const TIMEOUT_CLOCK_VALUE: ClockValue = ClockValue::new( - (30001 * CLOCK_FREQUENCY_HZ as isize) / 1000, + (30001 * CLOCK_FREQUENCY_HZ as i64) / 1000, CLOCK_FREQUENCY_HZ, ); @@ -423,8 +423,8 @@ mod test { let application = [0x0A; 32]; 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); + //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 and private key are missing assert_eq!(response, Err(Ctap1StatusCode::SW_INTERNAL_EXCEPTION)); @@ -434,8 +434,8 @@ mod test { .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); + //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_INTERNAL_EXCEPTION)); @@ -445,8 +445,8 @@ mod test { .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); + //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); @@ -488,8 +488,8 @@ mod test { let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); 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); + //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, TIMEOUT_CLOCK_VALUE); assert_eq!(response, Err(Ctap1StatusCode::SW_COND_USE_NOT_SATISFIED)); @@ -636,8 +636,8 @@ mod test { let message = create_authenticate_message(&application, Ctap1Flags::EnforceUpAndSign, &key_handle); - ctap_state.u2f_up_state.consume_up(START_CLOCK_VALUE); - ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE); + //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], 0x01); @@ -689,8 +689,8 @@ mod test { let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); 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); + //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); assert_eq!(response, Err(Ctap1StatusCode::SW_WRONG_DATA)); } @@ -706,8 +706,8 @@ mod test { let dummy_user_presence = |_| panic!("Unexpected user presence check in CTAP1"); 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); + //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, TIMEOUT_CLOCK_VALUE); assert_eq!(response, Err(Ctap1StatusCode::SW_COND_USE_NOT_SATISFIED)); diff --git a/apps/vault/src/ctap/data_formats.rs b/apps/vault/src/ctap/data_formats.rs index 9e1ea7d3b..eeea4da9b 100644 --- a/apps/vault/src/ctap/data_formats.rs +++ b/apps/vault/src/ctap/data_formats.rs @@ -140,7 +140,7 @@ impl TryFrom for PublicKeyCredentialType { // https://www.w3.org/TR/webauthn/#dictdef-publickeycredentialparameters #[derive(PartialEq)] -#[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))] +// #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))] #[derive(Debug)] pub struct PublicKeyCredentialParameter { pub cred_type: PublicKeyCredentialType, diff --git a/apps/vault/src/ctap/mod.rs b/apps/vault/src/ctap/mod.rs index 58bab96ef..37db4fc1e 100644 --- a/apps/vault/src/ctap/mod.rs +++ b/apps/vault/src/ctap/mod.rs @@ -472,7 +472,7 @@ where // (self.check_user_presence)(cid)?; let mut desc = String::from(t!("vault.fido.exclude_list", xous::LANG)); desc.push_str( - &format!("\nRelying Party: {}\nUser name: {}", + &format!("\n\nRelying Party: {}\nUser name: {}", if let Some(name) = rp.rp_name {name.to_string()} else {rp_id.to_string()}, if let Some(name) = user.user_display_name {name} else {user.user_name.unwrap_or("*Unspecified*".to_string())}, ) @@ -523,7 +523,7 @@ where let mut make_cred_desc = String::from(t!("vault.fido.make_credential", xous::LANG)); let alt_name = user.user_name.as_deref().unwrap_or("*Unspecified*"); make_cred_desc.push_str( - &format!("\nRelying Party: {}\nUser name: {}", + &format!("\n\nRelying Party: {}\nUser name: {}", // prefer the "friendly name" over the technical name if let Some(name) = &rp.rp_name {name.to_string()} else {rp_id.to_string()}, if let Some(name) = &user.user_display_name {name} else {alt_name} @@ -815,7 +815,14 @@ where // For CTAP 2.1, it was moved to a later protocol step. if options.up { //(self.check_user_presence)(cid)?; - if crate::ux::request_permission_blocking("process_get_assertion requests user presence".to_string(), cid).is_none() { + let mut desc = String::from(t!("vault.fido.get_assertion", xous::LANG)); + desc.push_str( + &format!("\n\nRelying Party: {}\nClient data hash: {}", + rp_id, + hex::encode(&client_data_hash), + ) + ); + if crate::ux::request_permission_blocking(desc, cid).is_none() { return Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED) } } diff --git a/apps/vault/src/ctap/pin_protocol_v1.rs b/apps/vault/src/ctap/pin_protocol_v1.rs index b225d7cff..e8f23a91b 100644 --- a/apps/vault/src/ctap/pin_protocol_v1.rs +++ b/apps/vault/src/ctap/pin_protocol_v1.rs @@ -41,8 +41,10 @@ const PIN_TOKEN_LENGTH: usize = 32; /// Returns LEFT(HMAC(hmac_key, hmac_contents), 16) == pin_auth). fn verify_pin_auth(hmac_key: &[u8], hmac_contents: &[u8], pin_auth: &[u8]) -> bool { if pin_auth.len() != PIN_AUTH_LENGTH { + log::info!("pin_auth length is wrong: {}", pin_auth.len()); return false; } + log::info!("key {:?}, contents: {:?}, pin: {:?}", hmac_key, hmac_contents, pin_auth); verify_hmac_256_first_128bits::( hmac_key, hmac_contents, @@ -247,10 +249,14 @@ impl PinProtocolV1 { pin_auth: &[u8], authenticated_message: &[u8], ) -> Result<[u8; 32], Ctap2StatusCode> { + log::info!("cosekey: {:?}", key_agreement); let pk: ctap_crypto::ecdh::PubKey = CoseKey::try_into(key_agreement)?; + log::info!("pk: {:?}", pk); let shared_secret = self.key_agreement_key.exchange_x_sha256(&pk); + log::info!("shared_secret: {:?}", shared_secret); if !verify_pin_auth(&shared_secret, authenticated_message, pin_auth) { + log::info!("pin_auth invalid"); return Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID); } @@ -287,10 +293,12 @@ impl PinProtocolV1 { new_pin_enc: Vec, ) -> Result<(), Ctap2StatusCode> { if persistent_store.pin_hash()?.is_some() { + log::info!("pin_hash() is_some()"); return Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID); } let pin_decryption_key = self.exchange_decryption_key(key_agreement, &pin_auth, &new_pin_enc)?; + log::info!("exchanged decryption key"); check_and_store_new_pin(persistent_store, &pin_decryption_key, new_pin_enc)?; persistent_store.reset_pin_retries()?; Ok(()) diff --git a/apps/vault/src/ctap/storage.rs b/apps/vault/src/ctap/storage.rs index 4b57c3281..8674a9a11 100644 --- a/apps/vault/src/ctap/storage.rs +++ b/apps/vault/src/ctap/storage.rs @@ -623,7 +623,10 @@ openssl asn1parse -in opensk_cert.pem -inform pem } } Err(e) => match e.kind() { - std::io::ErrorKind::NotFound => Ok(None), + std::io::ErrorKind::NotFound => { + log::info!("pin_hash not found"); + Ok(None) + }, _ => Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR), // PDDB internal error } } diff --git a/apps/vault/src/ctap/timed_permission.rs b/apps/vault/src/ctap/timed_permission.rs index f937fcbc0..6cd8db87b 100644 --- a/apps/vault/src/ctap/timed_permission.rs +++ b/apps/vault/src/ctap/timed_permission.rs @@ -106,12 +106,13 @@ mod test { const CLOCK_FREQUENCY_HZ: usize = 1000; const ZERO: ClockValue = ClockValue::new(0, CLOCK_FREQUENCY_HZ); - const BIG_POSITIVE: ClockValue = ClockValue::new(isize::MAX / 1000 - 1, CLOCK_FREQUENCY_HZ); + const BIG_POSITIVE: ClockValue = ClockValue::new(i64::MAX / 1000 - 1, CLOCK_FREQUENCY_HZ); const NEGATIVE: ClockValue = ClockValue::new(-1, CLOCK_FREQUENCY_HZ); - const SMALL_NEGATIVE: ClockValue = ClockValue::new(isize::MIN / 1000 + 1, CLOCK_FREQUENCY_HZ); + const SMALL_NEGATIVE: ClockValue = ClockValue::new(i64::MIN / 1000 + 1, CLOCK_FREQUENCY_HZ); const REQUEST_DURATION: Duration = Duration::from_ms(1000); const PRESENCE_DURATION: Duration = Duration::from_ms(1000); + /* // ux tests not valid in xous fn grant_up_when_needed(start_time: ClockValue) { let mut u2f_state = U2fUserPresenceState::new(REQUEST_DURATION, PRESENCE_DURATION); assert!(!u2f_state.consume_up(start_time)); @@ -136,8 +137,8 @@ mod test { u2f_state.grant_up(start_time); // The timeout excludes equality, so it should be over at this instant. assert!(!u2f_state.consume_up(start_time.wrapping_add(PRESENCE_DURATION))); - } - + } */ +/* #[test] fn test_grant_up_timeout() { grant_up_timeout(ZERO); @@ -168,5 +169,5 @@ mod test { u2f_state.grant_up(ZERO); assert!(!u2f_state.is_up_needed(ZERO)); assert!(!u2f_state.consume_up(ZERO)); - } + }*/ } diff --git a/apps/vault/src/ux.rs b/apps/vault/src/ux.rs index d5df17b60..b8af2e964 100644 --- a/apps/vault/src/ux.rs +++ b/apps/vault/src/ux.rs @@ -315,7 +315,7 @@ pub(crate) fn start_ux_thread() { if deferred { last_timer = 1 + (prompt_expiration_ms - tt.elapsed_ms() as i64 - 1) / 1000; request_str.push_str( - &format!("\n\n⚠ {}{} ⚠", + &format!("\n\n⚠ {}{} ⚠\n", last_timer, t!("vault.fido.countdown", xous::LANG) )); @@ -388,13 +388,12 @@ pub(crate) fn start_ux_thread() { log::info!("new_timer: {}", new_timer); let mut request_str = String::from(&request_str_base); request_str.push_str( - &format!("{}\n\n⚠ {}{} ⚠", - &request_str_base, + &format!("\n\n⚠ {}{} ⚠\n", new_timer, t!("vault.fido.countdown", xous::LANG) )); modals.dynamic_notification_update( - None, + Some(t!("vault.u2freq", xous::LANG)), Some(&request_str), ).unwrap(); last_timer = new_timer;