Merge pull request #23 from bitwarden/PM-8571-CONTRIB-PM-7144-add-sup…

…port-for-incremental-counters

Add support for incremental counters

CHANGELOG.md

@@ -1,5 +1,17 @@
 # Changelog
 
+## Unreleased
+
+### passkey-authenticator
+
+- Added: support for signature counters
+	- ⚠ BREAKING: Add `update_credential` function to `CredentialStore` ([#23](https://github.com/1Password/passkey-rs/pull/23)).
+	- Add `make_credentials_with_signature_counter` to `Authenticator`.
+
+### passkey-client
+
+- Changed: The `Client` no longer hardcodes the UV value sent to the `Authenticator` ([#22](https://github.com/1Password/passkey-rs/pull/22)).
+
 ## Passkey v0.2.0
 ### passkey-types v0.2.0
 

passkey-authenticator/src/authenticator.rs

@@ -27,6 +27,13 @@ pub struct Authenticator<S, U> {
 
     /// The display name given when a [`webauthn::CredentialPropertiesOutput`] is requested
     display_name: Option<String>,
+
+    /// Value to control whether the authenticator will save new credentials with a signature counter.
+    /// The default value is `false`.
+    ///
+    /// NOTE: Using a counter with a credential that will sync is not recommended and can cause friction
+    /// with the distributed nature of synced keys. It can also cause issues with backup and restore functionality.
+    make_credentials_with_signature_counter: bool,
 }
 
 impl<S, U> Authenticator<S, U>
@@ -47,6 +54,7 @@ where
             ],
             user_validation: user,
             display_name: None,
+            make_credentials_with_signature_counter: false,
         }
     }
 
@@ -60,6 +68,19 @@ where
         self.display_name.as_ref()
     }
 
