diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 4dea18bf..ad2d2c3c 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -18,12 +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 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_cable.rs b/libwebauthn/examples/webauthn_cable.rs index f42595c2..1c8b0cda 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(); @@ -100,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/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/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 04ba2eb5..5193da88 100644 --- a/libwebauthn/src/management/authenticator_config.rs +++ b/libwebauthn/src/management/authenticator_config.rs @@ -164,28 +164,26 @@ impl Ctap2UserVerifiableRequest for Ctap2AuthenticatorConfigRequest { fn calculate_and_set_uv_auth( &mut self, - uv_proto: &Box, + 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 { - 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..edf74b71 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,32 +296,27 @@ impl Ctap2UserVerifiableRequest for Ctap2BioEnrollmentRequest { fn calculate_and_set_uv_auth( &mut self, - uv_proto: &Box, + 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 { - 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..b803bca5 100644 --- a/libwebauthn/src/management/credential_management.rs +++ b/libwebauthn/src/management/credential_management.rs @@ -276,26 +276,27 @@ 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]; + ) -> 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 { - 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..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, @@ -215,7 +228,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..b7ca054b 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, @@ -357,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; @@ -642,7 +643,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 +655,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..b409120a 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 { @@ -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) { @@ -123,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()) } } @@ -143,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 @@ -172,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(), + }, + )) } } @@ -209,17 +235,17 @@ 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)] 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" @@ -252,6 +278,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) { @@ -280,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) } } @@ -350,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]; @@ -368,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 { - let hk = Hkdf::::new(salt, &ikm); +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] @@ -396,13 +433,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)); } @@ -417,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( @@ -441,7 +483,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(); @@ -460,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(), @@ -472,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 a2b7ccfa..9aae6a38 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 { @@ -149,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/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..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,15 +101,15 @@ 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) } } -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/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/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..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; @@ -214,10 +217,12 @@ 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]; + ) -> 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/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..d70f9a62 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( @@ -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, @@ -419,16 +419,20 @@ 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()); + ) -> 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 { @@ -514,7 +518,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 +540,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..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}; @@ -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,16 +329,20 @@ 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()); + ) -> 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 888e1b05..8941689a 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, @@ -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 1a87fe9b..0bfc2707 100644 --- a/libwebauthn/src/transport/ble/btleplug/manager.rs +++ b/libwebauthn/src/transport/ble/btleplug/manager.rs @@ -119,10 +119,15 @@ async fn get_adapter() -> Result { .await .or(Err(Error::Unavailable))? .into_iter() - .nth(0) + .next() .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> { @@ -151,8 +156,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 +194,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 { @@ -221,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/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/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/device.rs b/libwebauthn/src/transport/ble/device.rs index b94d528f..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() @@ -59,9 +64,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..a8b03267 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 { @@ -144,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 { @@ -153,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 { @@ -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/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/advertisement.rs b/libwebauthn/src/transport/cable/advertisement.rs index 2ba5246d..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 @@ -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..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 }) } } @@ -62,13 +63,14 @@ 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 tunnel_id = &derive( + let routing_id_str = hex::encode(proximity_output.advert.routing_id); + let tunnel_id_full = derive( qr_device.qr_code.qr_secret.as_ref(), None, KeyPurpose::TunnelID, - )[..16]; - let tunnel_id_str = hex::encode(&tunnel_id); + ).map_err(|_| TransportError::InvalidKey)?; + let tunnel_id = &tunnel_id_full[..16]; + let tunnel_id_str = hex::encode(tunnel_id); let connection_type = CableTunnelConnectionType::QrCode { routing_id: routing_id_str, @@ -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 54682ce5..98f30b26 100644 --- a/libwebauthn/src/transport/cable/crypto.rs +++ b/libwebauthn/src/transport/cable/crypto.rs @@ -1,25 +1,28 @@ -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, TunnelID = 2, - PSK = 3, + 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 60d97d33..bd60d92d 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) } @@ -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/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 {} diff --git a/libwebauthn/src/transport/cable/qr_code_device.rs b/libwebauthn/src/transport/cable/qr_code_device.rs index fcfa8f39..06bb9f55 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 { - let serialized = cbor::to_vec(&self).unwrap(); - format!("FIDO:/{}", digit_encode(&serialized)) +impl std::fmt::Display for CableQrCode { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + 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,25 +115,25 @@ 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::default().fill_bytes(&mut qr_secret); + OsRng.fill_bytes(&mut qr_secret); let current_unix_time = SystemTime::now() .duration_since(SystemTime::UNIX_EPOCH) .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 48801736..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; @@ -200,7 +203,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 +238,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 +288,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(), }; @@ -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] }, @@ -581,7 +588,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 +715,7 @@ async fn connection_recv( private_key, tunnel_domain, &linking_info, - &noise_state, + noise_state, ) { Ok(known_device) => { debug!(?device_id, "Updating known device"); @@ -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 3280b09d..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) => { @@ -449,7 +468,6 @@ impl Channel for HidChannel<'_> { } async fn close(&mut self) { - () } async fn apdu_send( 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 7282730d..43e40b01 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,19 +132,18 @@ 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 { - 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 { @@ -147,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 { @@ -250,7 +255,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 +272,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..88096097 100644 --- a/libwebauthn/src/transport/nfc/device.rs +++ b/libwebauthn/src/transport/nfc/device.rs @@ -81,22 +81,14 @@ 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 - let protocols = chan.supported_protocols().await.unwrap(); + let protocols = chan.supported_protocols().await?; Ok(protocols.fido2 || protocols.u2f) } - inner::(device).await.is_ok() + inner(device).await.is_ok() } #[instrument] @@ -117,7 +109,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/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 191d91f1..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<'tx> Deref for PcscCard { +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<'tx> 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))), }; @@ -114,13 +110,8 @@ 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::>()) + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result { + 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.rs b/libwebauthn/src/webauthn.rs index 57a632a4..9b8dcc56 100644 --- a/libwebauthn/src/webauthn.rs +++ b/libwebauthn/src/webauthn.rs @@ -61,24 +61,6 @@ pub trait WebAuthn { &mut self, op: &GetAssertionRequest, ) -> Result; - async fn _webauthn_make_credential_fido2( - &mut self, - op: &MakeCredentialRequest, - ) -> Result; - async fn _webauthn_make_credential_u2f( - &mut self, - op: &MakeCredentialRequest, - ) -> Result; - - async fn _webauthn_get_assertion_fido2( - &mut self, - op: &GetAssertionRequest, - ) -> Result; - async fn _webauthn_get_assertion_u2f( - &mut self, - op: &GetAssertionRequest, - ) -> Result; - async fn _negotiate_protocol(&mut self, allow_u2f: bool) -> Result; } #[async_trait] @@ -92,194 +74,194 @@ where op: &MakeCredentialRequest, ) -> Result { trace!(?op, "WebAuthn MakeCredential request"); - let protocol = self._negotiate_protocol(op.is_downgradable()).await?; + let protocol = negotiate_protocol(self, op.is_downgradable()).await?; match protocol { - FidoProtocol::FIDO2 => self._webauthn_make_credential_fido2(op).await, - FidoProtocol::U2F => self._webauthn_make_credential_u2f(op).await, + FidoProtocol::FIDO2 => make_credential_fido2(self, op).await, + FidoProtocol::U2F => make_credential_u2f(self, op).await, } } - async fn _webauthn_make_credential_fido2( - &mut self, - op: &MakeCredentialRequest, - ) -> Result { - let get_info_response = self.ctap2_get_info().await?; - let mut ctap2_request = - Ctap2MakeCredentialRequest::from_webauthn_request(op, &get_info_response)?; - if Self::supports_preflight() { - if let Some(exclude_list) = &op.exclude { - let filtered_exclude_list = - ctap2_preflight(self, exclude_list, &op.client_data_hash(), &op.relying_party.id).await; - ctap2_request.exclude = Some(filtered_exclude_list); - } - } - let response = loop { - let uv_auth_used = - user_verification(self, op.user_verification, &mut ctap2_request, op.timeout) - .await?; - - // We've already sent out this update, in case we used builtin UV - // but in all other cases, we need to touch the device now. - if uv_auth_used - != UsedPinUvAuthToken::NewlyCalculated( - Ctap2UserVerificationOperation::GetPinUvAuthTokenUsingUvWithPermissions, - ) - { - self.send_ux_update(UvUpdate::PresenceRequired.into()).await; - } - handle_errors!( - self, - self.ctap2_make_credential(&ctap2_request, op.timeout).await, - uv_auth_used, - op.timeout - ) - }?; - let make_cred = response.into_make_credential_output(op, Some(&get_info_response)); - Ok(make_cred) - } - - async fn _webauthn_make_credential_u2f( - &mut self, - op: &MakeCredentialRequest, - ) -> Result { - let register_request: RegisterRequest = op.try_downgrade()?; - - self.ctap1_register(®ister_request) - .await? - .try_upgrade(op) - } - #[instrument(skip_all, fields(dev = % self))] async fn webauthn_get_assertion( &mut self, op: &GetAssertionRequest, ) -> Result { trace!(?op, "WebAuthn GetAssertion request"); - let protocol = self._negotiate_protocol(op.is_downgradable()).await?; + let protocol = negotiate_protocol(self, op.is_downgradable()).await?; match protocol { - FidoProtocol::FIDO2 => self._webauthn_get_assertion_fido2(op).await, - FidoProtocol::U2F => self._webauthn_get_assertion_u2f(op).await, + FidoProtocol::FIDO2 => get_assertion_fido2(self, op).await, + FidoProtocol::U2F => get_assertion_u2f(self, op).await, } } +} - async fn _webauthn_get_assertion_fido2( - &mut self, - op: &GetAssertionRequest, - ) -> Result { - let get_info_response = self.ctap2_get_info().await?; - let mut ctap2_request = - Ctap2GetAssertionRequest::from_webauthn_request(op, &get_info_response)?; +async fn make_credential_fido2( + channel: &mut C, + op: &MakeCredentialRequest, +) -> Result { + let get_info_response = channel.ctap2_get_info().await?; + let mut ctap2_request = + Ctap2MakeCredentialRequest::from_webauthn_request(op, &get_info_response)?; + if C::supports_preflight() { + if let Some(exclude_list) = &op.exclude { + let filtered_exclude_list = + ctap2_preflight(channel, exclude_list, &op.client_data_hash(), &op.relying_party.id).await; + ctap2_request.exclude = Some(filtered_exclude_list); + } + } + let response = loop { + let uv_auth_used = + user_verification(channel, op.user_verification, &mut ctap2_request, op.timeout) + .await?; - if Self::supports_preflight() { - let filtered_allow_list = - ctap2_preflight(self, &op.allow, &op.client_data_hash(), &op.relying_party_id).await; - if filtered_allow_list.is_empty() && !op.allow.is_empty() { - // We filtered out everything in preflight, meaning none of the allowed - // credentials are present on this device. So we error out here - // But the spec requires some form of user interaction, so we run a - // dummy request, ignore the result and error out. - warn!("Preflight removed all credentials from the allow-list. Sending dummy request and erroring out."); - let dummy_request: Ctap2MakeCredentialRequest = Ctap2MakeCredentialRequest::dummy(); - self.send_ux_update(UvUpdate::PresenceRequired.into()).await; - let _ = self.ctap2_make_credential(&dummy_request, op.timeout).await; - return Err(Error::Ctap(CtapError::NoCredentials)); - } - ctap2_request.allow = filtered_allow_list; + // We've already sent out this update, in case we used builtin UV + // but in all other cases, we need to touch the device now. + if uv_auth_used + != UsedPinUvAuthToken::NewlyCalculated( + Ctap2UserVerificationOperation::GetPinUvAuthTokenUsingUvWithPermissions, + ) + { + channel.send_ux_update(UvUpdate::PresenceRequired.into()).await; } + handle_errors!( + channel, + channel.ctap2_make_credential(&ctap2_request, op.timeout).await, + uv_auth_used, + op.timeout + ) + }?; + let make_cred = response.into_make_credential_output(op, Some(&get_info_response)); + Ok(make_cred) +} - let response = loop { - let uv_auth_used = - user_verification(self, op.user_verification, &mut ctap2_request, op.timeout) - .await?; - // Order is important here! - // We can error out in calculate_hmac() with PlatformError's, - // so only send out the UvUpdate after we are done with hmac. - if let Some(auth_data) = self.get_auth_data() { - if let Some(e) = ctap2_request.extensions.as_mut() { - e.calculate_hmac(&op.allow, auth_data)?; - } - } +async fn make_credential_u2f( + channel: &mut C, + op: &MakeCredentialRequest, +) -> Result { + let register_request: RegisterRequest = op.try_downgrade()?; - // We've already sent out this update, in case we used builtin UV - // but in all other cases, we need to touch the device now. - if uv_auth_used - != UsedPinUvAuthToken::NewlyCalculated( - Ctap2UserVerificationOperation::GetPinUvAuthTokenUsingUvWithPermissions, - ) - { - self.send_ux_update(UvUpdate::PresenceRequired.into()).await; + channel.ctap1_register(®ister_request) + .await? + .try_upgrade(op) +} + +async fn get_assertion_fido2( + channel: &mut C, + op: &GetAssertionRequest, +) -> Result { + let get_info_response = channel.ctap2_get_info().await?; + let mut ctap2_request = + Ctap2GetAssertionRequest::from_webauthn_request(op, &get_info_response)?; + + if C::supports_preflight() { + let filtered_allow_list = + ctap2_preflight(channel, &op.allow, &op.client_data_hash(), &op.relying_party_id).await; + if filtered_allow_list.is_empty() && !op.allow.is_empty() { + // We filtered out everything in preflight, meaning none of the allowed + // credentials are present on this device. So we error out here + // But the spec requires some form of user interaction, so we run a + // dummy request, ignore the result and error out. + warn!("Preflight removed all credentials from the allow-list. Sending dummy request and erroring out."); + let dummy_request: Ctap2MakeCredentialRequest = Ctap2MakeCredentialRequest::dummy(); + channel.send_ux_update(UvUpdate::PresenceRequired.into()).await; + let _ = channel.ctap2_make_credential(&dummy_request, op.timeout).await; + return Err(Error::Ctap(CtapError::NoCredentials)); + } + ctap2_request.allow = filtered_allow_list; + } + + let response = loop { + let uv_auth_used = + user_verification(channel, op.user_verification, &mut ctap2_request, op.timeout) + .await?; + // Order is important here! + // We can error out in calculate_hmac() with PlatformError's, + // so only send out the UvUpdate after we are done with hmac. + if let Some(auth_data) = channel.get_auth_data() { + if let Some(e) = ctap2_request.extensions.as_mut() { + e.calculate_hmac(&op.allow, auth_data)?; } + } - handle_errors!( - self, - self.ctap2_get_assertion(&ctap2_request, op.timeout).await, - uv_auth_used, - op.timeout + // We've already sent out this update, in case we used builtin UV + // but in all other cases, we need to touch the device now. + if uv_auth_used + != UsedPinUvAuthToken::NewlyCalculated( + Ctap2UserVerificationOperation::GetPinUvAuthTokenUsingUvWithPermissions, ) - }?; - let count = response.credentials_count.unwrap_or(1); - let mut assertions = vec![response.into_assertion_output(op, self.get_auth_data())]; - for i in 1..count { - debug!({ i }, "Fetching additional credential"); - // GetNextAssertion doesn't use PinUVAuthToken, so we don't need to check uv_auth_used here - let response = self.ctap2_get_next_assertion(op.timeout).await?; - assertions.push(response.into_assertion_output(op, self.get_auth_data())); + { + channel.send_ux_update(UvUpdate::PresenceRequired.into()).await; } - Ok(assertions.as_slice().into()) + + handle_errors!( + channel, + channel.ctap2_get_assertion(&ctap2_request, op.timeout).await, + uv_auth_used, + op.timeout + ) + }?; + let count = response.credentials_count.unwrap_or(1); + let mut assertions = vec![response.into_assertion_output(op, channel.get_auth_data())]; + for i in 1..count { + debug!({ i }, "Fetching additional credential"); + // GetNextAssertion doesn't use PinUVAuthToken, so we don't need to check uv_auth_used here + let response = channel.ctap2_get_next_assertion(op.timeout).await?; + assertions.push(response.into_assertion_output(op, channel.get_auth_data())); } + Ok(assertions.as_slice().into()) +} - async fn _webauthn_get_assertion_u2f( - &mut self, - op: &GetAssertionRequest, - ) -> Result { - let sign_requests: Vec = op.try_downgrade()?; +async fn get_assertion_u2f( + channel: &mut C, + op: &GetAssertionRequest, +) -> Result { + let sign_requests: Vec = op.try_downgrade()?; - for sign_request in sign_requests { - match self.ctap1_sign(&sign_request).await { - Ok(response) => { - debug!("Found successful candidate in allowList"); - return response.try_upgrade(&sign_request); - } - Err(Error::Ctap(CtapError::NoCredentials)) => { - debug!("No credentials found, trying with the next."); - } - Err(err) => { - error!( - ?err, - "Unexpected error whilst trying each credential in allowList." - ); - return Err(err); - } + for sign_request in sign_requests { + match channel.ctap1_sign(&sign_request).await { + Ok(response) => { + debug!("Found successful candidate in allowList"); + return response.try_upgrade(&sign_request); + } + Err(Error::Ctap(CtapError::NoCredentials)) => { + debug!("No credentials found, trying with the next."); + } + Err(err) => { + error!( + ?err, + "Unexpected error whilst trying each credential in allowList." + ); + return Err(err); } } - warn!("None of the credentials in the original request's allowList were found."); - Err(Error::Ctap(CtapError::NoCredentials)) } + warn!("None of the credentials in the original request's allowList were found."); + Err(Error::Ctap(CtapError::NoCredentials)) +} - #[instrument(skip_all)] - async fn _negotiate_protocol(&mut self, allow_u2f: bool) -> Result { - let supported = self.supported_protocols().await?; - if !supported.u2f && !supported.fido2 { - return Err(Error::Transport(TransportError::NegotiationFailed)); - } +#[instrument(skip_all)] +async fn negotiate_protocol(channel: &mut C, allow_u2f: bool) -> Result { + let supported = channel.supported_protocols().await?; + if !supported.u2f && !supported.fido2 { + return Err(Error::Transport(TransportError::NegotiationFailed)); + } - if !allow_u2f && !supported.fido2 { - return Err(Error::Transport(TransportError::NegotiationFailed)); - } + if !allow_u2f && !supported.fido2 { + return Err(Error::Transport(TransportError::NegotiationFailed)); + } - let fido_protocol = if supported.fido2 { - FidoProtocol::FIDO2 - } else { - // Ensure CTAP1 version is reported correctly. - self.ctap1_version().await?; - FidoProtocol::U2F - }; + let fido_protocol = if supported.fido2 { + FidoProtocol::FIDO2 + } else { + // Ensure CTAP1 version is reported correctly. + channel.ctap1_version().await?; + FidoProtocol::U2F + }; - if fido_protocol == FidoProtocol::U2F { - warn!("Negotiated protocol downgrade from FIDO2 to FIDO U2F"); - } else { - debug!("Selected protocol: {:?}", fido_protocol); - } - Ok(fido_protocol) + if fido_protocol == FidoProtocol::U2F { + warn!("Negotiated protocol downgrade from FIDO2 to FIDO U2F"); + } else { + debug!("Selected protocol: {:?}", fido_protocol); } + Ok(fido_protocol) } 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 d25c77d8..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, 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. @@ -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, 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 +368,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 @@ -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];