From 6bc256f2d655d0afaf75a6a51a285c18237f7774 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Tue, 3 Mar 2026 10:15:43 +0000 Subject: [PATCH 1/5] chore(clippy): Fix all clippy warnings --- .github/workflows/rust.yml | 2 ++ libwebauthn/examples/bio_enrollment_hid.rs | 8 ++--- libwebauthn/examples/change_pin_hid.rs | 17 ++++++----- libwebauthn/examples/u2f_ble.rs | 4 +-- libwebauthn/examples/u2f_hid.rs | 4 +-- libwebauthn/examples/webauthn_nfc.rs | 10 ++++--- .../examples/webauthn_preflight_hid.rs | 5 ++-- .../src/management/authenticator_config.rs | 4 +-- libwebauthn/src/management/bio_enrollment.rs | 7 ++--- .../src/management/credential_management.rs | 4 +-- libwebauthn/src/ops/u2f.rs | 2 +- libwebauthn/src/ops/webauthn/get_assertion.rs | 7 ++--- libwebauthn/src/ops/webauthn/idl/base64url.rs | 2 +- .../src/ops/webauthn/make_credential.rs | 4 +-- libwebauthn/src/pin.rs | 20 ++++++++++--- libwebauthn/src/proto/ctap1/apdu/request.rs | 22 ++++---------- libwebauthn/src/proto/ctap1/model.rs | 13 +++++---- libwebauthn/src/proto/ctap1/protocol.rs | 4 +-- libwebauthn/src/proto/ctap2/cbor/response.rs | 2 +- libwebauthn/src/proto/ctap2/cbor/serde.rs | 2 +- libwebauthn/src/proto/ctap2/model.rs | 2 +- .../src/proto/ctap2/model/bio_enrollment.rs | 14 ++++----- .../src/proto/ctap2/model/get_assertion.rs | 29 ++++++++----------- .../src/proto/ctap2/model/make_credential.rs | 6 ++-- .../src/transport/ble/btleplug/connection.rs | 2 +- .../src/transport/ble/btleplug/manager.rs | 7 ++--- libwebauthn/src/transport/ble/device.rs | 6 ++-- libwebauthn/src/transport/ble/framing.rs | 22 +++++++++----- .../src/transport/cable/advertisement.rs | 2 +- libwebauthn/src/transport/cable/channel.rs | 4 +-- .../src/transport/cable/connection_stages.rs | 6 ++-- libwebauthn/src/transport/cable/crypto.rs | 2 +- .../src/transport/cable/known_devices.rs | 6 ++-- .../src/transport/cable/qr_code_device.rs | 8 ++--- libwebauthn/src/transport/cable/tunnel.rs | 10 +++---- libwebauthn/src/transport/hid/channel.rs | 1 - libwebauthn/src/transport/hid/framing.rs | 22 +++++++++----- libwebauthn/src/transport/mock/channel.rs | 6 ++++ libwebauthn/src/transport/mod.rs | 1 + libwebauthn/src/transport/nfc/channel.rs | 3 +- libwebauthn/src/transport/nfc/commands.rs | 2 +- libwebauthn/src/transport/nfc/device.rs | 14 +++------ libwebauthn/src/transport/nfc/pcsc/mod.rs | 4 +-- libwebauthn/src/webauthn/pin_uv_auth_token.rs | 8 ++--- 44 files changed, 169 insertions(+), 161 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 4dea18bf..3ea66300 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -19,6 +19,8 @@ jobs: run: sudo apt-get update - name: Install system dependencies run: sudo apt-get install libudev-dev libdbus-1-dev libsodium-dev + - name: Clippy + run: cargo clippy --all-targets --all-features -- -D warnings - name: Build run: cargo build - name: Run tests diff --git a/libwebauthn/examples/bio_enrollment_hid.rs b/libwebauthn/examples/bio_enrollment_hid.rs index 30095533..62f4443d 100644 --- a/libwebauthn/examples/bio_enrollment_hid.rs +++ b/libwebauthn/examples/bio_enrollment_hid.rs @@ -147,7 +147,7 @@ pub async fn main() -> Result<(), Box> { println!("Devices found: {:?}", devices); for mut device in devices { - println!("Selected HID authenticator: {}", &device); + println!("Selected HID authenticator: {}", device); let mut channel = device.channel().await?; channel.wink(TIMEOUT).await?; @@ -199,7 +199,7 @@ pub async fn main() -> Result<(), Box> { let idx = ask_for_user_input(enrollments.len()); channel .remove_bio_enrollment( - &enrollments[idx].template_id.as_ref().unwrap(), + enrollments[idx].template_id.as_ref().unwrap(), TIMEOUT, ) .await @@ -229,7 +229,7 @@ pub async fn main() -> Result<(), Box> { let new_name: String = read!("{}\n"); channel .rename_bio_enrollment( - &enrollments[idx].template_id.as_ref().unwrap(), + enrollments[idx].template_id.as_ref().unwrap(), &new_name, TIMEOUT, ) @@ -266,7 +266,7 @@ pub async fn main() -> Result<(), Box> { Err(err) => break 'outer Err(err), }; } - Ok(format!("Success!")) + Ok("Success!".to_string()) } }; match action { diff --git a/libwebauthn/examples/change_pin_hid.rs b/libwebauthn/examples/change_pin_hid.rs index 6c086d8f..4b38a13b 100644 --- a/libwebauthn/examples/change_pin_hid.rs +++ b/libwebauthn/examples/change_pin_hid.rs @@ -72,7 +72,7 @@ pub async fn main() -> Result<(), Box> { println!("Devices found: {:?}", devices); for mut device in devices { - println!("Selected HID authenticator: {}", &device); + println!("Selected HID authenticator: {}", device); let mut channel = device.channel().await?; channel.wink(TIMEOUT).await?; @@ -80,7 +80,7 @@ pub async fn main() -> Result<(), Box> { io::stdout().flush().unwrap(); let new_pin: String = read!("{}\n"); - if &new_pin == "" { + if new_pin.is_empty() { println!("PIN: No PIN provided, cancelling operation."); return Ok(()); } @@ -88,21 +88,22 @@ pub async fn main() -> Result<(), Box> { let state_recv = channel.get_ux_update_receiver(); tokio::spawn(handle_updates(state_recv)); - let response = loop { + loop { match channel.change_pin(new_pin.clone(), TIMEOUT).await { - Ok(response) => break Ok(response), + Ok(response) => { + println!("WebAuthn response: {response:?}"); + break; + } Err(WebAuthnError::Ctap(ctap_error)) => { if ctap_error.is_retryable_user_error() { println!("Oops, try again! Error: {}", ctap_error); continue; } - break Err(WebAuthnError::Ctap(ctap_error)); + panic!("{:?}", WebAuthnError::Ctap(ctap_error)); } - Err(err) => break Err(err), + Err(err) => panic!("{:?}", err), }; } - .unwrap(); - println!("WebAuthn response: {:?}", response); } Ok(()) diff --git a/libwebauthn/examples/u2f_ble.rs b/libwebauthn/examples/u2f_ble.rs index 66007d52..def97700 100644 --- a/libwebauthn/examples/u2f_ble.rs +++ b/libwebauthn/examples/u2f_ble.rs @@ -44,7 +44,7 @@ pub async fn main() -> Result<(), Box> { // Registration ceremony println!("Registration request sent (timeout: {:?}).", TIMEOUT); let register_request = - RegisterRequest::new_u2f_v2(&APP_ID, &challenge, vec![], TIMEOUT, false); + RegisterRequest::new_u2f_v2(APP_ID, challenge, vec![], TIMEOUT, false); let state_recv = channel.get_ux_update_receiver(); tokio::spawn(handle_updates(state_recv)); @@ -56,7 +56,7 @@ pub async fn main() -> Result<(), Box> { println!("Signature request sent (timeout: {:?} seconds).", TIMEOUT); let new_key = response.as_registered_key()?; let sign_request = - SignRequest::new(&APP_ID, &challenge, &new_key.key_handle, TIMEOUT, true); + SignRequest::new(APP_ID, challenge, &new_key.key_handle, TIMEOUT, true); let response = channel.u2f_sign(&sign_request).await?; println!("Response: {:?}", response); } diff --git a/libwebauthn/examples/u2f_hid.rs b/libwebauthn/examples/u2f_hid.rs index 58be0e22..63b0cca9 100644 --- a/libwebauthn/examples/u2f_hid.rs +++ b/libwebauthn/examples/u2f_hid.rs @@ -47,7 +47,7 @@ pub async fn main() -> Result<(), Box> { // Registration ceremony println!("Registration request sent (timeout: {:?}).", TIMEOUT); let register_request = - RegisterRequest::new_u2f_v2(&APP_ID, &challenge, vec![], TIMEOUT, false); + RegisterRequest::new_u2f_v2(APP_ID, challenge, vec![], TIMEOUT, false); let state_recv = channel.get_ux_update_receiver(); tokio::spawn(handle_updates(state_recv)); @@ -59,7 +59,7 @@ pub async fn main() -> Result<(), Box> { println!("Signature request sent (timeout: {:?} seconds).", TIMEOUT); let new_key = response.as_registered_key()?; let sign_request = - SignRequest::new(&APP_ID, &challenge, &new_key.key_handle, TIMEOUT, true); + SignRequest::new(APP_ID, challenge, &new_key.key_handle, TIMEOUT, true); let response = channel.u2f_sign(&sign_request).await?; println!("Response: {:?}", response); } diff --git a/libwebauthn/examples/webauthn_nfc.rs b/libwebauthn/examples/webauthn_nfc.rs index b6133320..59b8e53f 100644 --- a/libwebauthn/examples/webauthn_nfc.rs +++ b/libwebauthn/examples/webauthn_nfc.rs @@ -1,4 +1,3 @@ -use std::convert::TryInto; use std::error::Error; use std::io::{self, Write}; use std::time::Duration; @@ -85,13 +84,14 @@ pub async fn main() -> Result<(), Box> { let challenge: [u8; 32] = thread_rng().gen(); if let Some(mut device) = device { - println!("Selected NFC authenticator: {}", &device); + println!("Selected NFC authenticator: {}", device); let mut channel = device.channel().await?; // Make Credentials ceremony let make_credentials_request = MakeCredentialRequest { + challenge: Vec::from(challenge), origin: "example.org".to_owned(), - hash: Vec::from(challenge), + cross_origin: None, relying_party: Ctap2PublicKeyCredentialRpEntity::new("example.org", "example.org"), user: Ctap2PublicKeyCredentialUserEntity::new(&user_id, "mario.rossi", "Mario Rossi"), resident_key: Some(ResidentKeyRequirement::Discouraged), @@ -128,7 +128,9 @@ pub async fn main() -> Result<(), Box> { (&response.authenticator_data).try_into().unwrap(); let get_assertion = GetAssertionRequest { relying_party_id: "example.org".to_owned(), - hash: Vec::from(challenge), + challenge: Vec::from(challenge), + origin: "example.org".to_owned(), + cross_origin: None, allow: vec![credential], user_verification: UserVerificationRequirement::Discouraged, extensions: None, diff --git a/libwebauthn/examples/webauthn_preflight_hid.rs b/libwebauthn/examples/webauthn_preflight_hid.rs index 8d439751..abb4751a 100644 --- a/libwebauthn/examples/webauthn_preflight_hid.rs +++ b/libwebauthn/examples/webauthn_preflight_hid.rs @@ -1,4 +1,3 @@ -use std::convert::TryInto; use std::error::Error; use std::io::{self, Write}; use std::time::Duration; @@ -85,7 +84,7 @@ pub async fn main() -> Result<(), Box> { let user_id: [u8; 32] = thread_rng().gen(); for mut device in devices { - println!("Selected HID authenticator: {}", &device); + println!("Selected HID authenticator: {}", device); let mut channel = device.channel().await?; channel.wink(TIMEOUT).await?; @@ -165,7 +164,7 @@ async fn make_credential_call( origin: "example.org".to_owned(), cross_origin: None, relying_party: Ctap2PublicKeyCredentialRpEntity::new("example.org", "example.org"), - user: Ctap2PublicKeyCredentialUserEntity::new(&user_id, "mario.rossi", "Mario Rossi"), + user: Ctap2PublicKeyCredentialUserEntity::new(user_id, "mario.rossi", "Mario Rossi"), resident_key: Some(ResidentKeyRequirement::Discouraged), user_verification: UserVerificationRequirement::Preferred, algorithms: vec![Ctap2CredentialType::default()], diff --git a/libwebauthn/src/management/authenticator_config.rs b/libwebauthn/src/management/authenticator_config.rs index 04ba2eb5..032d5712 100644 --- a/libwebauthn/src/management/authenticator_config.rs +++ b/libwebauthn/src/management/authenticator_config.rs @@ -164,7 +164,7 @@ impl Ctap2UserVerifiableRequest for Ctap2AuthenticatorConfigRequest { fn calculate_and_set_uv_auth( &mut self, - uv_proto: &Box, + uv_proto: &dyn PinUvAuthProtocol, uv_auth_token: &[u8], ) { // pinUvAuthParam (0x04): the result of calling @@ -185,7 +185,7 @@ impl Ctap2UserVerifiableRequest for Ctap2AuthenticatorConfigRequest { } fn permissions(&self) -> Ctap2AuthTokenPermissionRole { - return Ctap2AuthTokenPermissionRole::AUTHENTICATOR_CONFIGURATION; + Ctap2AuthTokenPermissionRole::AUTHENTICATOR_CONFIGURATION } fn permissions_rpid(&self) -> Option<&str> { diff --git a/libwebauthn/src/management/bio_enrollment.rs b/libwebauthn/src/management/bio_enrollment.rs index c3710de7..ed960706 100644 --- a/libwebauthn/src/management/bio_enrollment.rs +++ b/libwebauthn/src/management/bio_enrollment.rs @@ -104,8 +104,7 @@ where Ok(Ctap2BioEnrollmentFingerprintSensorInfo { fingerprint_kind, max_capture_samples_required_for_enroll: resp - .max_capture_samples_required_for_enroll - .clone(), + .max_capture_samples_required_for_enroll, max_template_friendly_name: resp.max_template_friendly_name, }) } @@ -297,7 +296,7 @@ impl Ctap2UserVerifiableRequest for Ctap2BioEnrollmentRequest { fn calculate_and_set_uv_auth( &mut self, - uv_proto: &Box, + uv_proto: &dyn PinUvAuthProtocol, uv_auth_token: &[u8], ) { // pinUvAuthParam (0x05): authenticate(pinUvAuthToken, fingerprint (0x01) || enumerateEnrollments (0x04)). @@ -322,7 +321,7 @@ impl Ctap2UserVerifiableRequest for Ctap2BioEnrollmentRequest { } fn permissions(&self) -> Ctap2AuthTokenPermissionRole { - return Ctap2AuthTokenPermissionRole::BIO_ENROLLMENT; + Ctap2AuthTokenPermissionRole::BIO_ENROLLMENT } fn permissions_rpid(&self) -> Option<&str> { diff --git a/libwebauthn/src/management/credential_management.rs b/libwebauthn/src/management/credential_management.rs index 9b3c483a..23f1533b 100644 --- a/libwebauthn/src/management/credential_management.rs +++ b/libwebauthn/src/management/credential_management.rs @@ -276,7 +276,7 @@ impl Ctap2UserVerifiableRequest for Ctap2CredentialManagementRequest { fn calculate_and_set_uv_auth( &mut self, - uv_proto: &Box, + uv_proto: &dyn PinUvAuthProtocol, uv_auth_token: &[u8], ) { let mut data = vec![self.subcommand.unwrap() as u8]; @@ -295,7 +295,7 @@ impl Ctap2UserVerifiableRequest for Ctap2CredentialManagementRequest { } fn permissions(&self) -> Ctap2AuthTokenPermissionRole { - return Ctap2AuthTokenPermissionRole::CREDENTIAL_MANAGEMENT; + Ctap2AuthTokenPermissionRole::CREDENTIAL_MANAGEMENT } fn permissions_rpid(&self) -> Option<&str> { diff --git a/libwebauthn/src/ops/u2f.rs b/libwebauthn/src/ops/u2f.rs index 9c1f7296..8a677ce9 100644 --- a/libwebauthn/src/ops/u2f.rs +++ b/libwebauthn/src/ops/u2f.rs @@ -215,7 +215,7 @@ impl UpgradableResponse for SignResponse { } else { UserVerificationRequirement::Preferred }, - timeout: request.timeout.clone(), + timeout: request.timeout, }; let upgraded_response = [response.into_assertion_output(&orig_request, None)] .as_slice() diff --git a/libwebauthn/src/ops/webauthn/get_assertion.rs b/libwebauthn/src/ops/webauthn/get_assertion.rs index 4d47add0..1f093ccd 100644 --- a/libwebauthn/src/ops/webauthn/get_assertion.rs +++ b/libwebauthn/src/ops/webauthn/get_assertion.rs @@ -107,7 +107,6 @@ impl WebAuthnIDL for GetAssertionRequest { sequence hints = []; AuthenticationExtensionsClientInputsJSON extensions; }; */ - impl FromIdlModel for GetAssertionRequest { @@ -342,7 +341,7 @@ impl Ctap2HMACGetSecretOutput { pub(crate) fn decrypt_output( &self, shared_secret: &[u8], - uv_proto: &Box, + uv_proto: &dyn PinUvAuthProtocol, ) -> Option { let output = match uv_proto.decrypt(shared_secret, &self.encrypted_output) { Ok(o) => o, @@ -642,7 +641,7 @@ mod tests { #[test] fn test_request_from_json_invalid_rp_id() { let rpid = RelyingPartyId::try_from("example.org").unwrap(); - let req_json = json_field_add(&REQUEST_BASE_JSON, "rpId", r#""example.org.""#); + let req_json = json_field_add(REQUEST_BASE_JSON, "rpId", r#""example.org.""#); let result = GetAssertionRequest::from_json(&rpid, &req_json); assert!(matches!( @@ -654,7 +653,7 @@ mod tests { #[test] fn test_request_from_json_mismatching_rp_id() { let rpid = RelyingPartyId::try_from("example.org").unwrap(); - let req_json = json_field_add(&REQUEST_BASE_JSON, "rpId", r#""other.example.org""#); + let req_json = json_field_add(REQUEST_BASE_JSON, "rpId", r#""other.example.org""#); let result = GetAssertionRequest::from_json(&rpid, &req_json); assert!(matches!( diff --git a/libwebauthn/src/ops/webauthn/idl/base64url.rs b/libwebauthn/src/ops/webauthn/idl/base64url.rs index ebd805f7..d652e531 100644 --- a/libwebauthn/src/ops/webauthn/idl/base64url.rs +++ b/libwebauthn/src/ops/webauthn/idl/base64url.rs @@ -40,7 +40,7 @@ impl<'de> Deserialize<'de> for Base64UrlString { let s: String = Deserialize::deserialize(deserializer)?; base64_url::decode(&s) .map_err(serde::de::Error::custom) - .map(|bytes| Base64UrlString(bytes)) + .map(Base64UrlString) } } diff --git a/libwebauthn/src/ops/webauthn/make_credential.rs b/libwebauthn/src/ops/webauthn/make_credential.rs index 85af17a9..b7d65cd2 100644 --- a/libwebauthn/src/ops/webauthn/make_credential.rs +++ b/libwebauthn/src/ops/webauthn/make_credential.rs @@ -612,7 +612,7 @@ impl DowngradableRequest for MakeCredentialRequest { .exclude .as_ref() .unwrap_or(&vec![]) - .into_iter() + .iter() .map(|exclude| Ctap1RegisteredKey { version: Ctap1Version::U2fV2, key_handle: exclude.id.to_vec(), @@ -621,7 +621,7 @@ impl DowngradableRequest for MakeCredentialRequest { None => None, Some(ctap2_transports) => { let transports: Result, _> = - ctap2_transports.into_iter().map(|t| t.try_into()).collect(); + ctap2_transports.iter().map(|t| t.try_into()).collect(); transports.ok() } } diff --git a/libwebauthn/src/pin.rs b/libwebauthn/src/pin.rs index d8988fed..6fa71f80 100644 --- a/libwebauthn/src/pin.rs +++ b/libwebauthn/src/pin.rs @@ -95,6 +95,12 @@ pub struct PinUvAuthProtocolOne { public_key: P256PublicKey, } +impl Default for PinUvAuthProtocolOne { + fn default() -> Self { + Self::new() + } +} + impl PinUvAuthProtocolOne { pub fn new() -> Self { let private_key = if cfg!(test) { @@ -252,6 +258,12 @@ pub struct PinUvAuthProtocolTwo { public_key: P256PublicKey, } +impl Default for PinUvAuthProtocolTwo { + fn default() -> Self { + Self::new() + } +} + impl PinUvAuthProtocolTwo { pub fn new() -> Self { let private_key = if cfg!(test) { @@ -375,7 +387,7 @@ pub fn hmac_sha256(key: &[u8], message: &[u8]) -> Vec { } pub fn hkdf_sha256(salt: Option<&[u8]>, ikm: &[u8], info: &[u8]) -> Vec { - let hk = Hkdf::::new(salt, &ikm); + let hk = Hkdf::::new(salt, ikm); let mut okm = [0u8; 32]; // fixed L = 32 hk.expand(info, &mut okm) .expect("32 is a valid length for Sha256 to output"); @@ -396,13 +408,13 @@ where let get_info_response = self.ctap2_get_info().await?; // If the minPINLength member of the authenticatorGetInfo response is absent, then let platformMinPINLengthInCodePoints be 4. - if new_pin.as_bytes().len() < get_info_response.min_pin_length.unwrap_or(4) as usize { + if new_pin.len() < get_info_response.min_pin_length.unwrap_or(4) as usize { // If platformCollectedPinLengthInCodePoints is less than platformMinPINLengthInCodePoints then the platform SHOULD display a "PIN too short" error message to the user. return Err(Error::Platform(PlatformError::PinTooShort)); } // If the byte length of "newPin" is greater than the max UTF-8 representation limit of 63 bytes, then the platform SHOULD display a "PIN too long" error message to the user. - if new_pin.as_bytes().len() >= 64 { + if new_pin.len() >= 64 { return Err(Error::Platform(PlatformError::PinTooLong)); } @@ -441,7 +453,7 @@ where // In preparation for obtaining pinUvAuthToken, the platform: // * Obtains a shared secret. - let (public_key, shared_secret) = obtain_shared_secret(self, &uv_proto, timeout).await?; + let (public_key, shared_secret) = obtain_shared_secret(self, uv_proto.as_ref(), timeout).await?; // paddedPin is newPin padded on the right with 0x00 bytes to make it 64 bytes long. (Since the maximum length of newPin is 63 bytes, there is always at least one byte of padding.) let mut padded_new_pin = new_pin.as_bytes().to_vec(); diff --git a/libwebauthn/src/proto/ctap1/apdu/request.rs b/libwebauthn/src/proto/ctap1/apdu/request.rs index a2b7ccfa..794fc7d8 100644 --- a/libwebauthn/src/proto/ctap1/apdu/request.rs +++ b/libwebauthn/src/proto/ctap1/apdu/request.rs @@ -15,6 +15,8 @@ const U2F_REGISTER: u8 = 0x01; const U2F_AUTHENTICATE: u8 = 0x02; const U2F_VERSION: u8 = 0x03; +const CLA: u8 = 0x00; + const _CONTROL_BYTE_CHECK_ONLY: u8 = 0x07; const CONTROL_BYTE_ENFORCE_UP_AND_SIGN: u8 = 0x03; const CONTROL_BYTE_DONT_ENFORCE_UP_AND_SIGN: u8 = 0x08; @@ -40,21 +42,13 @@ impl ApduRequest { ins, p1, p2, - data: if let Some(bytes) = data { - Some(Vec::from(bytes)) - } else { - None - }, + data: data.map(Vec::from), response_max_length, } } pub fn raw_short(&self) -> Result, IOError> { - let mut raw: Vec = Vec::new(); - raw.push(0x00); // CLA - raw.push(self.ins); - raw.push(self.p1); - raw.push(self.p2); + let mut raw: Vec = vec![CLA, self.ins, self.p1, self.p2]; if let Some(data) = &self.data { if data.len() > APDU_SHORT_MAX_DATA { @@ -65,7 +59,7 @@ impl ApduRequest { data.len() ), )); - } else if data.len() == 0 { + } else if data.is_empty() { return Err(IOError::new( IOErrorKind::InvalidData, "Cannot serialize an empty payload.", @@ -94,11 +88,7 @@ impl ApduRequest { } pub fn raw_long(&self) -> Result, IOError> { - let mut raw: Vec = Vec::new(); - raw.push(0x00); // CLA - raw.push(self.ins); - raw.push(self.p1); - raw.push(self.p2); + let mut raw: Vec = vec![CLA, self.ins, self.p1, self.p2]; if let Some(data) = &self.data { if data.len() > APDI_LONG_MAX_DATA { diff --git a/libwebauthn/src/proto/ctap1/model.rs b/libwebauthn/src/proto/ctap1/model.rs index ff6e12b1..9a6a192a 100644 --- a/libwebauthn/src/proto/ctap1/model.rs +++ b/libwebauthn/src/proto/ctap1/model.rs @@ -182,7 +182,7 @@ impl Ctap1SignRequest { let app_id_hash = hasher.finalize().to_vec(); Ctap1SignRequest { - app_id_hash: app_id_hash, + app_id_hash, challenge: Vec::from(challenge), key_handle: Vec::from(key_handle), timeout, @@ -209,6 +209,12 @@ impl Ctap1SignRequest { #[derive(Debug)] pub struct Ctap1VersionRequest {} +impl Default for Ctap1VersionRequest { + fn default() -> Self { + Self::new() + } +} + impl Ctap1VersionRequest { pub fn new() -> Ctap1VersionRequest { Ctap1VersionRequest {} @@ -279,10 +285,7 @@ impl TryFrom for Ctap1SignResponse { ))?; let mut cursor = IOCursor::new(data); - let user_presence_verified = match cursor.read_u8()? { - 0x01 => true, - _ => false, - }; + let user_presence_verified = cursor.read_u8()? == 0x01; let counter = cursor.read_u32::()?; let mut signature = vec![]; diff --git a/libwebauthn/src/proto/ctap1/protocol.rs b/libwebauthn/src/proto/ctap1/protocol.rs index 49b5ebc3..6dcbb302 100644 --- a/libwebauthn/src/proto/ctap1/protocol.rs +++ b/libwebauthn/src/proto/ctap1/protocol.rs @@ -108,8 +108,8 @@ where } } -async fn send_apdu_request_wait_uv<'c, C: Channel>( - channel: &'c mut C, +async fn send_apdu_request_wait_uv( + channel: &mut C, request: &ApduRequest, timeout: Duration, ) -> Result { diff --git a/libwebauthn/src/proto/ctap2/cbor/response.rs b/libwebauthn/src/proto/ctap2/cbor/response.rs index abc1c052..fc1a4572 100644 --- a/libwebauthn/src/proto/ctap2/cbor/response.rs +++ b/libwebauthn/src/proto/ctap2/cbor/response.rs @@ -25,7 +25,7 @@ impl CborResponse { impl TryFrom<&Vec> for CborResponse { type Error = IOError; fn try_from(packet: &Vec) -> Result { - if packet.len() < 1 { + if packet.is_empty() { return Err(IOError::new( IOErrorKind::InvalidData, "Cbor response packets must contain at least 1 byte.", diff --git a/libwebauthn/src/proto/ctap2/cbor/serde.rs b/libwebauthn/src/proto/ctap2/cbor/serde.rs index d065d1c1..940f21ba 100644 --- a/libwebauthn/src/proto/ctap2/cbor/serde.rs +++ b/libwebauthn/src/proto/ctap2/cbor/serde.rs @@ -33,7 +33,7 @@ where R: std::io::Read, { let mut deserializer = serde_cbor::Deserializer::from_reader(reader); - return T::deserialize(&mut deserializer).map_err(CborError::from); + T::deserialize(&mut deserializer).map_err(CborError::from) } pub(crate) fn from_slice(slice: &[u8]) -> Result diff --git a/libwebauthn/src/proto/ctap2/model.rs b/libwebauthn/src/proto/ctap2/model.rs index 84373b4a..a0ecc7d5 100644 --- a/libwebauthn/src/proto/ctap2/model.rs +++ b/libwebauthn/src/proto/ctap2/model.rs @@ -214,7 +214,7 @@ pub trait Ctap2UserVerifiableRequest { fn ensure_uv_set(&mut self); fn calculate_and_set_uv_auth( &mut self, - uv_proto: &Box, + uv_proto: &dyn PinUvAuthProtocol, uv_auth_token: &[u8], ); fn client_data_hash(&self) -> &[u8]; diff --git a/libwebauthn/src/proto/ctap2/model/bio_enrollment.rs b/libwebauthn/src/proto/ctap2/model/bio_enrollment.rs index 48c26200..b7191973 100644 --- a/libwebauthn/src/proto/ctap2/model/bio_enrollment.rs +++ b/libwebauthn/src/proto/ctap2/model/bio_enrollment.rs @@ -226,15 +226,11 @@ impl Ctap2BioEnrollmentRequest { } pub fn new_start_new_enrollment(enrollment_timeout: Option) -> Self { - let subcommand_params = if let Some(time) = enrollment_timeout { - Some(Ctap2BioEnrollmentParams { - template_id: None, - template_friendly_name: None, - timeout_milliseconds: Some(time.as_millis() as u64), - }) - } else { - None - }; + let subcommand_params = enrollment_timeout.map(|time| Ctap2BioEnrollmentParams { + template_id: None, + template_friendly_name: None, + timeout_milliseconds: Some(time.as_millis() as u64), + }); Ctap2BioEnrollmentRequest { modality: Some(Ctap2BioEnrollmentModality::Fingerprint), diff --git a/libwebauthn/src/proto/ctap2/model/get_assertion.rs b/libwebauthn/src/proto/ctap2/model/get_assertion.rs index e14cedb2..d829480a 100644 --- a/libwebauthn/src/proto/ctap2/model/get_assertion.rs +++ b/libwebauthn/src/proto/ctap2/model/get_assertion.rs @@ -144,7 +144,7 @@ impl Ctap2GetAssertionRequest { ) -> bool { extensions .as_ref() - .map_or(true, |extensions| extensions.skip_serializing()) + .is_none_or(|extensions| extensions.skip_serializing()) } pub(crate) fn from_webauthn_request( @@ -419,7 +419,7 @@ impl Ctap2UserVerifiableRequest for Ctap2GetAssertionRequest { fn calculate_and_set_uv_auth( &mut self, - uv_proto: &Box, + uv_proto: &dyn PinUvAuthProtocol, uv_auth_token: &[u8], ) { let uv_auth_param = uv_proto.authenticate(uv_auth_token, self.client_data_hash()); @@ -514,7 +514,7 @@ impl Ctap2GetAssertionResponseExtensions { let decrypted_hmac = self.hmac_secret.as_ref().and_then(|x| { if let Some(auth_data) = auth_data { let uv_proto = auth_data.protocol_version.create_protocol_object(); - x.decrypt_output(&auth_data.shared_secret, &uv_proto) + x.decrypt_output(&auth_data.shared_secret, uv_proto.as_ref()) } else { None } @@ -536,23 +536,18 @@ impl Ctap2GetAssertionResponseExtensions { }); // LargeBlobs was requested - let large_blob = match request + let large_blob = request .extensions .as_ref() .and_then(|ext| ext.large_blob.as_ref()) - { - None => None, - Some(GetAssertionLargeBlobExtension::Read) => { - Some(GetAssertionLargeBlobExtensionOutput { - blob: response - .large_blob_key - .as_ref() - .map(|x| x.clone().into_vec()), - // Not yet supported - // written: None, - }) - } - }; + .map(|_| GetAssertionLargeBlobExtensionOutput { + blob: response + .large_blob_key + .as_ref() + .map(|x| x.clone().into_vec()), + // Not yet supported + // written: None, + }); GetAssertionResponseUnsignedExtensions { hmac_get_secret: None, diff --git a/libwebauthn/src/proto/ctap2/model/make_credential.rs b/libwebauthn/src/proto/ctap2/model/make_credential.rs index 1bcb005e..59127acd 100644 --- a/libwebauthn/src/proto/ctap2/model/make_credential.rs +++ b/libwebauthn/src/proto/ctap2/model/make_credential.rs @@ -109,7 +109,7 @@ impl Ctap2MakeCredentialRequest { } pub fn skip_serializing_options(options: &Option) -> bool { - options.map_or(true, |options| options.skip_serializing()) + options.is_none_or(|options| options.skip_serializing()) } pub fn skip_serializing_extensions( @@ -117,7 +117,7 @@ impl Ctap2MakeCredentialRequest { ) -> bool { extensions .as_ref() - .map_or(true, |extensions| extensions.skip_serializing()) + .is_none_or(|extensions| extensions.skip_serializing()) } pub(crate) fn from_webauthn_request( @@ -329,7 +329,7 @@ impl Ctap2UserVerifiableRequest for Ctap2MakeCredentialRequest { fn calculate_and_set_uv_auth( &mut self, - uv_proto: &Box, + uv_proto: &dyn PinUvAuthProtocol, uv_auth_token: &[u8], ) { let uv_auth_param = uv_proto.authenticate(uv_auth_token, self.client_data_hash()); diff --git a/libwebauthn/src/transport/ble/btleplug/connection.rs b/libwebauthn/src/transport/ble/btleplug/connection.rs index 888e1b05..9b9fdeac 100644 --- a/libwebauthn/src/transport/ble/btleplug/connection.rs +++ b/libwebauthn/src/transport/ble/btleplug/connection.rs @@ -78,7 +78,7 @@ impl Connection { } pub(crate) async fn select_fido_revision(&self, revision: &FidoRevision) -> Result<(), Error> { - let ack: u8 = revision.clone() as u8; + let ack: u8 = *revision as u8; self.peripheral .write( &self.services.service_revision_bitfield, diff --git a/libwebauthn/src/transport/ble/btleplug/manager.rs b/libwebauthn/src/transport/ble/btleplug/manager.rs index 1a87fe9b..9fcb7fd5 100644 --- a/libwebauthn/src/transport/ble/btleplug/manager.rs +++ b/libwebauthn/src/transport/ble/btleplug/manager.rs @@ -119,7 +119,7 @@ async fn get_adapter() -> Result { .await .or(Err(Error::Unavailable))? .into_iter() - .nth(0) + .next() .ok_or(Error::PoweredOff) } @@ -151,8 +151,7 @@ pub async fn list_fido_devices() -> Result, Error> { .filter(|p| { p.services() .iter() - .find(|s| s.uuid == FIDO_PROFILE_UUID) - .is_some() + .any(|s| s.uuid == FIDO_PROFILE_UUID) }) .collect(); let with_properties = discover_properties(peripherals) @@ -190,7 +189,7 @@ pub async fn supported_fido_revisions( .read(&services.service_revision_bitfield) .await .or(Err(Error::ConnectionFailed))?; - let bitfield = revision.iter().next().ok_or(Error::OperationFailed)?; + let bitfield = revision.first().ok_or(Error::OperationFailed)?; debug!(?revision, "Supported revision bitfield"); let supported = SupportedRevisions { diff --git a/libwebauthn/src/transport/ble/device.rs b/libwebauthn/src/transport/ble/device.rs index b94d528f..7c0d9d1e 100644 --- a/libwebauthn/src/transport/ble/device.rs +++ b/libwebauthn/src/transport/ble/device.rs @@ -59,9 +59,9 @@ impl From<&BtleplugFidoDevice> for BleDevice { } } -impl Into for &BleDevice { - fn into(self) -> BtleplugFidoDevice { - self.btleplug_device.clone() +impl From<&BleDevice> for BtleplugFidoDevice { + fn from(device: &BleDevice) -> Self { + device.btleplug_device.clone() } } diff --git a/libwebauthn/src/transport/ble/framing.rs b/libwebauthn/src/transport/ble/framing.rs index ec2176d3..6e404739 100644 --- a/libwebauthn/src/transport/ble/framing.rs +++ b/libwebauthn/src/transport/ble/framing.rs @@ -47,7 +47,7 @@ impl BleFrame { } let length = self.data.len() as u16; - let mut message = self.data.as_slice().into_iter().cloned().peekable(); + let mut message = self.data.as_slice().iter().cloned().peekable(); let mut fragments = vec![]; // Initial fragment @@ -89,13 +89,19 @@ pub struct BleFrameParser { fragments: Vec>, } +impl Default for BleFrameParser { + fn default() -> Self { + Self::new() + } +} + impl BleFrameParser { pub fn new() -> Self { Self { fragments: vec![] } } pub fn update(&mut self, fragment: &[u8]) -> Result { - if (self.fragments.len() == 0 && fragment.len() < INITIAL_FRAGMENT_MIN_LENGTH) + if (self.fragments.is_empty() && fragment.len() < INITIAL_FRAGMENT_MIN_LENGTH) || fragment.len() < CONT_FRAGMENT_MIN_LENGTH { return Err(IOError::new( @@ -105,11 +111,11 @@ impl BleFrameParser { } self.fragments.push(Vec::from(fragment)); - return if self.more_fragments_needed() { + if self.more_fragments_needed() { Ok(BleFrameParserResult::MoreFragmentsExpected) } else { Ok(BleFrameParserResult::Done) - }; + } } pub fn frame(&self) -> Result { @@ -202,7 +208,7 @@ mod tests { let mut parser = BleFrameParser::new(); assert_eq!( parser - .update(&vec![0x83, 0x00, 0x04, 0x0A, 0x0B, 0x0C, 0x0D]) + .update(&[0x83, 0x00, 0x04, 0x0A, 0x0B, 0x0C, 0x0D]) .unwrap(), BleFrameParserResult::Done ); @@ -213,15 +219,15 @@ mod tests { fn parse_multiple_fragments() { let mut parser = BleFrameParser::new(); assert_eq!( - parser.update(&vec![0x83, 0x00, 0x05, 0x0A]).unwrap(), + parser.update(&[0x83, 0x00, 0x05, 0x0A]).unwrap(), BleFrameParserResult::MoreFragmentsExpected ); assert_eq!( - parser.update(&vec![0x00, 0x0B, 0x0C, 0x0D]).unwrap(), + parser.update(&[0x00, 0x0B, 0x0C, 0x0D]).unwrap(), BleFrameParserResult::MoreFragmentsExpected ); assert_eq!( - parser.update(&vec![0x01, 0x0E]).unwrap(), + parser.update(&[0x01, 0x0E]).unwrap(), BleFrameParserResult::Done ); assert_eq!( diff --git a/libwebauthn/src/transport/cable/advertisement.rs b/libwebauthn/src/transport/cable/advertisement.rs index 2ba5246d..7d8bfae6 100644 --- a/libwebauthn/src/transport/cable/advertisement.rs +++ b/libwebauthn/src/transport/cable/advertisement.rs @@ -65,7 +65,7 @@ pub(crate) async fn await_advertisement( }; trace!(?device, ?data, ?eid_key); - let Some(decrypted) = trial_decrypt_advert(&eid_key, &data) else { + let Some(decrypted) = trial_decrypt_advert(eid_key, &data) else { warn!(?device, "Trial decrypt failed, ignoring"); continue; }; diff --git a/libwebauthn/src/transport/cable/channel.rs b/libwebauthn/src/transport/cable/channel.rs index 48cbd1f7..adcf1c93 100644 --- a/libwebauthn/src/transport/cable/channel.rs +++ b/libwebauthn/src/transport/cable/channel.rs @@ -115,7 +115,7 @@ impl From for CableUxUpdate { } #[async_trait] -impl<'d> Channel for CableChannel { +impl Channel for CableChannel { type UxUpdate = CableUxUpdate; async fn supported_protocols(&self) -> Result { @@ -186,7 +186,7 @@ impl<'d> Channel for CableChannel { } } -impl<'d> Ctap2AuthTokenStore for CableChannel { +impl Ctap2AuthTokenStore for CableChannel { fn store_auth_data(&mut self, _auth_token_data: AuthTokenData) {} fn get_auth_data(&self) -> Option<&AuthTokenData> { diff --git a/libwebauthn/src/transport/cable/connection_stages.rs b/libwebauthn/src/transport/cable/connection_stages.rs index 2ac834c8..8e95e8d8 100644 --- a/libwebauthn/src/transport/cable/connection_stages.rs +++ b/libwebauthn/src/transport/cable/connection_stages.rs @@ -62,13 +62,13 @@ impl ConnectionInput { ) -> Result { let tunnel_domain = decode_tunnel_domain_from_advert(&proximity_output.advert)?; - let routing_id_str = hex::encode(&proximity_output.advert.routing_id); + let routing_id_str = hex::encode(proximity_output.advert.routing_id); let tunnel_id = &derive( qr_device.qr_code.qr_secret.as_ref(), None, KeyPurpose::TunnelID, )[..16]; - let tunnel_id_str = hex::encode(&tunnel_id); + let tunnel_id_str = hex::encode(tunnel_id); let connection_type = CableTunnelConnectionType::QrCode { routing_id: routing_id_str, @@ -310,7 +310,7 @@ pub(crate) async fn handshake_stage( fn derive_psk(secret: &[u8], advert_plaintext: &[u8]) -> [u8; 32] { let mut psk: [u8; 32] = [0u8; 32]; - psk.copy_from_slice(&derive(secret, Some(advert_plaintext), KeyPurpose::PSK)[..32]); + psk.copy_from_slice(&derive(secret, Some(advert_plaintext), KeyPurpose::Psk)[..32]); psk } diff --git a/libwebauthn/src/transport/cable/crypto.rs b/libwebauthn/src/transport/cable/crypto.rs index 54682ce5..9bfcf730 100644 --- a/libwebauthn/src/transport/cable/crypto.rs +++ b/libwebauthn/src/transport/cable/crypto.rs @@ -9,7 +9,7 @@ use crate::pin::hmac_sha256; pub enum KeyPurpose { EIDKey = 1, TunnelID = 2, - PSK = 3, + Psk = 3, } pub fn derive(secret: &[u8], salt: Option<&[u8]>, purpose: KeyPurpose) -> [u8; 64] { diff --git a/libwebauthn/src/transport/cable/known_devices.rs b/libwebauthn/src/transport/cable/known_devices.rs index 60d97d33..a9b9a3dd 100644 --- a/libwebauthn/src/transport/cable/known_devices.rs +++ b/libwebauthn/src/transport/cable/known_devices.rs @@ -93,7 +93,7 @@ pub struct CableKnownDeviceInfo { impl From<&CableLinkingInfo> for CableKnownDeviceId { fn from(linking_info: &CableLinkingInfo) -> Self { - hex::encode(&linking_info.authenticator_public_key) + hex::encode(linking_info.authenticator_public_key.as_slice()) } } @@ -136,7 +136,7 @@ impl Display for CableKnownDevice { f, "{} ({})", &self.device_info.name, - hex::encode(&self.device_info.public_key) + hex::encode(self.device_info.public_key) ) } } @@ -153,7 +153,7 @@ impl CableKnownDevice { let device = CableKnownDevice { hint, device_info: device_info.clone(), - store: store, + store, }; Ok(device) } diff --git a/libwebauthn/src/transport/cable/qr_code_device.rs b/libwebauthn/src/transport/cable/qr_code_device.rs index fcfa8f39..6c9bed47 100644 --- a/libwebauthn/src/transport/cable/qr_code_device.rs +++ b/libwebauthn/src/transport/cable/qr_code_device.rs @@ -73,10 +73,10 @@ pub struct CableQrCode { pub supports_non_discoverable_mc: Option, } -impl ToString for CableQrCode { - fn to_string(&self) -> String { +impl std::fmt::Display for CableQrCode { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let serialized = cbor::to_vec(&self).unwrap(); - format!("FIDO:/{}", digit_encode(&serialized)) + write!(f, "FIDO:/{}", digit_encode(&serialized)) } } @@ -126,7 +126,7 @@ impl CableQrCodeDevice { .try_into() .unwrap(); let mut qr_secret = [0u8; 16]; - OsRng::default().fill_bytes(&mut qr_secret); + OsRng.fill_bytes(&mut qr_secret); let current_unix_time = SystemTime::now() .duration_since(SystemTime::UNIX_EPOCH) diff --git a/libwebauthn/src/transport/cable/tunnel.rs b/libwebauthn/src/transport/cable/tunnel.rs index 48801736..4755350a 100644 --- a/libwebauthn/src/transport/cable/tunnel.rs +++ b/libwebauthn/src/transport/cable/tunnel.rs @@ -200,7 +200,7 @@ impl std::fmt::Debug for CableTunnelConnectionType { } } -pub(crate) async fn connect<'d>( +pub(crate) async fn connect( tunnel_domain: &str, connection_type: &CableTunnelConnectionType, ) -> Result>, TransportError> { @@ -235,7 +235,7 @@ pub(crate) async fn connect<'d>( cbor::to_vec(client_payload).or(Err(TransportError::InvalidEndpoint))?; request.headers_mut().insert( "X-caBLE-Client-Payload", - hex::encode(&client_payload) + hex::encode(client_payload) .parse() .or(Err(TransportError::InvalidEndpoint))?, ); @@ -285,7 +285,7 @@ pub(crate) async fn do_handshake( .. } => Builder::new("Noise_NKpsk0_P256_AESGCM_SHA256".parse()?) .prologue(CABLE_PROLOGUE_STATE_ASSISTED)? - .remote_public_key(&authenticator_public_key)? + .remote_public_key(authenticator_public_key)? .psk(0, &psk)? .build_initiator(), }; @@ -581,7 +581,7 @@ async fn connection_recv_update(message: &[u8]) -> Result = match serde_cbor::from_slice(&message) { + let update_message: BTreeMap = match serde_cbor::from_slice(message) { Ok(update_message) => update_message, Err(e) => { error!(?e, "Failed to decode update message"); @@ -708,7 +708,7 @@ async fn connection_recv( private_key, tunnel_domain, &linking_info, - &noise_state, + noise_state, ) { Ok(known_device) => { debug!(?device_id, "Updating known device"); diff --git a/libwebauthn/src/transport/hid/channel.rs b/libwebauthn/src/transport/hid/channel.rs index 3280b09d..35a94863 100644 --- a/libwebauthn/src/transport/hid/channel.rs +++ b/libwebauthn/src/transport/hid/channel.rs @@ -449,7 +449,6 @@ impl Channel for HidChannel<'_> { } async fn close(&mut self) { - () } async fn apdu_send( diff --git a/libwebauthn/src/transport/hid/framing.rs b/libwebauthn/src/transport/hid/framing.rs index 7282730d..11208e03 100644 --- a/libwebauthn/src/transport/hid/framing.rs +++ b/libwebauthn/src/transport/hid/framing.rs @@ -53,7 +53,7 @@ impl HidMessage { )); } - let mut payload = self.payload.as_slice().into_iter().cloned().peekable(); + let mut payload = self.payload.as_slice().iter().cloned().peekable(); let mut packets = vec![]; // Initial fragment @@ -106,13 +106,19 @@ pub struct HidMessageParser { packets: Vec>, } +impl Default for HidMessageParser { + fn default() -> Self { + Self::new() + } +} + impl HidMessageParser { pub fn new() -> Self { Self { packets: vec![] } } pub fn update(&mut self, packet: &[u8]) -> Result { - if (self.packets.len() == 0 && packet.len() < PACKET_INITIAL_HEADER_SIZE) + if (self.packets.is_empty() && packet.len() < PACKET_INITIAL_HEADER_SIZE) || packet.len() < PACKET_CONT_HEADER_SIZE + 1 { error!("Packet length in invalid"); @@ -126,11 +132,11 @@ impl HidMessageParser { } else { self.packets.push(Vec::from(packet)); } - return if self.more_packets_needed() { + if self.more_packets_needed() { Ok(HidMessageParserState::MorePacketsExpected) } else { Ok(HidMessageParserState::Done) - }; + } } fn more_packets_needed(&self) -> bool { @@ -250,7 +256,7 @@ mod tests { let mut parser = HidMessageParser::new(); assert_eq!( parser - .update(&vec![ + .update(&[ 0xC0, 0xC1, 0xC2, 0xC3, 0x83, 0x00, 0x04, 0x0A, 0x0B, 0x0C, 0x0D, ]) .unwrap(), @@ -267,19 +273,19 @@ mod tests { let mut parser = HidMessageParser::new(); assert_eq!( parser - .update(&vec![0xC0, 0xC1, 0xC2, 0xC3, 0x83, 0x00, 0x05, 0x0A]) + .update(&[0xC0, 0xC1, 0xC2, 0xC3, 0x83, 0x00, 0x05, 0x0A]) .unwrap(), HidMessageParserState::MorePacketsExpected ); assert_eq!( parser - .update(&vec![0xC0, 0xC1, 0xC2, 0xC3, 0x00, 0x0B, 0x0C]) + .update(&[0xC0, 0xC1, 0xC2, 0xC3, 0x00, 0x0B, 0x0C]) .unwrap(), HidMessageParserState::MorePacketsExpected ); assert_eq!( parser - .update(&vec![0xC0, 0xC1, 0xC2, 0xC3, 0x01, 0x0D, 0x0E, 0xFF]) // excess byte + .update(&[0xC0, 0xC1, 0xC2, 0xC3, 0x01, 0x0D, 0x0E, 0xFF]) // excess byte .unwrap(), HidMessageParserState::Done ); diff --git a/libwebauthn/src/transport/mock/channel.rs b/libwebauthn/src/transport/mock/channel.rs index f6ad459e..43936bc8 100644 --- a/libwebauthn/src/transport/mock/channel.rs +++ b/libwebauthn/src/transport/mock/channel.rs @@ -21,6 +21,12 @@ pub struct MockChannel { ux_update_sender: broadcast::Sender, } +impl Default for MockChannel { + fn default() -> Self { + Self::new() + } +} + impl MockChannel { pub fn new() -> Self { let (ux_update_sender, _) = broadcast::channel(16); diff --git a/libwebauthn/src/transport/mod.rs b/libwebauthn/src/transport/mod.rs index 9b725d4a..1d6ea8c4 100644 --- a/libwebauthn/src/transport/mod.rs +++ b/libwebauthn/src/transport/mod.rs @@ -16,6 +16,7 @@ pub mod mock; pub mod virt; mod channel; +#[allow(clippy::module_inception)] mod transport; pub(crate) use channel::{AuthTokenData, Ctap2AuthTokenPermission}; diff --git a/libwebauthn/src/transport/nfc/channel.rs b/libwebauthn/src/transport/nfc/channel.rs index d703af7b..6408e612 100644 --- a/libwebauthn/src/transport/nfc/channel.rs +++ b/libwebauthn/src/transport/nfc/channel.rs @@ -139,7 +139,7 @@ where #[instrument(skip_all)] pub async fn wink(&mut self, _timeout: Duration) -> Result { warn!("WINK capability is not supported"); - return Ok(false); + Ok(false) } pub async fn select_fido2(&mut self) -> Result<(), Error> { @@ -234,7 +234,6 @@ where } async fn close(&mut self) { - () } #[instrument(level = Level::DEBUG, skip_all)] diff --git a/libwebauthn/src/transport/nfc/commands.rs b/libwebauthn/src/transport/nfc/commands.rs index 3a799b35..b011d2ea 100644 --- a/libwebauthn/src/transport/nfc/commands.rs +++ b/libwebauthn/src/transport/nfc/commands.rs @@ -96,6 +96,6 @@ impl<'a> From<&'a ApduRequest> for Command<'a> { impl_into_vec!(CtapMsgCommand<'a>); /// Constructs a `GET MSG` command. -pub fn command_ctap_msg(has_more: bool, payload: &[u8]) -> CtapMsgCommand { +pub fn command_ctap_msg(has_more: bool, payload: &[u8]) -> CtapMsgCommand<'_> { CtapMsgCommand::new(has_more, payload) } diff --git a/libwebauthn/src/transport/nfc/device.rs b/libwebauthn/src/transport/nfc/device.rs index 2ddb2275..90ceebba 100644 --- a/libwebauthn/src/transport/nfc/device.rs +++ b/libwebauthn/src/transport/nfc/device.rs @@ -81,14 +81,8 @@ impl<'d> Device<'d, Nfc, NfcChannel> for NfcDevice { } } -async fn is_fido(device: &NfcDevice) -> bool -where - Ctx: fmt::Debug + fmt::Display + Copy + Send + Sync, -{ - async fn inner(device: &NfcDevice) -> Result - where - Ctx: fmt::Debug + fmt::Display + Copy + Send + Sync, - { +async fn is_fido(device: &NfcDevice) -> bool { + async fn inner(device: &NfcDevice) -> Result { let chan = device.channel_sync().await?; // We fill the struct within channel_sync() and the call cannot fail for NFC, // so unwrap is fine here @@ -96,7 +90,7 @@ where Ok(protocols.fido2 || protocols.u2f) } - inner::(device).await.is_ok() + inner(device).await.is_ok() } #[instrument] @@ -117,7 +111,7 @@ pub async fn get_nfc_device() -> Result, Error> { for list_devices in list_devices_fns { for device in list_devices()? { - if is_fido::(&device).await { + if is_fido(&device).await { return Ok(Some(device)); } } diff --git a/libwebauthn/src/transport/nfc/pcsc/mod.rs b/libwebauthn/src/transport/nfc/pcsc/mod.rs index 191d91f1..229bab0e 100644 --- a/libwebauthn/src/transport/nfc/pcsc/mod.rs +++ b/libwebauthn/src/transport/nfc/pcsc/mod.rs @@ -22,7 +22,7 @@ pub struct PcscCard { pub card: Option, } -impl<'tx> Deref for PcscCard { +impl Deref for PcscCard { type Target = pcsc::Card; fn deref(&self) -> &pcsc::Card { @@ -114,7 +114,7 @@ impl Channel { } impl fmt::Display for Channel { - fn fmt(&self, f: &mut fmt::Formatter) -> std::fmt::Result { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result { let card = self.card.lock().unwrap(); let (names_len, atr_len) = card.status2_len().unwrap(); let mut names_buf = vec![0; names_len]; diff --git a/libwebauthn/src/webauthn/pin_uv_auth_token.rs b/libwebauthn/src/webauthn/pin_uv_auth_token.rs index d25c77d8..a38ca6a2 100644 --- a/libwebauthn/src/webauthn/pin_uv_auth_token.rs +++ b/libwebauthn/src/webauthn/pin_uv_auth_token.rs @@ -74,7 +74,7 @@ where ctap2_request.permissions_rpid(), ); if let Some(uv_auth_token) = channel.get_uv_auth_token(&token_identifier) { - ctap2_request.calculate_and_set_uv_auth(&uv_proto, uv_auth_token); + ctap2_request.calculate_and_set_uv_auth(uv_proto.as_ref(), uv_auth_token); return Ok(UsedPinUvAuthToken::FromStorage); } } @@ -214,7 +214,7 @@ where // In preparation for obtaining pinUvAuthToken, the platform: // * Obtains a shared secret. - let (public_key, shared_secret) = obtain_shared_secret(channel, &uv_proto, timeout).await?; + let (public_key, shared_secret) = obtain_shared_secret(channel, uv_proto.as_ref(), timeout).await?; // Then the platform obtains a pinUvAuthToken from the authenticator, with the mc (and likely also with the ga) // permission (see "pre-flight", mentioned above), using the selected operation. @@ -349,7 +349,7 @@ where // If successful, the platform creates the pinUvAuthParam parameter by calling // authenticate(pinUvAuthToken, clientDataHash), and goes to Step 1.1.1. // Sets the pinUvAuthProtocol parameter to the value as selected when it obtained the shared secret. - ctap2_request.calculate_and_set_uv_auth(&uv_proto, uv_auth_token.as_slice()); + ctap2_request.calculate_and_set_uv_auth(uv_proto.as_ref(), uv_auth_token.as_slice()); Ok(UsedPinUvAuthToken::NewlyCalculated(uv_operation)) } @@ -359,7 +359,7 @@ where pub(crate) async fn obtain_shared_secret( channel: &mut C, - pin_proto: &Box, + pin_proto: &dyn PinUvAuthProtocol, timeout: Duration, ) -> Result<(PublicKey, Vec), Error> where From 8a3cff6e9fd990be31023f98b87b2999aa007992 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Tue, 3 Mar 2026 21:24:49 +0000 Subject: [PATCH 2/5] feat(cable): add transport availability API --- libwebauthn/examples/webauthn_cable.rs | 6 ++++++ libwebauthn/src/transport/ble/btleplug/manager.rs | 5 +++++ libwebauthn/src/transport/ble/btleplug/mod.rs | 3 ++- libwebauthn/src/transport/ble/device.rs | 5 +++++ libwebauthn/src/transport/ble/mod.rs | 1 + libwebauthn/src/transport/cable/mod.rs | 6 ++++++ 6 files changed, 25 insertions(+), 1 deletion(-) diff --git a/libwebauthn/examples/webauthn_cable.rs b/libwebauthn/examples/webauthn_cable.rs index f42595c2..93d1cb8d 100644 --- a/libwebauthn/examples/webauthn_cable.rs +++ b/libwebauthn/examples/webauthn_cable.rs @@ -4,6 +4,7 @@ use std::sync::Arc; use std::time::Duration; use libwebauthn::pin::PinRequestReason; +use libwebauthn::transport::cable::is_available; use libwebauthn::transport::cable::channel::{CableUpdate, CableUxUpdate}; use libwebauthn::transport::cable::known_devices::{ CableKnownDevice, ClientPayloadHint, EphemeralDeviceInfoStore, @@ -91,6 +92,11 @@ async fn handle_updates(mut state_recv: Receiver) { pub async fn main() -> Result<(), Box> { setup_logging(); + if !is_available().await { + eprintln!("No Bluetooth adapter found. Cable/Hybrid transport is unavailable."); + return Err("Cable transport not available".into()); + } + let device_info_store = Arc::new(EphemeralDeviceInfoStore::default()); let user_id: [u8; 32] = thread_rng().gen(); let challenge: [u8; 32] = thread_rng().gen(); diff --git a/libwebauthn/src/transport/ble/btleplug/manager.rs b/libwebauthn/src/transport/ble/btleplug/manager.rs index 9fcb7fd5..d51a7b99 100644 --- a/libwebauthn/src/transport/ble/btleplug/manager.rs +++ b/libwebauthn/src/transport/ble/btleplug/manager.rs @@ -123,6 +123,11 @@ async fn get_adapter() -> Result { .ok_or(Error::PoweredOff) } +/// Checks if a Bluetooth adapter is available on the system. +pub async fn is_available() -> bool { + get_adapter().await.is_ok() +} + async fn discover_properties( peripherals: Vec, ) -> Result, Error> { diff --git a/libwebauthn/src/transport/ble/btleplug/mod.rs b/libwebauthn/src/transport/ble/btleplug/mod.rs index bd347606..28aab4a6 100644 --- a/libwebauthn/src/transport/ble/btleplug/mod.rs +++ b/libwebauthn/src/transport/ble/btleplug/mod.rs @@ -8,5 +8,6 @@ pub use connection::Connection; pub use device::FidoDevice; pub use error::Error; pub use manager::{ - connect, list_fido_devices, start_discovery_for_service_data, supported_fido_revisions, + connect, is_available, list_fido_devices, start_discovery_for_service_data, + supported_fido_revisions, }; diff --git a/libwebauthn/src/transport/ble/device.rs b/libwebauthn/src/transport/ble/device.rs index 7c0d9d1e..c567d18a 100644 --- a/libwebauthn/src/transport/ble/device.rs +++ b/libwebauthn/src/transport/ble/device.rs @@ -15,6 +15,11 @@ use super::btleplug::{supported_fido_revisions, FidoDevice as BtleplugFidoDevice use super::channel::BleChannel; use super::{btleplug, Ble}; +/// Checks if a Bluetooth adapter is available on the system. +pub async fn is_available() -> bool { + btleplug::is_available().await +} + #[instrument] pub async fn list_devices() -> Result, Error> { let devices: Vec<_> = btleplug::list_fido_devices() diff --git a/libwebauthn/src/transport/ble/mod.rs b/libwebauthn/src/transport/ble/mod.rs index 0659c03d..dcd6f92f 100644 --- a/libwebauthn/src/transport/ble/mod.rs +++ b/libwebauthn/src/transport/ble/mod.rs @@ -5,6 +5,7 @@ pub mod channel; pub mod device; pub mod framing; +pub use device::is_available; pub use device::list_devices; pub use device::BleDevice; diff --git a/libwebauthn/src/transport/cable/mod.rs b/libwebauthn/src/transport/cable/mod.rs index 7e7f583e..118e5a87 100644 --- a/libwebauthn/src/transport/cable/mod.rs +++ b/libwebauthn/src/transport/cable/mod.rs @@ -13,6 +13,12 @@ pub mod tunnel; use super::Transport; pub use digit_encode::digit_encode; +/// Checks if the Cable/Hybrid transport is available on the system. +/// Cable depends on a Bluetooth adapter for BLE advertisement discovery. +pub async fn is_available() -> bool { + super::ble::is_available().await +} + pub struct Cable {} impl Transport for Cable {} unsafe impl Send for Cable {} From 8e6f67047623fac5c0a27a20a28a9aaf4a6557d5 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Tue, 3 Mar 2026 21:28:21 +0000 Subject: [PATCH 3/5] Install NFC dependencies before --all-features builds --- .github/workflows/rust.yml | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 3ea66300..ad2d2c3c 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -18,14 +18,10 @@ jobs: - name: Update apt cache run: sudo apt-get update - name: Install system dependencies - run: sudo apt-get install libudev-dev libdbus-1-dev libsodium-dev + run: sudo apt-get install libudev-dev libdbus-1-dev libsodium-dev libnfc-dev libpcsclite-dev - name: Clippy run: cargo clippy --all-targets --all-features -- -D warnings - name: Build - run: cargo build + run: cargo build --all-targets --all-features - name: Run tests run: cargo test --verbose - - name: Install nfc dependencies - run: sudo apt-get install libnfc-dev libpcsclite-dev - - name: Build with nfc-support - run: cargo build --features libnfc,pcsc From c205cb59b685fb219bdfa30641fbce30ab5571c3 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Tue, 3 Mar 2026 21:33:55 +0000 Subject: [PATCH 4/5] Additional failures on CI only --- libwebauthn/src/pin.rs | 2 +- libwebauthn/src/proto/ctap1/apdu/response.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libwebauthn/src/pin.rs b/libwebauthn/src/pin.rs index 6fa71f80..5e651ad2 100644 --- a/libwebauthn/src/pin.rs +++ b/libwebauthn/src/pin.rs @@ -225,7 +225,7 @@ impl PinUvAuthProtocol for PinUvAuthProtocolOne { fn decrypt(&self, key: &[u8], ciphertext: &[u8]) -> Result, Error> { // If the size of demCiphertext is not a multiple of the AES block length, return error. // Otherwise return the AES-256-CBC decryption of demCiphertext using an all-zero IV. - if ciphertext.len() % 16 != 0 { + if !ciphertext.len().is_multiple_of(16) { error!( ?ciphertext, "Ciphertext length is not a multiple of AES block length" diff --git a/libwebauthn/src/proto/ctap1/apdu/response.rs b/libwebauthn/src/proto/ctap1/apdu/response.rs index 071b4aee..e442a302 100644 --- a/libwebauthn/src/proto/ctap1/apdu/response.rs +++ b/libwebauthn/src/proto/ctap1/apdu/response.rs @@ -33,7 +33,7 @@ impl ApduResponse { pub fn status(&self) -> Result { let mut cursor = IOCursor::new(vec![self.sw1, self.sw2]); - let code = cursor.read_u16::().unwrap() as u16; + let code = cursor.read_u16::().unwrap(); code.try_into().or(Err(IOError::new( IOErrorKind::InvalidData, From a9ffa2e3704fed0d2871e6310fd65800964c6679 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Tue, 3 Mar 2026 23:00:37 +0000 Subject: [PATCH 5/5] clean(clippy): do not panic in production code paths --- libwebauthn/examples/webauthn_cable.rs | 2 +- libwebauthn/src/fido.rs | 25 +++-- libwebauthn/src/lib.rs | 8 ++ .../src/management/authenticator_config.rs | 10 +- libwebauthn/src/management/bio_enrollment.rs | 21 ++-- .../src/management/credential_management.rs | 15 +-- libwebauthn/src/ops/u2f.rs | 45 ++++++--- libwebauthn/src/ops/webauthn/get_assertion.rs | 4 +- libwebauthn/src/pin.rs | 98 ++++++++++++------- libwebauthn/src/proto/ctap1/apdu/request.rs | 2 +- libwebauthn/src/proto/ctap1/apdu/response.rs | 2 +- libwebauthn/src/proto/ctap1/protocol.rs | 4 +- libwebauthn/src/proto/ctap2/cbor/request.rs | 67 +++++++------ libwebauthn/src/proto/ctap2/model.rs | 11 ++- .../src/proto/ctap2/model/get_assertion.rs | 14 ++- .../src/proto/ctap2/model/make_credential.rs | 14 ++- libwebauthn/src/proto/ctap2/protocol.rs | 12 +-- .../src/transport/ble/btleplug/connection.rs | 2 +- .../src/transport/ble/btleplug/manager.rs | 11 ++- libwebauthn/src/transport/ble/channel.rs | 4 +- libwebauthn/src/transport/ble/framing.rs | 4 +- .../src/transport/cable/advertisement.rs | 4 +- .../src/transport/cable/connection_stages.rs | 40 ++++---- libwebauthn/src/transport/cable/crypto.rs | 19 ++-- .../src/transport/cable/known_devices.rs | 12 ++- .../src/transport/cable/qr_code_device.rs | 28 +++--- libwebauthn/src/transport/cable/tunnel.rs | 18 +++- libwebauthn/src/transport/hid/channel.rs | 35 +++++-- libwebauthn/src/transport/hid/device.rs | 13 ++- libwebauthn/src/transport/hid/framing.rs | 9 +- libwebauthn/src/transport/nfc/device.rs | 4 +- libwebauthn/src/transport/nfc/libnfc/mod.rs | 32 +++--- libwebauthn/src/transport/nfc/pcsc/mod.rs | 55 +++++------ libwebauthn/src/webauthn/error.rs | 2 + libwebauthn/src/webauthn/pin_uv_auth_token.rs | 43 ++++---- 35 files changed, 408 insertions(+), 281 deletions(-) diff --git a/libwebauthn/examples/webauthn_cable.rs b/libwebauthn/examples/webauthn_cable.rs index 93d1cb8d..1c8b0cda 100644 --- a/libwebauthn/examples/webauthn_cable.rs +++ b/libwebauthn/examples/webauthn_cable.rs @@ -106,7 +106,7 @@ pub async fn main() -> Result<(), Box> { let mut device: CableQrCodeDevice = CableQrCodeDevice::new_persistent( QrCodeOperationHint::MakeCredential, device_info_store.clone(), - ); + )?; println!("Created QR code, awaiting for advertisement."); let qr_code = QrCode::new(device.qr_code.to_string()).unwrap(); diff --git a/libwebauthn/src/fido.rs b/libwebauthn/src/fido.rs index 532a08a1..94c8c1d1 100644 --- a/libwebauthn/src/fido.rs +++ b/libwebauthn/src/fido.rs @@ -196,10 +196,16 @@ impl<'de, T: DeserializeOwned> Deserialize<'de> for AuthenticatorData { let mut cursor = Cursor::new(&data); let mut rp_id_hash = [0u8; 32]; - cursor.read_exact(&mut rp_id_hash).unwrap(); // We checked the length - let flags_raw = cursor.read_u8().unwrap(); // We checked the length + cursor + .read_exact(&mut rp_id_hash) + .map_err(|e| DesError::custom(format!("failed to read rp_id_hash: {e}")))?; + let flags_raw = cursor + .read_u8() + .map_err(|e| DesError::custom(format!("failed to read flags: {e}")))?; let flags = AuthenticatorDataFlags::from_bits_truncate(flags_raw); - let signature_count = cursor.read_u32::().unwrap(); // We checked the length + let signature_count = cursor + .read_u32::() + .map_err(|e| DesError::custom(format!("failed to read signature_count: {e}")))?; let attested_credential = if flags.contains(AuthenticatorDataFlags::ATTESTED_CREDENTIALS) { @@ -209,13 +215,20 @@ impl<'de, T: DeserializeOwned> Deserialize<'de> for AuthenticatorData { } let mut aaguid = [0u8; 16]; - cursor.read_exact(&mut aaguid).unwrap(); // We checked the length - let credential_id_len = cursor.read_u16::().unwrap() as usize; // We checked the length + cursor + .read_exact(&mut aaguid) + .map_err(|e| DesError::custom(format!("failed to read aaguid: {e}")))?; + let credential_id_len = cursor + .read_u16::() + .map_err(|e| DesError::custom(format!("failed to read credential_id_len: {e}")))? + as usize; if data.len() < 55 + credential_id_len { return Err(DesError::invalid_length(data.len(), &"55+L")); } let mut credential_id = vec![0u8; credential_id_len]; - cursor.read_exact(&mut credential_id).unwrap(); // We checked the length + cursor + .read_exact(&mut credential_id) + .map_err(|e| DesError::custom(format!("failed to read credential_id: {e}")))?; let credential_public_key: PublicKey = cbor::from_cursor(&mut cursor).map_err(DesError::custom)?; diff --git a/libwebauthn/src/lib.rs b/libwebauthn/src/lib.rs index 23ec8dd9..f66f5993 100644 --- a/libwebauthn/src/lib.rs +++ b/libwebauthn/src/lib.rs @@ -1,3 +1,11 @@ +// Deny panic-inducing patterns in production code. +// Tests are allowed to use unwrap/expect/panic for convenience. +#![cfg_attr(not(test), deny(clippy::unwrap_used))] +#![cfg_attr(not(test), deny(clippy::expect_used))] +#![cfg_attr(not(test), deny(clippy::panic))] +#![cfg_attr(not(test), deny(clippy::todo))] +#![cfg_attr(not(test), deny(clippy::unreachable))] + pub mod fido; pub mod management; pub mod ops; diff --git a/libwebauthn/src/management/authenticator_config.rs b/libwebauthn/src/management/authenticator_config.rs index 032d5712..5193da88 100644 --- a/libwebauthn/src/management/authenticator_config.rs +++ b/libwebauthn/src/management/authenticator_config.rs @@ -166,23 +166,21 @@ impl Ctap2UserVerifiableRequest for Ctap2AuthenticatorConfigRequest { &mut self, uv_proto: &dyn PinUvAuthProtocol, uv_auth_token: &[u8], - ) { + ) -> Result<(), Error> { // pinUvAuthParam (0x04): the result of calling // authenticate(pinUvAuthToken, 32×0xff || 0x0d || uint8(subCommand) || subCommandParams). let mut data = vec![0xff; 32]; data.push(0x0D); data.push(self.subcommand as u8); if self.subcommand == Ctap2AuthenticatorConfigCommand::SetMinPINLength { - data.extend(cbor::to_vec(&self.subcommand_params).unwrap()); + data.extend(cbor::to_vec(&self.subcommand_params)?); } - let uv_auth_param = uv_proto.authenticate(uv_auth_token, &data); + let uv_auth_param = uv_proto.authenticate(uv_auth_token, &data)?; self.protocol = Some(uv_proto.version()); self.uv_auth_param = Some(ByteBuf::from(uv_auth_param)); + Ok(()) } - fn client_data_hash(&self) -> &[u8] { - unreachable!() - } fn permissions(&self) -> Ctap2AuthTokenPermissionRole { Ctap2AuthTokenPermissionRole::AUTHENTICATOR_CONFIGURATION diff --git a/libwebauthn/src/management/bio_enrollment.rs b/libwebauthn/src/management/bio_enrollment.rs index ed960706..edf74b71 100644 --- a/libwebauthn/src/management/bio_enrollment.rs +++ b/libwebauthn/src/management/bio_enrollment.rs @@ -298,27 +298,22 @@ impl Ctap2UserVerifiableRequest for Ctap2BioEnrollmentRequest { &mut self, uv_proto: &dyn PinUvAuthProtocol, uv_auth_token: &[u8], - ) { + ) -> Result<(), Error> { // pinUvAuthParam (0x05): authenticate(pinUvAuthToken, fingerprint (0x01) || enumerateEnrollments (0x04)). - let mut data = match self.subcommand { - None => unreachable!(), - Some(x) => { - let data = vec![Ctap2BioEnrollmentModality::Fingerprint as u8, x as u8]; - data - } - }; + let subcommand = self + .subcommand + .ok_or(Error::Platform(PlatformError::InvalidDeviceResponse))?; + let mut data = vec![Ctap2BioEnrollmentModality::Fingerprint as u8, subcommand as u8]; // e.g. "Authenticator calls verify(pinUvAuthToken, fingerprint (0x01) || removeEnrollment (0x06) || subCommandParams, pinUvAuthParam)" if let Some(params) = &self.subcommand_params { - data.extend(cbor::to_vec(¶ms).unwrap()); + data.extend(cbor::to_vec(¶ms)?); } - let uv_auth_param = uv_proto.authenticate(uv_auth_token, &data); + let uv_auth_param = uv_proto.authenticate(uv_auth_token, &data)?; self.protocol = Some(uv_proto.version()); self.uv_auth_param = Some(ByteBuf::from(uv_auth_param)); + Ok(()) } - fn client_data_hash(&self) -> &[u8] { - unreachable!() - } fn permissions(&self) -> Ctap2AuthTokenPermissionRole { Ctap2AuthTokenPermissionRole::BIO_ENROLLMENT diff --git a/libwebauthn/src/management/credential_management.rs b/libwebauthn/src/management/credential_management.rs index 23f1533b..b803bca5 100644 --- a/libwebauthn/src/management/credential_management.rs +++ b/libwebauthn/src/management/credential_management.rs @@ -278,21 +278,22 @@ impl Ctap2UserVerifiableRequest for Ctap2CredentialManagementRequest { &mut self, uv_proto: &dyn PinUvAuthProtocol, uv_auth_token: &[u8], - ) { - let mut data = vec![self.subcommand.unwrap() as u8]; + ) -> Result<(), Error> { + let subcommand = self + .subcommand + .ok_or(Error::Platform(PlatformError::InvalidDeviceResponse))?; + let mut data = vec![subcommand as u8]; // e.g. pinUvAuthParam (0x04): authenticate(pinUvAuthToken, enumerateCredentialsBegin (0x04) || subCommandParams). if let Some(params) = &self.subcommand_params { - data.extend(cbor::to_vec(¶ms).unwrap()); + data.extend(cbor::to_vec(¶ms)?); } - let uv_auth_param = uv_proto.authenticate(uv_auth_token, &data); + let uv_auth_param = uv_proto.authenticate(uv_auth_token, &data)?; self.protocol = Some(uv_proto.version()); self.uv_auth_param = Some(ByteBuf::from(uv_auth_param)); + Ok(()) } - fn client_data_hash(&self) -> &[u8] { - unreachable!() - } fn permissions(&self) -> Ctap2AuthTokenPermissionRole { Ctap2AuthTokenPermissionRole::CREDENTIAL_MANAGEMENT diff --git a/libwebauthn/src/ops/u2f.rs b/libwebauthn/src/ops/u2f.rs index 8a677ce9..51df4c94 100644 --- a/libwebauthn/src/ops/u2f.rs +++ b/libwebauthn/src/ops/u2f.rs @@ -19,7 +19,7 @@ use crate::proto::ctap2::{ Ctap2AttestationStatement, Ctap2GetAssertionResponse, Ctap2MakeCredentialResponse, Ctap2PublicKeyCredentialDescriptor, Ctap2PublicKeyCredentialType, FidoU2fAttestationStmt, }; -use crate::webauthn::{CtapError, Error}; +use crate::webauthn::{CtapError, Error, PlatformError}; // FIDO U2F operations can be aliased to CTAP1 requests, as they have no other representation. pub type RegisterRequest = Ctap1RegisterRequest; @@ -61,20 +61,30 @@ impl UpgradableResponse for Regis error!(?self.public_key, "Failed to parse public key as SEC-1 v2 encoded point"); return Err(Error::Ctap(CtapError::Other)); }; - let x: heapless::Vec = heapless::Vec::from_slice( - encoded_point - .x() - .expect("Not the identity point") - .as_bytes(), - ) - .unwrap(); - let y: heapless::Vec = heapless::Vec::from_slice( - encoded_point - .y() - .expect("Not identity nor compressed") - .as_bytes(), - ) - .unwrap(); + let x_bytes = encoded_point.x().ok_or_else(|| { + error!("Public key is the identity point"); + Error::Platform(PlatformError::CryptoError( + "public key is the identity point".into(), + )) + })?; + let y_bytes = encoded_point.y().ok_or_else(|| { + error!("Public key is identity or compressed"); + Error::Platform(PlatformError::CryptoError( + "public key is identity or compressed".into(), + )) + })?; + let x: heapless::Vec = + heapless::Vec::from_slice(x_bytes.as_bytes()).map_err(|_| { + Error::Platform(PlatformError::CryptoError( + "x coordinate exceeds 32 bytes".into(), + )) + })?; + let y: heapless::Vec = + heapless::Vec::from_slice(y_bytes.as_bytes()).map_err(|_| { + Error::Platform(PlatformError::CryptoError( + "y coordinate exceeds 32 bytes".into(), + )) + })?; let cose_public_key = cose::PublicKey::P256Key(cose::P256PublicKey { x: x.into(), y: y.into(), @@ -173,7 +183,10 @@ impl UpgradableResponse for SignResponse { // 1 Flags Initialized with flags' value. // 4 Signature counter (signCount) Initialized with signCount bytes. let authenticator_data = AuthenticatorData { - rp_id_hash: request.app_id_hash.clone().try_into().unwrap(), + rp_id_hash: request.app_id_hash.clone().try_into().map_err(|_| { + error!("app_id_hash has invalid length, expected 32 bytes"); + Error::Platform(PlatformError::InvalidDeviceResponse) + })?, flags, signature_count, attested_credential: None, diff --git a/libwebauthn/src/ops/webauthn/get_assertion.rs b/libwebauthn/src/ops/webauthn/get_assertion.rs index 1f093ccd..b7ca054b 100644 --- a/libwebauthn/src/ops/webauthn/get_assertion.rs +++ b/libwebauthn/src/ops/webauthn/get_assertion.rs @@ -356,7 +356,9 @@ impl Ctap2HMACGetSecretOutput { } else if output.len() == 64 { let (o1, o2) = output.split_at(32); res.output1.copy_from_slice(o1); - res.output2 = Some(o2.try_into().unwrap()); + let mut output2 = [0u8; 32]; + output2.copy_from_slice(o2); + res.output2 = Some(output2); } else { error!("Failed to split HMAC Secret outputs. Unexpected output length: {}. Skipping HMAC extension", output.len()); return None; diff --git a/libwebauthn/src/pin.rs b/libwebauthn/src/pin.rs index 6fa71f80..fb38ad0a 100644 --- a/libwebauthn/src/pin.rs +++ b/libwebauthn/src/pin.rs @@ -71,13 +71,13 @@ pub trait PinUvAuthProtocol: Send + Sync { // authenticate(key, message) → signature // Computes a MAC of the given message. - fn authenticate(&self, key: &[u8], message: &[u8]) -> Vec; + fn authenticate(&self, key: &[u8], message: &[u8]) -> Result, Error>; } trait ECPrivateKeyPinUvAuthProtocol { fn private_key(&self) -> &EphemeralSecret; fn public_key(&self) -> &P256PublicKey; - fn kdf(&self, bytes: &[u8]) -> Vec; + fn kdf(&self, bytes: &[u8]) -> Result, Error>; } /// Common functionality between ECDH-based PIN/UV auth protocols (1 & 2) @@ -87,7 +87,7 @@ trait ECDHPinUvAuthProtocol { &self, peer_public_key: &cose::PublicKey, ) -> Result<(cose::PublicKey, Vec), Error>; - fn get_public_key(&self) -> cose::PublicKey; + fn get_public_key(&self) -> Result; } pub struct PinUvAuthProtocolOne { @@ -129,10 +129,10 @@ impl ECPrivateKeyPinUvAuthProtocol for PinUvAuthProtocolOne { } /// kdf(Z) → sharedSecret - fn kdf(&self, bytes: &[u8]) -> Vec { + fn kdf(&self, bytes: &[u8]) -> Result, Error> { let mut hasher = Sha256::default(); hasher.update(bytes); - hasher.finalize().to_vec() + Ok(hasher.finalize().to_vec()) } } @@ -149,7 +149,7 @@ where let shared_secret = self.ecdh(peer_public_key)?; // Return(getPublicKey(), sharedSecret) - Ok((self.get_public_key(), shared_secret)) + Ok((self.get_public_key()?, shared_secret)) } /// ecdh(peerCoseKey) → sharedSecret | error @@ -178,22 +178,42 @@ where let shared = self.private_key().diffie_hellman(&peer_public_key); // Return kdf(Z). - Ok(self.kdf(shared.raw_secret_bytes().as_bytes())) + self.kdf(shared.raw_secret_bytes().as_bytes()) } /// getPublicKey() - fn get_public_key(&self) -> cose::PublicKey { + fn get_public_key(&self) -> Result { let point = EncodedPoint::from(self.public_key()); + let x_bytes = point.x().ok_or_else(|| { + error!("Public key is the identity point"); + Error::Platform(PlatformError::CryptoError( + "public key is the identity point".into(), + )) + })?; + let y_bytes = point.y().ok_or_else(|| { + error!("Public key is identity or compressed"); + Error::Platform(PlatformError::CryptoError( + "public key is identity or compressed".into(), + )) + })?; let x: heapless::Vec = - heapless::Vec::from_slice(point.x().expect("Not the identity point").as_bytes()) - .unwrap(); + heapless::Vec::from_slice(x_bytes.as_bytes()).map_err(|_| { + Error::Platform(PlatformError::CryptoError( + "x coordinate exceeds 32 bytes".into(), + )) + })?; let y: heapless::Vec = - heapless::Vec::from_slice(point.y().expect("Not identity nor compressed").as_bytes()) - .unwrap(); - cose::PublicKey::EcdhEsHkdf256Key(cose::EcdhEsHkdf256PublicKey { - x: x.into(), - y: y.into(), - }) + heapless::Vec::from_slice(y_bytes.as_bytes()).map_err(|_| { + Error::Platform(PlatformError::CryptoError( + "y coordinate exceeds 32 bytes".into(), + )) + })?; + Ok(cose::PublicKey::EcdhEsHkdf256Key( + cose::EcdhEsHkdf256PublicKey { + x: x.into(), + y: y.into(), + }, + )) } } @@ -215,10 +235,10 @@ impl PinUvAuthProtocol for PinUvAuthProtocolOne { } #[instrument(skip_all)] - fn authenticate(&self, key: &[u8], message: &[u8]) -> Vec { + fn authenticate(&self, key: &[u8], message: &[u8]) -> Result, Error> { // Return the first 16 bytes of the result of computing HMAC-SHA-256 with the given key and message. - let hmac = hmac_sha256(key, message); - Vec::from(&hmac[..16]) + let hmac = hmac_sha256(key, message)?; + Ok(Vec::from(&hmac[..16])) } #[instrument(skip_all)] @@ -292,14 +312,14 @@ impl ECPrivateKeyPinUvAuthProtocol for PinUvAuthProtocolTwo { } /// kdf(Z) → sharedSecret - fn kdf(&self, ikm: &[u8]) -> Vec { + fn kdf(&self, ikm: &[u8]) -> Result, Error> { // Returns: // HKDF-SHA-256(salt = 32 zero bytes, IKM = Z, L = 32, info = "CTAP2 HMAC key") || // HKDF-SHA-256(salt = 32 zero bytes, IKM = Z, L = 32, info = "CTAP2 AES key") let salt: &[u8] = &[0u8; 32]; - let mut output = hkdf_sha256(Some(salt), ikm, "CTAP2 HMAC key".as_bytes()); - output.extend(hkdf_sha256(Some(salt), ikm, "CTAP2 AES key".as_bytes())); - output + let mut output = hkdf_sha256(Some(salt), ikm, "CTAP2 HMAC key".as_bytes())?; + output.extend(hkdf_sha256(Some(salt), ikm, "CTAP2 AES key".as_bytes())?); + Ok(output) } } @@ -362,7 +382,7 @@ impl PinUvAuthProtocol for PinUvAuthProtocolTwo { Ok(plaintext) } - fn authenticate(&self, key: &[u8], message: &[u8]) -> Vec { + fn authenticate(&self, key: &[u8], message: &[u8]) -> Result, Error> { // If key is longer than 32 bytes, discard the excess. (This selects the HMAC-key portion of the shared secret. // When key is the pinUvAuthToken, it is exactly 32 bytes long and thus this step has no effect.) let key = &key[..32]; @@ -380,18 +400,23 @@ pub fn pin_hash(pin: &[u8]) -> Vec { Vec::from(&hashed[..16]) } -pub fn hmac_sha256(key: &[u8], message: &[u8]) -> Vec { - let mut hmac = HmacSha256::new_from_slice(key).expect("Any key size is valid"); +pub fn hmac_sha256(key: &[u8], message: &[u8]) -> Result, Error> { + let mut hmac = HmacSha256::new_from_slice(key).map_err(|e| { + error!("HMAC key error: {e}"); + Error::Platform(PlatformError::CryptoError(format!("HMAC key error: {e}"))) + })?; hmac.update(message); - hmac.finalize().into_bytes().to_vec() + Ok(hmac.finalize().into_bytes().to_vec()) } -pub fn hkdf_sha256(salt: Option<&[u8]>, ikm: &[u8], info: &[u8]) -> Vec { +pub fn hkdf_sha256(salt: Option<&[u8]>, ikm: &[u8], info: &[u8]) -> Result, Error> { let hk = Hkdf::::new(salt, ikm); let mut okm = [0u8; 32]; // fixed L = 32 - hk.expand(info, &mut okm) - .expect("32 is a valid length for Sha256 to output"); - Vec::from(okm) + hk.expand(info, &mut okm).map_err(|e| { + error!("HKDF expand error: {e}"); + Error::Platform(PlatformError::CryptoError(format!("HKDF expand error: {e}"))) + })?; + Ok(Vec::from(okm)) } #[async_trait] @@ -429,7 +454,12 @@ where return Err(Error::Ctap(CtapError::Other)); }; - let current_pin = match get_info_response.options.as_ref().unwrap().get("clientPin") { + let current_pin = match get_info_response + .options + .as_ref() + .ok_or(Error::Platform(PlatformError::InvalidDeviceResponse))? + .get("clientPin") + { // Obtaining the current PIN, if one is set Some(true) => Some( obtain_pin( @@ -472,7 +502,7 @@ where let uv_auth_param = uv_proto.authenticate( &shared_secret, &[new_pin_enc.as_slice(), pin_hash_enc.as_slice()].concat(), - ); + )?; Ctap2ClientPinRequest::new_change_pin( uv_proto.version(), @@ -484,7 +514,7 @@ where } None => { // pinUvAuthParam: the result of calling authenticate(shared secret, newPinEnc). - let uv_auth_param = uv_proto.authenticate(&shared_secret, &new_pin_enc); + let uv_auth_param = uv_proto.authenticate(&shared_secret, &new_pin_enc)?; Ctap2ClientPinRequest::new_set_pin( uv_proto.version(), diff --git a/libwebauthn/src/proto/ctap1/apdu/request.rs b/libwebauthn/src/proto/ctap1/apdu/request.rs index 794fc7d8..9aae6a38 100644 --- a/libwebauthn/src/proto/ctap1/apdu/request.rs +++ b/libwebauthn/src/proto/ctap1/apdu/request.rs @@ -139,7 +139,7 @@ impl From<&Ctap1SignRequest> for ApduRequest { }; let mut data = request.challenge.clone(); data.extend(&request.app_id_hash); - data.write_u8(request.key_handle.len() as u8).unwrap(); + data.push(request.key_handle.len() as u8); data.extend(&request.key_handle); Self::new(U2F_AUTHENTICATE, p1, 0x00, Some(&data), Some(APDU_SHORT_LE)) } diff --git a/libwebauthn/src/proto/ctap1/apdu/response.rs b/libwebauthn/src/proto/ctap1/apdu/response.rs index 071b4aee..95a9289d 100644 --- a/libwebauthn/src/proto/ctap1/apdu/response.rs +++ b/libwebauthn/src/proto/ctap1/apdu/response.rs @@ -33,7 +33,7 @@ impl ApduResponse { pub fn status(&self) -> Result { let mut cursor = IOCursor::new(vec![self.sw1, self.sw2]); - let code = cursor.read_u16::().unwrap() as u16; + let code = cursor.read_u16::()?; code.try_into().or(Err(IOError::new( IOErrorKind::InvalidData, diff --git a/libwebauthn/src/proto/ctap1/protocol.rs b/libwebauthn/src/proto/ctap1/protocol.rs index 6dcbb302..f4c913b0 100644 --- a/libwebauthn/src/proto/ctap1/protocol.rs +++ b/libwebauthn/src/proto/ctap1/protocol.rs @@ -81,7 +81,7 @@ where return Err(Error::Ctap(CtapError::from(status))); } - let response: Ctap1RegisterResponse = apdu_response.try_into().unwrap(); + let response: Ctap1RegisterResponse = apdu_response.try_into().or(Err(CtapError::Other))?; debug!("CTAP1 register response"); trace!(?response); Ok(response) @@ -101,7 +101,7 @@ where return Err(Error::Ctap(CtapError::from(status))); } - let response: Ctap1SignResponse = apdu_response.try_into().unwrap(); + let response: Ctap1SignResponse = apdu_response.try_into().or(Err(CtapError::Other))?; debug!({ ?response.user_presence_verified }, "CTAP1 sign response received"); trace!(?response); Ok(response) diff --git a/libwebauthn/src/proto/ctap2/cbor/request.rs b/libwebauthn/src/proto/ctap2/cbor/request.rs index 86dc4343..03828a62 100644 --- a/libwebauthn/src/proto/ctap2/cbor/request.rs +++ b/libwebauthn/src/proto/ctap2/cbor/request.rs @@ -8,6 +8,7 @@ use crate::proto::ctap2::model::Ctap2MakeCredentialRequest; use crate::proto::ctap2::Ctap2AuthenticatorConfigRequest; use crate::proto::ctap2::Ctap2BioEnrollmentRequest; use crate::proto::ctap2::Ctap2CredentialManagementRequest; +use crate::webauthn::Error; #[derive(Debug, Clone, PartialEq)] pub struct CborRequest { @@ -36,66 +37,72 @@ impl CborRequest { } } -impl From<&Ctap2MakeCredentialRequest> for CborRequest { - fn from(request: &Ctap2MakeCredentialRequest) -> CborRequest { - CborRequest { +impl TryFrom<&Ctap2MakeCredentialRequest> for CborRequest { + type Error = Error; + fn try_from(request: &Ctap2MakeCredentialRequest) -> Result { + Ok(CborRequest { command: Ctap2CommandCode::AuthenticatorMakeCredential, - encoded_data: cbor::to_vec(&request).unwrap(), - } + encoded_data: cbor::to_vec(&request)?, + }) } } -impl From<&Ctap2GetAssertionRequest> for CborRequest { - fn from(request: &Ctap2GetAssertionRequest) -> CborRequest { - CborRequest { +impl TryFrom<&Ctap2GetAssertionRequest> for CborRequest { + type Error = Error; + fn try_from(request: &Ctap2GetAssertionRequest) -> Result { + Ok(CborRequest { command: Ctap2CommandCode::AuthenticatorGetAssertion, - encoded_data: cbor::to_vec(&request).unwrap(), - } + encoded_data: cbor::to_vec(&request)?, + }) } } -impl From<&Ctap2ClientPinRequest> for CborRequest { - fn from(request: &Ctap2ClientPinRequest) -> CborRequest { - CborRequest { +impl TryFrom<&Ctap2ClientPinRequest> for CborRequest { + type Error = Error; + fn try_from(request: &Ctap2ClientPinRequest) -> Result { + Ok(CborRequest { command: Ctap2CommandCode::AuthenticatorClientPin, - encoded_data: cbor::to_vec(&request).unwrap(), - } + encoded_data: cbor::to_vec(&request)?, + }) } } -impl From<&Ctap2AuthenticatorConfigRequest> for CborRequest { - fn from(request: &Ctap2AuthenticatorConfigRequest) -> CborRequest { - CborRequest { +impl TryFrom<&Ctap2AuthenticatorConfigRequest> for CborRequest { + type Error = Error; + fn try_from(request: &Ctap2AuthenticatorConfigRequest) -> Result { + Ok(CborRequest { command: Ctap2CommandCode::AuthenticatorConfig, - encoded_data: cbor::to_vec(&request).unwrap(), - } + encoded_data: cbor::to_vec(&request)?, + }) } } -impl From<&Ctap2BioEnrollmentRequest> for CborRequest { - fn from(request: &Ctap2BioEnrollmentRequest) -> CborRequest { +impl TryFrom<&Ctap2BioEnrollmentRequest> for CborRequest { + type Error = Error; + fn try_from(request: &Ctap2BioEnrollmentRequest) -> Result { let command = if request.use_legacy_preview { Ctap2CommandCode::AuthenticatorBioEnrollmentPreview } else { Ctap2CommandCode::AuthenticatorBioEnrollment }; - CborRequest { + Ok(CborRequest { command, - encoded_data: cbor::to_vec(&request).unwrap(), - } + encoded_data: cbor::to_vec(&request)?, + }) } } -impl From<&Ctap2CredentialManagementRequest> for CborRequest { - fn from(request: &Ctap2CredentialManagementRequest) -> CborRequest { +impl TryFrom<&Ctap2CredentialManagementRequest> for CborRequest { + type Error = Error; + fn try_from(request: &Ctap2CredentialManagementRequest) -> Result { let command = if request.use_legacy_preview { Ctap2CommandCode::AuthenticatorCredentialManagementPreview } else { Ctap2CommandCode::AuthenticatorCredentialManagement }; - CborRequest { + Ok(CborRequest { command, - encoded_data: cbor::to_vec(&request).unwrap(), - } + encoded_data: cbor::to_vec(&request)?, + }) } } diff --git a/libwebauthn/src/proto/ctap2/model.rs b/libwebauthn/src/proto/ctap2/model.rs index a0ecc7d5..63c49d6e 100644 --- a/libwebauthn/src/proto/ctap2/model.rs +++ b/libwebauthn/src/proto/ctap2/model.rs @@ -1,5 +1,8 @@ use crate::proto::ctap1::Ctap1Transport; -use crate::{ops::webauthn::idl::create::PublicKeyCredentialUserEntity, pin::PinUvAuthProtocol}; +use crate::{ + ops::webauthn::idl::create::PublicKeyCredentialUserEntity, pin::PinUvAuthProtocol, + webauthn::Error, +}; use num_enum::{IntoPrimitive, TryFromPrimitive}; use serde_bytes::ByteBuf; @@ -216,8 +219,10 @@ pub trait Ctap2UserVerifiableRequest { &mut self, uv_proto: &dyn PinUvAuthProtocol, uv_auth_token: &[u8], - ); - fn client_data_hash(&self) -> &[u8]; + ) -> Result<(), Error>; + fn client_data_hash(&self) -> Option<&[u8]> { + None + } fn permissions(&self) -> Ctap2AuthTokenPermissionRole; fn permissions_rpid(&self) -> Option<&str>; fn can_use_uv(&self, info: &Ctap2GetInfoResponse) -> bool; diff --git a/libwebauthn/src/proto/ctap2/model/get_assertion.rs b/libwebauthn/src/proto/ctap2/model/get_assertion.rs index d829480a..d70f9a62 100644 --- a/libwebauthn/src/proto/ctap2/model/get_assertion.rs +++ b/libwebauthn/src/proto/ctap2/model/get_assertion.rs @@ -273,7 +273,7 @@ impl Ctap2GetAssertionRequestExtensions { return Ok(()); }; - let salt_auth = ByteBuf::from(uv_proto.authenticate(&auth_data.shared_secret, &salt_enc)); + let salt_auth = ByteBuf::from(uv_proto.authenticate(&auth_data.shared_secret, &salt_enc)?); self.hmac_secret = Some(CalculatedHMACGetSecretInput { public_key, @@ -421,14 +421,18 @@ impl Ctap2UserVerifiableRequest for Ctap2GetAssertionRequest { &mut self, uv_proto: &dyn PinUvAuthProtocol, uv_auth_token: &[u8], - ) { - let uv_auth_param = uv_proto.authenticate(uv_auth_token, self.client_data_hash()); + ) -> Result<(), Error> { + let hash = self + .client_data_hash() + .ok_or(Error::Platform(PlatformError::InvalidDeviceResponse))?; + let uv_auth_param = uv_proto.authenticate(uv_auth_token, hash)?; self.pin_auth_proto = Some(uv_proto.version() as u32); self.pin_auth_param = Some(ByteBuf::from(uv_auth_param)); + Ok(()) } - fn client_data_hash(&self) -> &[u8] { - self.client_data_hash.as_slice() + fn client_data_hash(&self) -> Option<&[u8]> { + Some(self.client_data_hash.as_slice()) } fn permissions(&self) -> Ctap2AuthTokenPermissionRole { diff --git a/libwebauthn/src/proto/ctap2/model/make_credential.rs b/libwebauthn/src/proto/ctap2/model/make_credential.rs index 59127acd..db798d83 100644 --- a/libwebauthn/src/proto/ctap2/model/make_credential.rs +++ b/libwebauthn/src/proto/ctap2/model/make_credential.rs @@ -13,7 +13,7 @@ use crate::{ }, pin::PinUvAuthProtocol, proto::CtapError, - webauthn::Error, + webauthn::{Error, PlatformError}, }; use ctap_types::ctap2::credential_management::CredentialProtectionPolicy as Ctap2CredentialProtectionPolicy; use serde::{Deserialize, Serialize}; @@ -331,14 +331,18 @@ impl Ctap2UserVerifiableRequest for Ctap2MakeCredentialRequest { &mut self, uv_proto: &dyn PinUvAuthProtocol, uv_auth_token: &[u8], - ) { - let uv_auth_param = uv_proto.authenticate(uv_auth_token, self.client_data_hash()); + ) -> Result<(), Error> { + let hash = self + .client_data_hash() + .ok_or(Error::Platform(PlatformError::InvalidDeviceResponse))?; + let uv_auth_param = uv_proto.authenticate(uv_auth_token, hash)?; self.pin_auth_proto = Some(uv_proto.version() as u32); self.pin_auth_param = Some(ByteBuf::from(uv_auth_param)); + Ok(()) } - fn client_data_hash(&self) -> &[u8] { - self.hash.as_slice() + fn client_data_hash(&self) -> Option<&[u8]> { + Some(self.hash.as_slice()) } fn permissions(&self) -> Ctap2AuthTokenPermissionRole { diff --git a/libwebauthn/src/proto/ctap2/protocol.rs b/libwebauthn/src/proto/ctap2/protocol.rs index f6dd5eeb..3376adb2 100644 --- a/libwebauthn/src/proto/ctap2/protocol.rs +++ b/libwebauthn/src/proto/ctap2/protocol.rs @@ -103,7 +103,7 @@ where timeout: Duration, ) -> Result { trace!(?request); - self.cbor_send(&request.into(), timeout).await?; + self.cbor_send(&request.try_into()?, timeout).await?; let cbor_response = self.cbor_recv(timeout).await?; match cbor_response.status_code { CtapError::Ok => (), @@ -124,7 +124,7 @@ where timeout: Duration, ) -> Result { trace!(?request); - self.cbor_send(&request.into(), timeout).await?; + self.cbor_send(&request.try_into()?, timeout).await?; let cbor_response = self.cbor_recv(timeout).await?; match cbor_response.status_code { CtapError::Ok => (), @@ -179,7 +179,7 @@ where timeout: Duration, ) -> Result { trace!(?request); - self.cbor_send(&request.into(), timeout).await?; + self.cbor_send(&request.try_into()?, timeout).await?; let cbor_response = self.cbor_recv(timeout).await?; match cbor_response.status_code { CtapError::Ok => (), @@ -205,7 +205,7 @@ where timeout: Duration, ) -> Result<(), Error> { trace!(?request); - self.cbor_send(&request.into(), timeout).await?; + self.cbor_send(&request.try_into()?, timeout).await?; let cbor_response = self.cbor_recv(timeout).await?; match cbor_response.status_code { CtapError::Ok => { @@ -228,7 +228,7 @@ where timeout: Duration, ) -> Result { trace!(?request); - self.cbor_send(&request.into(), timeout).await?; + self.cbor_send(&request.try_into()?, timeout).await?; let cbor_response = self.cbor_recv(timeout).await?; match cbor_response.status_code { CtapError::Ok => (), @@ -254,7 +254,7 @@ where timeout: Duration, ) -> Result { trace!(?request); - self.cbor_send(&request.into(), timeout).await?; + self.cbor_send(&request.try_into()?, timeout).await?; let cbor_response = self.cbor_recv(timeout).await?; match cbor_response.status_code { CtapError::Ok => (), diff --git a/libwebauthn/src/transport/ble/btleplug/connection.rs b/libwebauthn/src/transport/ble/btleplug/connection.rs index 9b9fdeac..8941689a 100644 --- a/libwebauthn/src/transport/ble/btleplug/connection.rs +++ b/libwebauthn/src/transport/ble/btleplug/connection.rs @@ -104,7 +104,7 @@ impl Connection { let status = parser.update(&fragment).or(Err(Error::InvalidFraming))?; match status { BleFrameParserResult::Done => { - let frame = parser.frame().unwrap(); + let frame = parser.frame().or(Err(Error::InvalidFraming))?; trace!(?frame, "Received frame"); match frame.cmd { BleCommand::Keepalive => { diff --git a/libwebauthn/src/transport/ble/btleplug/manager.rs b/libwebauthn/src/transport/ble/btleplug/manager.rs index d51a7b99..0bfc2707 100644 --- a/libwebauthn/src/transport/ble/btleplug/manager.rs +++ b/libwebauthn/src/transport/ble/btleplug/manager.rs @@ -225,16 +225,19 @@ pub async fn connect( } async fn discover_services(peripheral: &Peripheral) -> Result { - let control_point_uuid = Uuid::parse_str(FIDO_CONTROL_POINT_UUID).unwrap(); + let control_point_uuid = + Uuid::parse_str(FIDO_CONTROL_POINT_UUID).or(Err(Error::OperationFailed))?; let control_point = get_gatt_characteristic(peripheral, control_point_uuid)?; - let control_point_length_uuid = Uuid::parse_str(FIDO_CONTROL_POINT_LENGTH_UUID).unwrap(); + let control_point_length_uuid = + Uuid::parse_str(FIDO_CONTROL_POINT_LENGTH_UUID).or(Err(Error::OperationFailed))?; let control_point_length = get_gatt_characteristic(peripheral, control_point_length_uuid)?; - let status_uuid = Uuid::parse_str(FIDO_STATUS_UUID).unwrap(); + let status_uuid = Uuid::parse_str(FIDO_STATUS_UUID).or(Err(Error::OperationFailed))?; let status = get_gatt_characteristic(peripheral, status_uuid)?; - let service_revision_bitfield_uuid = Uuid::parse_str(FIDO_REVISION_BITFIELD_UUID).unwrap(); + let service_revision_bitfield_uuid = + Uuid::parse_str(FIDO_REVISION_BITFIELD_UUID).or(Err(Error::OperationFailed))?; let service_revision_bitfield = get_gatt_characteristic(peripheral, service_revision_bitfield_uuid)?; diff --git a/libwebauthn/src/transport/ble/channel.rs b/libwebauthn/src/transport/ble/channel.rs index c052c7c4..73d24c35 100644 --- a/libwebauthn/src/transport/ble/channel.rs +++ b/libwebauthn/src/transport/ble/channel.rs @@ -81,8 +81,8 @@ impl<'a> Channel for BleChannel<'a> { } async fn close(&mut self) { - let _x = self.device; - todo!() + // BLE disconnection is handled when the Connection is dropped + trace!("BLE channel close() called — no-op, cleanup on drop"); } #[instrument(level = Level::DEBUG, skip_all)] diff --git a/libwebauthn/src/transport/ble/framing.rs b/libwebauthn/src/transport/ble/framing.rs index 6e404739..a8b03267 100644 --- a/libwebauthn/src/transport/ble/framing.rs +++ b/libwebauthn/src/transport/ble/framing.rs @@ -150,7 +150,7 @@ impl BleFrameParser { return true; } - self.expected_bytes().unwrap() > self.data_len() + self.expected_bytes().unwrap_or(0) > self.data_len() } fn expected_bytes(&self) -> Option { @@ -159,7 +159,7 @@ impl BleFrameParser { } let mut cursor = IOCursor::new(vec![self.fragments[0][1], self.fragments[0][2]]); - Some(cursor.read_u16::().unwrap() as usize) + Some(cursor.read_u16::().ok()? as usize) } fn data_len(&self) -> usize { diff --git a/libwebauthn/src/transport/cable/advertisement.rs b/libwebauthn/src/transport/cable/advertisement.rs index 7d8bfae6..e0ab7fcd 100644 --- a/libwebauthn/src/transport/cable/advertisement.rs +++ b/libwebauthn/src/transport/cable/advertisement.rs @@ -42,8 +42,8 @@ pub(crate) async fn await_advertisement( eid_key: &[u8], ) -> Result<(FidoDevice, DecryptedAdvert), TransportError> { let uuids = &[ - Uuid::parse_str(CABLE_UUID_FIDO).unwrap(), - Uuid::parse_str(CABLE_UUID_GOOGLE).unwrap(), // Deprecated, but may still be in use. + Uuid::parse_str(CABLE_UUID_FIDO).or(Err(TransportError::InvalidEndpoint))?, + Uuid::parse_str(CABLE_UUID_GOOGLE).or(Err(TransportError::InvalidEndpoint))?, // Deprecated, but may still be in use. ]; let stream = btleplug::manager::start_discovery_for_service_data(uuids) .await diff --git a/libwebauthn/src/transport/cable/connection_stages.rs b/libwebauthn/src/transport/cable/connection_stages.rs index 8e95e8d8..0a02aada 100644 --- a/libwebauthn/src/transport/cable/connection_stages.rs +++ b/libwebauthn/src/transport/cable/connection_stages.rs @@ -12,6 +12,7 @@ use super::tunnel::{self, CableTunnelConnectionType, TunnelNoiseState}; use crate::proto::ctap2::cbor::{CborRequest, CborResponse}; use crate::transport::ble::btleplug::FidoDevice; use crate::transport::error::TransportError; +use crate::webauthn::error::Error; use std::sync::Arc; #[derive(Debug)] @@ -20,25 +21,25 @@ pub(crate) struct ProximityCheckInput { } impl ProximityCheckInput { - pub fn new_for_qr_code(qr_device: &CableQrCodeDevice) -> Self { + pub fn new_for_qr_code(qr_device: &CableQrCodeDevice) -> Result { let eid_key: [u8; 64] = derive( qr_device.qr_code.qr_secret.as_ref(), None, KeyPurpose::EIDKey, - ); - Self { eid_key } + )?; + Ok(Self { eid_key }) } pub fn new_for_known_device( known_device: &CableKnownDevice, client_nonce: &ClientNonce, - ) -> Self { + ) -> Result { let eid_key: [u8; 64] = derive( &known_device.device_info.link_secret, Some(client_nonce), KeyPurpose::EIDKey, - ); - Self { eid_key } + )?; + Ok(Self { eid_key }) } } @@ -63,11 +64,12 @@ impl ConnectionInput { let tunnel_domain = decode_tunnel_domain_from_advert(&proximity_output.advert)?; let routing_id_str = hex::encode(proximity_output.advert.routing_id); - let tunnel_id = &derive( + let tunnel_id_full = derive( qr_device.qr_code.qr_secret.as_ref(), None, KeyPurpose::TunnelID, - )[..16]; + ).map_err(|_| TransportError::InvalidKey)?; + let tunnel_id = &tunnel_id_full[..16]; let tunnel_id_str = hex::encode(tunnel_id); let connection_type = CableTunnelConnectionType::QrCode { @@ -126,31 +128,31 @@ impl HandshakeInput { qr_device: &CableQrCodeDevice, connection_output: ConnectionOutput, proximity_output: ProximityCheckOutput, - ) -> Self { + ) -> Result { let advert_plaintext = &proximity_output.advert.plaintext; - let psk = derive_psk(qr_device.qr_code.qr_secret.as_ref(), advert_plaintext); - Self { + let psk = derive_psk(qr_device.qr_code.qr_secret.as_ref(), advert_plaintext)?; + Ok(Self { ws_stream: connection_output.ws_stream, psk, connection_type: connection_output.connection_type, tunnel_domain: connection_output.tunnel_domain, - } + }) } pub fn new_for_known_device( known_device: &CableKnownDevice, connection_output: ConnectionOutput, proximity_output: ProximityCheckOutput, - ) -> Self { + ) -> Result { let link_secret = known_device.device_info.link_secret; let advert_plaintext = proximity_output.advert.plaintext; - let psk = derive_psk(&link_secret, &advert_plaintext); - Self { + let psk = derive_psk(&link_secret, &advert_plaintext)?; + Ok(Self { ws_stream: connection_output.ws_stream, psk, connection_type: connection_output.connection_type, tunnel_domain: connection_output.tunnel_domain, - } + }) } } @@ -308,10 +310,10 @@ pub(crate) async fn handshake_stage( }) } -fn derive_psk(secret: &[u8], advert_plaintext: &[u8]) -> [u8; 32] { +fn derive_psk(secret: &[u8], advert_plaintext: &[u8]) -> Result<[u8; 32], Error> { let mut psk: [u8; 32] = [0u8; 32]; - psk.copy_from_slice(&derive(secret, Some(advert_plaintext), KeyPurpose::Psk)[..32]); - psk + psk.copy_from_slice(&derive(secret, Some(advert_plaintext), KeyPurpose::Psk)?[..32]); + Ok(psk) } pub(crate) fn decode_tunnel_domain_from_advert( diff --git a/libwebauthn/src/transport/cable/crypto.rs b/libwebauthn/src/transport/cable/crypto.rs index 9bfcf730..98f30b26 100644 --- a/libwebauthn/src/transport/cable/crypto.rs +++ b/libwebauthn/src/transport/cable/crypto.rs @@ -1,10 +1,12 @@ -use aes::cipher::{BlockDecrypt, KeyInit}; +use aes::cipher::{generic_array::GenericArray, BlockDecrypt, KeyInit}; use aes::{Aes256, Block}; use hkdf::Hkdf; use sha2::Sha256; use tracing::{instrument, warn}; use crate::pin::hmac_sha256; +use crate::transport::error::TransportError; +use crate::webauthn::error::Error; pub enum KeyPurpose { EIDKey = 1, @@ -12,14 +14,15 @@ pub enum KeyPurpose { Psk = 3, } -pub fn derive(secret: &[u8], salt: Option<&[u8]>, purpose: KeyPurpose) -> [u8; 64] { +pub fn derive(secret: &[u8], salt: Option<&[u8]>, purpose: KeyPurpose) -> Result<[u8; 64], Error> { let mut purpose32 = [0u8; 4]; purpose32[0] = purpose as u8; let hkdf = Hkdf::::new(salt, secret); let mut output = [0u8; 64]; - hkdf.expand(&purpose32, &mut output).unwrap(); - output + hkdf.expand(&purpose32, &mut output) + .map_err(|_| Error::Transport(TransportError::InvalidKey))?; + Ok(output) } fn reserved_bits_are_zero(plaintext: &[u8]) -> bool { @@ -38,14 +41,14 @@ pub fn trial_decrypt_advert(eid_key: &[u8], candidate_advert: &[u8]) -> Option<[ return None; } - let expected_tag = hmac_sha256(&eid_key[32..], &candidate_advert[..16]); + let expected_tag = hmac_sha256(&eid_key[32..], &candidate_advert[..16]).ok()?; if expected_tag[..4] != candidate_advert[16..] { warn!({ expected = ?expected_tag[..4], actual = ?candidate_advert[16..] }, "candidate advert HMAC tag does not match"); return None; } - let cipher = Aes256::new_from_slice(&eid_key[..32]).unwrap(); + let cipher = Aes256::new(GenericArray::from_slice(&eid_key[..32])); let mut block = Block::clone_from_slice(&candidate_advert[..16]); cipher.decrypt_block(&mut block); @@ -70,7 +73,7 @@ mod tests { .unwrap() .try_into() .unwrap(); - let output = derive(&input, None, KeyPurpose::EIDKey).to_vec(); + let output = derive(&input, None, KeyPurpose::EIDKey).unwrap().to_vec(); let expected = hex::decode("efafab5b2c84a11c80e3ad0770353138b414a859ccd3afcc99e3d3250dba65084ede8e38e75432617c0ccae1ffe5d8143df0db0cd6d296f489419cd6411ee505").unwrap(); assert_eq!(output, expected); } @@ -82,7 +85,7 @@ mod tests { .try_into() .unwrap(); let salt = hex::decode("ffeeddccbbaa998877665544332211").unwrap(); - let output = derive(&input, Some(&salt), KeyPurpose::EIDKey).to_vec(); + let output = derive(&input, Some(&salt), KeyPurpose::EIDKey).unwrap().to_vec(); let expected = hex::decode("168cf3dd220a7907f8bac30f559be92a3b6d937fe5594beeaf1e50e35976b7d654dd550e22ae4c801b9d1cdbf0d2b1472daa1328661eb889acae3023b7ffa509").unwrap(); assert_eq!(output, expected); } diff --git a/libwebauthn/src/transport/cable/known_devices.rs b/libwebauthn/src/transport/cable/known_devices.rs index a9b9a3dd..bd60d92d 100644 --- a/libwebauthn/src/transport/cable/known_devices.rs +++ b/libwebauthn/src/transport/cable/known_devices.rs @@ -162,7 +162,7 @@ impl CableKnownDevice { async fn connection( known_device: &CableKnownDevice, ux_sender: &super::connection_stages::MpscUxUpdateSender, - ) -> Result { + ) -> Result { let client_nonce = rand::random::(); // Stage 1: Connection (no proximity check needed for known devices) @@ -171,12 +171,12 @@ impl CableKnownDevice { // Stage 2: Proximity check (after connection for known devices) let proximity_input = - ProximityCheckInput::new_for_known_device(known_device, &client_nonce); + ProximityCheckInput::new_for_known_device(known_device, &client_nonce)?; let proximity_output = proximity_check_stage(proximity_input, ux_sender).await?; // Stage 3: Handshake let handshake_input = - HandshakeInput::new_for_known_device(known_device, connection_output, proximity_output); + HandshakeInput::new_for_known_device(known_device, connection_output, proximity_output)?; let handshake_output = handshake_stage(handshake_input, ux_sender).await?; Ok(handshake_output) @@ -204,7 +204,11 @@ impl<'d> Device<'d, Cable, CableChannel> for CableKnownDevice { let handshake_output = match Self::connection(&known_device, &ux_sender).await { Ok(handshake_output) => handshake_output, Err(e) => { - ux_sender.send_error(e).await; + let transport_err = match e { + Error::Transport(t) => t, + _ => TransportError::ConnectionFailed, + }; + ux_sender.send_error(transport_err).await; return; } }; diff --git a/libwebauthn/src/transport/cable/qr_code_device.rs b/libwebauthn/src/transport/cable/qr_code_device.rs index 6c9bed47..06bb9f55 100644 --- a/libwebauthn/src/transport/cable/qr_code_device.rs +++ b/libwebauthn/src/transport/cable/qr_code_device.rs @@ -75,7 +75,7 @@ pub struct CableQrCode { impl std::fmt::Display for CableQrCode { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let serialized = cbor::to_vec(&self).unwrap(); + let serialized = cbor::to_vec(&self).map_err(|_| std::fmt::Error)?; write!(f, "FIDO:/{}", digit_encode(&serialized)) } } @@ -107,7 +107,7 @@ impl CableQrCodeDevice { pub fn new_persistent( hint: QrCodeOperationHint, store: Arc, - ) -> Self { + ) -> Result { Self::new(hint, true, Some(store)) } @@ -115,16 +115,16 @@ impl CableQrCodeDevice { hint: QrCodeOperationHint, state_assisted: bool, store: Option>, - ) -> Self { + ) -> Result { let private_key_scalar = NonZeroScalar::random(&mut OsRng); - let private_key = SecretKey::from_bytes(&private_key_scalar.to_bytes()).unwrap(); + let private_key = SecretKey::from(private_key_scalar); let public_key: [u8; 33] = private_key .public_key() .as_affine() .to_encoded_point(true) .as_bytes() .try_into() - .unwrap(); + .map_err(|_| Error::Transport(TransportError::InvalidKey))?; let mut qr_secret = [0u8; 16]; OsRng.fill_bytes(&mut qr_secret); @@ -133,7 +133,7 @@ impl CableQrCodeDevice { .ok() .map(|t| t.as_secs()); - Self { + Ok(Self { qr_code: CableQrCode { public_key: ByteArray::from(public_key), qr_secret: ByteArray::from(qr_secret), @@ -148,14 +148,14 @@ impl CableQrCodeDevice { }, private_key: private_key_scalar, store, - } + }) } } impl CableQrCodeDevice { /// Generates a QR code, without any known-device store. A device scanning this QR code /// will not be persisted. - pub fn new_transient(hint: QrCodeOperationHint) -> Self { + pub fn new_transient(hint: QrCodeOperationHint) -> Result { Self::new(hint, false, None) } @@ -163,9 +163,9 @@ impl CableQrCodeDevice { async fn connection( qr_device: &CableQrCodeDevice, ux_sender: &MpscUxUpdateSender, - ) -> Result { + ) -> Result { // Stage 1: Proximity check - let proximity_input = ProximityCheckInput::new_for_qr_code(qr_device); + let proximity_input = ProximityCheckInput::new_for_qr_code(qr_device)?; let proximity_output = proximity_check_stage(proximity_input, ux_sender).await?; // Stage 2: Connection @@ -174,7 +174,7 @@ impl CableQrCodeDevice { // Stage 3: Handshake let handshake_input = - HandshakeInput::new_for_qr_code(qr_device, connection_output, proximity_output); + HandshakeInput::new_for_qr_code(qr_device, connection_output, proximity_output)?; let handshake_output = handshake_stage(handshake_input, ux_sender).await?; Ok(handshake_output) @@ -210,7 +210,11 @@ impl<'d> Device<'d, Cable, CableChannel> for CableQrCodeDevice { let handshake_output = match Self::connection(&qr_device, &ux_sender).await { Ok(handshake_output) => handshake_output, Err(e) => { - ux_sender.send_error(e).await; + let transport_err = match e { + Error::Transport(t) => t, + _ => TransportError::ConnectionFailed, + }; + ux_sender.send_error(transport_err).await; return; } }; diff --git a/libwebauthn/src/transport/cable/tunnel.rs b/libwebauthn/src/transport/cable/tunnel.rs index 4755350a..c5de81fb 100644 --- a/libwebauthn/src/transport/cable/tunnel.rs +++ b/libwebauthn/src/transport/cable/tunnel.rs @@ -145,7 +145,10 @@ pub fn decode_tunnel_server_domain(encoded: u16) -> Option { hasher.update(&sha_input); let digest = hasher.finalize(); - let mut v = u64::from_le_bytes(digest[..8].try_into().unwrap()); + let mut v = u64::from_le_bytes([ + digest[0], digest[1], digest[2], digest[3], + digest[4], digest[5], digest[6], digest[7], + ]); let tld_index = v & 3; v >>= 2; @@ -355,9 +358,13 @@ pub(crate) async fn do_handshake( } let mut payload = [0u8; 1024]; - let payload_len = noise_handshake - .read_message(&response, &mut payload) - .unwrap(); + let payload_len = match noise_handshake.read_message(&response, &mut payload) { + Ok(len) => len, + Err(e) => { + error!(?e, "Failed to read handshake response message"); + return Err(TransportError::ConnectionFailed); + } + }; debug!( { handshake = ?payload[..payload_len] }, @@ -762,7 +769,8 @@ fn parse_known_device( .raw_secret_bytes() .to_vec(); - let mut hmac = Hmac::::new_from_slice(&shared_secret).expect("Any key size is valid"); + let mut hmac = Hmac::::new_from_slice(&shared_secret) + .map_err(|_| Error::Transport(TransportError::InvalidKey))?; hmac.update(&noise_state.handshake_hash); let expected_mac = hmac.finalize().into_bytes().to_vec(); diff --git a/libwebauthn/src/transport/hid/channel.rs b/libwebauthn/src/transport/hid/channel.rs index 35a94863..d2a6af1e 100644 --- a/libwebauthn/src/transport/hid/channel.rs +++ b/libwebauthn/src/transport/hid/channel.rs @@ -195,15 +195,31 @@ impl<'d> HidChannel<'d> { } let mut cursor = IOCursor::new(response.payload); - cursor.seek(SeekFrom::Start(8)).unwrap(); + cursor + .seek(SeekFrom::Start(8)) + .map_err(|e| Error::Transport(TransportError::IoError(e.kind())))?; let init = InitResponse { - cid: cursor.read_u32::().unwrap(), - protocol_version: cursor.read_u8().unwrap(), - version_major: cursor.read_u8().unwrap(), - version_minor: cursor.read_u8().unwrap(), - version_build: cursor.read_u8().unwrap(), - caps: Caps::from_bits_truncate(cursor.read_u8().unwrap()), + cid: cursor + .read_u32::() + .map_err(|e| Error::Transport(TransportError::IoError(e.kind())))?, + protocol_version: cursor + .read_u8() + .map_err(|e| Error::Transport(TransportError::IoError(e.kind())))?, + version_major: cursor + .read_u8() + .map_err(|e| Error::Transport(TransportError::IoError(e.kind())))?, + version_minor: cursor + .read_u8() + .map_err(|e| Error::Transport(TransportError::IoError(e.kind())))?, + version_build: cursor + .read_u8() + .map_err(|e| Error::Transport(TransportError::IoError(e.kind())))?, + caps: Caps::from_bits_truncate( + cursor + .read_u8() + .map_err(|e| Error::Transport(TransportError::IoError(e.kind())))?, + ), }; debug!(?init, "Device init complete"); @@ -345,7 +361,10 @@ impl<'d> HidChannel<'d> { Self::hid_recv_hidapi(device, cancel_rx, timeout) }) .await - .expect("HID read not to panic.") + .map_err(|e| { + warn!(?e, "HID read task failed"); + Error::Transport(TransportError::ConnectionLost) + })? } #[cfg(test)] OpenHidDevice::VirtualDevice(virt_device) => { diff --git a/libwebauthn/src/transport/hid/device.rs b/libwebauthn/src/transport/hid/device.rs index 5e70ab7d..6c399e26 100644 --- a/libwebauthn/src/transport/hid/device.rs +++ b/libwebauthn/src/transport/hid/device.rs @@ -34,13 +34,12 @@ impl From<&DeviceInfo> for HidDevice { impl fmt::Display for HidDevice { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match &self.backend { - HidBackendDevice::HidApiDevice(dev) => write!( - f, - "{:} {:} (r{:?})", - dev.manufacturer_string().unwrap(), - dev.product_string().unwrap(), - dev.release_number() - ), + HidBackendDevice::HidApiDevice(dev) => { + let manufacturer = dev.manufacturer_string().unwrap_or_default(); + let product = dev.product_string().unwrap_or_default(); + let name = [manufacturer, product].join(" "); + write!(f, "{} (r{:?})", name.trim(), dev.release_number()) + } #[cfg(test)] HidBackendDevice::VirtualDevice => write!(f, "virtual fido-authenticator"), } diff --git a/libwebauthn/src/transport/hid/framing.rs b/libwebauthn/src/transport/hid/framing.rs index 11208e03..43e40b01 100644 --- a/libwebauthn/src/transport/hid/framing.rs +++ b/libwebauthn/src/transport/hid/framing.rs @@ -140,11 +140,10 @@ impl HidMessageParser { } fn more_packets_needed(&self) -> bool { - if self.packets.is_empty() { - return true; + match self.expected_bytes() { + None => true, + Some(expected) => expected > self.payload_len(), } - - self.expected_bytes().unwrap() > self.payload_len() } fn expected_bytes(&self) -> Option { @@ -153,7 +152,7 @@ impl HidMessageParser { } let mut cursor = IOCursor::new(vec![self.packets[0][5], self.packets[0][6]]); - Some(cursor.read_u16::().unwrap() as usize) + Some(cursor.read_u16::().ok()? as usize) } fn payload_len(&self) -> usize { diff --git a/libwebauthn/src/transport/nfc/device.rs b/libwebauthn/src/transport/nfc/device.rs index 90ceebba..88096097 100644 --- a/libwebauthn/src/transport/nfc/device.rs +++ b/libwebauthn/src/transport/nfc/device.rs @@ -84,9 +84,7 @@ impl<'d> Device<'d, Nfc, NfcChannel> for NfcDevice { async fn is_fido(device: &NfcDevice) -> bool { async fn inner(device: &NfcDevice) -> Result { let chan = device.channel_sync().await?; - // We fill the struct within channel_sync() and the call cannot fail for NFC, - // so unwrap is fine here - let protocols = chan.supported_protocols().await.unwrap(); + let protocols = chan.supported_protocols().await?; Ok(protocols.fido2 || protocols.u2f) } diff --git a/libwebauthn/src/transport/nfc/libnfc/mod.rs b/libwebauthn/src/transport/nfc/libnfc/mod.rs index 7328931e..f77d7188 100644 --- a/libwebauthn/src/transport/nfc/libnfc/mod.rs +++ b/libwebauthn/src/transport/nfc/libnfc/mod.rs @@ -72,10 +72,13 @@ impl Info { pub fn channel(&self) -> Result, Error> { let context = nfc1::Context::new().map_err(map_error)?; - let mut chan = Channel::new(self, context); + let mut chan = Channel::new(self, context)?; { - let mut device = chan.device.lock().unwrap(); + let mut device = chan + .device + .lock() + .map_err(|_| Error::Transport(TransportError::ConnectionFailed))?; device.initiator_init()?; device.set_property_bool(nfc1::Property::InfiniteSelect, false)?; @@ -93,20 +96,21 @@ impl Info { } pub struct Channel { + name: String, device: Arc>, } unsafe impl Send for Channel {} impl Channel { - pub fn new(info: &Info, mut context: nfc1::Context) -> Self { - let device = context - .open_with_connstring(&info.connstring) - .expect("opened device"); + pub fn new(info: &Info, mut context: nfc1::Context) -> Result { + let mut device = context.open_with_connstring(&info.connstring)?; + let name = device.name().to_owned(); - Self { + Ok(Self { + name, device: Arc::new(Mutex::new(device)), - } + }) } fn initiator_select_passive_target_ex( @@ -133,7 +137,10 @@ impl Channel { } fn connect_to_target(&mut self) -> Result { - let mut device = self.device.lock().unwrap(); + let mut device = self + .device + .lock() + .map_err(|_| Error::Transport(TransportError::ConnectionFailed))?; // Assume baudrates are already sorted higher to lower let baudrates = device.get_supported_baud_rate(nfc1::Mode::Initiator, MODULATION_TYPE)?; let modulations = baudrates @@ -189,7 +196,7 @@ where let rapdu = self .device .lock() - .unwrap() + .map_err(|_| HandleError::Nfc(Box::new(std::io::Error::other("mutex poisoned"))))? .initiator_transceive_bytes(command, len, timeout) .map_err(|e| HandleError::Nfc(Box::new(e)))?; @@ -207,8 +214,7 @@ where impl fmt::Display for Channel { fn fmt(&self, f: &mut fmt::Formatter) -> std::fmt::Result { - let mut device = self.device.lock().unwrap(); - write!(f, "{}", device.name()) + write!(f, "{}", self.name) } } @@ -229,7 +235,7 @@ pub(crate) fn list_devices() -> Result, Error> { nfc1::Context::new().map_err(|_| Error::Transport(TransportError::TransportUnavailable))?; let devices = context .list_devices(MAX_DEVICES) - .expect("libnfc devices") + .map_err(|_| Error::Transport(TransportError::TransportUnavailable))? .iter() .map(|x| NfcDevice::new_libnfc(Info::new(x))) .collect::>(); diff --git a/libwebauthn/src/transport/nfc/pcsc/mod.rs b/libwebauthn/src/transport/nfc/pcsc/mod.rs index 229bab0e..ea67888c 100644 --- a/libwebauthn/src/transport/nfc/pcsc/mod.rs +++ b/libwebauthn/src/transport/nfc/pcsc/mod.rs @@ -16,15 +16,17 @@ use tracing::{debug, info, instrument, trace}; #[derive(Clone, Debug)] pub struct Info { name: CString, + display_name: String, } pub struct PcscCard { - pub card: Option, + card: Option, } impl Deref for PcscCard { type Target = pcsc::Card; + #[allow(clippy::unwrap_used)] // The Option is always Some; it is only taken in Drop. fn deref(&self) -> &pcsc::Card { self.card.as_ref().unwrap() } @@ -34,33 +36,23 @@ impl Deref for PcscCard { // card has to be powered down instead. impl Drop for PcscCard { fn drop(&mut self) { - let _ = PcscCard::disconnect(self.card.take()); + if let Some(card) = self.card.take() { + debug!("Disconnect card"); + let _ = card.disconnect(pcsc::Disposition::UnpowerCard); + } } } impl PcscCard { pub fn new(card: pcsc::Card) -> Self { - PcscCard { card: Some(card) } - } - - fn map_disconnect_error(pair: (pcsc::Card, pcsc::Error)) -> Error { - let (_card, _err) = pair; - Error::Transport(TransportError::InvalidFraming) - } - - fn disconnect(card: Option) -> Result<(), Error> { - match card { - Some(card) => { - debug!("Disconnect card"); - card.disconnect(pcsc::Disposition::UnpowerCard) - .map_err(PcscCard::map_disconnect_error) - } - None => Ok(()), + PcscCard { + card: Some(card), } } } pub struct Channel { + name: String, card: Arc>, } @@ -68,7 +60,7 @@ unsafe impl Send for Channel {} impl fmt::Display for Info { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:?}", self.name) + write!(f, "{}", self.display_name) } } @@ -86,8 +78,11 @@ impl From for Error { impl Info { pub fn new(name: &CStr) -> Self { + let cstring = name.to_owned(); + let display_name = cstring.to_string_lossy().into_owned(); Info { - name: CStr::into_c_string(name.into()), + name: cstring, + display_name, } } @@ -106,6 +101,7 @@ impl Channel { let card = context.connect(&info.name, pcsc::ShareMode::Shared, pcsc::Protocols::ANY)?; let chan = Self { + name: info.display_name.clone(), card: Arc::new(Mutex::new(PcscCard::new(card))), }; @@ -115,12 +111,7 @@ impl Channel { impl fmt::Display for Channel { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result { - let card = self.card.lock().unwrap(); - let (names_len, atr_len) = card.status2_len().unwrap(); - let mut names_buf = vec![0; names_len]; - let mut atr_buf = vec![0; atr_len]; - let status = card.status2(&mut names_buf, &mut atr_buf).unwrap(); - write!(f, "{:?}", status.reader_names().collect::>()) + write!(f, "{}", self.name) } } @@ -138,10 +129,11 @@ where ) -> apdu_core::Result { trace!("TX: {:?}", command); - let rapdu = self + let card = self .card .lock() - .unwrap() + .map_err(|_| HandleError::Nfc(Box::new(std::io::Error::other("mutex poisoned"))))?; + let rapdu = card .transmit(command, response) .map_err(|e| HandleError::Nfc(Box::new(e)))?; @@ -161,15 +153,14 @@ pub(crate) fn is_nfc_available() -> bool { #[instrument] pub(crate) fn list_devices() -> Result, Error> { - let ctx = pcsc::Context::establish(pcsc::Scope::User).expect("PC/SC context"); - let len = ctx.list_readers_len().expect("PC/SC readers len"); + let ctx = pcsc::Context::establish(pcsc::Scope::User)?; + let len = ctx.list_readers_len()?; if len == 0 { return Err(Error::Transport(TransportError::TransportUnavailable)); } let mut readers_buf = vec![0; len]; let devices = ctx - .list_readers(&mut readers_buf) - .expect("PC/SC readers") + .list_readers(&mut readers_buf)? .map(|x| NfcDevice::new_pcsc(Info::new(x))) .collect::>(); diff --git a/libwebauthn/src/webauthn/error.rs b/libwebauthn/src/webauthn/error.rs index e7c3b6e7..9ae405a2 100644 --- a/libwebauthn/src/webauthn/error.rs +++ b/libwebauthn/src/webauthn/error.rs @@ -35,6 +35,8 @@ pub enum PlatformError { SyntaxError, #[error("cbor serialization error: {0}")] CborError(#[from] CborError), + #[error("crypto error: {0}")] + CryptoError(String), #[error("cancelled by user")] Cancelled, } diff --git a/libwebauthn/src/webauthn/pin_uv_auth_token.rs b/libwebauthn/src/webauthn/pin_uv_auth_token.rs index a38ca6a2..0d7d5d96 100644 --- a/libwebauthn/src/webauthn/pin_uv_auth_token.rs +++ b/libwebauthn/src/webauthn/pin_uv_auth_token.rs @@ -74,7 +74,7 @@ where ctap2_request.permissions_rpid(), ); if let Some(uv_auth_token) = channel.get_uv_auth_token(&token_identifier) { - ctap2_request.calculate_and_set_uv_auth(uv_proto.as_ref(), uv_auth_token); + ctap2_request.calculate_and_set_uv_auth(uv_proto.as_ref(), uv_auth_token)?; return Ok(UsedPinUvAuthToken::FromStorage); } } @@ -230,14 +230,20 @@ where Ctap2ClientPinRequest::new_get_pin_token( uv_proto.version(), public_key.clone(), - &uv_proto.encrypt(&shared_secret, &pin_hash(&pin.unwrap()))?, + &uv_proto.encrypt(&shared_secret, &pin_hash(&pin.ok_or_else(|| { + error!("PIN expected but not available"); + Error::Ctap(CtapError::PINRequired) + })?))?, ) } Ctap2UserVerificationOperation::GetPinUvAuthTokenUsingPinWithPermissions => { Ctap2ClientPinRequest::new_get_pin_token_with_perm( uv_proto.version(), public_key.clone(), - &uv_proto.encrypt(&shared_secret, &pin_hash(&pin.unwrap()))?, + &uv_proto.encrypt(&shared_secret, &pin_hash(&pin.ok_or_else(|| { + error!("PIN expected but not available"); + Error::Ctap(CtapError::PINRequired) + })?))?, ctap2_request.permissions(), ctap2_request.permissions_rpid(), ) @@ -320,7 +326,10 @@ where | Ctap2UserVerificationOperation::GetPinUvAuthTokenUsingPinWithPermissions | Ctap2UserVerificationOperation::GetPinToken => { { - let token_response = token_response.unwrap(); + let token_response = token_response.ok_or_else(|| { + error!("Expected token response but got None"); + Error::Ctap(CtapError::Other) + })?; let Some(encrypted_pin_uv_auth_token) = token_response.pin_uv_auth_token else { error!("Client PIN response did not include a PIN UV auth token"); return Err(Error::Ctap(CtapError::Other)); @@ -349,7 +358,7 @@ where // If successful, the platform creates the pinUvAuthParam parameter by calling // authenticate(pinUvAuthToken, clientDataHash), and goes to Step 1.1.1. // Sets the pinUvAuthProtocol parameter to the value as selected when it obtained the shared secret. - ctap2_request.calculate_and_set_uv_auth(uv_proto.as_ref(), uv_auth_token.as_slice()); + ctap2_request.calculate_and_set_uv_auth(uv_proto.as_ref(), uv_auth_token.as_slice())?; Ok(UsedPinUvAuthToken::NewlyCalculated(uv_operation)) } @@ -748,9 +757,9 @@ mod test { let info_resp = CborResponse::new_success_from_slice(to_vec(&info).unwrap().as_slice()); channel.push_command_pair(info_req, info_resp); - let pin_req = CborRequest::from(&Ctap2ClientPinRequest::new_get_key_agreement( + let pin_req = CborRequest::try_from(&Ctap2ClientPinRequest::new_get_key_agreement( Ctap2PinUvAuthProtocol::One, - )); + )).unwrap(); let pin_resp = CborResponse::new_success_from_slice( to_vec(&Ctap2ClientPinResponse { key_agreement: Some(get_key_agreement()), @@ -822,9 +831,9 @@ mod test { channel.push_command_pair(info_req, info_resp); // Queueing KeyAgreement request and response - let key_agreement_req = CborRequest::from( + let key_agreement_req = CborRequest::try_from( &Ctap2ClientPinRequest::new_get_key_agreement(Ctap2PinUvAuthProtocol::One), - ); + ).unwrap(); let key_agreement_resp = CborResponse::new_success_from_slice( to_vec(&Ctap2ClientPinResponse { key_agreement: Some(get_key_agreement()), @@ -844,12 +853,12 @@ mod test { let pin_protocol = PinUvAuthProtocolOne::new(); let (public_key, shared_secret) = pin_protocol.encapsulate(&get_key_agreement()).unwrap(); - let pin_req = CborRequest::from(&Ctap2ClientPinRequest::new_get_uv_token_with_perm( + let pin_req = CborRequest::try_from(&Ctap2ClientPinRequest::new_get_uv_token_with_perm( Ctap2PinUvAuthProtocol::One, public_key, getassertion.permissions(), getassertion.permissions_rpid(), - )); + )).unwrap(); // We do here what the device would need to do, i.e. generate a new random // pinUvAuthToken (here all 5's), then encrypt it using the shared_secret. let token = [5; 32]; @@ -937,9 +946,9 @@ mod test { channel.push_command_pair(info_req, info_resp); // Queueing PinRetries request and response - let pin_retries_req = CborRequest::from(&Ctap2ClientPinRequest::new_get_pin_retries( + let pin_retries_req = CborRequest::try_from(&Ctap2ClientPinRequest::new_get_pin_retries( Some(Ctap2PinUvAuthProtocol::One), - )); + )).unwrap(); let pin_retries_resp = CborResponse::new_success_from_slice( to_vec(&Ctap2ClientPinResponse { key_agreement: None, @@ -954,9 +963,9 @@ mod test { channel.push_command_pair(pin_retries_req, pin_retries_resp); // Queueing KeyAgreement request and response - let key_agreement_req = CborRequest::from( + let key_agreement_req = CborRequest::try_from( &Ctap2ClientPinRequest::new_get_key_agreement(Ctap2PinUvAuthProtocol::One), - ); + ).unwrap(); let key_agreement_resp = CborResponse::new_success_from_slice( to_vec(&Ctap2ClientPinResponse { key_agreement: Some(get_key_agreement()), @@ -979,13 +988,13 @@ mod test { let pin_hash_enc = pin_protocol .encrypt(&shared_secret, &pin_hash("1234".as_bytes())) .unwrap(); - let pin_req = CborRequest::from(&Ctap2ClientPinRequest::new_get_pin_token_with_perm( + let pin_req = CborRequest::try_from(&Ctap2ClientPinRequest::new_get_pin_token_with_perm( Ctap2PinUvAuthProtocol::One, public_key, &pin_hash_enc, getassertion.permissions(), getassertion.permissions_rpid(), - )); + )).unwrap(); // We do here what the device would need to do, i.e. generate a new random // pinUvAuthToken (here all 5's), then encrypt it using the shared_secret. let token = [5; 32];