+    /// Set whether the authenticator should save new credentials with a signature counter.
+    ///
+    /// NOTE: Using a counter with a credential that will sync is not recommended and can cause friction
+    /// with the distributed nature of synced keys. It can also cause issues with backup and restore functionality.
+    pub fn set_make_credentials_with_signature_counter(&mut self, value: bool) {
+        self.make_credentials_with_signature_counter = value;
+    }
+
+    /// Get whether the authenticator will save new credentials with a signature counter.
+    pub fn make_credentials_with_signature_counter(&self) -> bool {
+        self.make_credentials_with_signature_counter
+    }
+
     /// Access the [`CredentialStore`] to look into what is stored.
     pub fn store(&self) -> &S {
         &self.store

passkey-authenticator/src/authenticator/get_assertion.rs

@@ -19,7 +19,7 @@ where
     /// This method is used by a host to request cryptographic proof of user authentication as well
     /// as user consent to a given transaction, using a previously generated credential that is
     /// bound to the authenticator and relying party identifier.
-    pub async fn get_assertion(&self, input: Request) -> Result<Response, StatusCode> {
+    pub async fn get_assertion(&mut self, input: Request) -> Result<Response, StatusCode> {
         // 1. Locate all credentials that are eligible for retrieval under the specified criteria:
         //     1. If an allowList is present and is non-empty, locate all denoted credentials
         //        present on this authenticator and bound to the specified rpId.
@@ -72,7 +72,7 @@ where
         let flags = self.check_user(&input.options).await?;
 
         // 8. If no credentials were located in step 1, return CTAP2_ERR_NO_CREDENTIALS.
-        let credential = maybe_credential?
+        let mut credential = maybe_credential?
             .into_iter()
             .next()
             .ok_or(Ctap2Error::NoCredentials)?
@@ -107,6 +107,17 @@ where
         //        the authenticator), terminate this procedure and return the
         //        CTAP2_ERR_OPERATION_DENIED error.
 
+        // [WebAuthn-9]. Increment the credential associated signature counter or the global signature
+        //               counter value, depending on which approach is implemented by the authenticator,
+        //               by some positive value. If the authenticator does not implement a signature
+        //               counter, let the signature counter value remain constant at zero.
+        if let Some(counter) = credential.counter {
+            credential.counter = Some(counter + 1);
+            self.store_mut()
+                .update_credential(credential.clone())
+                .await?;
+        }
+
         // 12. Sign the clientDataHash along with authData with the selected credential.
         //     Let signature be the assertion signature of the concatenation `authenticatorData` ||
         //     `clien_data_hash` using the privateKey of selectedCredential. A simple, undelimited
@@ -141,3 +152,86 @@ where
         })
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use coset::{CborSerializable, CoseKey};
+    use passkey_types::{
+        ctap2::{
+            get_assertion::{Options, Request},
+            Aaguid,
+        },
+        Passkey,
+    };
+
+    use crate::{Authenticator, MockUserValidationMethod};
+
+    fn create_passkey() -> Passkey {
+        Passkey {
+            key: private_key_for_testing(),
+            credential_id: Default::default(),
+            rp_id: "example.com".into(),
+            user_handle: None,
+            counter: None,
+        }
+    }
+
+    fn good_request() -> Request {
+        Request {
+            rp_id: "example.com".into(),
+            client_data_hash: vec![0; 32].into(),
+            allow_list: None,
+            extensions: None,
+            pin_auth: None,
+            pin_protocol: None,
+            options: Options {
+                up: true,
+                uv: true,
+                rk: true,
+            },
+        }
+    }
+
+    fn private_key_for_testing() -> CoseKey {
+        // Hardcoded CoseKey for testing purposes
+        let bytes = vec![
+            166, 1, 2, 3, 38, 32, 1, 33, 88, 32, 200, 30, 161, 146, 196, 121, 165, 149, 92, 232,
+            49, 48, 245, 253, 73, 234, 204, 3, 209, 153, 166, 77, 59, 232, 70, 16, 206, 77, 84,
+            156, 28, 77, 34, 88, 32, 82, 141, 165, 28, 241, 82, 31, 33, 183, 206, 29, 91, 93, 111,
+            216, 216, 26, 62, 211, 49, 191, 86, 238, 118, 241, 124, 131, 106, 214, 95, 170, 160,
+            35, 88, 32, 147, 171, 4, 49, 68, 170, 47, 51, 74, 211, 94, 40, 212, 244, 95, 55, 154,
+            92, 171, 241, 0, 55, 84, 151, 79, 244, 151, 198, 135, 45, 97, 238,
+        ];
+
+        CoseKey::from_slice(bytes.as_slice()).unwrap()
+    }
+
+    #[tokio::test]
+    async fn get_assertion_increments_signature_counter_when_counter_is_some() {
+        // Arrange
+        let request = good_request();
+        let store = Some(Passkey {
+            counter: Some(9000),
+            ..create_passkey()
+        });
+        let mut authenticator = Authenticator::new(
+            Aaguid::new_empty(),
+            store,
+            MockUserValidationMethod::verified_user(1),
+        );
+
+        // Act
+        let response = authenticator.get_assertion(request).await.unwrap();
+
+        // Assert
+        assert_eq!(response.auth_data.counter.unwrap(), 9001);
+        assert_eq!(
+            authenticator
+                .store()
+                .as_ref()
+                .and_then(|c| c.counter)
+                .unwrap(),
+            9001
+        );
+    }
+}

passkey-authenticator/src/authenticator/make_credential.rs

@@ -107,7 +107,7 @@ where
             rp_id: input.rp.id.clone(),
             credential_id: credential_id.into(),
             user_handle: input.options.rk.then_some(input.user.id.clone()),
-            counter: None,
+            counter: self.make_credentials_with_signature_counter.then_some(0),
         };
 
         // 10. If "rk" in options parameter is set to true:
@@ -270,4 +270,22 @@ mod tests {
 
         assert_eq!(err, Ctap2Error::UnsupportedAlgorithm.into());
     }
+
+    #[tokio::test]
+    async fn make_credential_counter_is_some_0_when_counters_are_enabled() {
+        // Arrange
+        let shared_store = Arc::new(Mutex::new(None));
+        let user_mock = MockUserValidationMethod::verified_user(1);
+        let request = good_request();
+        let mut authenticator =
+            Authenticator::new(Aaguid::new_empty(), shared_store.clone(), user_mock);
+        authenticator.set_make_credentials_with_signature_counter(true);
+
+        // Act
+        authenticator.make_credential(request).await.unwrap();
+
+        // Assert
+        let store = shared_store.lock().await;
+        assert_eq!(store.as_ref().and_then(|c| c.counter).unwrap(), 0);
+    }
 }

