diff --git a/apps/desktop/desktop_native/win_webauthn/src/plugin/com.rs b/apps/desktop/desktop_native/win_webauthn/src/plugin/com.rs index 95ff453b147..7607b6693f7 100644 --- a/apps/desktop/desktop_native/win_webauthn/src/plugin/com.rs +++ b/apps/desktop/desktop_native/win_webauthn/src/plugin/com.rs @@ -24,7 +24,10 @@ use super::types::{ }; use super::PluginAuthenticator; -use crate::{plugin::crypto, ErrorKind, WinWebAuthnError}; +use crate::{ + plugin::{crypto, PluginMakeCredentialRequest}, + ErrorKind, WinWebAuthnError, +}; static HANDLER: OnceLock<(GUID, Arc)> = OnceLock::new(); @@ -109,7 +112,8 @@ impl IPluginAuthenticator_Impl for PluginAuthenticatorComObject_Impl { return E_INVALIDARG; } - let registration_request = match op_request_ptr.try_into() { + // SAFETY: we received the pointer from Windows, so we trust that the values are set properly. + let registration_request = match PluginMakeCredentialRequest::from_ptr(op_request_ptr) { Ok(r) => r, Err(err) => { tracing::error!("Could not deserialize MakeCredential request: {err}"); diff --git a/apps/desktop/desktop_native/win_webauthn/src/plugin/types.rs b/apps/desktop/desktop_native/win_webauthn/src/plugin/types.rs index 252670cdc72..da60043312b 100644 --- a/apps/desktop/desktop_native/win_webauthn/src/plugin/types.rs +++ b/apps/desktop/desktop_native/win_webauthn/src/plugin/types.rs @@ -14,7 +14,7 @@ use windows::{ use crate::{ plugin::crypto, - types::UserId, + types::{RpEntityInformation, UserEntityInformation, UserId}, util::{webauthn_call, WindowsString}, CredentialId, ErrorKind, WinWebAuthnError, }; @@ -412,20 +412,19 @@ impl PluginMakeCredentialRequest { } } - pub fn rp_information(&self) -> Option<&WEBAUTHN_RP_ENTITY_INFORMATION> { + pub fn rp_information(&self) -> RpEntityInformation<'_> { let ptr = self.as_ref().pRpInformation; - if ptr.is_null() { - return None; - } - unsafe { Some(&*ptr) } + // SAFETY: if this is constructed using Self::from_ptr(), the caller must ensure that pRpInformation is valid. + unsafe { RpEntityInformation::new(ptr.as_ref().expect("pRpInformation to be non-null")) } } - pub fn user_information(&self) -> Option<&WEBAUTHN_USER_ENTITY_INFORMATION> { + pub fn user_information(&self) -> UserEntityInformation<'_> { + // SAFETY: if this is constructed using Self::from_ptr(), the caller must ensure that pUserInformation is valid. let ptr = self.as_ref().pUserInformation; - if ptr.is_null() { - return None; + assert!(!ptr.is_null()); + unsafe { + UserEntityInformation::new(ptr.as_ref().expect("pUserInformation to be non-null")) } - unsafe { Some(&*ptr) } } pub fn pub_key_cred_params(&self) -> WEBAUTHN_COSE_CREDENTIAL_PARAMETERS { @@ -455,6 +454,54 @@ impl PluginMakeCredentialRequest { } unsafe { Some(*ptr) } } + + /// # Safety + /// When calling this method, callers must ensure: + /// - `ptr` must be convertible to a reference. + /// - pbEncodedRequest must be non-null and have the length specified in cbEncodedRequest. + pub(super) unsafe fn from_ptr( + ptr: NonNull, + ) -> Result { + let request = ptr.as_ref(); + if !matches!(request.requestType, WebAuthnPluginRequestType::CTAP2_CBOR) { + return Err(WinWebAuthnError::new( + ErrorKind::Serialization, + "Unknown plugin operation request type", + )); + } + let request_slice = + std::slice::from_raw_parts(request.pbEncodedRequest, request.cbEncodedRequest as usize); + let request_hash = crypto::hash_sha256(request_slice).map_err(|err| { + WinWebAuthnError::with_cause(ErrorKind::WindowsInternal, "failed to hash request", err) + })?; + let mut registration_request = MaybeUninit::uninit(); + webauthn_decode_make_credential_request( + request.cbEncodedRequest, + request.pbEncodedRequest, + registration_request.as_mut_ptr(), + )? + .ok() + .map_err(|err| { + WinWebAuthnError::with_cause( + ErrorKind::WindowsInternal, + "Failed to decode get assertion request", + err, + ) + })?; + + let registration_request = registration_request.assume_init(); + Ok(Self { + inner: registration_request as *const WEBAUTHN_CTAPCBOR_MAKE_CREDENTIAL_REQUEST, + window_handle: request.hWnd, + transaction_id: request.transactionId, + request_signature: std::slice::from_raw_parts( + request.pbRequestSignature, + request.cbEncodedRequest as usize, + ) + .to_vec(), + request_hash, + }) + } } impl AsRef for PluginMakeCredentialRequest { @@ -474,60 +521,6 @@ impl Drop for PluginMakeCredentialRequest { } } -impl TryFrom> for PluginMakeCredentialRequest { - type Error = WinWebAuthnError; - - fn try_from(value: NonNull) -> Result { - unsafe { - let request = value.as_ref(); - if !matches!(request.requestType, WebAuthnPluginRequestType::CTAP2_CBOR) { - return Err(WinWebAuthnError::new( - ErrorKind::Serialization, - "Unknown plugin operation request type", - )); - } - let request_slice = std::slice::from_raw_parts( - request.pbEncodedRequest, - request.cbEncodedRequest as usize, - ); - let request_hash = crypto::hash_sha256(request_slice).map_err(|err| { - WinWebAuthnError::with_cause( - ErrorKind::WindowsInternal, - "failed to hash request", - err, - ) - })?; - let mut registration_request = MaybeUninit::uninit(); - webauthn_decode_make_credential_request( - request.cbEncodedRequest, - request.pbEncodedRequest, - registration_request.as_mut_ptr(), - )? - .ok() - .map_err(|err| { - WinWebAuthnError::with_cause( - ErrorKind::WindowsInternal, - "Failed to decode get assertion request", - err, - ) - })?; - - let registration_request = registration_request.assume_init(); - Ok(Self { - inner: registration_request as *const WEBAUTHN_CTAPCBOR_MAKE_CREDENTIAL_REQUEST, - window_handle: request.hWnd, - transaction_id: request.transactionId, - request_signature: std::slice::from_raw_parts( - request.pbRequestSignature, - request.cbEncodedRequest as usize, - ) - .to_vec(), - request_hash, - }) - } - } -} - // Windows API function signatures for decoding make credential requests webauthn_call!("WebAuthNDecodeMakeCredentialRequest" as fn webauthn_decode_make_credential_request( cbEncoded: u32, diff --git a/apps/desktop/desktop_native/win_webauthn/src/types/mod.rs b/apps/desktop/desktop_native/win_webauthn/src/types/mod.rs index 103f657b7e8..70bfebeb05f 100644 --- a/apps/desktop/desktop_native/win_webauthn/src/types/mod.rs +++ b/apps/desktop/desktop_native/win_webauthn/src/types/mod.rs @@ -3,7 +3,7 @@ #![allow(non_snake_case)] #![allow(non_camel_case_types)] -use std::{collections::HashSet, fmt::Display, ptr::NonNull}; +use std::{collections::HashSet, fmt::Display, marker::PhantomData, num::NonZeroU32, ptr::NonNull}; use ciborium::Value; use windows_core::PCWSTR; @@ -168,102 +168,159 @@ impl From<&CtapVersion> for String { #[repr(C)] #[derive(Debug, Copy, Clone)] -pub struct WEBAUTHN_RP_ENTITY_INFORMATION { +pub(crate) struct WEBAUTHN_RP_ENTITY_INFORMATION { + /// Version of this structure, to allow for modifications in the future. + /// This field is required and should be set to CURRENT_VERSION above. dwVersion: u32, - pwszId: *const u16, // PCWSTR + + /// Identifier for the RP. This field is required. + pwszId: NonNull, // PCWSTR + + /// Contains the friendly name of the Relying Party, such as "Acme + /// Corporation", "Widgets Inc" or "Awesome Site". + /// + /// This member is deprecated in WebAuthn Level 3 because many clients do not display it, but it + /// remains a required dictionary member for backwards compatibility. Relying + /// Parties MAY, as a safe default, set this equal to the RP ID. pwszName: *const u16, // PCWSTR - pwszIcon: *const u16, // PCWSTR + + /// Optional URL pointing to RP's logo. + /// + /// This field was removed in WebAuthn Level 2. Keeping this here for proper struct sizing. + #[deprecated] + _pwszIcon: *const u16, // PCWSTR } -impl WEBAUTHN_RP_ENTITY_INFORMATION { - /// Relying party ID. - pub fn id(&self) -> Result { - if self.pwszId.is_null() { - return Err(WinWebAuthnError::new( - ErrorKind::WindowsInternal, - "Received invalid RP ID", - )); - } - unsafe { - PCWSTR(self.pwszId).to_string().map_err(|err| { - WinWebAuthnError::with_cause(ErrorKind::WindowsInternal, "Invalid RP ID", err) - }) +/// A wrapper around WEBAUTHN_RP_ENTITY_INFORMATION. +pub struct RpEntityInformation<'a> { + ptr: NonNull, + _phantom: PhantomData<&'a WEBAUTHN_RP_ENTITY_INFORMATION>, +} + +impl RpEntityInformation<'_> { + /// # Safety + /// When calling this method, you must ensure that + /// - the pointer is convertible to a reference, + /// - pwszId points to a valid null-terminated UTF-16 string. + /// - pwszName is null or points to a valid null-terminated UTF-16 string. + pub(crate) unsafe fn new(ptr: &WEBAUTHN_RP_ENTITY_INFORMATION) -> Self { + Self { + ptr: NonNull::from_ref(ptr), + _phantom: PhantomData, } } - /// Relying party name. - pub fn name(&self) -> Result { - if self.pwszName.is_null() { - return Err(WinWebAuthnError::new( - ErrorKind::WindowsInternal, - "Received invalid RP name", - )); - } + /// Identifier for the RP. + pub fn id(&self) -> String { + // SAFETY: If the caller upholds the constraints of the struct in + // Self::new(), then pwszId is valid UTF-16. unsafe { - PCWSTR(self.pwszName).to_string().map_err(|err| { - WinWebAuthnError::with_cause(ErrorKind::WindowsInternal, "Invalid RP name", err) - }) + assert!(self.ptr.is_aligned()); + PCWSTR(self.ptr.as_ref().pwszId.as_ptr()) + .to_string() + .expect("valid null-terminated UTF-16 string") + } + } + + /// Contains the friendly name of the Relying Party, such as "Acme + /// Corporation", "Widgets Inc" or "Awesome Site". + pub fn name(&self) -> Option { + // SAFETY: If the caller upholds the constraints of the struct in + // Self::new(), then pwszName is either null or valid UTF-16. + unsafe { + if self.ptr.as_ref().pwszName.is_null() { + return None; + } + let s = PCWSTR(self.ptr.as_ref().pwszName) + .to_string() + .expect("null-terminated UTF-16 string or null"); + + // WebAuthn Level 3 deprecates the use of the `name` field, so verify whether this is empty or not. + if s.is_empty() { + None + } else { + Some(s) + } } } } #[repr(C)] #[derive(Debug, Copy, Clone)] -pub struct WEBAUTHN_USER_ENTITY_INFORMATION { +pub(crate) struct WEBAUTHN_USER_ENTITY_INFORMATION { + /// Version of this structure, to allow for modifications in the future. + /// This field is required and should be set to CURRENT_VERSION above. pub dwVersion: u32, - pub cbId: u32, // DWORD - pub pbId: *const u8, // PBYTE - pub pwszName: *const u16, // PCWSTR - pub pwszIcon: *const u16, // PCWSTR - pub pwszDisplayName: *const u16, // PCWSTR + + /// Identifier for the User. This field is required. + pub cbId: NonZeroU32, // DWORD + pub pbId: NonNull, // PBYTE + + /// Contains a detailed name for this account, such as "john.p.smith@example.com". + pub pwszName: NonNull, // PCWSTR + + /// Optional URL that can be used to retrieve an image containing the user's current avatar, + /// or a data URI that contains the image data. + #[deprecated] + pub pwszIcon: Option>, // PCWSTR + + /// Contains the friendly name associated with the user account by the Relying Party, such as "John P. Smith". + pub pwszDisplayName: NonNull, // PCWSTR } -impl WEBAUTHN_USER_ENTITY_INFORMATION { - /// User handle. - pub fn id(&self) -> Result<&[u8], WinWebAuthnError> { - if self.cbId == 0 || self.pbId.is_null() { - return Err(WinWebAuthnError::new( - ErrorKind::WindowsInternal, - "Received invalid user ID", - )); +pub struct UserEntityInformation<'a> { + ptr: NonNull, + _phantom: PhantomData<&'a WEBAUTHN_USER_ENTITY_INFORMATION>, +} + +impl UserEntityInformation<'_> { + /// # Safety + /// When calling this method, the caller must ensure that + /// - `ptr` is convertible to a reference, + /// - pbId is non-null and points to a valid memory allocation with length of cbId. + /// - pwszName is non-null and points to a valid null-terminated UTF-16 string. + /// - pwszDisplayName is non-null and points to a valid null-terminated UTF-16 string. + pub(crate) unsafe fn new(ptr: &WEBAUTHN_USER_ENTITY_INFORMATION) -> Self { + Self { + ptr: NonNull::from_ref(ptr), + _phantom: PhantomData, + } + } + + /// User handle. + pub fn id(&self) -> &[u8] { + // SAFETY: If the caller upholds the constraints on Self::new(), then pbId + // is non-null and points to valid memory. + unsafe { + let ptr = self.ptr.as_ref(); + std::slice::from_raw_parts(ptr.pbId.as_ptr(), ptr.cbId.get() as usize) } - unsafe { Ok(std::slice::from_raw_parts(self.pbId, self.cbId as usize)) } } /// User name. - pub fn name(&self) -> Result { - if self.pwszName.is_null() { - return Err(WinWebAuthnError::new( - ErrorKind::WindowsInternal, - "Received invalid user name", - )); - } + pub fn name(&self) -> String { + // SAFETY: If the caller upholds the constraints on Self::new(), then ID + // is non-null and points to valid memory. unsafe { - PCWSTR(self.pwszName).to_string().map_err(|err| { - WinWebAuthnError::with_cause(ErrorKind::WindowsInternal, "Invalid user name", err) - }) + let ptr = self.ptr.as_ref(); + PCWSTR(ptr.pwszName.as_ptr()) + .to_string() + .expect("valid UTF-16 string") } } /// User display name. - pub fn display_name(&self) -> Result { - if self.pwszDisplayName.is_null() { - return Err(WinWebAuthnError::new( - ErrorKind::WindowsInternal, - "Received invalid user name", - )); - } + pub fn display_name(&self) -> String { unsafe { - PCWSTR(self.pwszDisplayName).to_string().map_err(|err| { - WinWebAuthnError::with_cause( - ErrorKind::WindowsInternal, - "Invalid user display name", - err, - ) - }) + let ptr = self.ptr.as_ref(); + + PCWSTR(ptr.pwszDisplayName.as_ptr()) + .to_string() + .expect("valid UTF-16 string") } } } + #[repr(C)] #[derive(Debug, Copy, Clone)] pub struct WEBAUTHN_COSE_CREDENTIAL_PARAMETER { diff --git a/apps/desktop/desktop_native/windows_plugin_authenticator/src/make_credential.rs b/apps/desktop/desktop_native/windows_plugin_authenticator/src/make_credential.rs index c5eead87817..b98afbf179f 100644 --- a/apps/desktop/desktop_native/windows_plugin_authenticator/src/make_credential.rs +++ b/apps/desktop/desktop_native/windows_plugin_authenticator/src/make_credential.rs @@ -25,27 +25,18 @@ pub fn make_credential( ) -> Result, Box> { tracing::debug!("=== PluginMakeCredential() called ==="); // Extract RP information - let rp_info = request - .rp_information() - .ok_or_else(|| "RP information is null".to_string())?; + let rp_info = request.rp_information(); - let rpid = rp_info.id()?; + let rpid = rp_info.id(); // let rp_name = rp_info.name().unwrap_or_else(|| String::new()); // Extract user information - let user = request - .user_information() - .ok_or_else(|| "User information is null".to_string())?; + let user = request.user_information(); - let user_handle = user - .id() - .map_err(|err| format!("User ID is required for registration: {err}"))? - .to_vec(); + let user_handle = user.id().to_vec(); - let user_name = user - .name() - .map_err(|err| format!("User name is required for registration: {err}"))?; + let user_name = user.name(); // let user_display_name = user.display_name();