diff --git a/credentialsd-ui/src/dbus.rs b/credentialsd-ui/src/dbus.rs index 859d3b6..6f31e54 100644 --- a/credentialsd-ui/src/dbus.rs +++ b/credentialsd-ui/src/dbus.rs @@ -36,13 +36,11 @@ pub struct CredentialPortalBackend { #[derive(Debug, Clone)] pub(crate) struct UiContext { parent_window: Option, - origin: String, r#type: Operation, devices: Vec, app_id: String, app_display_name: String, app_pid: u32, - app_path: String, options: PortalBackendOptions, } @@ -56,12 +54,11 @@ impl CredentialPortalBackend { #[zbus(object_server)] object_server: &ObjectServer, handle: OwnedObjectPath, parent_window: Optional, - origin: String, + _origin: String, r#type: Operation, devices: Vec, app_id: String, app_pid: u32, - app_path: String, options: PortalBackendOptions, ) -> fdo::Result<()> { let Some(sender) = header.sender().map(|h| h.to_owned()) else { @@ -130,13 +127,11 @@ impl CredentialPortalBackend { let ui_context = UiContext { parent_window: parent_window.into(), - origin, r#type, devices, app_id, app_display_name, app_pid, - app_path, options, }; let ui_events_forwarder_task = Arc::new(AsyncMutex::new(None)); @@ -251,20 +246,28 @@ impl CeremonyObject { }; })); - // Assuming this is a PublicKey request, require the rp_id - let rp_id = self - .ui_context - .options - .rp_id - .as_ref() - .ok_or_else(|| { - { - fdo::Error::InvalidArgs( - "rp_id is required for public key credential requests".to_string(), - ) - } - })? - .to_string(); + // TODO: + // - calculate the registrable domain of the origin's hostname using Public Suffix List. + // - if rp_id does not match origin, then send both origin's domain and the + // domain and RP ID, and follow the guidance in WebAuthn level 3 for + // displaying dialogs for cross-origin ceremonies. + // https://www.w3.org/TR/webauthn-3/#sctn-cross-origin-use + let rp_id = match self.ui_context.r#type { + Operation::PublicKeyCreate | Operation::PublicKeyGet => self + .ui_context + .options + .rp_id + .as_ref() + .ok_or_else(|| { + { + fdo::Error::InvalidArgs( + "rp_id is required for public key credential requests".to_string(), + ) + } + })? + .to_string(), + }; + let req = ( ViewRequest { operation: self.ui_context.r#type.clone(), diff --git a/credentialsd-ui/src/gui/view_model/mod.rs b/credentialsd-ui/src/gui/view_model/mod.rs index 9ee4537..816f49b 100644 --- a/credentialsd-ui/src/gui/view_model/mod.rs +++ b/credentialsd-ui/src/gui/view_model/mod.rs @@ -90,15 +90,15 @@ impl ViewModel { // TRANSLATORS: %s1 is the "relying party" (e.g.: domain name) where the request is coming from // TRANSLATORS: %s2 is the application name (e.g.: firefox) where the request is coming from, must be left untouched to make the name bold // TRANSLATORS: %i1 is the process ID of the requesting application - // TRANSLATORS: %s3 is the absolute path (think: /usr/bin/firefox) of the requesting application - gettext("\"%s2\" (process ID: %i1, binary: %s3) is asking to create a credential to register at \"%s1\". Only proceed if you trust this process.") + // TRANSLATORS: %s3 is the app ID (think: org.mozilla.firefox) of the requesting application + gettext("\"%s2\" (process ID: %i1, app ID: %s3) is asking to create a credential to register at \"%s1\". Only proceed if you trust this process.") } Operation::PublicKeyGet => { // TRANSLATORS: %s1 is the "relying party" (think: domain name) where the request is coming from // TRANSLATORS: %s2 is the application name (e.g.: firefox) where the request is coming from, must be left untouched to make the name bold // TRANSLATORS: %i1 is the process ID of the requesting application - // TRANSLATORS: %s3 is the absolute path (think: /usr/bin/firefox) of the requesting application - gettext("\"%s2\" (process ID: %i1, binary: %s3) is asking to use a credential to sign in to \"%s1\". Only proceed if you trust this process.") + // TRANSLATORS: %s3 is the app ID (think: org.mozilla.firefox) of the requesting application + gettext("\"%s2\" (process ID: %i1, app ID: %s3) is asking to use a credential to sign in to \"%s1\". Only proceed if you trust this process.") } } .to_string(); diff --git a/credentialsd/src/dbus/flow_control.rs b/credentialsd/src/dbus/flow_control.rs index 045a30f..ed20433 100644 --- a/credentialsd/src/dbus/flow_control.rs +++ b/credentialsd/src/dbus/flow_control.rs @@ -11,9 +11,7 @@ use async_trait::async_trait; use credentialsd_common::server::{BackgroundEvent, WindowHandle}; use credentialsd_common::{ memfd::read_secret, - model::{ - Error as CredentialServiceError, Operation, PortalBackendOptions, UserInteractedEvent, - }, + model::{Error as CredentialServiceError, PortalBackendOptions, UserInteractedEvent}, }; use futures_lite::{Stream, StreamExt}; use tokio::sync::mpsc::Receiver; @@ -92,28 +90,12 @@ async fn handle Result { let (request_tx, request_rx) = oneshot::channel(); let request_id = svc.lock().await.init_request(&msg, request_tx).await?; - let operation = match &msg { - CredentialRequest::CreatePublicKeyCredentialRequest(_) => Operation::PublicKeyCreate, - CredentialRequest::GetPublicKeyCredentialRequest(_) => Operation::PublicKeyGet, - }; - let rp_id = match &msg { - CredentialRequest::CreatePublicKeyCredentialRequest(r) => r.relying_party.id.clone(), - CredentialRequest::GetPublicKeyCredentialRequest(r) => r.relying_party_id.clone(), - }; + let operation = msg.operation(); + let rp_id = msg.relying_party_id().to_string(); - // TODO: pass origin to this method so we can do this correctly. - let origin = match &msg { - CredentialRequest::CreatePublicKeyCredentialRequest(r) => r.origin.clone(), - CredentialRequest::GetPublicKeyCredentialRequest(r) => { - format!("https://{}", r.relying_party_id.clone()) - } - }; + let origin = msg.origin().to_string(); - // TODO: pass top_origin to this method so we can do this correctly. - let top_origin = match &msg { - CredentialRequest::CreatePublicKeyCredentialRequest(r) => None, - CredentialRequest::GetPublicKeyCredentialRequest(r) => None, - }; + let top_origin = msg.top_origin().map(|o| o.to_string()); let initial_devices = svc .lock() .await @@ -123,7 +105,6 @@ async fn handle, app_id: String, app_pid: u32, - app_path: String, options: PortalBackendOptions, ) -> impl Future>> + Send; } @@ -50,7 +49,6 @@ trait UiControlService2 { devices: Vec, app_id: String, app_pid: u32, - app_path: String, options: PortalBackendOptions, ) -> fdo::Result<()>; } @@ -118,7 +116,6 @@ impl UiController for UiControlServiceClient { devices: Vec, app_id: String, app_pid: u32, - app_path: String, options: PortalBackendOptions, ) -> Result> { let ceremony = CeremonyObjectProxy::new(&self.conn, handle.clone()).await?; @@ -138,7 +135,6 @@ impl UiController for UiControlServiceClient { devices, app_id, app_pid, - app_path, options, ) .await?; diff --git a/credentialsd/src/gateway/dbus.rs b/credentialsd/src/gateway/dbus.rs index b4e4355..9242869 100644 --- a/credentialsd/src/gateway/dbus.rs +++ b/credentialsd/src/gateway/dbus.rs @@ -103,8 +103,6 @@ impl CredentialPortalGateway { ); let request = CreateCredentialRequest { - origin: Some(origin.clone()), - is_same_origin: Some(top_origin.is_none()), r#type: cred_type.to_string(), public_key: Some(CreatePublicKeyCredentialRequest { request_json }), }; @@ -167,8 +165,6 @@ impl CredentialPortalGateway { ); let request = GetCredentialRequest { - origin: Some(origin), - is_same_origin: Some(top_origin.is_none()), public_key: Some(GetPublicKeyCredentialRequest { request_json }), }; diff --git a/credentialsd/src/gateway/mod.rs b/credentialsd/src/gateway/mod.rs index 149a433..75a90d8 100644 --- a/credentialsd/src/gateway/mod.rs +++ b/credentialsd/src/gateway/mod.rs @@ -62,8 +62,6 @@ impl From for ClientDetails { fn from(value: RequestContext) -> Self { ClientDetails { app_id: value.app_id.as_ref().to_string(), - // TODO: put path in RequestContext - path: value.app_id.as_ref().to_string(), pid: value.pid, } } @@ -347,8 +345,6 @@ fn check_origin_from_privileged_client( #[derive(Clone, Debug, DeserializeDict, Type)] #[zvariant(signature = "dict")] pub struct GetCredentialRequest { - pub origin: Option, - pub is_same_origin: Option, #[zvariant(rename = "publicKey")] pub public_key: Option, } @@ -386,8 +382,6 @@ impl From for GetCredentialResponse { #[derive(Clone, Debug, DeserializeDict, Type)] #[zvariant(signature = "dict")] pub struct CreateCredentialRequest { - pub origin: Option, - pub is_same_origin: Option, #[zvariant(rename = "type")] pub r#type: String, #[zvariant(rename = "publicKey")] diff --git a/credentialsd/src/model.rs b/credentialsd/src/model.rs index 95c4004..921016f 100644 --- a/credentialsd/src/model.rs +++ b/credentialsd/src/model.rs @@ -1,3 +1,4 @@ +use credentialsd_common::model::Operation; use libwebauthn::ops::webauthn::{ Assertion, GetAssertionRequest, MakeCredentialRequest, MakeCredentialResponse, }; @@ -8,6 +9,36 @@ pub enum CredentialRequest { GetPublicKeyCredentialRequest(GetAssertionRequest), } +impl CredentialRequest { + pub fn operation(&self) -> Operation { + match self { + Self::CreatePublicKeyCredentialRequest(_) => Operation::PublicKeyCreate, + Self::GetPublicKeyCredentialRequest(_) => Operation::PublicKeyGet, + } + } + + pub fn relying_party_id(&self) -> &str { + match self { + Self::CreatePublicKeyCredentialRequest(r) => r.relying_party.id.as_str(), + Self::GetPublicKeyCredentialRequest(r) => r.relying_party_id.as_str(), + } + } + + pub fn origin(&self) -> &str { + match self { + Self::CreatePublicKeyCredentialRequest(r) => r.origin.as_str(), + Self::GetPublicKeyCredentialRequest(r) => r.origin.as_str(), + } + } + + pub fn top_origin(&self) -> Option<&str> { + match self { + Self::CreatePublicKeyCredentialRequest(r) => r.top_origin.as_deref(), + Self::GetPublicKeyCredentialRequest(r) => r.top_origin.as_deref(), + } + } +} + #[derive(Clone, Debug)] pub enum CredentialResponse { CreatePublicKeyCredentialResponse(Box), @@ -74,6 +105,5 @@ impl GetAssertionResponseInternal { pub struct ClientDetails { pub app_id: String, - pub path: String, pub pid: u32, } diff --git a/credentialsd/src/webauthn.rs b/credentialsd/src/webauthn.rs index 85754d4..a59ffff 100644 --- a/credentialsd/src/webauthn.rs +++ b/credentialsd/src/webauthn.rs @@ -145,16 +145,6 @@ pub(crate) enum NavigationContext { CrossOrigin((Origin, Origin)), } -impl NavigationContext { - /// Retrieve the origin from the context. - pub(crate) fn origin(&self) -> &Origin { - match self { - NavigationContext::SameOrigin(origin) => origin, - NavigationContext::CrossOrigin((origin, _)) => origin, - } - } -} - #[derive(Debug)] pub(crate) enum OriginParseError { InvalidScheme,