passkey-authenticator/src/credential_store.rs

@@ -26,13 +26,16 @@ pub trait CredentialStore {
         rp_id: &str,
     ) -> Result<Vec<Self::PasskeyItem>, StatusCode>;
 
-    /// Save the new/updated credential into your store
+    /// Save the new credential into your store
     async fn save_credential(
         &mut self,
         cred: Passkey,
         user: PublicKeyCredentialUserEntity,
         rp: PublicKeyCredentialRpEntity,
     ) -> Result<(), StatusCode>;
+
+    /// Update the credential in your store
+    async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode>;
 }
 
 /// In-memory store for Passkeys
@@ -71,6 +74,11 @@ impl CredentialStore for MemoryStore {
         self.insert(cred.credential_id.clone().into(), cred);
         Ok(())
     }
+
+    async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode> {
+        self.insert(cred.credential_id.clone().into(), cred);
+        Ok(())
+    }
 }
 
 #[async_trait::async_trait]
@@ -103,6 +111,11 @@ impl CredentialStore for Option<Passkey> {
         self.replace(cred);
         Ok(())
     }
+
+    async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode> {
+        self.replace(cred);
+        Ok(())
+    }
 }
 
 #[cfg(any(feature = "tokio", test))]
@@ -128,6 +141,10 @@ impl<S: CredentialStore<PasskeyItem = Passkey> + Send + Sync> CredentialStore
     ) -> Result<(), StatusCode> {
         self.lock().await.save_credential(cred, user, rp).await
     }
+
+    async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode> {
+        self.lock().await.update_credential(cred).await
+    }
 }
 
 #[cfg(any(feature = "tokio", test))]
@@ -153,6 +170,10 @@ impl<S: CredentialStore<PasskeyItem = Passkey> + Send + Sync> CredentialStore
     ) -> Result<(), StatusCode> {
         self.write().await.save_credential(cred, user, rp).await
     }
+
+    async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode> {
+        self.write().await.update_credential(cred).await
+    }
 }
 
 #[cfg(any(feature = "tokio", test))]
@@ -178,6 +199,10 @@ impl<S: CredentialStore<PasskeyItem = Passkey> + Send + Sync> CredentialStore
     ) -> Result<(), StatusCode> {
         self.lock().await.save_credential(cred, user, rp).await
     }
+
+    async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode> {
+        self.lock().await.update_credential(cred).await
+    }
 }
 
 #[cfg(any(feature = "tokio", test))]
@@ -203,4 +228,8 @@ impl<S: CredentialStore<PasskeyItem = Passkey> + Send + Sync> CredentialStore
     ) -> Result<(), StatusCode> {
         self.write().await.save_credential(cred, user, rp).await
     }
+
+    async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode> {
+        self.write().await.update_credential(cred).await
+    }
 }

passkey-client/Cargo.toml

@@ -30,5 +30,8 @@ coset = "0.3"
 
 [dev-dependencies]
 coset = "0.3"
-passkey-authenticator = { path = "../passkey-authenticator", features = ["tokio", "testable"] }
+passkey-authenticator = { path = "../passkey-authenticator", features = [
+  "tokio",
+  "testable",
+] }
 tokio = { version = "1", features = ["macros", "rt"] }

passkey-client/src/lib.rs

@@ -297,7 +297,7 @@ where
     ///
     /// Returns either an [`webauthn::AuthenticatedPublicKeyCredential`] on success or some [`WebauthnError`].
     pub async fn authenticate(
-        &self,
+        &mut self,
         origin: &Url,
         request: webauthn::CredentialRequestOptions,
         client_data_hash: Option<Vec<u8>>,

passkey/Cargo.toml

@@ -17,7 +17,9 @@ workspace = true
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [package.metadata.cargo-udeps.ignore]
-development = ["tokio-test"] # Only used for async doctests. Cargo udeps can't check.:
+development = [
+    "tokio-test",
+] # Only used for async doctests. Cargo udeps can't check.:
 
 [dependencies]
 passkey-authenticator = { path = "../passkey-authenticator", version = "0.2" }