Test canonical encoding of serialized structs

This patch makes sure that all structs use the canonical CBOR encoding
as defined and required by § 8 Message Encoding of the CTAP
specification [0].

Structs using serde_indexed::SerializeIndexed are not tested because
in serde-indexed v0.1.1, the correct order is automatically enforced.

[0] https://fidoalliance.org/specs/fido-v2.2-ps-20250714/fido-client-to-authenticator-protocol-v2.2-ps-20250714.html#message-encoding

Author: robin-nitrokey

.github/workflows/ci.yml

@@ -64,6 +64,8 @@ jobs:
         override: true
     - name: Run tests
       run: cargo test
+    - name: Run tests with all features
+      run: cargo test --all-features
 
   clippy:
     name: Run clippy

Cargo.toml

@@ -22,6 +22,9 @@ serde_bytes = { version = "0.11.14", default-features = false }
 serde_repr = "0.1"
 
 [dev-dependencies]
+cbor-smol = "0.5"
+ciborium = "0.2"
+hex = "0.4"
 hex-literal = "0.4.1"
 serde_test = "1.0.176"
 

src/ctap2.rs

@@ -552,3 +552,18 @@ impl<A: Authenticator> crate::Rpc<Error, Request<'_>, Response> for A {
         self.call_ctap2(request)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_authenticator_options_canonical() {
+        let options = AuthenticatorOptions {
+            uv: Some(true),
+            up: Some(true),
+            rk: Some(true),
+        };
+        crate::test::assert_canonical_cbor(&options);
+    }
+}

src/ctap2/get_assertion.rs

@@ -163,3 +163,43 @@ impl ResponseBuilder {
 #[derive(Clone, Debug, Eq, PartialEq, Deserialize, Serialize)]
 #[non_exhaustive]
 pub struct UnsignedExtensionOutputs {}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_extensions_input_canonical() {
+        let input = ExtensionsInput {
+            hmac_secret: Some(HmacSecretInput {
+                key_agreement: EcdhEsHkdf256PublicKey {
+                    x: [0xff; 32].try_into().unwrap(),
+                    y: [0xff; 32].try_into().unwrap(),
+                },
+                salt_enc: [0xff; 80].try_into().unwrap(),
+                salt_auth: [0xff; 32].try_into().unwrap(),
+                pin_protocol: Some(1),
+            }),
+            large_blob_key: Some(true),
+            #[cfg(feature = "third-party-payment")]
+            third_party_payment: Some(true),
+        };
+        crate::test::assert_canonical_cbor(&input);
+    }
+
+    #[test]
+    fn test_extensions_output_canonical() {
+        let output = ExtensionsOutput {
+            hmac_secret: Some([0xff; 80].try_into().unwrap()),
+            #[cfg(feature = "third-party-payment")]
+            third_party_payment: Some(true),
+        };
+        crate::test::assert_canonical_cbor(&output);
+    }
+
+    #[test]
+    fn test_unsigned_extension_outputs_canonical() {
+        let outputs = UnsignedExtensionOutputs {};
+        crate::test::assert_canonical_cbor(&outputs);
+    }
+}

src/ctap2/get_info.rs

@@ -606,4 +606,55 @@ mod tests {
             ],
         );
     }
+
+    #[test]
+    fn test_ctap_options_canonical() {
+        let options = CtapOptions {
+            #[cfg(feature = "get-info-full")]
+            ep: Some(true),
+            rk: false,
+            up: true,
+            uv: Some(true),
+            plat: Some(true),
+            #[cfg(feature = "get-info-full")]
+            uv_acfg: Some(true),
+            #[cfg(feature = "get-info-full")]
+            always_uv: Some(true),
+            cred_mgmt: Some(true),
+            #[cfg(feature = "get-info-full")]
+            authnr_cfg: Some(true),
+            #[cfg(feature = "get-info-full")]
+            bio_enroll: Some(true),
+            client_pin: Some(true),
+            large_blobs: Some(true),
+            #[cfg(feature = "get-info-full")]
+            uv_bio_enroll: Some(true),
+            pin_uv_auth_token: Some(true),
+            #[cfg(feature = "get-info-full")]
+            set_min_pin_length: Some(true),
+            #[cfg(feature = "get-info-full")]
+            make_cred_uv_not_rqd: Some(true),
+            #[cfg(feature = "get-info-full")]
+            credential_mgmt_preview: Some(true),
+            #[cfg(feature = "get-info-full")]
+            user_verification_mgmt_preview: Some(true),
+            #[cfg(feature = "get-info-full")]
+            no_mc_ga_permissions_with_client_pin: Some(true),
+        };
+        crate::test::assert_canonical_cbor(&options);
+    }
+
+    #[test]
+    #[cfg(feature = "get-info-full")]
+    fn test_certifications_canonical() {
+        let certifications = Certifications {
+            fips_cmpv2: Some(1),
+            fips_cmpv3: Some(1),
+            fips_cmpv2_phy: Some(1),
+            fips_cmpv3_phy: Some(1),
+            cc_eal: Some(1),
+            fido: Some(1),
+        };
+        crate::test::assert_canonical_cbor(&certifications);
+    }
 }

