bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2026-02-22 04:14:04 +00:00
Code Issues Releases Wiki Activity

Document safety for credential metadata iterators

Browse Source
This commit is contained in:
Isaiah Inuwa
2025-12-17 15:39:35 -06:00
parent 7e45ba5dad
commit 17d2e848f6
6 changed files with 91 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
apps/desktop/desktop_native/win_webauthn/src/plugin/types.rs
View File

@@ -14,16 +14,18 @@ use windows::{
use crate::{
plugin::crypto,
types::{RpEntityInformation, UserEntityInformation, UserId},
types::{
CredentialEx, RpEntityInformation, UserEntityInformation, UserId,
WEBAUTHN_COSE_CREDENTIAL_PARAMETER,
},
util::{webauthn_call, WindowsString},
CredentialId, ErrorKind, WinWebAuthnError,
};
use crate::types::{
AuthenticatorInfo, CredentialList, CtapTransport, HmacSecretSalt,
WebAuthnExtensionMakeCredentialOutput, WEBAUTHN_COSE_CREDENTIAL_PARAMETERS,
WEBAUTHN_CREDENTIAL_ATTESTATION, WEBAUTHN_CREDENTIAL_LIST, WEBAUTHN_EXTENSIONS,
WEBAUTHN_RP_ENTITY_INFORMATION, WEBAUTHN_USER_ENTITY_INFORMATION,
AuthenticatorInfo, CtapTransport, HmacSecretSalt, WebAuthnExtensionMakeCredentialOutput,
WEBAUTHN_COSE_CREDENTIAL_PARAMETERS, WEBAUTHN_CREDENTIAL_ATTESTATION, WEBAUTHN_CREDENTIAL_LIST,
WEBAUTHN_EXTENSIONS, WEBAUTHN_RP_ENTITY_INFORMATION, WEBAUTHN_USER_ENTITY_INFORMATION,
};
use super::Clsid;
@@ -378,7 +380,7 @@ pub(super) struct WEBAUTHN_CTAPCBOR_MAKE_CREDENTIAL_REQUEST {
pub pbClientDataHash: *const u8,
pub pRpInformation: *const WEBAUTHN_RP_ENTITY_INFORMATION,
pub pUserInformation: *const WEBAUTHN_USER_ENTITY_INFORMATION,
pub WebAuthNCredentialParameters: WEBAUTHN_COSE_CREDENTIAL_PARAMETERS, // Matches C++ sample
pub WebAuthNCredentialParameters: WEBAUTHN_COSE_CREDENTIAL_PARAMETERS,
pub CredentialList: WEBAUTHN_CREDENTIAL_LIST,
pub cbCborExtensionsMap: u32,
pub pbCborExtensionsMap: *const u8,
@@ -414,12 +416,12 @@ impl PluginMakeCredentialRequest {
pub fn rp_information(&self) -> RpEntityInformation<'_> {
let ptr = self.as_ref().pRpInformation;
// SAFETY: if this is constructed using Self::from_ptr(), the caller must ensure that pRpInformation is valid.
// SAFETY: When 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) -> UserEntityInformation<'_> {
// SAFETY: if this is constructed using Self::from_ptr(), the caller must ensure that pUserInformation is valid.
// SAFETY: When this is constructed using Self::from_ptr(), the caller must ensure that pUserInformation is valid.
let ptr = self.as_ref().pUserInformation;
assert!(!ptr.is_null());
unsafe {
@@ -427,12 +429,14 @@ impl PluginMakeCredentialRequest {
}
}
pub fn pub_key_cred_params(&self) -> WEBAUTHN_COSE_CREDENTIAL_PARAMETERS {
self.as_ref().WebAuthNCredentialParameters
pub fn pub_key_cred_params(&self) -> impl Iterator<Item = &WEBAUTHN_COSE_CREDENTIAL_PARAMETER> {
// SAFETY: When this is constructed from Self::from_ptr(), the Windows decode API constructs valid pointers.
unsafe { self.as_ref().WebAuthNCredentialParameters.iter() }
}
pub fn exclude_credentials(&self) -> CredentialList {
self.as_ref().CredentialList
pub fn exclude_credentials(&self) -> impl Iterator<Item = CredentialEx<'_>> {
// SAFETY: When this is constructed from Self::from_ptr(), the Windows decode API constructs valid pointers.
unsafe { self.as_ref().CredentialList.iter() }
}
/// CTAP CBOR extensions map
@@ -459,6 +463,7 @@ impl PluginMakeCredentialRequest {
/// 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.
/// - pbEncodedRequest must point to a valid CTAP MakeCredential bytes.
pub(super) unsafe fn from_ptr(
ptr: NonNull<WEBAUTHN_PLUGIN_OPERATION_REQUEST>,
) -> Result<PluginMakeCredentialRequest, WinWebAuthnError> {
@@ -803,8 +808,9 @@ impl PluginGetAssertionRequest {
}
}
pub fn allow_credentials(&self) -> CredentialList {
self.as_ref().CredentialList
pub fn allow_credentials(&self) -> impl Iterator<Item = CredentialEx<'_>> {
// SAFETY: When this is constructed from Self::from_ptr(), the Windows decode API constructs valid pointers.
unsafe { self.as_ref().CredentialList.iter() }
}
// TODO: Support extensions

139
apps/desktop/desktop_native/win_webauthn/src/types/mod.rs
View File

@@ -324,27 +324,23 @@ impl UserEntityInformation<'_> {
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct WEBAUTHN_COSE_CREDENTIAL_PARAMETER {
pub dwVersion: u32,
pub pwszCredentialType: *const u16, // LPCWSTR
pub lAlg: i32, // LONG - COSE algorithm identifier
dwVersion: u32,
pwszCredentialType: NonNull<u16>, // LPCWSTR
lAlg: i32, // LONG - COSE algorithm identifier
}
impl WEBAUTHN_COSE_CREDENTIAL_PARAMETER {
pub fn credential_type(&self) -> Result<String, WinWebAuthnError> {
if self.pwszCredentialType.is_null() {
return Err(WinWebAuthnError::new(
ErrorKind::WindowsInternal,
"Invalid credential type",
));
}
unsafe {
PCWSTR(self.pwszCredentialType).to_string().map_err(|err| {
WinWebAuthnError::with_cause(
ErrorKind::WindowsInternal,
"Invalid credential type",
err,
)
})
PCWSTR(self.pwszCredentialType.as_ptr())
.to_string()
.map_err(|err| {
WinWebAuthnError::with_cause(
ErrorKind::WindowsInternal,
"Invalid credential type",
err,
)
})
}
}
pub fn alg(&self) -> i32 {
@@ -354,19 +350,21 @@ impl WEBAUTHN_COSE_CREDENTIAL_PARAMETER {
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct WEBAUTHN_COSE_CREDENTIAL_PARAMETERS {
pub(crate) struct WEBAUTHN_COSE_CREDENTIAL_PARAMETERS {
cCredentialParameters: u32,
pCredentialParameters: *const WEBAUTHN_COSE_CREDENTIAL_PARAMETER,
}
impl WEBAUTHN_COSE_CREDENTIAL_PARAMETERS {
pub fn iter(&self) -> ArrayPointerIterator<'_, WEBAUTHN_COSE_CREDENTIAL_PARAMETER> {
unsafe {
ArrayPointerIterator::new(
self.pCredentialParameters,
self.cCredentialParameters as usize,
)
}
/// # Safety
/// The caller must ensure that the pCredentialParameters is either null or
/// marks the beginning of a list whose count is represented accurately by
/// cCredentialParameters.
pub unsafe fn iter(&self) -> ArrayPointerIterator<'_, WEBAUTHN_COSE_CREDENTIAL_PARAMETER> {
ArrayPointerIterator::new(
self.pCredentialParameters,
self.cCredentialParameters as usize,
)
}
}
@@ -614,7 +612,7 @@ impl TryFrom<Vec<u8>> for CredentialId {
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct WEBAUTHN_CREDENTIAL_EX {
pub(crate) struct WEBAUTHN_CREDENTIAL_EX {
dwVersion: u32,
cbId: u32,
pbId: *const u8,
@@ -622,36 +620,47 @@ pub struct WEBAUTHN_CREDENTIAL_EX {
dwTransports: u32,
}
impl WEBAUTHN_CREDENTIAL_EX {
pub struct CredentialEx<'a> {
inner: &'a WEBAUTHN_CREDENTIAL_EX,
}
impl CredentialEx<'_> {
pub fn credential_id(&self) -> Option<&[u8]> {
if self.cbId == 0 || self.pbId.is_null() {
if self.inner.cbId == 0 || self.inner.pbId.is_null() {
None
} else {
unsafe { Some(std::slice::from_raw_parts(self.pbId, self.cbId as usize)) }
unsafe {
Some(std::slice::from_raw_parts(
self.inner.pbId,
self.inner.cbId as usize,
))
}
}
}
pub fn credential_type(&self) -> Result<String, WinWebAuthnError> {
if self.pwszCredentialType.is_null() {
if self.inner.pwszCredentialType.is_null() {
return Err(WinWebAuthnError::new(
ErrorKind::WindowsInternal,
"Received invalid credential ID",
));
}
unsafe {
PCWSTR(self.pwszCredentialType).to_string().map_err(|err| {
WinWebAuthnError::with_cause(
ErrorKind::WindowsInternal,
"Invalid credential ID",
err,
)
})
PCWSTR(self.inner.pwszCredentialType)
.to_string()
.map_err(|err| {
WinWebAuthnError::with_cause(
ErrorKind::WindowsInternal,
"Invalid credential ID",
err,
)
})
}
}
pub fn transports(&self) -> Vec<CtapTransport> {
let mut transports = Vec::new();
let mut t = self.dwTransports;
let mut t = self.inner.dwTransports;
if t == 0 {
return transports;
};
@@ -677,23 +686,6 @@ impl WEBAUTHN_CREDENTIAL_EX {
}
}
pub struct CredentialEx {
inner: NonNull<WEBAUTHN_CREDENTIAL_EX>,
}
impl AsRef<WEBAUTHN_CREDENTIAL_EX> for CredentialEx {
fn as_ref(&self) -> &WEBAUTHN_CREDENTIAL_EX {
// SAFETY: We initialize memory manually in constructors.
unsafe { self.inner.as_ref() }
}
}
impl From<NonNull<WEBAUTHN_CREDENTIAL_EX>> for CredentialEx {
fn from(value: NonNull<WEBAUTHN_CREDENTIAL_EX>) -> Self {
Self { inner: value }
}
}
#[repr(u32)]
#[derive(Clone, Copy)]
pub enum CtapTransport {
@@ -708,30 +700,13 @@ pub enum CtapTransport {
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct CredentialList {
pub(crate) struct WEBAUTHN_CREDENTIAL_LIST {
pub cCredentials: u32,
pub ppCredentials: *const *const WEBAUTHN_CREDENTIAL_EX,
}
pub(crate) type WEBAUTHN_CREDENTIAL_LIST = CredentialList;
pub struct CredentialListIterator<'a> {
inner: ArrayPointerIterator<'a, *const WEBAUTHN_CREDENTIAL_EX>,
}
impl<'a> Iterator for CredentialListIterator<'a> {
type Item = &'a WEBAUTHN_CREDENTIAL_EX;
fn next(&mut self) -> Option<Self::Item> {
let item = self.inner.next()?;
// SAFETY: This type can only be constructed from this library using
// responses from Windows APIs, and we trust that the pointer and length
// of each inner item of the array is valid.
unsafe { item.as_ref() }
}
}
impl CredentialList {
pub fn iter(&self) -> CredentialListIterator<'_> {
impl WEBAUTHN_CREDENTIAL_LIST {
pub unsafe fn iter(&self) -> CredentialListIterator<'_> {
// SAFETY: This type can only be constructed from this library using
// responses from Windows APIs. The pointer is checked for null safety
// on construction.
@@ -743,6 +718,22 @@ impl CredentialList {
}
}
pub struct CredentialListIterator<'a> {
inner: ArrayPointerIterator<'a, *const WEBAUTHN_CREDENTIAL_EX>,
}
impl<'a> Iterator for CredentialListIterator<'a> {
type Item = CredentialEx<'a>;
fn next(&mut self) -> Option<Self::Item> {
let item = self.inner.next()?;
// SAFETY: This type can only be constructed from this library using
// responses from Windows APIs, and we trust that the pointer and length
// of each inner item of the array is valid.
unsafe { item.as_ref().map(|inner| CredentialEx { inner }) }
}
}
#[cfg(test)]
mod tests {
use super::*;

3
apps/desktop/desktop_native/win_webauthn/src/util.rs
View File

@@ -72,7 +72,8 @@ pub struct ArrayPointerIterator<'a, T> {
}
impl<T> ArrayPointerIterator<'_, T> {
/// Safety constraints: The caller must ensure that the pointer and length is
/// # Safety
/// The caller must ensure that the pointer and length is
/// valid. A null pointer returns an empty iterator.
pub unsafe fn new(data: *const T, len: usize) -> Self {
let slice = if !data.is_null() {

4
apps/desktop/desktop_native/windows_plugin_authenticator/src/assert.rs
View File

@@ -39,9 +39,7 @@ pub fn get_assertion(
// Extract allowed credentials from credential list
let allowed_credential_ids: Vec<Vec<u8>> = request
.allow_credentials()
.iter()
.filter_map(|cred| cred.credential_id())
.map(|id| id.to_vec())
.filter_map(|cred| cred.credential_id().map(|id| id.to_vec()))
.collect();
let client_window_handle = request.window_handle.0.addr().to_le_bytes().to_vec();

3
apps/desktop/desktop_native/windows_plugin_authenticator/src/main.rs
View File

@@ -302,7 +302,8 @@ impl PluginAuthenticator for BitwardenPluginAuthenticator {
let is_unlocked = get_lock_status(&client).map_or(false, |response| response.is_unlocked);
// Don't mess with the window unless we're going to need it: if the
// vault is locked or if we need to show credential selection dialog.
let needs_ui = !is_unlocked || request.allow_credentials().cCredentials != 1;
let needs_ui = !is_unlocked || request.allow_credentials().count() != 1;
if needs_ui {
unsafe {
let plugin_window = get_window_details(&client)?;

5
apps/desktop/desktop_native/windows_plugin_authenticator/src/make_credential.rs
View File

@@ -49,7 +49,6 @@ pub fn make_credential(
// Extract supported algorithms
let supported_algorithms: Vec<i32> = request
.pub_key_cred_params()
.iter()
.map(|params| params.alg())
.collect();
@@ -66,9 +65,7 @@ pub fn make_credential(
// Extract excluded credentials from credential list
let excluded_credentials: Vec<Vec<u8>> = request
.exclude_credentials()
.iter()
.filter_map(|cred| cred.credential_id())
.map(|id| id.to_vec())
.filter_map(|cred| cred.credential_id().map(|id| id.to_vec()))
.collect();
if !excluded_credentials.is_empty() {
tracing::debug!(