Parse slot_id in use from token state (#546)

* Parse slot_id in use from token state

Support switching to multi-PIN feature and making the default slots 8.
But the command to switch to multi-PIN isn't exposed yet because the
implementation isn't ready.

Main change of this commit is to cache the slot_id in use inside token
state, and retrieve it from token state when needed.

* Fix once_cell dependency (#548)

* fixed version of once_cell

* fixes comments

* removes unnecessary fuzz dependency

* Fix styles

Co-authored-by: kaczmarczyck <43844792+kaczmarczyck@users.noreply.github.com>

Cargo.toml

@@ -45,6 +45,11 @@ sk-cbor = { path = "libraries/cbor" }
 uuid = { version = "0.8", features = ["v4"] }
 openssl = "0.10.36"
 
+# We explicitly lock the version of those transitive dependencies because their
+# Cargo.toml don't parse with the nightly compiler used by Tock. Remove this
+# whole block once CTAP is a library.
+once_cell = "=1.14" # transitive dependency of openssl
+
 [profile.dev]
 panic = "abort"
 lto = true # Link Time Optimization usually reduces size of binaries and static libraries

libraries/crypto/Cargo.toml

@@ -25,6 +25,7 @@ regex = { version = "1", optional = true }
 # Cargo.toml don't parse with the nightly compiler used by Tock. Remove this
 # whole block once CTAP is a library.
 bumpalo = "=3.8.0" # transitive dependency of ring
+once_cell = { version = "=1.14", optional = true } # transitive dependency of ring
 
 [features]
 std = ["hex", "ring", "rng256/std", "untrusted", "serde", "serde_json", "regex"]

src/api/customization.rs

@@ -294,7 +294,7 @@ pub const DEFAULT_CUSTOMIZATION: CustomizationImpl = CustomizationImpl {
     max_large_blob_array_size: 2048,
     max_rp_ids_length: 8,
     max_supported_resident_keys: 150,
-    slot_count: 1,
+    slot_count: 8,
 };
 
 impl Customization for CustomizationImpl {

src/ctap/client_pin.rs

@@ -344,9 +344,8 @@ impl ClientPin {
 
         self.pin_protocol_v1.reset_pin_uv_auth_token(env.rng());
         self.pin_protocol_v2.reset_pin_uv_auth_token(env.rng());
-        // TODO: pass the slot_id parameter and store it in the state too.
         self.pin_uv_auth_token_state
-            .begin_using_pin_uv_auth_token(now);
+            .begin_using_pin_uv_auth_token(now, slot_id);
         self.pin_uv_auth_token_state.set_default_permissions();
         let pin_uv_auth_token = shared_secret.encrypt(
             env.rng(),
@@ -581,9 +580,23 @@ impl ClientPin {
         self.pin_uv_auth_token_state.has_permissions_rp_id(rp_id)
     }
 
+    /// Get the slot_id_in_use of the current pin_uv_auth_token_state if multi-PIN
+    /// feature is enabled. Otherwise return the default slot (0).
+    pub fn get_slot_id_in_use_or_default(
+        &self,
+        env: &mut impl Env,
+    ) -> Result<Option<usize>, Ctap2StatusCode> {
+        if storage::has_multi_pin(env)? {
+            Ok(self.pin_uv_auth_token_state.slot_id_in_use())
+        } else {
+            Ok(Some(0))
+        }
+    }
+
     #[cfg(test)]
     pub fn new_test(
         env: &mut impl Env,
+        slot_id: usize,
         key_agreement_key: crypto::ecdh::SecKey,
         pin_uv_auth_token: [u8; PIN_TOKEN_LENGTH],
         pin_uv_auth_protocol: PinUvAuthProtocol,
@@ -594,7 +607,7 @@ impl ClientPin {
         };
         let mut pin_uv_auth_token_state = PinUvAuthTokenState::new();
         pin_uv_auth_token_state.set_permissions(0xFF);
-        pin_uv_auth_token_state.begin_using_pin_uv_auth_token(CtapInstant::new(0));
+        pin_uv_auth_token_state.begin_using_pin_uv_auth_token(CtapInstant::new(0), slot_id);
         ClientPin {
             pin_protocol_v1: PinProtocol::new_test(key_agreement_key_v1, pin_uv_auth_token),
             pin_protocol_v2: PinProtocol::new_test(key_agreement_key_v2, pin_uv_auth_token),
@@ -637,6 +650,7 @@ mod test {
     /// tests using the wrong combination of PIN protocol and shared secret
     /// should fail.
     fn create_client_pin_and_shared_secret(
+        slot_id: usize,
         pin_uv_auth_protocol: PinUvAuthProtocol,
     ) -> (ClientPin, Box<dyn SharedSecret>) {
         let mut env = TestEnv::new();
@@ -646,6 +660,7 @@ mod test {
         let pin_uv_auth_token = [0x91; PIN_TOKEN_LENGTH];
         let client_pin = ClientPin::new_test(
             &mut env,
+            slot_id,
             key_agreement_key,
             pin_uv_auth_token,
             pin_uv_auth_protocol,
@@ -665,7 +680,9 @@ mod test {
         sub_command: ClientPinSubCommand,
     ) -> (ClientPin, AuthenticatorClientPinParameters) {
         let mut env = TestEnv::new();
-        let (client_pin, shared_secret) = create_client_pin_and_shared_secret(pin_uv_auth_protocol);
+        // TODO: Make slot_id a passed parameter once we include it in AuthenticatorClientPinParameters.
+        let (client_pin, shared_secret) =
+            create_client_pin_and_shared_secret(0, pin_uv_auth_protocol);
 
         let pin = b"1234";
         let mut padded_pin = [0u8; 64];
@@ -1284,7 +1301,8 @@ mod test {
         salt: Vec<u8>,
     ) -> Result<Vec<u8>, Ctap2StatusCode> {
         let mut env = TestEnv::new();
-        let (client_pin, shared_secret) = create_client_pin_and_shared_secret(pin_uv_auth_protocol);
+        let (client_pin, shared_secret) =
+            create_client_pin_and_shared_secret(0, pin_uv_auth_protocol);
 
         let salt_enc = shared_secret.as_ref().encrypt(env.rng(), &salt).unwrap();
         let salt_auth = shared_secret.authenticate(&salt_enc);
@@ -1302,7 +1320,8 @@ mod test {
 
     fn test_helper_process_hmac_secret_bad_salt_auth(pin_uv_auth_protocol: PinUvAuthProtocol) {
         let mut env = TestEnv::new();
-        let (client_pin, shared_secret) = create_client_pin_and_shared_secret(pin_uv_auth_protocol);
+        let (client_pin, shared_secret) =
+            create_client_pin_and_shared_secret(0, pin_uv_auth_protocol);
         let cred_random = [0xC9; 32];
 
         let salt_enc = vec![0x01; 32];
@@ -1515,7 +1534,7 @@ mod test {
         let message = [0xAA];
         client_pin
             .pin_uv_auth_token_state
-            .begin_using_pin_uv_auth_token(CtapInstant::new(0));
+            .begin_using_pin_uv_auth_token(CtapInstant::new(0), 0);
 
         let pin_uv_auth_token_v1 = client_pin
             .get_pin_protocol(PinUvAuthProtocol::V1)
@@ -1532,6 +1551,7 @@ mod test {
         let pin_uv_auth_param_v2_from_v1_token =
             authenticate_pin_uv_auth_token(pin_uv_auth_token_v1, &message, PinUvAuthProtocol::V2);
 
+        assert_eq!(client_pin.pin_uv_auth_token_state.slot_id_in_use(), Some(0));
         assert_eq!(
             client_pin.verify_pin_uv_auth_token(
                 &message,

src/ctap/config_command.rs

@@ -91,10 +91,12 @@ pub fn process_config(
         pin_uv_auth_protocol,
     } = params;
 
-    // TODO: Get the slot id from active token state when multi-PIN is enabled.
+    let slot_id = client_pin.get_slot_id_in_use_or_default(env)?;
-    let slot_id = Some(0);
     let enforce_uv =
         !matches!(sub_command, ConfigSubCommand::ToggleAlwaysUv) && storage::has_always_uv(env)?;
+    // If multi-PIN feature is enabled, no PIN is in use, and the command is to turn off alwaysUv,
+    // the PIN check will be skipped here but an OPERATION_DENIED will still be returned later,
+    // which is correct behavior.
     if (slot_id.is_some() && storage::pin_hash(env, slot_id.unwrap())?.is_some()) || enforce_uv {
         let pin_uv_auth_param =
             pin_uv_auth_param.ok_or(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED)?;
@@ -143,6 +145,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let mut client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,
@@ -174,6 +177,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let mut client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,
@@ -213,6 +217,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let mut client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             pin_uv_auth_protocol,
@@ -287,6 +292,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let mut client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,
@@ -335,6 +341,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let mut client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,
@@ -415,6 +422,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let mut client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,
@@ -442,6 +450,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let mut client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,
@@ -477,6 +486,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let mut client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,

src/ctap/credential_management.rs

@@ -394,6 +394,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             pin_uv_auth_protocol,
@@ -480,6 +481,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,
@@ -578,6 +580,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,
@@ -663,6 +666,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,
@@ -769,6 +773,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,
@@ -843,6 +848,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,

src/ctap/large_blobs.rs

@@ -87,9 +87,9 @@ impl LargeBlobs {
             if offset != self.expected_next_offset {
                 return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_SEQ);
             }
-            // TODO: Get the slot id from active token state when multi-PIN is enabled.
+            // We only check whether slot 0 has a PIN here because if multi-PIN is enabled,
-            let slot_id = 0;
+            // has_always_uv will always be true and the condition will always pass.
-            if storage::pin_hash(env, slot_id)?.is_some() || storage::has_always_uv(env)? {
+            if storage::pin_hash(env, 0)?.is_some() || storage::has_always_uv(env)? {
                 let pin_uv_auth_param =
                     pin_uv_auth_param.ok_or(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED)?;
                 let pin_uv_auth_protocol =
@@ -151,6 +151,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let mut client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,
@@ -186,6 +187,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let mut client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,
@@ -252,6 +254,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let mut client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,
@@ -302,6 +305,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let mut client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,
@@ -352,6 +356,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let mut client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,
@@ -379,6 +384,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let mut client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,
@@ -411,6 +417,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let mut client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             pin_uv_auth_protocol,

src/ctap/mod.rs

@@ -778,8 +778,7 @@ impl CtapState {
             enterprise_attestation,
         } = make_credential_params;
 
-        // TODO: Get the slot id from active token state when multi-PIN is enabled.
+        let slot_id = self.client_pin.get_slot_id_in_use_or_default(env)?;
-        let slot_id = Some(0);
 
         self.pin_uv_auth_precheck(
             env,
@@ -1175,8 +1174,7 @@ impl CtapState {
             pin_uv_auth_protocol,
         } = get_assertion_params;
 
-        // TODO: Get the slot id from active token state when multi-PIN is enabled.
+        let slot_id = self.client_pin.get_slot_id_in_use_or_default(env)?;
-        let slot_id = Some(0);
 
         self.pin_uv_auth_precheck(
             env,
@@ -2154,6 +2152,7 @@ mod test {
         let pin_uv_auth_token = [0x91; PIN_TOKEN_LENGTH];
         let client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             pin_uv_auth_protocol,
@@ -3067,6 +3066,7 @@ mod test {
         let pin_uv_auth_token = [0x88; 32];
         let client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             pin_uv_auth_protocol,
@@ -3800,6 +3800,7 @@ mod test {
         let pin_uv_auth_token = [0x55; 32];
         let client_pin = ClientPin::new_test(
             &mut env,
+            0,
             key_agreement_key,
             pin_uv_auth_token,
             PinUvAuthProtocol::V1,

src/ctap/storage.rs

@@ -612,6 +612,25 @@ pub fn toggle_always_uv(env: &mut impl Env) -> Result<(), Ctap2StatusCode> {
     }
 }
 
+/// Returns whether multi-PIN is enabled.
+pub fn has_multi_pin(env: &mut impl Env) -> Result<bool, Ctap2StatusCode> {
+    match env.store().find(key::MULTI_PIN)? {
+        None => Ok(false),
+        Some(value) if value.is_empty() => Ok(true),
+        _ => Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR),
+    }
+}
+
+// TODO: Call this in config_commands after the whole multi-PIN feature is ready.
+// Before that, this function should stay private, only for testing purpose.
+/// Enables multi-PIN, when disabled.
+fn _enable_multi_pin(env: &mut impl Env) -> Result<(), Ctap2StatusCode> {
+    if !has_multi_pin(env)? {
+        env.store().insert(key::MULTI_PIN, &[])?;
+    }
+    Ok(())
+}
+
 impl From<persistent_store::StoreError> for Ctap2StatusCode {
     fn from(error: persistent_store::StoreError) -> Ctap2StatusCode {
         use persistent_store::StoreError;
@@ -1241,6 +1260,15 @@ mod test {
         }
     }
 
+    #[test]
+    fn test_multi_pin() {
+        let mut env = TestEnv::new();
+
+        assert!(!has_multi_pin(&mut env).unwrap());
+        assert_eq!(_enable_multi_pin(&mut env), Ok(()));
+        assert!(has_multi_pin(&mut env).unwrap());
+    }
+
     #[test]
     fn test_serialize_deserialize_credential() {
         let mut env = TestEnv::new();

src/ctap/storage/key.rs

@@ -73,6 +73,11 @@ make_partition! {
     // - When adding a (non-persistent) key below this message, make sure its value is bigger or
     //   equal than NUM_PERSISTENT_KEYS.
 
+    /// Whether multi-PIN is enabled.
+    ///
+    /// The value must be empty. Only presence of the value matters.
+    MULTI_PIN = 983;
+
     // Start of key arrays for multi-PIN feature: these fields are separated for each slots, so
     // a unique key is needed for each slot. However, we reuse the existing fields and rename them
     // to `FIRST_{KEY_NAME}` so the upgrade is backward compatible.

src/ctap/token_state.rs

@@ -41,7 +41,7 @@ pub struct PinUvAuthTokenState {
     permissions_rp_id: Option<String>,
     usage_timer: TimedPermission,
     user_verified: bool,
-    in_use: bool,
+    slot_id_in_use: Option<usize>,
 }
 
 impl PinUvAuthTokenState {
@@ -52,13 +52,18 @@ impl PinUvAuthTokenState {
             permissions_rp_id: None,
             usage_timer: TimedPermission::waiting(),
             user_verified: false,
-            in_use: false,
+            slot_id_in_use: None,
         }
     }
 
     /// Returns whether the pinUvAuthToken is active.
     pub fn is_in_use(&self) -> bool {
-        self.in_use
+        self.slot_id_in_use.is_some()
+    }
+
+    /// Returns the slot id in use.
+    pub fn slot_id_in_use(&self) -> Option<usize> {
+        self.slot_id_in_use
     }
 
     /// Checks if the permission is granted.
@@ -113,15 +118,15 @@ impl PinUvAuthTokenState {
     }
 
     /// Starts the timer for pinUvAuthToken usage.
-    pub fn begin_using_pin_uv_auth_token(&mut self, now: CtapInstant) {
+    pub fn begin_using_pin_uv_auth_token(&mut self, now: CtapInstant, slot_id: usize) {
         self.user_verified = true;
         self.usage_timer = TimedPermission::granted(now, INITIAL_USAGE_TIME_LIMIT);
-        self.in_use = true;
+        self.slot_id_in_use = Some(slot_id);
     }
 
     /// Updates the usage timer, and disables the pinUvAuthToken on timeout.
     pub fn pin_uv_auth_token_usage_timer_observer(&mut self, now: CtapInstant) {
-        if !self.in_use {
+        if !self.is_in_use() {
             return;
         }
         self.usage_timer = self.usage_timer.check_expiration(now);
@@ -132,7 +137,7 @@ impl PinUvAuthTokenState {
 
     /// Returns whether the user is verified.
     pub fn get_user_verified_flag_value(&self) -> bool {
-        self.in_use && self.user_verified
+        self.is_in_use() && self.user_verified
     }
 
     /// Consumes the user verification.
@@ -151,7 +156,7 @@ impl PinUvAuthTokenState {
         self.permissions_set = 0;
         self.usage_timer = TimedPermission::waiting();
         self.user_verified = false;
-        self.in_use = false;
+        self.slot_id_in_use = None;
     }
 }
 
@@ -164,24 +169,28 @@ mod test {
     fn test_observer() {
         let mut token_state = PinUvAuthTokenState::new();
         let mut now: CtapInstant = CtapInstant::new(0);
-        token_state.begin_using_pin_uv_auth_token(now);
+        token_state.begin_using_pin_uv_auth_token(now, 0);
         assert!(token_state.is_in_use());
+        assert_eq!(token_state.slot_id_in_use(), Some(0));
         now = now + Milliseconds(100_u32);
         token_state.pin_uv_auth_token_usage_timer_observer(now);
         assert!(token_state.is_in_use());
         now = now + INITIAL_USAGE_TIME_LIMIT;
         token_state.pin_uv_auth_token_usage_timer_observer(now);
         assert!(!token_state.is_in_use());
+        assert!(token_state.slot_id_in_use().is_none());
     }
 
     #[test]
     fn test_stop() {
         let mut token_state = PinUvAuthTokenState::new();
         let now: CtapInstant = CtapInstant::new(0);
-        token_state.begin_using_pin_uv_auth_token(now);
+        token_state.begin_using_pin_uv_auth_token(now, 0);
         assert!(token_state.is_in_use());
+        assert_eq!(token_state.slot_id_in_use(), Some(0));
         token_state.stop_using_pin_uv_auth_token();
         assert!(!token_state.is_in_use());
+        assert!(token_state.slot_id_in_use().is_none());
     }
 
     #[test]
@@ -265,11 +274,11 @@ mod test {
         let mut token_state = PinUvAuthTokenState::new();
         assert!(!token_state.get_user_verified_flag_value());
         let now: CtapInstant = CtapInstant::new(0);
-        token_state.begin_using_pin_uv_auth_token(now);
+        token_state.begin_using_pin_uv_auth_token(now, 0);
         assert!(token_state.get_user_verified_flag_value());
         token_state.clear_user_verified_flag();
         assert!(!token_state.get_user_verified_flag_value());
-        token_state.begin_using_pin_uv_auth_token(now);
+        token_state.begin_using_pin_uv_auth_token(now, 0);
         assert!(token_state.get_user_verified_flag_value());
         token_state.stop_using_pin_uv_auth_token();
         assert!(!token_state.get_user_verified_flag_value());