From 243dca10ee814d14aa3137e65168d4a88c4a0091 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Sun, 21 Jun 2026 19:12:48 -0500 Subject: [PATCH 1/2] common: Move memfd ops to shared lib --- Cargo.lock | 10 +-- credentialsd-common/Cargo.toml | 2 + credentialsd-common/src/lib.rs | 1 + credentialsd-common/src/memfd.rs | 118 ++++++++++++++++++++++++++ credentialsd-ui/Cargo.toml | 1 - credentialsd-ui/src/client.rs | 76 ++--------------- credentialsd/Cargo.toml | 1 - credentialsd/src/dbus/flow_control.rs | 72 ++-------------- 8 files changed, 140 insertions(+), 141 deletions(-) create mode 100644 credentialsd-common/src/memfd.rs diff --git a/Cargo.lock b/Cargo.lock index 3ac56848..fb8fcaff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -799,7 +799,6 @@ dependencies = [ "futures", "futures-lite", "gio 0.21.5", - "libc", "libwebauthn", "nfc1", "rand 0.9.2", @@ -817,7 +816,9 @@ name = "credentialsd-common" version = "0.2.0" dependencies = [ "futures-lite", + "libc", "serde", + "zeroize", "zvariant", ] @@ -832,7 +833,6 @@ dependencies = [ "gettext-rs", "gio-unix", "gtk4", - "libc", "qrcode", "serde", "tracing", @@ -3184,7 +3184,7 @@ dependencies = [ "once_cell", "socket2", "tracing", - "windows-sys 0.52.0", + "windows-sys 0.60.2", ] [[package]] @@ -5210,9 +5210,9 @@ dependencies = [ [[package]] name = "zeroize" -version = "1.8.2" +version = "1.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b97154e67e32c85465826e8bcc1c59429aaaf107c1e4a9e53c8d8ccd5eff88d0" +checksum = "e13c156562582aa81c60cb29407084cdb54c4164760106ab78e6c5b0858cf64e" dependencies = [ "zeroize_derive", ] diff --git a/credentialsd-common/Cargo.toml b/credentialsd-common/Cargo.toml index 66f1c2c9..fc0490a9 100644 --- a/credentialsd-common/Cargo.toml +++ b/credentialsd-common/Cargo.toml @@ -7,5 +7,7 @@ license = "LGPL-3.0-only" [dependencies] futures-lite.workspace = true +libc.workspace = true serde = { workspace = true, features = ["derive"] } +zeroize = "1.9.0" zvariant.workspace = true diff --git a/credentialsd-common/src/lib.rs b/credentialsd-common/src/lib.rs index cd7f939f..f6ad9561 100644 --- a/credentialsd-common/src/lib.rs +++ b/credentialsd-common/src/lib.rs @@ -1,3 +1,4 @@ pub mod client; +pub mod memfd; pub mod model; pub mod server; diff --git a/credentialsd-common/src/memfd.rs b/credentialsd-common/src/memfd.rs new file mode 100644 index 00000000..52cfb0cf --- /dev/null +++ b/credentialsd-common/src/memfd.rs @@ -0,0 +1,118 @@ +use std::{ + io::{self, ErrorKind}, + mem::MaybeUninit, + os::{ + fd::{AsRawFd, FromRawFd, OwnedFd}, + raw::c_void, + }, +}; + +use libc::{ + ftruncate, mmap, off_t, SYS_memfd_secret, MAP_SHARED, O_CLOEXEC, PROT_READ, PROT_WRITE, +}; +use zeroize::Zeroize; + +pub fn read_secret(pin_fd: std::os::fd::OwnedFd) -> Result { + // Get pin length + let len = { + let mut stat_buf = MaybeUninit::::uninit(); + let res = unsafe { libc::fstat(pin_fd.as_raw_fd(), stat_buf.as_mut_ptr()) }; + if res == -1 { + return Err(io::Error::last_os_error()); + } + let stat_buf = unsafe { stat_buf.assume_init() }; + usize::try_from(stat_buf.st_size) + .map_err(|_| io::Error::new(ErrorKind::FileTooLarge, "pin is too large"))? + }; + + // map the memory from the file descriptor + let ptr = unsafe { + let ptr = libc::mmap( + std::ptr::null_mut(), + 4096, + PROT_READ | PROT_WRITE, + MAP_SHARED, + pin_fd.as_raw_fd(), + 0, + ); + if ptr == usize::MAX as *mut c_void { + return Err(std::io::Error::last_os_error()); + } + ptr as *const u8 + }; + + // Copy the bytes. + let buf = unsafe { + let mut buf: Vec = Vec::with_capacity(len); + ptr.copy_to_nonoverlapping(buf.as_mut_ptr().cast(), len); + buf.set_len(len); + buf + }; + + // Clean up mapping + unsafe { + if libc::munmap(ptr as *mut c_void, 4096) == -1 { + return Err(std::io::Error::last_os_error()); + } + } + drop(pin_fd); + + String::from_utf8(buf).map_err(|_| { + std::io::Error::new( + std::io::ErrorKind::InvalidData, + "invalid UTF-8 data found in buffer", + ) + }) +} + +pub fn write_secret(mut secret: Vec, max_len: usize) -> Result { + let bytes_len = if secret.len() <= max_len { + secret.len() as u8 + } else { + return Err(io::Error::new( + ErrorKind::FileTooLarge, + "value is too large", + )); + }; + + // Open memfd_secret + let ret: i64 = unsafe { libc::syscall(SYS_memfd_secret, O_CLOEXEC) }; + if ret == -1 { + return Err(std::io::Error::last_os_error()); + } + let fd = i32::try_from(ret).map_err(|_| std::io::Error::other("invalid file descriptor"))?; + if unsafe { ftruncate(fd, bytes_len as off_t) } == -1 { + return Err(std::io::Error::last_os_error()); + } + + let ptr = unsafe { + let ptr = mmap( + std::ptr::null_mut(), + 4096, + PROT_READ | PROT_WRITE, + MAP_SHARED, + fd, + 0, + ); + if ptr == usize::MAX as *mut c_void { + return Err(std::io::Error::last_os_error()); + } + // ptr as *mut u8 + ptr + }; + + // Copy the data + unsafe { + ptr.copy_from_nonoverlapping(secret.as_ptr().cast(), secret.len()); + } + + // Cleanup + if unsafe { libc::munmap(ptr, 4096) } == -1 { + return Err(std::io::Error::last_os_error()); + } + secret.zeroize(); + drop(secret); + + let owned_fd = unsafe { OwnedFd::from_raw_fd(fd) }; + Ok(owned_fd) +} diff --git a/credentialsd-ui/Cargo.toml b/credentialsd-ui/Cargo.toml index b28c98c2..79eeaa23 100644 --- a/credentialsd-ui/Cargo.toml +++ b/credentialsd-ui/Cargo.toml @@ -16,7 +16,6 @@ futures-lite.workspace = true gettext-rs = { version = "0.7", features = ["gettext-system"] } gtk = { version = "0.10.3", package = "gtk4", features = ["v4_14"] } gdk-wayland = { version = "0.10.3", package = "gdk4-wayland", optional = true } -libc.workspace = true qrcode = "0.14.1" serde.workspace = true tracing.workspace = true diff --git a/credentialsd-ui/src/client.rs b/credentialsd-ui/src/client.rs index c421bb86..52cd93bb 100644 --- a/credentialsd-ui/src/client.rs +++ b/credentialsd-ui/src/client.rs @@ -1,21 +1,13 @@ -use std::{ - io::ErrorKind, - os::{ - fd::{FromRawFd, OwnedFd}, - raw::c_void, - }, -}; - use async_std::{ channel::{Receiver, Sender}, sync::Mutex as AsyncMutex, }; -use libc::{ - MAP_SHARED, O_CLOEXEC, PROT_READ, PROT_WRITE, SYS_memfd_secret, ftruncate, mmap, off_t, + +use credentialsd_common::{ + memfd::write_secret, model::UserInteractedEvent, server::BackgroundEvent, }; -use zeroize::Zeroize; -use credentialsd_common::{model::UserInteractedEvent, server::BackgroundEvent}; +const CTAP_CLIENT_SECRET_MAX_LEN: usize = 63; #[derive(Debug)] pub struct FlowControlClient { @@ -29,7 +21,7 @@ impl FlowControlClient { } pub async fn enter_client_pin(&mut self, pin: String) -> Result<(), ()> { - let fd = match write_secret(pin) { + let fd = match write_secret(pin.into_bytes(), CTAP_CLIENT_SECRET_MAX_LEN) { Ok(fd) => fd, Err(err) => { tracing::error!(%err, "Failed to write secret to file descriptor"); @@ -66,61 +58,3 @@ impl FlowControlClient { } } } - -fn write_secret(secret: String) -> Result { - let mut bytes = secret.into_bytes(); - // CTAP pins are maximum of 63 bytes, so they should all fit in a u8. - let bytes_len = if bytes.len() <= 63 { - bytes.len() as u8 - } else { - return Err(std::io::Error::new( - ErrorKind::FileTooLarge, - "value is too large", - )); - }; - - // Open memfd_secret - let ret: i64 = unsafe { libc::syscall(SYS_memfd_secret, O_CLOEXEC) }; - if ret == -1 { - tracing::debug!("Failed to create memfd_secret"); - return Err(std::io::Error::last_os_error()); - } - let fd = i32::try_from(ret).map_err(|_| std::io::Error::other("invalid file descriptor"))?; - if unsafe { ftruncate(fd, bytes_len as off_t) } == -1 { - tracing::debug!("Failed to ftruncate memfd_secret"); - return Err(std::io::Error::last_os_error()); - } - - let ptr = unsafe { - let ptr = mmap( - std::ptr::null_mut(), - 4096, - PROT_READ | PROT_WRITE, - MAP_SHARED, - fd, - 0, - ); - if ptr == usize::MAX as *mut c_void { - tracing::debug!("Failed to mmap memfd_secret"); - return Err(std::io::Error::last_os_error()); - } - // ptr as *mut u8 - ptr - }; - - // Copy the data - unsafe { - ptr.copy_from_nonoverlapping(bytes.as_ptr().cast(), bytes.len()); - } - - // Cleanup - if unsafe { libc::munmap(ptr, 4096) } == -1 { - tracing::debug!("Failed to unmap memfd_secret"); - return Err(std::io::Error::last_os_error()); - } - bytes.zeroize(); - drop(bytes); - - let owned_fd = unsafe { OwnedFd::from_raw_fd(fd) }; - Ok(owned_fd) -} diff --git a/credentialsd/Cargo.toml b/credentialsd/Cargo.toml index e8703b7a..10885c01 100644 --- a/credentialsd/Cargo.toml +++ b/credentialsd/Cargo.toml @@ -12,7 +12,6 @@ base64 = "0.22.1" credentialsd-common = { path = "../credentialsd-common" } futures = "0.3.32" futures-lite.workspace = true -libc.workspace = true libwebauthn = { version = "0.8.0", features = ["nfc-backend-libnfc", "nfc-backend-pcsc", "reqwest-related-origins-source"] } # 0.6.1 fails to build with non-vendored library. # https://github.com/alexrsagen/rs-nfc1/issues/15 diff --git a/credentialsd/src/dbus/flow_control.rs b/credentialsd/src/dbus/flow_control.rs index ce883674..3da16f6f 100644 --- a/credentialsd/src/dbus/flow_control.rs +++ b/credentialsd/src/dbus/flow_control.rs @@ -3,20 +3,20 @@ use std::{ fmt::Debug, - io::{self, ErrorKind}, - mem::MaybeUninit, - os::{fd::AsRawFd, raw::c_void}, + os::fd::OwnedFd, sync::{Arc, Mutex}, }; use async_trait::async_trait; -use credentialsd_common::model::{ - Error as CredentialServiceError, Operation, PortalBackendOptions, UserInteractedEvent, - WebAuthnError, -}; use credentialsd_common::server::{BackgroundEvent, WindowHandle}; +use credentialsd_common::{ + memfd::read_secret, + model::{ + Error as CredentialServiceError, Operation, PortalBackendOptions, UserInteractedEvent, + WebAuthnError, + }, +}; use futures_lite::{Stream, StreamExt}; -use libc::{MAP_SHARED, PROT_READ, PROT_WRITE}; use tokio::sync::mpsc::Receiver; use tokio::sync::oneshot; use tokio::sync::{mpsc::Sender, Mutex as AsyncMutex}; @@ -198,7 +198,7 @@ async fn handle { - let pin_fd = std::os::fd::OwnedFd::from(pin_fd); + let pin_fd = OwnedFd::from(pin_fd); let pin = match read_secret(pin_fd.into()) { Ok(pin) => pin, // TODO: need to send an error to the UI, cancel the request and terminate the loop. @@ -314,60 +314,6 @@ impl CredentialRequestController for CredentialRequestControllerClient { } } -fn read_secret(pin_fd: std::os::fd::OwnedFd) -> Result { - // Get pin length - let len = { - let mut stat_buf = MaybeUninit::::uninit(); - let res = unsafe { libc::fstat(pin_fd.as_raw_fd(), stat_buf.as_mut_ptr()) }; - if res == -1 { - return Err(io::Error::last_os_error()); - } - let stat_buf = unsafe { stat_buf.assume_init() }; - usize::try_from(stat_buf.st_size) - .map_err(|_| io::Error::new(ErrorKind::FileTooLarge, "pin is too large"))? - }; - - // map the memory from the file descriptor - let ptr = unsafe { - let ptr = libc::mmap( - std::ptr::null_mut(), - 4096, - PROT_READ | PROT_WRITE, - MAP_SHARED, - pin_fd.as_raw_fd(), - 0, - ); - if ptr == usize::MAX as *mut c_void { - return Err(std::io::Error::last_os_error()); - } - ptr as *const u8 - }; - - // Copy the bytes. - let buf = unsafe { - // let len = ptr.read() as usize; - let mut buf: Vec = Vec::with_capacity(len); - ptr.copy_to_nonoverlapping(buf.as_mut_ptr().cast(), len); - buf.set_len(len); - buf - }; - - // Clean up mapping - unsafe { - if libc::munmap(ptr as *mut c_void, 4096) == -1 { - return Err(std::io::Error::last_os_error()); - } - } - drop(pin_fd); - - String::from_utf8(buf).map_err(|_| { - std::io::Error::new( - std::io::ErrorKind::InvalidData, - "invalid UTF-8 data found in buffer", - ) - }) -} - #[cfg(test)] pub mod test { use std::{ From 8c4dfc5731baca8f7ba92a614c64f45164801b78 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Sun, 21 Jun 2026 19:12:48 -0500 Subject: [PATCH 2/2] common: Use memfd_secret for Hybrid QR code --- credentialsd-common/src/server.rs | 51 ++++++++++--------- credentialsd-ui/src/gui/view_model/mod.rs | 8 ++- credentialsd/src/credential_service/hybrid.rs | 19 +++++-- 3 files changed, 45 insertions(+), 33 deletions(-) diff --git a/credentialsd-common/src/server.rs b/credentialsd-common/src/server.rs index 0b06bb16..f785acb8 100644 --- a/credentialsd-common/src/server.rs +++ b/credentialsd-common/src/server.rs @@ -3,12 +3,12 @@ use std::{collections::HashMap, fmt::Display}; use serde::{ - Deserialize, Serialize, de::{DeserializeSeed, Error, Visitor}, + Deserialize, Serialize, }; use zvariant::{ - self, Array, DeserializeDict, DynamicDeserialize, Fd, NoneValue, Optional, OwnedFd, OwnedValue, - SerializeDict, Signature, Str, Structure, StructureBuilder, Type, Value, signature::Fields, + self, signature::Fields, Array, DeserializeDict, DynamicDeserialize, Fd, NoneValue, Optional, + OwnedFd, OwnedValue, SerializeDict, Signature, Str, Structure, StructureBuilder, Type, Value, }; use crate::model::{Device, Operation, RequestId, UserInteractedEvent}; @@ -55,7 +55,7 @@ const USER_INTERACTED_EVENT_CREDENTIAL_SELECTED: u32 = 0x05; const USER_INTERACTED_EVENT_REQUEST_CANCELLED: u32 = 0x06; /// Flattened enum BackgroundEvent for sending across D-Bus. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, PartialEq)] pub enum BackgroundEvent { CeremonyCompleted, NeedsPin { attempts_left: Option }, @@ -64,7 +64,7 @@ pub enum BackgroundEvent { SelectingCredential { creds: Vec }, HybridIdle, - HybridStarted(String), + HybridStarted(OwnedFd), HybridConnecting, HybridConnected, @@ -138,7 +138,7 @@ impl From<&BackgroundEvent> for Structure<'_> { Some(Value::U32(attempts_left.map(u32::from).unwrap_or(u32::MAX))) } BackgroundEvent::SelectingCredential { creds } => Some(Value::Array(creds.into())), - BackgroundEvent::HybridStarted(qr_data) => Some(Value::Str(qr_data.into())), + BackgroundEvent::HybridStarted(qr_data_fd) => Some(Value::Fd(qr_data_fd.into())), // Empty BackgroundEvent::CeremonyCompleted => None, BackgroundEvent::NeedsUserPresence => None, @@ -215,8 +215,8 @@ impl TryFrom<&Structure<'_>> for BackgroundEvent { BACKGROUND_EVENT_HYBRID_IDLE => Ok(Self::HybridIdle), BACKGROUND_EVENT_HYBRID_STARTED => { - let qr_data = value.downcast_ref::<&str>()?; - Ok(Self::HybridStarted(qr_data.to_string())) + let qr_data_fd = value.downcast_ref::()?.try_to_owned()?; + Ok(Self::HybridStarted(qr_data_fd.into())) } BACKGROUND_EVENT_HYBRID_CONNECTING => Ok(Self::HybridConnecting), BACKGROUND_EVENT_HYBRID_CONNECTED => Ok(Self::HybridConnected), @@ -636,9 +636,11 @@ fn tag_value_to_struct(tag: u32, value: Option>) -> Structure<'static> #[cfg(test)] mod test { + use std::os::fd::{FromRawFd, OwnedFd}; + use zvariant::{ - Type, serialized::{Context, Data, Format}, + Type, }; use super::{BackgroundEvent, Credential}; @@ -656,25 +658,28 @@ mod test { #[test] fn test_round_trip_background_hybrid_event() { - let event1 = BackgroundEvent::HybridStarted("FIDO:/1234".to_string()); + let mut fds = [0; 2]; + unsafe { + libc::pipe(fds.as_mut_ptr()); + } + // Wrap the raw fds into safe OwnedFd instances + let mock_fd = unsafe { OwnedFd::from_raw_fd(fds[0]) }; + let _mock_fd2 = unsafe { OwnedFd::from_raw_fd(fds[1]) }; + + println!("mock_fd: {mock_fd:?}"); + let event1 = BackgroundEvent::HybridStarted(mock_fd.into()); let ctx = zvariant::serialized::Context::new_dbus(zvariant::BE, 0); assert_eq!("(uv)", BackgroundEvent::SIGNATURE.to_string()); let data = zvariant::to_bytes(ctx, &event1).unwrap(); - let expected = b"\x00\x00\x00\x21\x01s\0\0\0\0\0\x0aFIDO:/1234\0"; + println!("data: {data:?}"); + // handle value is 0_u32 because it's the index into the list of fds in the message. Since + // there's only one, it will be 0. + let expected = b"\x00\x00\x00\x21\x01h\0\0\0\0\0\0"; assert_eq!(expected, data.bytes()); let event2 = data.deserialize().unwrap().0; - assert_eq!(event1, event2); - } - - #[test] - fn test_deserialize_background_hybrid_event() { - let bytes = b"\x00\x00\x00\x21\x01s\0\0\0\0\0\x0aFIDO:/1234\0"; - let data = Data::new(bytes, Context::new(Format::DBus, zvariant::BE, 0)); - let event: BackgroundEvent = data.deserialize().unwrap().0; - assert!(matches!( - event, - BackgroundEvent::HybridStarted(ref s) if s == "FIDO:/1234" - )); + // I believe that the fd is `dup()`'d through the serialization/deserialization process, so + // we can't compare the numbers for equality. + assert!(matches!(event2, BackgroundEvent::HybridStarted(_))); } #[test] diff --git a/credentialsd-ui/src/gui/view_model/mod.rs b/credentialsd-ui/src/gui/view_model/mod.rs index 3a5cb64b..70fcea45 100644 --- a/credentialsd-ui/src/gui/view_model/mod.rs +++ b/credentialsd-ui/src/gui/view_model/mod.rs @@ -7,6 +7,7 @@ use async_std::{ channel::{Receiver, Sender}, sync::Mutex as AsyncMutex, }; +use credentialsd_common::memfd::read_secret; use credentialsd_common::server::{BackgroundEvent, Credential}; use gettextrs::gettext; use serde::{Deserialize, Serialize}; @@ -33,10 +34,7 @@ pub(crate) struct ViewModel { devices: Vec, selected_device: Option, - // providers: Vec, - hybrid_qr_state: HybridState, hybrid_qr_code_data: Option>, - // hybrid_linked_state: HybridState, } impl ViewModel { @@ -67,7 +65,6 @@ impl ViewModel { subtitle: String::default(), devices, selected_device: None, - hybrid_qr_state: HybridState::default(), hybrid_qr_code_data: None, } } @@ -300,7 +297,8 @@ impl ViewModel { Event::Background(BackgroundEvent::HybridIdle) => { self.hybrid_qr_code_data = None; } - Event::Background(BackgroundEvent::HybridStarted(qr_code)) => { + Event::Background(BackgroundEvent::HybridStarted(qr_code_fd)) => { + let qr_code = read_secret(qr_code_fd.into()).unwrap(); self.hybrid_qr_code_data = Some(qr_code.clone().into_bytes()); self.tx_update .send(ViewUpdate::HybridNeedsQrCode(qr_code)) diff --git a/credentialsd/src/credential_service/hybrid.rs b/credentialsd/src/credential_service/hybrid.rs index 9ddc2d7e..2257f530 100644 --- a/credentialsd/src/credential_service/hybrid.rs +++ b/credentialsd/src/credential_service/hybrid.rs @@ -2,10 +2,11 @@ use core::panic; use std::fmt::Debug; use async_stream::stream; -use credentialsd_common::server::BackgroundEvent; use futures_lite::Stream; -use tokio::sync::broadcast; -use tokio::sync::mpsc::{self, Sender}; +use tokio::sync::{ + broadcast, + mpsc::{self, Sender}, +}; use tracing::{debug, error}; use libwebauthn::transport::cable::channel::{CableUpdate, CableUxUpdate}; @@ -15,7 +16,7 @@ use libwebauthn::transport::cable::qr_code_device::{ use libwebauthn::transport::{Channel, ChannelSettings, Device}; use libwebauthn::webauthn::{Error as WebAuthnError, WebAuthn}; -use credentialsd_common::model::Error; +use credentialsd_common::{memfd::write_secret, model::Error, server::BackgroundEvent}; use crate::model::CredentialRequest; @@ -234,7 +235,15 @@ impl From for credentialsd_common::model::HybridState { impl From<&HybridState> for BackgroundEvent { fn from(value: &HybridState) -> Self { match value { - HybridState::Init(qr_code) => BackgroundEvent::HybridStarted(qr_code.to_string()), + HybridState::Init(qr_code) => { + // This is just a rounded guess; FIDO QR codes shouldn't need more than this much data, even + // with PQ KEMs. + const CTAP_HYBRID_QR_CODE_MAX_LEN: usize = 4096; + let fd = write_secret(qr_code.clone().into_bytes(), CTAP_HYBRID_QR_CODE_MAX_LEN) + .unwrap(); + BackgroundEvent::HybridStarted(fd.into()) + } + HybridState::Connecting => BackgroundEvent::HybridConnecting, HybridState::Connected => BackgroundEvent::HybridConnected, HybridState::Completed => BackgroundEvent::CeremonyCompleted,