src/ctap2/make_credential.rs

@@ -177,4 +177,22 @@ mod tests {
             assert_ser_tokens(&format, &[Token::BorrowedStr(s)]);
         }
     }
+
+    #[test]
+    fn test_extensions_canonical() {
+        let extensions = Extensions {
+            cred_protect: Some(1),
+            hmac_secret: Some(true),
+            large_blob_key: Some(true),
+            #[cfg(feature = "third-party-payment")]
+            third_party_payment: Some(true),
+        };
+        crate::test::assert_canonical_cbor(&extensions);
+    }
+
+    #[test]
+    fn test_unsigned_extension_outputs_canonical() {
+        let outputs = UnsignedExtensionOutputs {};
+        crate::test::assert_canonical_cbor(&outputs);
+    }
 }

src/lib.rs

@@ -33,6 +33,8 @@ pub mod ctap2;
 pub(crate) mod operation;
 pub use cbor_smol as serde;
 pub mod sizes;
+#[cfg(test)]
+mod test;
 pub mod webauthn;
 
 pub use ctap2::{Error, Result};
@@ -50,9 +52,6 @@ impl Display for TryFromStrError {
     }
 }
 
-#[cfg(test)]
-mod tests {}
-
 /// Call a remote procedure with a request, receive a response, maybe.
 pub trait Rpc<Error, Request, Response> {
     fn call(&mut self, request: &Request) -> core::result::Result<Response, Error>;

src/test.rs

@@ -0,0 +1,126 @@
+use ciborium::Value;
+use serde::Serialize;
+
+fn parse_argument(additional_info: u8, value: &[u8]) -> (u64, usize) {
+    match additional_info {
+        0..24 => (additional_info.into(), 0),
+        24 => (value[0].into(), 1),
+        25 => {
+            let (argument, _) = value.split_first_chunk().unwrap();
+            (u16::from_be_bytes(*argument).into(), argument.len())
+        }
+        26 => {
+            let (argument, _) = value.split_first_chunk().unwrap();
+            (u32::from_be_bytes(*argument).into(), argument.len())
+        }
+        27 => {
+            let (argument, _) = value.split_first_chunk().unwrap();
+            (u64::from_be_bytes(*argument), argument.len())
+        }
+        28..=30 => panic!("reserved additional info: {additional_info}"),
+        31 => panic!("indefinite length items are not allowed in canonical CBOR"),
+        32.. => panic!("illegal additional info: {additional_info}"),
+    }
+}
+
+fn parse_value<'a>(data: &'a [u8], path: &str) -> &'a [u8] {
+    println!("Checking value {path}");
+
+    assert!(data.len() > 0, "CBOR value must not be empty");
+    let mut offset = 1;
+    let major_type = (data[0] & 0b11100000) >> 5;
+    let additional_info = data[0] & 0b00011111;
+    let (argument, n) = parse_argument(additional_info, &data[offset..]);
+    offset += n;
+
+    // if the argument encodes an integer, it must be encoded as short as possible
+    if major_type <= 5 {
+        let expected = if let Ok(argument) = u8::try_from(argument) {
+            if argument <= 23 {
+                argument
+            } else {
+                24
+            }
+        } else if argument <= u16::MAX.into() {
+            25
+        } else if argument <= u32::MAX.into() {
+            26
+        } else {
+            27
+        };
+        assert_eq!(
+            additional_info, expected,
+            "integer value {argument} must use additional info {expected}"
+        );
+    }
+
+    let argument = usize::try_from(argument).unwrap();
+    match major_type {
+        0 | 1 | 7 => {
+            // no additional restrictions, no additional data
+        }
+        2 => {
+            // byte strings: no additional restrictions, but additional data
+            offset += argument;
+        }
+        3 => {
+            // text strings: must be valid UTF-8
+            let s = &data[offset..][..argument];
+            assert!(
+                str::from_utf8(s).is_ok(),
+                "text must be valid UTF-8: {}",
+                String::from_utf8_lossy(s)
+            );
+            offset += argument;
+        }
+        4 => {
+            // arrays: must have valid items
+            for i in 0..argument {
+                let item = parse_value(&data[offset..], &format!("{path}[{i}]"));
+                offset += item.len();
+            }
+        }
+        5 => {
+            // maps: must have valid items and be sorted
+            let mut last_key = None;
+            for i in 0..argument {
+                let key = parse_value(&data[offset..], &format!("{path}[{i}].key"));
+                offset += key.len();
+
+                let parsed_key: Value = ciborium::from_reader(key).unwrap();
+                println!("{path}[{i}].key = {parsed_key:?}");
+
+                let value = parse_value(&data[offset..], &format!("{path}[{i}].value"));
+                offset += value.len();
+
+                if let Some(last_key) = last_key {
+                    assert!(
+                        last_key < key,
+                        "map keys must be in lexicographical order: keys[{}] = {}, keys[{i}] = {}",
+                        i - 1,
+                        hex::encode(last_key),
+                        hex::encode(key)
+                    );
+                }
+                last_key = Some(key);
+            }
+        }
+        6 => {
+            panic!("tags are not allowed in canonical CBOR");
+        }
+        8.. => panic!("illegal major type: {major_type}"),
+    };
+
+    &data[..offset]
+}
+
+pub fn assert_canonical_cbor<T: Serialize>(object: &T) {
+    let mut buffer = [0; 1024];
+    let serialized = cbor_smol::cbor_serialize(&object, &mut buffer).unwrap();
+    let value = parse_value(&serialized, "root");
+    assert_eq!(
+        value.len(),
+        serialized.len(),
+        "CBOR data must not contain trailing bytes"
+    );
+}

