diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 97aa2d30..e37e0c30 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -1375,9 +1375,9 @@ checksum = "914a755b7c2d4af2bdcff7ce1739e2db9a1b81a9b07123d8015786ae03c0980d" [[package]] name = "ctutils" -version = "0.4.3" +version = "0.4.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "03bb0e1cc970d482d121d9a1744999169b69a07470b3d644a7894e53fcaf4574" +checksum = "38d28fb6aa7dd680b3ffd1a3e64e115c177bdba63c0ae7f9cd48129ff05c4fa0" dependencies = [ "cmov", ] diff --git a/src-tauri/enterprise/fido2/src/hid.rs b/src-tauri/enterprise/fido2/src/hid.rs index 17ca8b93..92fa49a9 100644 --- a/src-tauri/enterprise/fido2/src/hid.rs +++ b/src-tauri/enterprise/fido2/src/hid.rs @@ -38,17 +38,22 @@ const CTAP2_ERR_PIN_REQUIRED: u8 = 0x36; const CTAP2_ERR_PIN_AUTH_BLOCKED: u8 = 0x34; const CTAP2_ERR_ACTION_TIMEOUT: u8 = 0x3A; -/// The crate surfaces the status only inside its error text, as `"0x31 CTAP2_ERR_PIN_INVALID ..."`. -/// Read back the leading byte, matching on the name conflates a rejected PIN with a missing one. -fn ctap_status(err: &impl std::fmt::Display) -> Option { - let text = err.to_string(); - let code = text.strip_prefix("0x")?.get(..2)?; +/// The crate surfaces the status only inside its error text, as +/// `"response_status err = 0x31 CTAP2_ERR_PIN_INVALID ..."`. Drop the prefix it adds. +fn ctap_text(text: &str) -> &str { + text.strip_prefix("response_status err = ").unwrap_or(text) +} + +/// Read back the status byte, matching on the name conflates a rejected PIN with a missing one. +fn ctap_status(text: &str) -> Option { + let code = ctap_text(text).strip_prefix("0x")?.get(..2)?; u8::from_str_radix(code, 16).ok() } /// Classify a CTAP failure. Everything recognised here is user-fixable. fn ceremony_error(err: &impl std::fmt::Display) -> Fido2Error { - match ctap_status(err) { + let text = err.to_string(); + match ctap_status(&text) { Some(CTAP2_ERR_USER_ACTION_TIMEOUT | CTAP2_ERR_ACTION_TIMEOUT) => Fido2Error::Timeout, Some(CTAP2_ERR_CREDENTIAL_EXCLUDED) => Fido2Error::CredentialExcluded, Some(CTAP2_ERR_NO_CREDENTIALS) => Fido2Error::NoCredentials, @@ -56,7 +61,7 @@ fn ceremony_error(err: &impl std::fmt::Display) -> Fido2Error { Some(CTAP2_ERR_PIN_INVALID) => Fido2Error::PinInvalid, Some(CTAP2_ERR_PIN_BLOCKED | CTAP2_ERR_PIN_AUTH_BLOCKED) => Fido2Error::PinBlocked, code => Fido2Error::Backend { - message: err.to_string(), + message: ctap_text(&text).to_string(), code: code.map(i32::from), }, } @@ -258,18 +263,18 @@ mod tests { /// The crate renders the status byte into its message and nowhere else. #[test] fn test_ctap_status_is_read_back_from_the_error_text() { - struct Rendered(&'static str); - impl std::fmt::Display for Rendered { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str(self.0) - } - } - assert_eq!( - ctap_status(&Rendered("0x31 CTAP2_ERR_PIN_INVALID PIN Invalid.")), + ctap_status("0x31 CTAP2_ERR_PIN_INVALID PIN Invalid."), Some(CTAP2_ERR_PIN_INVALID) ); - assert_eq!(ctap_status(&Rendered("device not found")), None); + assert_eq!( + ctap_status( + "response_status err = 0x19 CTAP2_ERR_CREDENTIAL_EXCLUDED Valid credential found \ + in the exclude list." + ), + Some(CTAP2_ERR_CREDENTIAL_EXCLUDED) + ); + assert_eq!(ctap_status("device not found"), None); } #[test] @@ -304,7 +309,7 @@ mod tests { f.write_str(&self.0) } } - let rendered = |code: u8| Rendered(format!("0x{code:02X} something")); + let rendered = |code: u8| Rendered(format!("response_status err = 0x{code:02X} something")); assert!(matches!( ceremony_error(&rendered(CTAP2_ERR_CREDENTIAL_EXCLUDED)), @@ -327,8 +332,8 @@ mod tests { ceremony_error(&rendered(0x7F)), Fido2Error::Backend { code: Some(0x7F), - .. - } + message, + } if message == "0x7F something" )); } } diff --git a/src-tauri/enterprise/fido2/src/windows/mod.rs b/src-tauri/enterprise/fido2/src/windows/mod.rs index aa1dec9f..c7681829 100644 --- a/src-tauri/enterprise/fido2/src/windows/mod.rs +++ b/src-tauri/enterprise/fido2/src/windows/mod.rs @@ -20,8 +20,8 @@ use windows::{ core::{BOOL, GUID, HRESULT}, Win32::{ Foundation::{ - ERROR_CANCELLED, ERROR_TIMEOUT, HWND, NTE_DEVICE_NOT_FOUND, NTE_INVALID_PARAMETER, - NTE_NOT_FOUND, NTE_USER_CANCELLED, + ERROR_CANCELLED, ERROR_TIMEOUT, HWND, NTE_DEVICE_NOT_FOUND, NTE_EXISTS, + NTE_INVALID_PARAMETER, NTE_NOT_FOUND, NTE_USER_CANCELLED, }, Networking::WindowsWebServices::{ WEBAUTHN_ASSERTION, WEBAUTHN_ASSERTION_VERSION_4, @@ -159,6 +159,9 @@ fn classify(name: &str, hr: HRESULT, cancelled: bool, timed_out: bool) -> Fido2E if hr == NTE_NOT_FOUND { return Fido2Error::NoCredentials; } + if hr == NTE_EXISTS { + return Fido2Error::CredentialExcluded; + } if hr == ERROR_TIMEOUT_HR { return Fido2Error::Timeout; } @@ -789,6 +792,10 @@ mod tests { classify("InvalidStateError", E_FAIL, false, false), Fido2Error::CredentialExcluded )); + assert!(matches!( + classify("", NTE_EXISTS, false, false), + Fido2Error::CredentialExcluded + )); } /// An unclassified failure still carries the status, so a report has something searchable. diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 880ee56f..e367836f 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -2419,7 +2419,7 @@ fn fido2_message(err: &Fido2Error, ceremony: &str) -> String { "Security key did not respond. Check that it is plugged in and try again".to_string() } Fido2Error::CredentialExcluded => { - "This security key is already registered for your account".to_string() + "This security key is already registered. Use a different key".to_string() } // Windows offers a phone and Windows Hello alongside the key, and only says which one // answered afterwards, so this is where that choice is turned back. Name the option.