From 2a563c82f162fe46ff8f12f2889dfccfdd55fcf1 Mon Sep 17 00:00:00 2001
From: Robin Krahl <robin@nitrokey.com>
Date: Wed, 15 Nov 2023 18:40:13 +0100
Subject: [PATCH] cose: Allow missing algorithms
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The algorithm field is optional, see RFC 8152 § 7:

   COSE_Key = {
       1 => tstr / int,          ; kty
       ? 2 => bstr,              ; kid
       ? 3 => tstr / int,        ; alg
       ? 4 => [+ (tstr / int) ], ; key_ops
       ? 5 => bstr,              ; Base IV
       * label => values
   }

   alg:  This parameter is used to restrict the algorithm that is used
      with the key.  If this parameter is present in the key structure,
      the application MUST verify that this algorithm matches the
      algorithm for which the key is being used.
---
 CHANGELOG.md  |  2 ++
 src/cose.rs   |  7 +++---
 tests/cose.rs | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 55c215b..b30bfa2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,7 +14,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Accept more than 12 algorithms ([#17][])
 - Add support for the `largeBlobKey` extension ([#18][])
 - Remove `AuthenticatorDataFlags::EMPTY` (use `AuthenticatorDataFlags::empty()` instead)
+- Allow missing algorithms in COSE keys ([#8][])
 
+[#8]: https://github.com/trussed-dev/ctap-types/pull/8
 [#9]: https://github.com/solokeys/ctap-types/issues/9
 [#30]: https://github.com/solokeys/fido-authenticator/issues/30
 [#13]: https://github.com/solokeys/ctap-types/issues/13
diff --git a/src/cose.rs b/src/cose.rs
index 86d2e9e..4e55c93 100644
--- a/src/cose.rs
+++ b/src/cose.rs
@@ -422,12 +422,13 @@ fn check_key_constants<K: PublicKeyConstants, E: serde::de::Error>(
     crv: Option<Crv>,
 ) -> Result<(), E> {
     let kty = kty.ok_or_else(|| E::missing_field("kty"))?;
-    let alg = alg.ok_or_else(|| E::missing_field("alg"))?;
     if kty != K::KTY {
         return Err(E::invalid_value(Unexpected::Signed(kty as _), &K::KTY));
     }
-    if alg != K::ALG {
-        return Err(E::invalid_value(Unexpected::Signed(alg as _), &K::ALG));
+    if let Some(alg) = alg {
+        if alg != K::ALG {
+            return Err(E::invalid_value(Unexpected::Signed(alg as _), &K::ALG));
+        }
     }
     if K::CRV != Crv::None {
         let crv = crv.ok_or_else(|| E::missing_field("crv"))?;
diff --git a/tests/cose.rs b/tests/cose.rs
index 89c8ca2..5e19325 100644
--- a/tests/cose.rs
+++ b/tests/cose.rs
@@ -54,6 +54,50 @@ fn test_de<T: DeserializeOwned + Debug + PartialEq>(s: &str, data: T) {
     assert_eq!(data, deserialized);
 }
 
+fn test_de_alg<T: Serialize + DeserializeOwned + Debug + PartialEq>(
+    data: T,
+    alg: Option<i8>,
+) -> bool {
+    let serialized_value = Value::serialized(&data).unwrap();
+    let mut fields = serialized_value.into_map().unwrap();
+    // this must be alg
+    assert_eq!(fields[1].0, Value::Integer(3.into()));
+
+    let expect_success = if let Some(alg) = alg {
+        // alg values may only work if they are correct
+        let alg = Value::Integer(alg.into());
+        if fields[1].1 == alg {
+            true
+        } else {
+            fields[1].1 = alg;
+            false
+        }
+    } else {
+        // deserialization without alg must work
+        fields.remove(1);
+        true
+    };
+
+    let (deserialized, serialized) = deserialize_map::<T>(fields);
+    let is_success = deserialized.is_ok() == expect_success;
+
+    if !is_success {
+        if alg.is_some() {
+            if expect_success {
+                println!("Expected correct deserialization for original algorithm");
+            } else {
+                println!("Expected error for invalid algorithm");
+            }
+        } else {
+            println!("Expected correct deserialization for missing algorithm");
+        }
+        println!("alg: {:?}", alg);
+        print_input_output(&data, &serialized, &deserialized);
+    }
+
+    is_success
+}
+
 fn test_de_order<T: Serialize + DeserializeOwned + Debug + PartialEq>(data: T) -> bool {
     let serialized_value = Value::serialized(&data).unwrap();
     let canonical_fields = serialized_value.into_map().unwrap();
@@ -166,4 +210,24 @@ quickcheck::quickcheck! {
             x: x.0,
         })
     }
+
+    fn de_alg_p256(x: Input, y: Input, alg: Option<i8>) -> bool {
+        test_de_alg(P256PublicKey {
+            x: x.0,
+            y: y.0,
+        }, alg)
+    }
+
+    fn de_alg_ecdh(x: Input, y: Input, alg: Option<i8>) -> bool {
+        test_de_alg(EcdhEsHkdf256PublicKey {
+            x: x.0,
+            y: y.0,
+        }, alg)
+    }
+
+    fn de_alg_ed25519(x: Input, alg: Option<i8>) -> bool {
+        test_de_alg(Ed25519PublicKey {
+            x: x.0,
+        }, alg)
+    }
 }