Client pin features

Cargo.toml

@@ -31,6 +31,7 @@ with_ctap2_1 = []
 
 [dev-dependencies]
 elf2tab = "0.4.0"
+enum-iterator = "0.6.0"
 
 [build-dependencies]
 openssl = "0.10"

README.md

@@ -103,6 +103,13 @@ a few things you can personalize:
     When changing the default, resident credentials become undiscoverable without
     user verification. This helps privacy, but can make usage less comfortable
     for credentials that need less protection.
+6.  Increase the default minimum length for PINs in `ctap/storage.rs`.
+    The current minimum is 4. Values from 4 to 63 are allowed. Requiring longer
+    PINs can help establish trust between users and relying parties. It makes
+    user verification harder to break, but less convenient.
+    NIST recommends at least 6-digit PINs in section 5.1.9.1:
+    https://pages.nist.gov/800-63-3/sp800-63b.html
+    You can add relying parties to the list of readers of the minimum PIN length.
 
 ### 3D printed enclosure
 

src/ctap/command.rs

@@ -278,12 +278,32 @@ pub struct AuthenticatorClientPinParameters {
     pub pin_auth: Option<Vec<u8>>,
     pub new_pin_enc: Option<Vec<u8>>,
     pub pin_hash_enc: Option<Vec<u8>>,
+    #[cfg(feature = "with_ctap2_1")]
+    pub min_pin_length: Option<u8>,
+    #[cfg(feature = "with_ctap2_1")]
+    pub min_pin_length_rp_ids: Option<Vec<String>>,
+    #[cfg(feature = "with_ctap2_1")]
+    pub permissions: Option<u8>,
+    #[cfg(feature = "with_ctap2_1")]
+    pub permissions_rp_id: Option<String>,
 }
 
 impl TryFrom<cbor::Value> for AuthenticatorClientPinParameters {
     type Error = Ctap2StatusCode;
 
     fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
+        #[cfg(not(feature = "with_ctap2_1"))]
+        destructure_cbor_map! {
+            let {
+                1 => pin_protocol,
+                2 => sub_command,
+                3 => key_agreement,
+                4 => pin_auth,
+                5 => new_pin_enc,
+                6 => pin_hash_enc,
+            } = extract_map(cbor_value)?;
+        }
+        #[cfg(feature = "with_ctap2_1")]
         destructure_cbor_map! {
             let {
                 1 => pin_protocol,
@@ -292,6 +312,10 @@ impl TryFrom<cbor::Value> for AuthenticatorClientPinParameters {
                 4 => pin_auth,
                 5 => new_pin_enc,
                 6 => pin_hash_enc,
+                7 => min_pin_length,
+                8 => min_pin_length_rp_ids,
+                9 => permissions,
+                10 => permissions_rp_id,
             } = extract_map(cbor_value)?;
         }
 
@@ -304,6 +328,32 @@ impl TryFrom<cbor::Value> for AuthenticatorClientPinParameters {
         let pin_auth = pin_auth.map(extract_byte_string).transpose()?;
         let new_pin_enc = new_pin_enc.map(extract_byte_string).transpose()?;
         let pin_hash_enc = pin_hash_enc.map(extract_byte_string).transpose()?;
+        #[cfg(feature = "with_ctap2_1")]
+        let min_pin_length = min_pin_length
+            .map(extract_unsigned)
+            .transpose()?
+            .map(u8::try_from)
+            .transpose()
+            .map_err(|_| Ctap2StatusCode::CTAP2_ERR_PIN_POLICY_VIOLATION)?;
+        #[cfg(feature = "with_ctap2_1")]
+        let min_pin_length_rp_ids = match min_pin_length_rp_ids {
+            Some(entry) => Some(
+                extract_array(entry)?
+                    .into_iter()
+                    .map(extract_text_string)
+                    .collect::<Result<Vec<String>, Ctap2StatusCode>>()?,
+            ),
+            None => None,
+        };
+        #[cfg(feature = "with_ctap2_1")]
+        // We expect a bit field of 8 bits, and drop everything else.
+        // This means we ignore extensions in future versions.
+        let permissions = permissions
+            .map(extract_unsigned)
+            .transpose()?
+            .map(|p| p as u8);
+        #[cfg(feature = "with_ctap2_1")]
+        let permissions_rp_id = permissions_rp_id.map(extract_text_string).transpose()?;
 
         Ok(AuthenticatorClientPinParameters {
             pin_protocol,
@@ -312,6 +362,14 @@ impl TryFrom<cbor::Value> for AuthenticatorClientPinParameters {
             pin_auth,
             new_pin_enc,
             pin_hash_enc,
+            #[cfg(feature = "with_ctap2_1")]
+            min_pin_length,
+            #[cfg(feature = "with_ctap2_1")]
+            min_pin_length_rp_ids,
+            #[cfg(feature = "with_ctap2_1")]
+            permissions,
+            #[cfg(feature = "with_ctap2_1")]
+            permissions_rp_id,
         })
     }
 }
@@ -434,6 +492,9 @@ mod test {
 
     #[test]
     fn test_from_cbor_client_pin_parameters() {
+        // TODO(kaczmarczyck) inline the #cfg when #128 is resolved:
+        // https://github.com/google/OpenSK/issues/128
+        #[cfg(not(feature = "with_ctap2_1"))]
         let cbor_value = cbor_map! {
             1 => 1,
             2 => ClientPinSubCommand::GetPinRetries,
@@ -442,6 +503,19 @@ mod test {
             5 => vec! [0xCC],
             6 => vec! [0xDD],
         };
+        #[cfg(feature = "with_ctap2_1")]
+        let cbor_value = cbor_map! {
+            1 => 1,
+            2 => ClientPinSubCommand::GetPinRetries,
+            3 => cbor_map!{},
+            4 => vec! [0xBB],
+            5 => vec! [0xCC],
+            6 => vec! [0xDD],
+            7 => 4,
+            8 => cbor_array!["example.com"],
+            9 => 0x03,
+            10 => "example.com",
+        };
         let returned_pin_protocol_parameters =
             AuthenticatorClientPinParameters::try_from(cbor_value).unwrap();
 
@@ -452,6 +526,14 @@ mod test {
             pin_auth: Some(vec![0xBB]),
             new_pin_enc: Some(vec![0xCC]),
             pin_hash_enc: Some(vec![0xDD]),
+            #[cfg(feature = "with_ctap2_1")]
+            min_pin_length: Some(4),
+            #[cfg(feature = "with_ctap2_1")]
+            min_pin_length_rp_ids: Some(vec!["example.com".to_string()]),
+            #[cfg(feature = "with_ctap2_1")]
+            permissions: Some(0x03),
+            #[cfg(feature = "with_ctap2_1")]
+            permissions_rp_id: Some("example.com".to_string()),
         };
 
         assert_eq!(

src/ctap/data_formats.rs

@@ -18,6 +18,8 @@ use alloc::string::String;
 use alloc::vec::Vec;
 use core::convert::TryFrom;
 use crypto::{ecdh, ecdsa};
+#[cfg(test)]
+use enum_iterator::IntoEnumIterator;
 
 // https://www.w3.org/TR/webauthn/#dictdef-publickeycredentialrpentity
 #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))]
@@ -167,6 +169,7 @@ impl From<PublicKeyCredentialParameter> for cbor::Value {
 
 // https://www.w3.org/TR/webauthn/#enumdef-authenticatortransport
 #[cfg_attr(any(test, feature = "debug_ctap"), derive(Clone, Debug, PartialEq))]
+#[cfg_attr(test, derive(IntoEnumIterator))]
 pub enum AuthenticatorTransport {
     Usb,
     Nfc,
@@ -453,6 +456,7 @@ impl TryFrom<cbor::Value> for SignatureAlgorithm {
 
 #[derive(Clone, Copy, PartialEq, PartialOrd)]
 #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))]
+#[cfg_attr(test, derive(IntoEnumIterator))]
 pub enum CredentialProtectionPolicy {
     UserVerificationOptional = 0x01,
     UserVerificationOptionalWithCredentialIdList = 0x02,
@@ -520,17 +524,16 @@ impl From<PublicKeyCredentialSourceField> for cbor::KeyType {
 
 impl From<PublicKeyCredentialSource> for cbor::Value {
     fn from(credential: PublicKeyCredentialSource) -> cbor::Value {
-        use PublicKeyCredentialSourceField::*;
         let mut private_key = [0u8; 32];
         credential.private_key.to_bytes(&mut private_key);
         cbor_map_options! {
-            CredentialId => Some(credential.credential_id),
-            PrivateKey => Some(private_key.to_vec()),
-            RpId => Some(credential.rp_id),
-            UserHandle => Some(credential.user_handle),
-            OtherUi => credential.other_ui,
-            CredRandom => credential.cred_random,
-            CredProtectPolicy => credential.cred_protect_policy,
+            PublicKeyCredentialSourceField::CredentialId => Some(credential.credential_id),
+            PublicKeyCredentialSourceField::PrivateKey => Some(private_key.to_vec()),
+            PublicKeyCredentialSourceField::RpId => Some(credential.rp_id),
+            PublicKeyCredentialSourceField::UserHandle => Some(credential.user_handle),
+            PublicKeyCredentialSourceField::OtherUi => credential.other_ui,
+            PublicKeyCredentialSourceField::CredRandom => credential.cred_random,
+            PublicKeyCredentialSourceField::CredProtectPolicy => credential.cred_protect_policy,
         }
     }
 }
@@ -539,18 +542,15 @@ impl TryFrom<cbor::Value> for PublicKeyCredentialSource {
     type Error = Ctap2StatusCode;
 
     fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
-        use PublicKeyCredentialSourceField::{
-            CredProtectPolicy, CredRandom, CredentialId, OtherUi, PrivateKey, RpId, UserHandle,
-        };
         destructure_cbor_map! {
             let {
-                CredentialId => credential_id,
-                PrivateKey => private_key,
-                RpId => rp_id,
-                UserHandle => user_handle,
-                OtherUi => other_ui,
-                CredRandom => cred_random,
-                CredProtectPolicy => cred_protect_policy,
+                PublicKeyCredentialSourceField::CredentialId => credential_id,
+                PublicKeyCredentialSourceField::PrivateKey => private_key,
+                PublicKeyCredentialSourceField::RpId => rp_id,
+                PublicKeyCredentialSourceField::UserHandle => user_handle,
+                PublicKeyCredentialSourceField::OtherUi => other_ui,
+                PublicKeyCredentialSourceField::CredRandom => cred_random,
+                PublicKeyCredentialSourceField::CredProtectPolicy => cred_protect_policy,
             } = extract_map(cbor_value)?;
         }
 
@@ -681,29 +681,27 @@ impl TryFrom<CoseKey> for ecdh::PubKey {
     }
 }
 
-#[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))]
+#[cfg_attr(any(test, feature = "debug_ctap"), derive(Clone, Debug, PartialEq))]
+#[cfg_attr(test, derive(IntoEnumIterator))]
 pub enum ClientPinSubCommand {
-    GetPinRetries,
-    GetKeyAgreement,
-    SetPin,
-    ChangePin,
-    GetPinUvAuthTokenUsingPin,
-    GetPinUvAuthTokenUsingUv,
-    GetUvRetries,
+    GetPinRetries = 0x01,
+    GetKeyAgreement = 0x02,
+    SetPin = 0x03,
+    ChangePin = 0x04,
+    GetPinToken = 0x05,
+    #[cfg(feature = "with_ctap2_1")]
+    GetPinUvAuthTokenUsingUvWithPermissions = 0x06,
+    #[cfg(feature = "with_ctap2_1")]
+    GetUvRetries = 0x07,
+    #[cfg(feature = "with_ctap2_1")]
+    SetMinPinLength = 0x08,
+    #[cfg(feature = "with_ctap2_1")]
+    GetPinUvAuthTokenUsingPinWithPermissions = 0x09,
 }
 
 impl From<ClientPinSubCommand> for cbor::Value {
     fn from(subcommand: ClientPinSubCommand) -> Self {
-        match subcommand {
-            ClientPinSubCommand::GetPinRetries => 0x01,
-            ClientPinSubCommand::GetKeyAgreement => 0x02,
-            ClientPinSubCommand::SetPin => 0x03,
-            ClientPinSubCommand::ChangePin => 0x04,
-            ClientPinSubCommand::GetPinUvAuthTokenUsingPin => 0x05,
-            ClientPinSubCommand::GetPinUvAuthTokenUsingUv => 0x06,
-            ClientPinSubCommand::GetUvRetries => 0x07,
-        }
-        .into()
+        (subcommand as u64).into()
     }
 }
 
@@ -717,10 +715,18 @@ impl TryFrom<cbor::Value> for ClientPinSubCommand {
             0x02 => Ok(ClientPinSubCommand::GetKeyAgreement),
             0x03 => Ok(ClientPinSubCommand::SetPin),
             0x04 => Ok(ClientPinSubCommand::ChangePin),
-            0x05 => Ok(ClientPinSubCommand::GetPinUvAuthTokenUsingPin),
-            0x06 => Ok(ClientPinSubCommand::GetPinUvAuthTokenUsingUv),
+            0x05 => Ok(ClientPinSubCommand::GetPinToken),
+            #[cfg(feature = "with_ctap2_1")]
+            0x06 => Ok(ClientPinSubCommand::GetPinUvAuthTokenUsingUvWithPermissions),
+            #[cfg(feature = "with_ctap2_1")]
             0x07 => Ok(ClientPinSubCommand::GetUvRetries),
-            // TODO(kaczmarczyck) what is the correct status code for this error?
+            #[cfg(feature = "with_ctap2_1")]
+            0x08 => Ok(ClientPinSubCommand::SetMinPinLength),
+            #[cfg(feature = "with_ctap2_1")]
+            0x09 => Ok(ClientPinSubCommand::GetPinUvAuthTokenUsingPinWithPermissions),
+            #[cfg(feature = "with_ctap2_1")]
+            _ => Err(Ctap2StatusCode::CTAP2_ERR_INVALID_SUBCOMMAND),
+            #[cfg(not(feature = "with_ctap2_1"))]
             _ => Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER),
         }
     }
@@ -1157,6 +1163,12 @@ mod test {
         let policy_error = CredentialProtectionPolicy::try_from(cbor_policy_error);
         let expected_error = Err(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE);
         assert_eq!(policy_error, expected_error);
+
+        for policy in CredentialProtectionPolicy::into_enum_iter() {
+            let created_cbor: cbor::Value = policy.into();
+            let reconstructed = CredentialProtectionPolicy::try_from(created_cbor).unwrap();
+            assert_eq!(policy, reconstructed);
+        }
     }
 
     #[test]
@@ -1171,6 +1183,12 @@ mod test {
         );
         let created_cbor: cbor::Value = authenticator_transport.unwrap().into();
         assert_eq!(created_cbor, cbor_authenticator_transport);
+
+        for transport in AuthenticatorTransport::into_enum_iter() {
+            let created_cbor: cbor::Value = transport.clone().into();
+            let reconstructed = AuthenticatorTransport::try_from(created_cbor).unwrap();
+            assert_eq!(transport, reconstructed);
+        }
     }
 
     #[test]
@@ -1313,6 +1331,12 @@ mod test {
         assert_eq!(sub_command, Ok(expected_sub_command));
         let created_cbor: cbor::Value = sub_command.unwrap().into();
         assert_eq!(created_cbor, cbor_sub_command);
+
+        for command in ClientPinSubCommand::into_enum_iter() {
+            let created_cbor: cbor::Value = command.clone().into();
+            let reconstructed = ClientPinSubCommand::try_from(created_cbor).unwrap();
+            assert_eq!(command, reconstructed);
+        }
     }
 
     #[test]