diff --git a/.idea/.gitignore b/.idea/.gitignore new file mode 100644 index 00000000..13566b81 --- /dev/null +++ b/.idea/.gitignore @@ -0,0 +1,8 @@ +# Default ignored files +/shelf/ +/workspace.xml +# Editor-based HTTP Client requests +/httpRequests/ +# Datasource local storage ignored files +/dataSources/ +/dataSources.local.xml diff --git a/.idea/linux-webauthn-platform-api.iml b/.idea/linux-webauthn-platform-api.iml new file mode 100644 index 00000000..5e2b0d78 --- /dev/null +++ b/.idea/linux-webauthn-platform-api.iml @@ -0,0 +1,12 @@ + + + + + + + + + + + + \ No newline at end of file diff --git a/.idea/modules.xml b/.idea/modules.xml new file mode 100644 index 00000000..41b72fe7 --- /dev/null +++ b/.idea/modules.xml @@ -0,0 +1,8 @@ + + + + + + + + \ No newline at end of file diff --git a/.idea/vcs.xml b/.idea/vcs.xml new file mode 100644 index 00000000..35eb1ddf --- /dev/null +++ b/.idea/vcs.xml @@ -0,0 +1,6 @@ + + + + + + \ No newline at end of file diff --git a/xyz-iinuwa-credential-manager-portal-gtk/Cargo.lock b/xyz-iinuwa-credential-manager-portal-gtk/Cargo.lock index 68a06f25..37b9b787 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/Cargo.lock +++ b/xyz-iinuwa-credential-manager-portal-gtk/Cargo.lock @@ -831,7 +831,7 @@ dependencies = [ "heapless-bytes", "iso7816", "serde", - "serde-indexed", + "serde-indexed 0.1.1", "serde_bytes", "serde_repr", ] @@ -1928,7 +1928,7 @@ dependencies = [ [[package]] name = "libwebauthn" version = "0.1.2" -source = "git+https://github.com/linux-credentials/libwebauthn?rev=528af8de3adfbc329b6bd6dca7bf48714e674f96#528af8de3adfbc329b6bd6dca7bf48714e674f96" +source = "git+https://github.com/linux-credentials/libwebauthn?rev=c61492dcc66cc53b33e9d3eb3377017019332964#c61492dcc66cc53b33e9d3eb3377017019332964" dependencies = [ "aes", "async-trait", @@ -1955,7 +1955,7 @@ dependencies = [ "p256", "rand 0.8.5", "serde", - "serde-indexed", + "serde-indexed 0.2.0", "serde_bytes", "serde_cbor", "serde_derive", @@ -2944,6 +2944,17 @@ dependencies = [ "syn", ] +[[package]] +name = "serde-indexed" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f68cf7478db8b81abcf71b6d195a34a4891bd3d39868731c4d73194d74ec7a3" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "serde-xml-rs" version = "0.6.0" @@ -3101,17 +3112,16 @@ checksum = "8917285742e9f3e1683f0a9c4e6b57960b7314d0b08d30d1ecd426713ee2eee9" [[package]] name = "snow" -version = "0.10.0-alpha.1" +version = "0.10.0-beta.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e55510cf1cf5094776adcdc1141531281f3a7e3f4840c02f0495b489b766f7aa" +checksum = "7c689cc478c547a02061bbf874efca827c63ef1112ad396957411326742dccc8" dependencies = [ "aes-gcm", "blake2", - "byteorder", "chacha20poly1305", "curve25519-dalek", + "getrandom 0.3.2", "p256", - "rand_core 0.6.4", "ring", "rustc_version", "sha2", diff --git a/xyz-iinuwa-credential-manager-portal-gtk/Cargo.toml b/xyz-iinuwa-credential-manager-portal-gtk/Cargo.toml index 7feec3fb..c7a1ca1e 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/Cargo.toml +++ b/xyz-iinuwa-credential-manager-portal-gtk/Cargo.toml @@ -20,7 +20,7 @@ serde_json = "1.0.140" tracing = "0.1.41" tracing-subscriber = "0.3" zbus = { version = "5.5.0", default-features = false, features = ["blocking-api", "tokio"] } -libwebauthn = { git = "https://github.com/linux-credentials/libwebauthn", rev = "528af8de3adfbc329b6bd6dca7bf48714e674f96" } +libwebauthn = { git = "https://github.com/linux-credentials/libwebauthn", rev = "c61492dcc66cc53b33e9d3eb3377017019332964" } async-trait = "0.1.88" tokio = { version = "1.45.0", features = ["rt-multi-thread"] } futures-lite = "2.6.0" diff --git a/xyz-iinuwa-credential-manager-portal-gtk/data/resources/ui/window.ui b/xyz-iinuwa-credential-manager-portal-gtk/data/resources/ui/window.ui index 3a554c77..555028b4 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/data/resources/ui/window.ui +++ b/xyz-iinuwa-credential-manager-portal-gtk/data/resources/ui/window.ui @@ -210,6 +210,31 @@ + + + failed + Something went wrong. + + + vertical + + + + + + ExampleApplicationWindow + + + + Something went wrong while retrieving a credential. Please try again later or use a different authenticator. + + + + + + + + diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/hybrid.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/hybrid.rs index 137f1ac5..ee3b5042 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/hybrid.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/hybrid.rs @@ -8,7 +8,7 @@ use libwebauthn::webauthn::{Error as WebAuthnError, WebAuthn}; use crate::dbus::CredentialRequest; -use super::AuthenticatorResponse; +use super::{AuthenticatorResponse, Error}; pub(crate) trait HybridHandler { fn start( @@ -30,6 +30,7 @@ impl HybridHandler for InternalHybridHandler { &self, request: &CredentialRequest, ) -> impl Stream + Unpin + Send + Sized + 'static { + tracing::debug!("Starting hybrid operation"); let request = request.clone(); let (tx, rx) = async_std::channel::unbounded(); tokio::spawn(async move { @@ -58,19 +59,31 @@ impl HybridHandler for InternalHybridHandler { if let Err(err) = tx.send(HybridStateInternal::Connected).await { tracing::error!("Failed to send caBLE update: {:?}", err) } - let response: AuthenticatorResponse = loop { + tracing::debug!("Polling hybrid channel for updates."); + let response: Result = loop { match &request { CredentialRequest::CreatePublicKeyCredentialRequest(make_request) => { match channel.webauthn_make_credential(make_request).await { Ok(response) => break Ok(response.into()), Err(WebAuthnError::Ctap(ctap_error)) => { if ctap_error.is_retryable_user_error() { - tracing::debug!("Oops, try again! Error: {}", ctap_error); + tracing::debug!("Retrying credential creation operation because of CTAP error: {:?}", ctap_error); continue; + } else { + tracing::error!( + "Received CTAP unrecoverable CTAP error: {:?}", + ctap_error + ); + break Err(Error::AuthenticatorError); } - break Err(WebAuthnError::Ctap(ctap_error)); } - Err(err) => break Err(err), + Err(err) => { + tracing::error!( + "Received unrecoverable error from authenticator: {:?}", + err + ); + break Err(Error::AuthenticatorError); + } }; } CredentialRequest::GetPublicKeyCredentialRequest(get_request) => { @@ -78,18 +91,32 @@ impl HybridHandler for InternalHybridHandler { Ok(response) => break Ok(response.into()), Err(WebAuthnError::Ctap(ctap_error)) => { if ctap_error.is_retryable_user_error() { - println!("Oops, try again! Error: {}", ctap_error); + tracing::debug!("Retrying assertion operation because of CTAP error: {:?}", ctap_error); continue; + } else { + tracing::error!( + "Received CTAP unrecoverable CTAP error: {:?}", + ctap_error + ); + break Err(Error::AuthenticatorError); } - break Err(WebAuthnError::Ctap(ctap_error)); } - Err(err) => break Err(err), + Err(err) => { + tracing::error!( + "Received unrecoverable error from authenticator: {:?}", + err + ); + break Err(Error::AuthenticatorError); + } }; } } - } - .unwrap(); - if let Err(err) = tx.send(HybridStateInternal::Completed(response)).await { + }; + let terminal_state = match response { + Ok(auth_response) => HybridStateInternal::Completed(auth_response), + Err(_) => HybridStateInternal::Failed, + }; + if let Err(err) = tx.send(terminal_state).await { tracing::error!("Failed to send caBLE update: {:?}", err) } }); @@ -102,6 +129,7 @@ impl HybridHandler for InternalHybridHandler { } } +/// Used to communicate privileged state between handler and credential service. #[derive(Clone, Debug)] pub(super) enum HybridStateInternal { /// Awaiting BLE advert from phone. Content is the FIDO string to be @@ -117,16 +145,20 @@ pub(super) enum HybridStateInternal { /// Authenticator data Completed(AuthenticatorResponse), + Failed, // TODO(cancellation) // This isn't actually sent from the server. #[allow(dead_code)] UserCancelled, } +// this is here to prevent making HybridStateInternal public to the whole crate. +/// Messages between hybrid handler and credential service. pub struct HybridEvent { pub(super) state: HybridStateInternal, } +/// Used to communicate privileged state between credential service and UI. #[derive(Clone, Debug)] pub enum HybridState { /// Awaiting BLE advert from phone. Content is the FIDO string to be displayed to the user, which contains QR secret @@ -139,9 +171,12 @@ pub enum HybridState { /// Tunnel is established, waiting for user to release credential on their device. Connected, - /// Authenticator data + /// Authenticator data has been received Completed, + /// Hybrid operation failed. + Failed, + // This isn't actually sent from the server. UserCancelled, } @@ -154,6 +189,7 @@ impl From for HybridState { HybridStateInternal::Connected => HybridState::Connected, HybridStateInternal::Completed(_) => HybridState::Completed, HybridStateInternal::UserCancelled => HybridState::UserCancelled, + HybridStateInternal::Failed => HybridState::Failed, } } } diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs index 998424bf..fe3f49cb 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/mod.rs @@ -194,6 +194,23 @@ enum AuthenticatorResponse { CredentialsAsserted(GetAssertionResponse), } +#[derive(Debug, Clone)] +pub enum Error { + /// Some unknown error with the authenticator occurred. + AuthenticatorError, + /// No matching credentials were found on the device. + NoCredentials, + /// Too many incorrect PIN attempts, and authenticator must be removed and + /// reinserted to continue any more PIN attempts. + /// + /// Note that this is different than exhausting the PIN count that fully + /// locks out the device. + PinAttemptsExhausted, + // TODO: We may want to hide the details on this variant from the public API. + /// Something went wrong with the credential service itself, not the authenticator. + Internal(String), +} + impl From for AuthenticatorResponse { fn from(value: MakeCredentialResponse) -> Self { Self::CredentialCreated(value) diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs index 64d3b275..5de60a82 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/credential_service/usb.rs @@ -5,6 +5,7 @@ use base64::{self, engine::general_purpose::URL_SAFE_NO_PAD, Engine}; use futures_lite::Stream; use libwebauthn::{ ops::webauthn::GetAssertionResponse, + proto::CtapError, transport::{hid::HidDevice, Device}, webauthn::{Error as WebAuthnError, WebAuthn}, UxUpdate, @@ -17,7 +18,7 @@ use crate::{ view_model::Credential, }; -use super::{AuthenticatorResponse, CredentialResponse}; +use super::{AuthenticatorResponse, CredentialResponse, Error}; pub(crate) trait UsbHandler { fn start( @@ -33,25 +34,40 @@ impl InProcessUsbHandler { async fn process( tx: Sender, cred_request: CredentialRequest, - ) -> Result<(), String> { + ) -> Result<(), Error> { let mut state = UsbStateInternal::Idle; let (signal_tx, mut signal_rx) = mpsc::channel(256); let (cred_tx, mut cred_rx) = mpsc::channel(1); debug!("polling for USB status"); + let mut failures = 0; + // act on current USB USB state, send state changes to the stream, and + // loop until a credential or error is returned. loop { tracing::debug!("current usb state: {:?}", state); let prev_usb_state = state; let next_usb_state = match prev_usb_state { UsbStateInternal::Idle | UsbStateInternal::Waiting => { - let mut hid_devices = - libwebauthn::transport::hid::list_devices().await.unwrap(); - if hid_devices.is_empty() { - let state = UsbStateInternal::Waiting; - Ok(state) - } else if hid_devices.len() == 1 { - Ok(UsbStateInternal::Connected(hid_devices.swap_remove(0))) - } else { - Ok(UsbStateInternal::SelectingDevice(hid_devices)) + match libwebauthn::transport::hid::list_devices().await { + Ok(mut hid_devices) => { + if hid_devices.is_empty() { + let state = UsbStateInternal::Waiting; + Ok(state) + } else if hid_devices.len() == 1 { + Ok(UsbStateInternal::Connected(hid_devices.swap_remove(0))) + } else { + Ok(UsbStateInternal::SelectingDevice(hid_devices)) + } + } + Err(err) => { + failures += 1; + if failures == 5 { + Err(Error::Internal(format!("Failed to list USB authenticators: {:?}. Cancelling USB state updates.", err))) + } else { + tracing::warn!("Failed to list USB authenticators: {:?}. Throttling USB state updates", err); + tokio::time::sleep(Duration::from_secs(1)).await; + Ok(prev_usb_state) + } + } } } UsbStateInternal::SelectingDevice(hid_devices) => { @@ -61,18 +77,36 @@ impl InProcessUsbHandler { for mut device in hid_devices { let tx = blinking_tx.clone(); tokio::spawn(async move { - let (mut channel, _state_rx) = device.channel().await.unwrap(); - let res = channel - .blink_and_wait_for_user_presence(Duration::from_secs(300)) - .await; - drop(channel); - match res { - Ok(true) => { - let _ = tx.send(Some(device)).await; - } - Ok(false) | Err(_) => { - let _ = tx.send(None).await; - } + let res = match device.channel().await { + Ok((ref mut channel, _)) => channel + .blink_and_wait_for_user_presence(Duration::from_secs(300)) + .await + .map_err(|err| { + format!( + "Failed to send wink request to authenticator: {:?}", + err + ) + }) + .and_then(|blinking| { + if blinking { + Ok(()) + } else { + Err("Authenticator was not able to blink".to_string()) + } + }), + Err(err) => Err(format!( + "Failed to create channel for USB authenticator: {:?}", + err + )), + } + .inspect_err(|err| tracing::warn!(err)) + .ok(); + + if let Err(err) = tx.send(res.map(|_| device)).await { + tracing::error!( + "Failed to send notification of wink response: {:?}", + err, + ); } }); } @@ -142,25 +176,30 @@ impl InProcessUsbHandler { } }, Some(Err(err)) => Err(err.clone()), - None => Err("Channel disconnected".to_string()), + None => Err(Error::Internal("Channel disconnected".to_string())), } } + // TODO: This match arm does basically the same thing as above, we + // should refactor this so we don't have to update things in two + // places. UsbStateInternal::NeedsPin { .. } | UsbStateInternal::NeedsUserVerification { .. } | UsbStateInternal::NeedsUserPresence => match signal_rx.recv().await { - Some(msg) => match msg? { - UsbUvMessage::NeedsPin { + Some(msg) => match msg { + Ok(UsbUvMessage::NeedsPin { attempts_left, pin_tx, - } => Ok(UsbStateInternal::NeedsPin { + }) => Ok(UsbStateInternal::NeedsPin { attempts_left, pin_tx, }), - UsbUvMessage::NeedsUserVerification { attempts_left } => { + Ok(UsbUvMessage::NeedsUserVerification { attempts_left }) => { Ok(UsbStateInternal::NeedsUserVerification { attempts_left }) } - UsbUvMessage::NeedsUserPresence => Ok(UsbStateInternal::NeedsUserPresence), - UsbUvMessage::ReceivedCredentials(response) => match response { + Ok(UsbUvMessage::NeedsUserPresence) => { + Ok(UsbStateInternal::NeedsUserPresence) + } + Ok(UsbUvMessage::ReceivedCredentials(response)) => match response { AuthenticatorResponse::CredentialCreated(make_credential_response) => { Ok(UsbStateInternal::Completed( CredentialResponse::from_make_credential( @@ -186,10 +225,10 @@ impl InProcessUsbHandler { } } }, + Err(err) => Err(err), }, - None => Err("USB UV handler channel closed".to_string()), + None => Err(Error::Internal("USB UV handler channel closed".to_string())), }, - UsbStateInternal::Completed(_) => Ok(prev_usb_state), UsbStateInternal::SelectCredential { response, cred_tx: _, @@ -222,22 +261,23 @@ impl InProcessUsbHandler { ), ), )), - None => Err("Selected credential not found.".to_string()), + None => Err(Error::NoCredentials), } } None => { tracing::debug!("cred channel closed before receiving cred from client."); - Err("Cred channel disconnected".to_string()) + Err(Error::Internal( + "Cred channel disconnected prematurely".to_string(), + )) } }, + UsbStateInternal::Completed(_) => break Ok(()), + UsbStateInternal::Failed(err) => break Err(err), }; - state = next_usb_state?; - tx.send(state.clone()) - .await - .map_err(|_| "Receiver channel closed".to_string())?; - if let UsbStateInternal::Completed(_) = state { - break Ok(()); - } + state = next_usb_state.map_or_else(|err| UsbStateInternal::Failed(err), |s| s); + tx.send(state.clone()).await.map_err(|_| { + Error::Internal("USB state channel receiver closed prematurely".to_string()) + })?; } } } @@ -245,70 +285,66 @@ impl InProcessUsbHandler { async fn handle_events( cred_request: &CredentialRequest, mut device: HidDevice, - signal_tx: &Sender>, + signal_tx: &Sender>, ) { - let (mut channel, state_rx) = device.channel().await.unwrap(); - let signal_tx2 = signal_tx.clone().downgrade(); - tokio::spawn(async move { - handle_usb_updates(&signal_tx2, state_rx).await; - debug!("Reached end of USB update task"); - }); - match cred_request { - CredentialRequest::CreatePublicKeyCredentialRequest(make_cred_request) => loop { - match channel.webauthn_make_credential(make_cred_request).await { - Ok(response) => { - notify_ceremony_completed( - signal_tx, - AuthenticatorResponse::CredentialCreated(response), - ) - .await; - break; - } - Err(WebAuthnError::Ctap(ctap_error)) if ctap_error.is_retryable_user_error() => { - warn!("Retrying WebAuthn make credential operation"); - continue; - } - Err(err) => { - notify_ceremony_failed(signal_tx, err.to_string()).await; - break; - } - }; - }, - CredentialRequest::GetPublicKeyCredentialRequest(get_cred_request) => loop { - match channel.webauthn_get_assertion(get_cred_request).await { - Ok(response) => { - notify_ceremony_completed( - signal_tx, - AuthenticatorResponse::CredentialsAsserted(response), - ) - .await; - break; - } - Err(WebAuthnError::Ctap(ctap_error)) if ctap_error.is_retryable_user_error() => { - warn!("Retrying WebAuthn get credential operation"); - continue; - } - Err(err) => { - notify_ceremony_failed(signal_tx, err.to_string()).await; - break; + let device_debug = device.to_string(); + match device.channel().await { + Err(err) => { + tracing::error!("Failed to open channel to USB authenticator, cannot receive user verification events: {:?}", err); + } + Ok((mut channel, state_rx)) => { + let signal_tx2 = signal_tx.clone().downgrade(); + tokio::spawn(async move { + handle_usb_updates(&signal_tx2, state_rx).await; + debug!("Reached end of USB update task"); + }); + tracing::debug!( + "Polling for credential from USB authenticator {}", + &device_debug + ); + let response: Result = loop { + let response = match cred_request { + CredentialRequest::CreatePublicKeyCredentialRequest(make_cred_request) => { + channel + .webauthn_make_credential(make_cred_request) + .await + .map(|response| UsbUvMessage::ReceivedCredentials(response.into())) + } + CredentialRequest::GetPublicKeyCredentialRequest(get_cred_request) => channel + .webauthn_get_assertion(get_cred_request) + .await + .map(|response| UsbUvMessage::ReceivedCredentials(response.into())), + }; + match response { + Ok(response) => { + tracing::debug!("Received credential from USB authenticator"); + break Ok(response); + } + Err(WebAuthnError::Ctap(ctap_error)) + if ctap_error.is_retryable_user_error() => + { + warn!("Retrying WebAuthn credential operation"); + continue; + } + Err(err) => { + tracing::warn!( + "Failed to make/get credential with USB authenticator: {:?}", + err + ); + break Err(err); + } } - }; - }, - }; -} - -async fn notify_ceremony_completed( - signal_tx: &Sender>, - response: AuthenticatorResponse, -) { - signal_tx - .send(Ok(UsbUvMessage::ReceivedCredentials(response))) - .await - .unwrap(); -} - -async fn notify_ceremony_failed(signal_tx: &Sender>, err: String) { - signal_tx.send(Err(err)).await.unwrap(); + } + .map_err(|err| match err { + WebAuthnError::Ctap(CtapError::PINAuthBlocked) => Error::PinAttemptsExhausted, + WebAuthnError::Ctap(CtapError::NoCredentials) => Error::NoCredentials, + _ => Error::AuthenticatorError, + }); + if let Err(err) = signal_tx.send(response).await { + tracing::error!("Failed to notify that ceremony completed: {:?}", err); + } + } + } } impl UsbHandler for InProcessUsbHandler { @@ -319,6 +355,8 @@ impl UsbHandler for InProcessUsbHandler { let request = request.clone(); let (tx, mut rx) = mpsc::channel(32); tokio::spawn(async move { + // TODO: instead of logging error here, push the errors into the + // stream so credential service can handle/forward them to the UI if let Err(err) = InProcessUsbHandler::process(tx, request).await { tracing::error!("Error getting credential from USB: {:?}", err); } @@ -331,10 +369,13 @@ impl UsbHandler for InProcessUsbHandler { } } +// this exists to prevent making UsbStateInternal type public to the whole crate. +/// A message between USB handler and credential service pub struct UsbEvent { pub(super) state: UsbStateInternal, } +/// Used to share internal state between handler and credential service #[derive(Clone, Debug, Default)] pub(super) enum UsbStateInternal { /// Not polling for FIDO USB device. @@ -344,6 +385,10 @@ pub(super) enum UsbStateInternal { /// Awaiting FIDO USB device to be plugged in. Waiting, + /// When we encounter multiple devices, we let all of them blink and continue + /// with the one that was tapped. + SelectingDevice(Vec), + /// USB device connected, prompt user to tap Connected(HidDevice), @@ -354,14 +399,12 @@ pub(super) enum UsbStateInternal { }, /// The device needs on-device user verification. - NeedsUserVerification { - attempts_left: Option, - }, + NeedsUserVerification { attempts_left: Option }, /// The device needs evidence of user presence (e.g. touch) to release the credential. NeedsUserPresence, - // Multiple credentials have been found and the user has to select which to use + /// Multiple credentials have been found and the user has to select which to use SelectCredential { response: GetAssertionResponse, cred_tx: mpsc::Sender, @@ -369,15 +412,15 @@ pub(super) enum UsbStateInternal { /// USB tapped, received credential Completed(CredentialResponse), + + /// There was an error while interacting with the authenticator. + Failed(Error), // TODO: implement cancellation // This isn't actually sent from the server. //UserCancelled, - - // When we encounter multiple devices, we let all of them blink and continue - // with the one that was tapped. - SelectingDevice(Vec), } +/// Used to share public state between credential service and UI. #[derive(Clone, Debug, Default)] pub enum UsbState { /// Not polling for FIDO USB device. @@ -387,6 +430,10 @@ pub enum UsbState { /// Awaiting FIDO USB device to be plugged in. Waiting, + // When we encounter multiple devices, we let all of them blink and continue + // with the one that was tapped. + SelectingDevice, + /// USB device connected, prompt user to tap Connected, @@ -403,23 +450,22 @@ pub enum UsbState { /// The device needs evidence of user presence (e.g. touch) to release the credential. NeedsUserPresence, - - /// USB tapped, received credential - Completed, // TODO: implement cancellation // This isn't actually sent from the server. //UserCancelled, - // When we encounter multiple devices, we let all of them blink and continue - // with the one that was tapped. - SelectingDevice, - // Multiple credentials have been found and the user has to select which to use // List of user-identities to decide which to use. SelectCredential { creds: Vec, cred_tx: mpsc::Sender, }, + + /// USB tapped, received credential + Completed, + + /// Interaction with the authenticator failed. + Failed(Error), } impl From for UsbState { @@ -474,12 +520,13 @@ impl From for UsbState { cred_tx, } } + UsbStateInternal::Failed(err) => UsbState::Failed(err), } } } async fn handle_usb_updates( - signal_tx: &WeakSender>, + signal_tx: &WeakSender>, mut state_rx: Receiver, ) { while let Some(msg) = state_rx.recv().await { @@ -489,25 +536,24 @@ async fn handle_usb_updates( }; match msg { UxUpdate::UvRetry { attempts_left } => { - signal_tx + if let Err(err) = signal_tx .send(Ok(UsbUvMessage::NeedsUserVerification { attempts_left })) .await - .unwrap(); + { + tracing::error!("Authenticator requested user verficiation, but we cannot relay the message to credential service: {:?}", err); + } } UxUpdate::PinRequired(pin_update) => { - if pin_update.attempts_left.is_some_and(|num| num <= 1) { - // TODO: cancel authenticator operation - signal_tx.send(Err("No more PIN attempts allowed. Select a different authenticator or try again later.".to_string())).await.unwrap(); - continue; - } let (pin_tx, mut pin_rx) = mpsc::channel(1); - signal_tx + if let Err(err) = signal_tx .send(Ok(UsbUvMessage::NeedsPin { pin_tx, attempts_left: pin_update.attempts_left, })) .await - .unwrap(); + { + tracing::error!("Authenticator requested a PIN from the user, but we cannot relay the message to the credential service: {:?}", err); + } match pin_rx.recv().await { Some(pin) => match pin_update.send_pin(&pin) { Ok(()) => {} @@ -517,16 +563,16 @@ async fn handle_usb_updates( } } UxUpdate::PresenceRequired => { - signal_tx - .send(Ok(UsbUvMessage::NeedsUserPresence)) - .await - .unwrap(); + if let Err(err) = signal_tx.send(Ok(UsbUvMessage::NeedsUserPresence)).await { + tracing::error!("Authenticator requested user presence, but we cannot relay the message to the credential service: {:?}", err); + } } } } debug!("USB update channel closed."); } +/// Messages sent between USB authenticator and handler for UV enum UsbUvMessage { NeedsPin { attempts_left: Option, diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/gtk/mod.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/gtk/mod.rs index 171ddfa8..b5f23254 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/gtk/mod.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/gtk/mod.rs @@ -48,6 +48,9 @@ mod imp { #[property(get, set)] pub completed: RefCell, + #[property(get, set)] + pub failed: RefCell, + // pub(super) vm: RefCell>, pub(super) rx: RefCell>>, pub(super) tx: RefCell>>, @@ -172,6 +175,11 @@ impl ViewModel { view_model.set_qr_spinner_visible(false); view_model.set_completed(true); } + ViewUpdate::Failed(error_msg) => { + view_model.set_qr_spinner_visible(false); + view_model.set_failed(true); + view_model.set_prompt(error_msg); + } } } Err(e) => { diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs index 27ee2657..c33b91b2 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/view_model/mod.rs @@ -10,7 +10,9 @@ use async_std::{ use tokio::sync::mpsc; use tracing::{error, info}; -use crate::credential_service::{CredentialServiceClient, UsbState}; +use crate::credential_service::{ + CredentialServiceClient, Error as CredentialServiceError, UsbState, +}; #[derive(Debug)] pub(crate) struct ViewModel @@ -166,35 +168,10 @@ impl ViewModel { async_std::task::spawn(async move { // TODO: add cancellation while let Some(usb_state) = stream.next().await { + // forward to background event loop tx.send(BackgroundEvent::UsbStateChanged(usb_state)) .await .unwrap(); - /* - Ok(usb_state) => { - let state = usb_state.into(); - if prev_state != state { - println!("{:?}", state); - tx.send(BackgroundEvent::UsbStateChanged(state.clone())) - .await - .unwrap(); - } - prev_state = state; - match prev_state { - UsbState::Completed => break, - UsbState::UserCancelled => break, - _ => {} - }; - async_std::task::sleep(Duration::from_millis(50)).await; - } - Err(err) => { - // TODO: move to error page - tracing::error!( - "There was an error trying to get credentials from USB: {}", - err - ); - break; - } - */ } }); } @@ -204,31 +181,10 @@ impl ViewModel { let mut stream = cred_service.lock().await.get_hybrid_credential().await; async_std::task::spawn(async move { while let Some(state) = stream.next().await { - let state = state.into(); - match state { - HybridState::Idle => {} - HybridState::Started(_) => { - tx.send(BackgroundEvent::HybridQrStateChanged(state)) - .await - .unwrap(); - } - HybridState::Connecting => { - tx.send(BackgroundEvent::HybridQrStateChanged(state)) - .await - .unwrap(); - } - HybridState::Connected => { - tx.send(BackgroundEvent::HybridQrStateChanged(state)) - .await - .unwrap(); - } - HybridState::Completed => { - tx.send(BackgroundEvent::HybridQrStateChanged(state)) - .await - .unwrap(); - } - HybridState::UserCancelled => break, - }; + // forward to background event loop + tx.send(BackgroundEvent::HybridQrStateChanged(state.into())) + .await + .unwrap(); } tracing::debug!("Broke out of hybrid QR state stream"); }); @@ -281,11 +237,7 @@ impl ViewModel { } } - Event::Background(BackgroundEvent::UsbPressed) => { - println!("UsbPressed"); - } Event::Background(BackgroundEvent::UsbStateChanged(state)) => { - // TODO: do we need to store the USB state? match state { UsbState::Connected => { info!("Found USB device") @@ -330,6 +282,18 @@ impl ViewModel { .await .unwrap(); } + // TODO: Provide more specific error messages using the wrapped Error. + UsbState::Failed(err) => { + let error_msg = String::from(match err { + CredentialServiceError::NoCredentials => "No matching credentials found on this authenticator.", + CredentialServiceError::PinAttemptsExhausted => "No more PIN attempts allowed. Try removing your device and plugging it back in.", + CredentialServiceError::AuthenticatorError | CredentialServiceError::Internal(_) => "Something went wrong while retrieving a credential. Please try again later or use a different authenticator.", + }); + self.tx_update + .send(ViewUpdate::Failed(error_msg)) + .await + .unwrap() + } } } Event::Background(BackgroundEvent::HybridQrStateChanged(state)) => { @@ -367,6 +331,10 @@ impl ViewModel { HybridState::UserCancelled => { self.hybrid_qr_code_data = None; } + HybridState::Failed => { + self.hybrid_qr_code_data = None; + self.tx_update.send(ViewUpdate::Failed(String::from("Something went wrong. Try again later or use a different authenticator."))).await.unwrap(); + } }; } }; @@ -386,20 +354,23 @@ pub enum ViewUpdate { SetTitle(String), SetDevices(Vec), SetCredentials(Vec), + WaitingForDevice(Device), + SelectingDevice, + UsbNeedsPin { attempts_left: Option }, UsbNeedsUserVerification { attempts_left: Option }, UsbNeedsUserPresence, - Completed, - SelectingDevice, HybridNeedsQrCode(String), HybridConnecting, HybridConnected, + + Completed, + Failed(String), } pub enum BackgroundEvent { - UsbPressed, UsbStateChanged(UsbState), HybridQrStateChanged(HybridState), } @@ -454,6 +425,9 @@ pub enum HybridState { // This isn't actually sent from the server. UserCancelled, + + /// Failed to receive a credential + Failed, } impl From for HybridState { @@ -468,6 +442,7 @@ impl From for HybridState { crate::credential_service::hybrid::HybridState::UserCancelled => { HybridState::UserCancelled } + crate::credential_service::hybrid::HybridState::Failed => HybridState::Failed, } } } diff --git a/xyz-iinuwa-credential-manager-portal-gtk/src/window.rs b/xyz-iinuwa-credential-manager-portal-gtk/src/window.rs index 2ec2f531..4da4aa50 100644 --- a/xyz-iinuwa-credential-manager-portal-gtk/src/window.rs +++ b/xyz-iinuwa-credential-manager-portal-gtk/src/window.rs @@ -190,6 +190,16 @@ impl ExampleApplicationWindow { } )); + view_model.connect_failed_notify(clone!( + #[weak] + stack, + move |vm| { + if vm.failed() { + stack.set_visible_child_name("failed"); + } + } + )); + view_model.connect_credentials_notify(clone!( #[weak] stack,