src/webauthn.rs

@@ -288,4 +288,52 @@ mod tests {
         assert_eq!(truncate::<5>(s), s);
         assert_eq!(truncate::<64>(s), s);
     }
+
+    #[test]
+    fn test_rp_entity_canonical() {
+        let rp_entity = PublicKeyCredentialRpEntity {
+            id: String::try_from("test").unwrap(),
+            name: Some(String::try_from("test").unwrap()),
+            icon: Some(Icon),
+        };
+        crate::test::assert_canonical_cbor(&rp_entity);
+    }
+
+    #[test]
+    fn test_user_entity_canonical() {
+        let user_entity = PublicKeyCredentialUserEntity {
+            id: Bytes::try_from(b"test").unwrap(),
+            icon: Some(String::try_from("test").unwrap()),
+            name: Some(String::try_from("test").unwrap()),
+            display_name: Some(String::try_from("test").unwrap()),
+        };
+        crate::test::assert_canonical_cbor(&user_entity);
+    }
+
+    #[test]
+    fn test_cred_parameters_canonical() {
+        let cred_parameters = PublicKeyCredentialParameters {
+            alg: -1,
+            key_type: String::try_from("public-key").unwrap(),
+        };
+        crate::test::assert_canonical_cbor(&cred_parameters);
+    }
+
+    #[test]
+    fn test_cred_descriptor_canonical() {
+        let cred_descriptor = PublicKeyCredentialDescriptor {
+            id: Bytes::try_from(b"test").unwrap(),
+            key_type: String::try_from("test").unwrap(),
+        };
+        crate::test::assert_canonical_cbor(&cred_descriptor);
+    }
+
+    #[test]
+    fn test_cred_descriptor_ref_canonical() {
+        let cred_descriptor_ref = PublicKeyCredentialDescriptorRef {
+            id: serde_bytes::Bytes::new(b"test"),
+            key_type: "test",
+        };
+        crate::test::assert_canonical_cbor(&cred_descriptor_ref);
+    }
 }