Skip to content

Commit

Permalink
cose: Allow missing algorithms
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
robin-nitrokey committed Nov 15, 2023
1 parent 07e8855 commit c448bb9
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 7 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Use references rather owned byte vectors to reduce the size of structs ([#16][])
- Accept more than 12 algorithms ([#17][])
- Add support for the `largeBlobKey` extension ([#18][])
- 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
Expand Down
7 changes: 4 additions & 3 deletions src/cose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,12 +413,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"))?;
Expand Down
71 changes: 67 additions & 4 deletions tests/cose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ impl Arbitrary for Input {
}
}

fn deserialize_map<T: DeserializeOwned>(
map: Vec<(Value, Value)>,
) -> (Result<T, cbor_smol::Error>, Vec<u8>) {
let map = Value::Map(map);
let mut serialized: Vec<u8> = Default::default();
ciborium::into_writer(&map, &mut serialized).unwrap();
(cbor_deserialize(&serialized), serialized)
}

fn test_serde<T: Serialize + DeserializeOwned + PartialEq>(data: T) -> bool {
let serialized: Bytes<1024> = cbor_serialize_bytes(&data).unwrap();
let deserialized: T = cbor_deserialize(&serialized).unwrap();
Expand All @@ -30,6 +39,43 @@ 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 success = deserialized.is_ok() == expect_success;

if !success {
println!("alg: {:?}", alg);
println!("expect_success: {}", expect_success);
println!("serialized: {}", hex::encode(&serialized));
println!("deserialized: {:?}", &data);
}

success
}

fn test_de_order<T: Serialize + DeserializeOwned + Debug>(data: T) -> bool {
let serialized_value = Value::serialized(&data).unwrap();
let canonical_fields = serialized_value.into_map().unwrap();
Expand All @@ -40,10 +86,7 @@ fn test_de_order<T: Serialize + DeserializeOwned + Debug>(data: T) -> bool {
.permutations(canonical_fields.len())
{
let is_canonical = fields == canonical_fields;
let map = Value::Map(fields);
let mut serialized: Vec<u8> = Default::default();
ciborium::into_writer(&map, &mut serialized).unwrap();
let deserialized = cbor_deserialize::<T>(&serialized);
let (deserialized, serialized) = deserialize_map::<T>(fields);

// only the canonical order should be accepted
if deserialized.is_ok() != is_canonical {
Expand Down Expand Up @@ -123,4 +166,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)
}
}

0 comments on commit c448bb9

Please sign in to comment.