Add more UV-negotiation tests (#149)

This PR adds another layer of testing opportunities, by introducing a
`TestChannel` that can be pre-filled with expected requests and
associated responses it should give.

I have used this to test `user_verification(..)` in various states of
complexity, and discovered a few bugs along the way.

Notable changes to make this possible:
- `PinUvAuthProtocol`s get a deterministic private key, to 'predict'
crypto results (for testing only!)
- Some (the ones I needed) Ctap2-response structs got a
`#[cfg_attr(test, derive(SerializeIndexed))]` to serialize them to CBOR,
before sending it back as if it were a real message from a device. This
should be easy to extend to other response structs, once they are
needed.
- Removed a second, subsequent `GetInfo`-call, by simply reusing the
result of the first call in `user_verification(..)`
- Fixed some corner cases in `user_verification(..)`, mostly around the
"shared secret only"-scenarios
- Renamed `ClientPinOnlyForSharedSecret` to `OnlyForSharedSecret` as
this is not bound to `ClientPin` anymore
- Added loads of tests. Still not exhaustive, though. More should be
written around full ceremony scenarios (I'll open tickets for some of
those, once this lands).

Author: msirringhaus

libwebauthn/src/ops/webauthn.rs

@@ -26,6 +26,14 @@ pub enum UserVerificationRequirement {
 }
 
 impl UserVerificationRequirement {
+    /// Check if user verification is discouraged
+    pub fn is_discouraged(&self) -> bool {
+        match self {
+            Self::Required | Self::Preferred => false,
+            Self::Discouraged => true,
+        }
+    }
+
     /// Check if user verification is preferred or required for this request
     pub fn is_preferred(&self) -> bool {
         match self {

libwebauthn/src/pin.rs

@@ -10,7 +10,7 @@ use p256::{
     ecdh::EphemeralSecret, elliptic_curve::sec1::FromEncodedPoint, EncodedPoint,
     PublicKey as P256PublicKey,
 };
-use rand::{rngs::OsRng, thread_rng, Rng};
+use rand::{rngs::OsRng, thread_rng, Rng, SeedableRng};
 use sha2::{Digest, Sha256};
 use tracing::{error, instrument, warn};
 use x509_parser::nom::AsBytes;
@@ -97,7 +97,14 @@ pub struct PinUvAuthProtocolOne {
 
 impl PinUvAuthProtocolOne {
     pub fn new() -> Self {
-        let private_key = EphemeralSecret::random(&mut OsRng);
+        let private_key = if cfg!(test) {
+            // For testing only!!
+            // We need a deterministic, seedable RNG to be able to "predict" crypto-operations
+            let mut rng = rand::rngs::StdRng::seed_from_u64(42);
+            EphemeralSecret::random(&mut rng)
+        } else {
+            EphemeralSecret::random(&mut OsRng)
+        };
         let public_key = private_key.public_key();
         Self {
             private_key,
@@ -247,7 +254,14 @@ pub struct PinUvAuthProtocolTwo {
 
 impl PinUvAuthProtocolTwo {
     pub fn new() -> Self {
-        let private_key = EphemeralSecret::random(&mut OsRng);
+        let private_key = if cfg!(test) {
+            // For testing only!!
+            // We need a deterministic, seedable RNG to be able to "predict" crypto-operations
+            let mut rng = rand::rngs::StdRng::seed_from_u64(42);
+            EphemeralSecret::random(&mut rng)
+        } else {
+            EphemeralSecret::random(&mut OsRng)
+        };
         let public_key = private_key.public_key();
         Self {
             private_key,

libwebauthn/src/proto/ctap2/cbor/request.rs

@@ -9,7 +9,7 @@ use crate::proto::ctap2::Ctap2AuthenticatorConfigRequest;
 use crate::proto::ctap2::Ctap2BioEnrollmentRequest;
 use crate::proto::ctap2::Ctap2CredentialManagementRequest;
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq)]
 pub struct CborRequest {
     pub command: Ctap2CommandCode,
     pub encoded_data: Vec<u8>,
@@ -18,7 +18,7 @@ pub struct CborRequest {
 impl CborRequest {
     pub fn new(command: Ctap2CommandCode) -> Self {
         Self {
-            command: command,
+            command,
             encoded_data: vec![],
         }
     }

libwebauthn/src/proto/ctap2/mod.rs

@@ -6,13 +6,17 @@ mod model;
 mod protocol;
 
 pub use model::Ctap2GetInfoResponse;
+#[cfg(test)]
+pub use model::Ctap2PinUvAuthProtocolCommand;
 pub use model::{
     Ctap2AttestationStatement, Ctap2AuthTokenPermissionRole, Ctap2COSEAlgorithmIdentifier,
-    Ctap2ClientPinRequest, Ctap2CommandCode, Ctap2CredentialType, Ctap2MakeCredentialOptions,
-    Ctap2PinUvAuthProtocol, Ctap2PublicKeyCredentialDescriptor, Ctap2PublicKeyCredentialRpEntity,
-    Ctap2PublicKeyCredentialType, Ctap2PublicKeyCredentialUserEntity, Ctap2Transport,
-    Ctap2UserVerifiableRequest, Ctap2UserVerificationOperation, FidoU2fAttestationStmt,
+    Ctap2ClientPinRequest, Ctap2ClientPinResponse, Ctap2CommandCode, Ctap2CredentialType,
+    Ctap2MakeCredentialOptions, Ctap2PinUvAuthProtocol, Ctap2PublicKeyCredentialDescriptor,
+    Ctap2PublicKeyCredentialRpEntity, Ctap2PublicKeyCredentialType,
+    Ctap2PublicKeyCredentialUserEntity, Ctap2Transport, Ctap2UserVerifiableRequest,
+    Ctap2UserVerificationOperation, FidoU2fAttestationStmt,
 };
+
 pub use model::{
     Ctap2AuthenticatorConfigCommand, Ctap2AuthenticatorConfigParams,
     Ctap2AuthenticatorConfigRequest,

libwebauthn/src/proto/ctap2/model.rs

@@ -19,10 +19,13 @@ pub use authenticator_config::{
     Ctap2AuthenticatorConfigRequest,
 };
 mod client_pin;
+#[cfg(test)]
+pub use client_pin::Ctap2PinUvAuthProtocolCommand;
 pub use client_pin::{
     Ctap2AuthTokenPermissionRole, Ctap2ClientPinRequest, Ctap2ClientPinResponse,
     Ctap2PinUvAuthProtocol,
 };
+
 mod make_credential;
 pub use make_credential::{
     Ctap2MakeCredentialOptions, Ctap2MakeCredentialRequest, Ctap2MakeCredentialResponse,
@@ -218,7 +221,7 @@ pub enum Ctap2UserVerificationOperation {
     GetPinUvAuthTokenUsingUvWithPermissions,
     GetPinUvAuthTokenUsingPinWithPermissions,
     GetPinToken,
-    ClientPinOnlyForSharedSecret,
+    OnlyForSharedSecret,
     LegacyUv,
 }
 

libwebauthn/src/proto/ctap2/model/client_pin.rs

@@ -221,6 +221,7 @@ pub enum Ctap2PinUvAuthProtocolCommand {
     GetPinUvAuthTokenUsingPinWithPermissions = 0x09,
 }
 
+#[cfg_attr(test, derive(SerializeIndexed))]
 #[derive(Debug, Clone, Default, DeserializeIndexed)]
 pub struct Ctap2ClientPinResponse {
     /// keyAgreement (0x01)

libwebauthn/src/proto/ctap2/model/get_info.rs

@@ -2,10 +2,13 @@ use std::collections::HashMap;
 
 use serde_bytes::ByteBuf;
 use serde_indexed::DeserializeIndexed;
+#[cfg(test)]
+use serde_indexed::SerializeIndexed;
 use tracing::debug;
 
 use super::{Ctap2CredentialType, Ctap2UserVerificationOperation};
 
+#[cfg_attr(test, derive(SerializeIndexed))]
 #[derive(Debug, Clone, DeserializeIndexed, Default)]
 pub struct Ctap2GetInfoResponse {
     /// versions (0x01)
@@ -227,7 +230,7 @@ impl Ctap2GetInfoResponse {
             // clientPIN exists, but is not enabled, aka PIN is not yet set on the device
             // We can use it for establishing a shared secret, but not for creating a pinUvAuthToken
             if self.option_exists("clientPin") && !self.option_enabled("clientPin") {
-                return Some(Ctap2UserVerificationOperation::ClientPinOnlyForSharedSecret);
+                return Some(Ctap2UserVerificationOperation::OnlyForSharedSecret);
             }
 
             // clientPin is not enabled (not supported or Pin not set) and
@@ -238,7 +241,7 @@ impl Ctap2GetInfoResponse {
                 && self.option_exists("uv")
                 && self.option_enabled("pinUvAuthToken")
             {
-                return Some(Ctap2UserVerificationOperation::ClientPinOnlyForSharedSecret);
+                return Some(Ctap2UserVerificationOperation::OnlyForSharedSecret);
             }
 
             // If we do have a PIN, check if we need to use legacy getPinToken or new getPinUvAuthToken..-command
@@ -355,11 +358,11 @@ mod test {
         assert!(info.can_establish_shared_secret());
         assert_eq!(
             info.uv_operation(false),
-            Some(Ctap2UserVerificationOperation::ClientPinOnlyForSharedSecret)
+            Some(Ctap2UserVerificationOperation::OnlyForSharedSecret)
         );
         assert_eq!(
             info.uv_operation(true),
-            Some(Ctap2UserVerificationOperation::ClientPinOnlyForSharedSecret)
+            Some(Ctap2UserVerificationOperation::OnlyForSharedSecret)
         );
     }
 
@@ -410,7 +413,7 @@ mod test {
         );
         assert_eq!(
             info.uv_operation(true),
-            Some(Ctap2UserVerificationOperation::ClientPinOnlyForSharedSecret)
+            Some(Ctap2UserVerificationOperation::OnlyForSharedSecret)
         );
     }
 
@@ -425,11 +428,11 @@ mod test {
         assert!(info.can_establish_shared_secret());
         assert_eq!(
             info.uv_operation(false),
-            Some(Ctap2UserVerificationOperation::ClientPinOnlyForSharedSecret)
+            Some(Ctap2UserVerificationOperation::OnlyForSharedSecret)
         );
         assert_eq!(
             info.uv_operation(true),
-            Some(Ctap2UserVerificationOperation::ClientPinOnlyForSharedSecret)
+            Some(Ctap2UserVerificationOperation::OnlyForSharedSecret)
         );
     }
 
@@ -463,7 +466,7 @@ mod test {
         );
         assert_eq!(
             info.uv_operation(true),
-            Some(Ctap2UserVerificationOperation::ClientPinOnlyForSharedSecret)
+            Some(Ctap2UserVerificationOperation::OnlyForSharedSecret)
         );
     }
 
@@ -495,11 +498,11 @@ mod test {
         assert!(info.can_establish_shared_secret());
         assert_eq!(
             info.uv_operation(false),
-            Some(Ctap2UserVerificationOperation::ClientPinOnlyForSharedSecret)
+            Some(Ctap2UserVerificationOperation::OnlyForSharedSecret)
         );
         assert_eq!(
             info.uv_operation(true),
-            Some(Ctap2UserVerificationOperation::ClientPinOnlyForSharedSecret)
+            Some(Ctap2UserVerificationOperation::OnlyForSharedSecret)
         );
     }
 
@@ -513,11 +516,11 @@ mod test {
         assert!(info.can_establish_shared_secret());
         assert_eq!(
             info.uv_operation(false),
-            Some(Ctap2UserVerificationOperation::ClientPinOnlyForSharedSecret)
+            Some(Ctap2UserVerificationOperation::OnlyForSharedSecret)
         );
         assert_eq!(
             info.uv_operation(true),
-            Some(Ctap2UserVerificationOperation::ClientPinOnlyForSharedSecret)
+            Some(Ctap2UserVerificationOperation::OnlyForSharedSecret)
         );
     }
 
@@ -547,7 +550,7 @@ mod test {
         );
         assert_eq!(
             info.uv_operation(true),
-            Some(Ctap2UserVerificationOperation::ClientPinOnlyForSharedSecret)
+            Some(Ctap2UserVerificationOperation::OnlyForSharedSecret)
         );
     }
 
@@ -564,11 +567,11 @@ mod test {
         assert!(info.can_establish_shared_secret());
         assert_eq!(
             info.uv_operation(false),
-            Some(Ctap2UserVerificationOperation::ClientPinOnlyForSharedSecret)
+            Some(Ctap2UserVerificationOperation::OnlyForSharedSecret)
         );
         assert_eq!(
             info.uv_operation(true),
-            Some(Ctap2UserVerificationOperation::ClientPinOnlyForSharedSecret)
+            Some(Ctap2UserVerificationOperation::OnlyForSharedSecret)
         );
     }
 }

libwebauthn/src/tests/channel.rs

@@ -0,0 +1,109 @@
+use async_trait::async_trait;
+use std::{collections::VecDeque, fmt::Display, time::Duration};
+use tokio::sync::broadcast;
+
+use crate::{
+    proto::{
+        ctap1::apdu::{ApduRequest, ApduResponse},
+        ctap2::cbor::{CborRequest, CborResponse},
+    },
+    transport::{
+        device::SupportedProtocols, AuthTokenData, Channel, ChannelStatus, Ctap2AuthTokenStore,
+    },
+    webauthn::Error,
+    UvUpdate,
+};
+
+pub struct TestChannel {
+    expected_requests: VecDeque<CborRequest>,
+    responses: VecDeque<CborResponse>,
+    auth_token_data: Option<AuthTokenData>,
+    ux_update_sender: broadcast::Sender<UvUpdate>,
+}
+
+impl TestChannel {
+    pub fn new() -> Self {
+        let (ux_update_sender, _) = broadcast::channel(16);
+        Self {
+            expected_requests: VecDeque::new(),
+            responses: VecDeque::new(),
+            auth_token_data: None,
+            ux_update_sender,
+        }
+    }
+
+    pub fn push_command_pair(&mut self, expected_request: CborRequest, response: CborResponse) {
+        self.expected_requests.push_front(expected_request);
+        self.responses.push_front(response);
+    }
+}
+
+impl Ctap2AuthTokenStore for TestChannel {
+    fn store_auth_data(&mut self, auth_token_data: AuthTokenData) {
+        self.auth_token_data = Some(auth_token_data);
+    }
+
+    fn get_auth_data(&self) -> Option<&AuthTokenData> {
+        self.auth_token_data.as_ref()
+    }
+
+    fn clear_uv_auth_token_store(&mut self) {
+        self.auth_token_data = None;
+    }
+}
+
+impl Display for TestChannel {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "TestChannel")
+    }
+}
+
+#[async_trait]
+impl Channel for TestChannel {
+    type UxUpdate = UvUpdate;
+
+    fn get_ux_update_sender(&self) -> &broadcast::Sender<Self::UxUpdate> {
+        &self.ux_update_sender
+    }
+
+    async fn supported_protocols(&self) -> Result<SupportedProtocols, Error> {
+        Ok(SupportedProtocols {
+            u2f: false,
+            fido2: true,
+        })
+    }
+    async fn status(&self) -> ChannelStatus {
+        unimplemented!();
+    }
+    async fn close(&mut self) {
+        unimplemented!();
+    }
+
+    async fn apdu_send(&self, _request: &ApduRequest, _timeout: Duration) -> Result<(), Error> {
+        unimplemented!();
+    }
+    async fn apdu_recv(&self, _timeout: Duration) -> Result<ApduResponse, Error> {
+        unimplemented!();
+    }
+
+    async fn cbor_send(&mut self, request: &CborRequest, _timeout: Duration) -> Result<(), Error> {
+        let expected = self
+            .expected_requests
+            .pop_back()
+            .expect("No expected request found, but one was sent");
+        assert_eq!(
+            &expected,
+            request,
+            "{} items still in the queue",
+            self.expected_requests.len()
+        );
+        Ok(())
+    }
+    async fn cbor_recv(&mut self, _timeout: Duration) -> Result<CborResponse, Error> {
+        let response = self
+            .responses
+            .pop_back()
+            .expect("No response found, but one was requested");
+        Ok(response)
+    }
+}

libwebauthn/src/tests/mod.rs

@@ -1,2 +1,3 @@
 pub mod basic_ctap1;
 pub mod basic_ctap2;
+pub mod channel;

libwebauthn/src/transport/mod.rs

@@ -12,5 +12,9 @@ mod transport;
 
 pub(crate) use channel::{AuthTokenData, Ctap2AuthTokenPermission};
 pub use channel::{Channel, Ctap2AuthTokenStore};
+
+#[cfg(test)]
+pub use channel::ChannelStatus;
+
 pub use device::Device;
 pub use transport::Transport;