From 68678ed4e1848db5c0fe2c99a7a1523b06a8e312 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 2 May 2024 10:31:21 +0200 Subject: [PATCH 1/4] Refactor COSE serialization and deserialization This patch introduces the RawPublicKey helper type that takes care of serialization and deserialization of the key type. The *PublicKey structs now only need to check if all required fields are present and have the correct value. It also adds extensive tests to make sure that serialization and deserialization work correctly. This patch is ported from: https://github.com/trussed-dev/ctap-types/pull/8 --- Cargo.toml | 10 +- src/lib.rs | 536 ++++++++++++++++++++++++++------------------------ tests/cose.rs | 170 ++++++++++++++++ 3 files changed, 460 insertions(+), 256 deletions(-) create mode 100644 tests/cose.rs diff --git a/Cargo.toml b/Cargo.toml index 083e3c0..c93a6d2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ categories = ["embedded", "encoding", "no-std"] keywords = ["cose", "cbor", "rust", "no-std"] repository = "https://github.com/ycrypto/cosey" readme = "README.md" -edition = "2018" +edition = "2021" [dependencies] heapless-bytes = "0.3.0" @@ -18,3 +18,11 @@ serde_repr = "0.1" version = "1.0" default-features = false features = ["derive"] + +[dev-dependencies] +cbor-smol = "0.4" +ciborium = "0.2.1" +hex = "0.4.3" +itertools = "0.12.0" +quickcheck = "1.0.3" +serde = "1" diff --git a/src/lib.rs b/src/lib.rs index 4707caf..5de79f2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -44,7 +44,11 @@ */ pub use heapless_bytes::Bytes; -use serde::Serialize; +use core::fmt::{self, Formatter}; +use serde::{ + de::{Error as _, Expected, MapAccess, Unexpected}, + Deserialize, Serialize, +}; use serde_repr::{Deserialize_repr, Serialize_repr}; #[repr(i8)] @@ -57,16 +61,41 @@ enum Label { Y = -3, } +struct TryFromIntError; + +impl TryFrom for Label { + type Error = TryFromIntError; + + fn try_from(label: i8) -> Result { + Ok(match label { + 1 => Self::Kty, + 3 => Self::Alg, + -1 => Self::Crv, + -2 => Self::X, + -3 => Self::Y, + _ => { + return Err(TryFromIntError); + } + }) + } +} + #[repr(i8)] -#[derive(Clone, Debug, Eq, PartialEq, Serialize_repr, Deserialize_repr)] +#[derive(Copy, Clone, Debug, Eq, PartialEq, Serialize_repr, Deserialize_repr)] enum Kty { Okp = 1, Ec2 = 2, Symmetric = 4, } +impl Expected for Kty { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "{}", *self as i8) + } +} + #[repr(i8)] -#[derive(Clone, Debug, Eq, PartialEq, Serialize_repr, Deserialize_repr)] +#[derive(Copy, Clone, Debug, Eq, PartialEq, Serialize_repr, Deserialize_repr)] enum Alg { Es256 = -7, // ECDSA with SHA-256 EdDsa = -8, @@ -86,8 +115,14 @@ enum Alg { EcdhEsHkdf256 = -25, // ES = ephemeral-static } +impl Expected for Alg { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "{}", *self as i8) + } +} + #[repr(i8)] -#[derive(Clone, Debug, Eq, PartialEq, Serialize_repr, Deserialize_repr)] +#[derive(Copy, Clone, Debug, Eq, PartialEq, Serialize_repr, Deserialize_repr)] enum Crv { None = 0, P256 = 1, @@ -99,6 +134,12 @@ enum Crv { // Ed448 = 7, } +impl Expected for Crv { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "{}", *self as i8) + } +} + // `Deserialize` can't be derived on untagged enum, // would need to "sniff" for correct (Kty, Alg, Crv) triple #[derive(Clone, Debug, Eq, PartialEq, Serialize)] @@ -134,13 +175,149 @@ impl From for PublicKey { } } +#[derive(Clone, Debug, Default)] +struct RawPublicKey { + kty: Option, + alg: Option, + crv: Option, + x: Option>, + y: Option>, +} + +impl<'de> Deserialize<'de> for RawPublicKey { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + struct IndexedVisitor; + impl<'de> serde::de::Visitor<'de> for IndexedVisitor { + type Value = RawPublicKey; + + fn expecting(&self, formatter: &mut core::fmt::Formatter) -> core::fmt::Result { + formatter.write_str("RawPublicKey") + } + + fn visit_map(self, mut map: V) -> Result + where + V: MapAccess<'de>, + { + #[derive(PartialEq)] + enum Key { + Label(Label), + Unknown(i8), + None, + } + + fn next_key<'a, V: MapAccess<'a>>(map: &mut V) -> Result { + let key: Option = map.next_key()?; + let key = match key { + Some(key) => match Label::try_from(key) { + Ok(label) => Key::Label(label), + Err(_) => Key::Unknown(key), + }, + None => Key::None, + }; + Ok(key) + } + + let mut public_key = RawPublicKey::default(); + + // As we cannot deserialize arbitrary values with cbor-smol, we do not support + // unknown keys before a known key. If there are unknown keys, they must be at the + // end. + + // only deserialize in canonical order + + let mut key = next_key(&mut map)?; + + if key == Key::Label(Label::Kty) { + public_key.kty = Some(map.next_value()?); + key = next_key(&mut map)?; + } + + if key == Key::Label(Label::Alg) { + public_key.alg = Some(map.next_value()?); + key = next_key(&mut map)?; + } + + if key == Key::Label(Label::Crv) { + public_key.crv = Some(map.next_value()?); + key = next_key(&mut map)?; + } + + if key == Key::Label(Label::X) { + public_key.x = Some(map.next_value()?); + key = next_key(&mut map)?; + } + + if key == Key::Label(Label::Y) { + public_key.y = Some(map.next_value()?); + key = next_key(&mut map)?; + } + + // if there is another key, it should be an unknown one + if matches!(key, Key::Label(_)) { + Err(serde::de::Error::custom( + "public key data in wrong order or with duplicates", + )) + } else { + Ok(public_key) + } + } + } + deserializer.deserialize_map(IndexedVisitor {}) + } +} + +impl Serialize for RawPublicKey { + fn serialize(&self, serializer: S) -> core::result::Result + where + S: serde::Serializer, + { + let is_set = [ + self.kty.is_some(), + self.alg.is_some(), + self.crv.is_some(), + self.x.is_some(), + self.y.is_some(), + ]; + let fields = is_set.into_iter().map(usize::from).sum(); + use serde::ser::SerializeMap; + let mut map = serializer.serialize_map(Some(fields))?; + + // 1: kty + if let Some(kty) = &self.kty { + map.serialize_entry(&(Label::Kty as i8), &(*kty as i8))?; + } + // 3: alg + if let Some(alg) = &self.alg { + map.serialize_entry(&(Label::Alg as i8), &(*alg as i8))?; + } + // -1: crv + if let Some(crv) = &self.crv { + map.serialize_entry(&(Label::Crv as i8), &(*crv as i8))?; + } + // -2: x + if let Some(x) = &self.x { + map.serialize_entry(&(Label::X as i8), x)?; + } + // -3: y + if let Some(y) = &self.y { + map.serialize_entry(&(Label::Y as i8), y)?; + } + + map.end() + } +} + trait PublicKeyConstants { const KTY: Kty; const ALG: Alg; const CRV: Crv; } -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq, Serialize)] +#[serde(into = "RawPublicKey")] pub struct P256PublicKey { pub x: Bytes<32>, pub y: Bytes<32>, @@ -152,7 +329,20 @@ impl PublicKeyConstants for P256PublicKey { const CRV: Crv = Crv::P256; } -#[derive(Clone, Debug, Eq, PartialEq)] +impl From for RawPublicKey { + fn from(key: P256PublicKey) -> Self { + Self { + kty: Some(P256PublicKey::KTY), + alg: Some(P256PublicKey::ALG), + crv: Some(P256PublicKey::CRV), + x: Some(key.x), + y: Some(key.y), + } + } +} + +#[derive(Clone, Debug, Eq, PartialEq, Serialize)] +#[serde(into = "RawPublicKey")] pub struct EcdhEsHkdf256PublicKey { pub x: Bytes<32>, pub y: Bytes<32>, @@ -164,7 +354,20 @@ impl PublicKeyConstants for EcdhEsHkdf256PublicKey { const CRV: Crv = Crv::P256; } -#[derive(Clone, Debug, Eq, PartialEq)] +impl From for RawPublicKey { + fn from(key: EcdhEsHkdf256PublicKey) -> Self { + Self { + kty: Some(EcdhEsHkdf256PublicKey::KTY), + alg: Some(EcdhEsHkdf256PublicKey::ALG), + crv: Some(EcdhEsHkdf256PublicKey::CRV), + x: Some(key.x), + y: Some(key.y), + } + } +} + +#[derive(Clone, Debug, Eq, PartialEq, Serialize)] +#[serde(into = "RawPublicKey")] pub struct Ed25519PublicKey { pub x: Bytes<32>, } @@ -175,7 +378,20 @@ impl PublicKeyConstants for Ed25519PublicKey { const CRV: Crv = Crv::Ed25519; } -#[derive(Clone, Debug, Default, Eq, PartialEq)] +impl From for RawPublicKey { + fn from(key: Ed25519PublicKey) -> Self { + Self { + kty: Some(Ed25519PublicKey::KTY), + alg: Some(Ed25519PublicKey::ALG), + crv: Some(Ed25519PublicKey::CRV), + x: Some(key.x), + y: None, + } + } +} + +#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize)] +#[serde(into = "RawPublicKey")] pub struct TotpPublicKey {} impl PublicKeyConstants for TotpPublicKey { @@ -184,109 +400,43 @@ impl PublicKeyConstants for TotpPublicKey { const CRV: Crv = Crv::None; } +impl From for RawPublicKey { + fn from(_key: TotpPublicKey) -> Self { + Self { + kty: Some(TotpPublicKey::KTY), + alg: Some(TotpPublicKey::ALG), + crv: None, + x: None, + y: None, + } + } +} + #[derive(Clone, Debug, Eq, PartialEq)] pub struct X25519PublicKey { pub pub_key: Bytes<32>, } -// impl serde::Serialize for PublicKey { -// fn serialize(&self, serializer: S) -> core::result::Result -// where -// S: serde::Serializer, -// { -// match self { -// PublicKey::P256Key(key) => key.serialize(serializer), -// PublicKey::EcdhEsHkdf256Key(key) => key.serialize(serializer), -// PublicKey::Ed25519Key(key) => key.serialize(serializer), -// } -// } -// } - -impl serde::Serialize for TotpPublicKey { - fn serialize(&self, serializer: S) -> core::result::Result - where - S: serde::Serializer, - { - use serde::ser::SerializeMap; - // let mut map = serializer.serialize_map(Some(3))?; - let mut map = serializer.serialize_map(Some(2))?; - - // 1: kty - map.serialize_entry(&(Label::Kty as i8), &(Self::KTY as i8))?; - // 3: alg - map.serialize_entry(&(Label::Alg as i8), &(Self::ALG as i8))?; - // // -1: crv - // map.serialize_entry(&(Label::Crv as i8), &(Self::CRV as i8))?; - - map.end() - } -} - -impl serde::Serialize for P256PublicKey { - fn serialize(&self, serializer: S) -> core::result::Result - where - S: serde::Serializer, - { - use serde::ser::SerializeMap; - let mut map = serializer.serialize_map(Some(5))?; - - // 1: kty - map.serialize_entry(&(Label::Kty as i8), &(Self::KTY as i8))?; - // 3: alg - map.serialize_entry(&(Label::Alg as i8), &(Self::ALG as i8))?; - // -1: crv - map.serialize_entry(&(Label::Crv as i8), &(Self::CRV as i8))?; - // -2: x - map.serialize_entry(&(Label::X as i8), &self.x)?; - // -3: y - map.serialize_entry(&(Label::Y as i8), &self.y)?; - - map.end() +fn check_key_constants( + kty: Option, + alg: Option, + crv: Option, +) -> 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)); } -} - -impl serde::Serialize for EcdhEsHkdf256PublicKey { - fn serialize(&self, serializer: S) -> core::result::Result - where - S: serde::Serializer, - { - use serde::ser::SerializeMap; - let mut map = serializer.serialize_map(Some(5))?; - - // 1: kty - map.serialize_entry(&(Label::Kty as i8), &(Self::KTY as i8))?; - // 3: alg - map.serialize_entry(&(Label::Alg as i8), &(Self::ALG as i8))?; - // -1: crv - map.serialize_entry(&(Label::Crv as i8), &(Self::CRV as i8))?; - // -2: x - map.serialize_entry(&(Label::X as i8), &self.x)?; - // -3: y - map.serialize_entry(&(Label::Y as i8), &self.y)?; - - map.end() + if alg != K::ALG { + return Err(E::invalid_value(Unexpected::Signed(alg as _), &K::ALG)); } -} - -impl serde::Serialize for Ed25519PublicKey { - fn serialize(&self, serializer: S) -> core::result::Result - where - S: serde::Serializer, - { - use serde::ser::SerializeMap; - let mut map = serializer.serialize_map(Some(4))?; - - // 1: kty - map.serialize_entry(&(Label::Kty as i8), &(Self::KTY as i8))?; - // 3: alg - map.serialize_entry(&(Label::Alg as i8), &(Self::ALG as i8))?; - // -1: crv - map.serialize_entry(&(Label::Crv as i8), &(Self::CRV as i8))?; - // -2: pub_key - map.serialize_entry(&(Label::X as i8), &self.x)?; - - map.end() + if K::CRV != Crv::None { + let crv = crv.ok_or_else(|| E::missing_field("crv"))?; + if crv != K::CRV { + return Err(E::invalid_value(Unexpected::Signed(crv as _), &K::CRV)); + } } + Ok(()) } impl<'de> serde::Deserialize<'de> for P256PublicKey { @@ -294,59 +444,17 @@ impl<'de> serde::Deserialize<'de> for P256PublicKey { where D: serde::Deserializer<'de>, { - struct IndexedVisitor; - impl<'de> serde::de::Visitor<'de> for IndexedVisitor { - type Value = P256PublicKey; - - fn expecting(&self, formatter: &mut core::fmt::Formatter) -> core::fmt::Result { - formatter.write_str("P256PublicKey") - } - - fn visit_map(self, mut map: V) -> Result - where - V: serde::de::MapAccess<'de>, - { - // implies kty-specific params - match (map.next_key()?, map.next_value()?) { - (Some(Label::Kty), Some(P256PublicKey::KTY)) => {} - _ => { - return Err(serde::de::Error::missing_field("kty")); - } - } - - // restricts key usage - check! - match (map.next_key()?, map.next_value()?) { - (Some(Label::Alg), Some(P256PublicKey::ALG)) => {} - _ => { - return Err(serde::de::Error::missing_field("alg")); - } - } - - match (map.next_key()?, map.next_value()?) { - (Some(Label::Crv), Some(P256PublicKey::CRV)) => {} - _ => { - return Err(serde::de::Error::missing_field("crv")); - } - } - - let x = match (map.next_key()?, map.next_value()?) { - (Some(Label::X), Some(bytes)) => bytes, - _ => { - return Err(serde::de::Error::missing_field("x")); - } - }; - - let y = match (map.next_key()?, map.next_value()?) { - (Some(Label::Y), Some(bytes)) => bytes, - _ => { - return Err(serde::de::Error::missing_field("y")); - } - }; - - Ok(P256PublicKey { x, y }) - } - } - deserializer.deserialize_map(IndexedVisitor {}) + let RawPublicKey { + kty, + alg, + crv, + x, + y, + } = RawPublicKey::deserialize(deserializer)?; + check_key_constants::(kty, alg, crv)?; + let x = x.ok_or_else(|| D::Error::missing_field("x"))?; + let y = y.ok_or_else(|| D::Error::missing_field("y"))?; + Ok(Self { x, y }) } } @@ -355,59 +463,17 @@ impl<'de> serde::Deserialize<'de> for EcdhEsHkdf256PublicKey { where D: serde::Deserializer<'de>, { - struct IndexedVisitor; - impl<'de> serde::de::Visitor<'de> for IndexedVisitor { - type Value = EcdhEsHkdf256PublicKey; - - fn expecting(&self, formatter: &mut core::fmt::Formatter) -> core::fmt::Result { - formatter.write_str("EcdhEsHkdf256PublicKey") - } - - fn visit_map(self, mut map: V) -> Result - where - V: serde::de::MapAccess<'de>, - { - // implies kty-specific params - match (map.next_key()?, map.next_value()?) { - (Some(Label::Kty), Some(EcdhEsHkdf256PublicKey::KTY)) => {} - _ => { - return Err(serde::de::Error::missing_field("kty")); - } - } - - // restricts key usage - check! - match (map.next_key()?, map.next_value()?) { - (Some(Label::Alg), Some(EcdhEsHkdf256PublicKey::ALG)) => {} - _ => { - return Err(serde::de::Error::missing_field("alg")); - } - } - - match (map.next_key()?, map.next_value()?) { - (Some(Label::Crv), Some(EcdhEsHkdf256PublicKey::CRV)) => {} - _ => { - return Err(serde::de::Error::missing_field("crv")); - } - } - - let x = match (map.next_key()?, map.next_value()?) { - (Some(Label::X), Some(bytes)) => bytes, - _ => { - return Err(serde::de::Error::missing_field("x")); - } - }; - - let y = match (map.next_key()?, map.next_value()?) { - (Some(Label::Y), Some(bytes)) => bytes, - _ => { - return Err(serde::de::Error::missing_field("y")); - } - }; - - Ok(EcdhEsHkdf256PublicKey { x, y }) - } - } - deserializer.deserialize_map(IndexedVisitor {}) + let RawPublicKey { + kty, + alg, + crv, + x, + y, + } = RawPublicKey::deserialize(deserializer)?; + check_key_constants::(kty, alg, crv)?; + let x = x.ok_or_else(|| D::Error::missing_field("x"))?; + let y = y.ok_or_else(|| D::Error::missing_field("y"))?; + Ok(Self { x, y }) } } @@ -416,51 +482,11 @@ impl<'de> serde::Deserialize<'de> for Ed25519PublicKey { where D: serde::Deserializer<'de>, { - struct IndexedVisitor; - impl<'de> serde::de::Visitor<'de> for IndexedVisitor { - type Value = Ed25519PublicKey; - - fn expecting(&self, formatter: &mut core::fmt::Formatter) -> core::fmt::Result { - formatter.write_str("Ed25519PublicKey") - } - - fn visit_map(self, mut map: V) -> Result - where - V: serde::de::MapAccess<'de>, - { - // implies kty-specific params - match (map.next_key()?, map.next_value()?) { - (Some(Label::Kty), Some(Ed25519PublicKey::KTY)) => {} - _ => { - return Err(serde::de::Error::missing_field("kty")); - } - } - - // restricts key usage - check! - match (map.next_key()?, map.next_value()?) { - (Some(Label::Alg), Some(Ed25519PublicKey::ALG)) => {} - _ => { - return Err(serde::de::Error::missing_field("alg")); - } - } - - match (map.next_key()?, map.next_value()?) { - (Some(Label::Crv), Some(Ed25519PublicKey::CRV)) => {} - _ => { - return Err(serde::de::Error::missing_field("crv")); - } - } - - let x = match (map.next_key()?, map.next_value()?) { - (Some(Label::X), Some(bytes)) => bytes, - _ => { - return Err(serde::de::Error::missing_field("x")); - } - }; - - Ok(Ed25519PublicKey { x }) - } - } - deserializer.deserialize_map(IndexedVisitor {}) + let RawPublicKey { + kty, alg, crv, x, .. + } = RawPublicKey::deserialize(deserializer)?; + check_key_constants::(kty, alg, crv)?; + let x = x.ok_or_else(|| D::Error::missing_field("x"))?; + Ok(Self { x }) } } diff --git a/tests/cose.rs b/tests/cose.rs new file mode 100644 index 0000000..6d8837c --- /dev/null +++ b/tests/cose.rs @@ -0,0 +1,170 @@ +use core::fmt::Debug; + +use cbor_smol::{cbor_deserialize, cbor_serialize_bytes}; +use ciborium::Value; +use cosey::{EcdhEsHkdf256PublicKey, Ed25519PublicKey, P256PublicKey}; +use heapless_bytes::Bytes; +use itertools::Itertools as _; +use quickcheck::{Arbitrary, Gen}; +use serde::{de::DeserializeOwned, Serialize}; + +#[derive(Clone, Debug)] +struct Input(Bytes<32>); + +impl Arbitrary for Input { + fn arbitrary(g: &mut Gen) -> Self { + let mut data = vec![0; 32]; + data.fill_with(|| u8::arbitrary(g)); + Self(Bytes::from_slice(&data).unwrap()) + } +} + +fn deserialize_map( + map: Vec<(Value, Value)>, +) -> (Result, Vec) { + let map = Value::Map(map); + let mut serialized: Vec = Default::default(); + ciborium::into_writer(&map, &mut serialized).unwrap(); + (cbor_deserialize(&serialized), serialized) +} + +fn print_input_output( + input: &T, + serialized: &[u8], + deserialized: &Result, +) { + println!("serialized:\n {}", hex::encode(serialized)); + println!("input:\n {:?}", input); + print!("deserialized:\n "); + if deserialized.as_ref() == Ok(input) { + println!("Ok(input)"); + } else { + println!("{:?}", deserialized); + } +} + +fn test_serde(data: T) -> bool { + let serialized: Bytes<1024> = cbor_serialize_bytes(&data).unwrap(); + let deserialized: T = cbor_deserialize(&serialized).unwrap(); + data == deserialized +} + +fn test_de(s: &str, data: T) { + let serialized = hex::decode(s).unwrap(); + let deserialized: T = cbor_deserialize(&serialized).unwrap(); + assert_eq!(data, deserialized); +} + +fn test_de_order(data: T) -> bool { + let serialized_value = Value::serialized(&data).unwrap(); + let canonical_fields = serialized_value.into_map().unwrap(); + + for fields in canonical_fields + .iter() + .cloned() + .permutations(canonical_fields.len()) + { + let is_canonical = fields == canonical_fields; + let (deserialized, serialized) = deserialize_map::(fields); + + // only the canonical order should be accepted + let is_success = if is_canonical { + Ok(&data) == deserialized.as_ref() + } else { + deserialized.is_err() + }; + + if !is_success { + if is_canonical { + println!("Expected correct deserialization for canonical order"); + } else { + println!("Expected error for non-canonical order"); + } + print_input_output(&data, &serialized, &deserialized); + return false; + } + } + + let mut fields = canonical_fields; + fields.push((Value::Integer(42.into()), Value::Text("foobar".to_owned()))); + fields.push((Value::Integer(24.into()), Value::Text("foobar".to_owned()))); + let (deserialized, serialized) = deserialize_map::(fields); + + // injecting an unsupported field should not change the result + let is_success = Ok(&data) == deserialized.as_ref(); + + if !is_success { + println!("Expected correct deserialization with unsupported fields"); + print_input_output(&data, &serialized, &deserialized); + } + + is_success +} + +#[test] +fn de_p256() { + let x = Bytes::from_slice(&[0xff; 32]).unwrap(); + let y = Bytes::from_slice(&[0xff; 32]).unwrap(); + let key = P256PublicKey { x, y }; + test_de("a5010203262001215820ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff225820ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", key); +} + +#[test] +fn de_ecdh() { + let x = Bytes::from_slice(&[0xff; 32]).unwrap(); + let y = Bytes::from_slice(&[0xff; 32]).unwrap(); + let key = EcdhEsHkdf256PublicKey { x, y }; + test_de("a501020338182001215820ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff225820ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", key); +} + +#[test] +fn de_ed25519() { + let x = Bytes::from_slice(&[0xff; 32]).unwrap(); + let key = Ed25519PublicKey { x }; + test_de( + "a4010103272006215820ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + key, + ); +} + +quickcheck::quickcheck! { + fn serde_p256(x: Input, y: Input) -> bool { + test_serde(P256PublicKey { + x: x.0, + y: y.0, + }) + } + + fn serde_ecdh(x: Input, y: Input) -> bool { + test_serde(EcdhEsHkdf256PublicKey { + x: x.0, + y: y.0, + }) + } + + fn serde_ed25519(x: Input) -> bool { + test_serde(Ed25519PublicKey { + x: x.0, + }) + } + + fn de_order_p256(x: Input, y: Input) -> bool { + test_de_order(P256PublicKey { + x: x.0, + y: y.0, + }) + } + + fn de_order_ecdh(x: Input, y: Input) -> bool { + test_de_order(EcdhEsHkdf256PublicKey { + x: x.0, + y: y.0, + }) + } + + fn de_order_ed25519(x: Input) -> bool { + test_de_order(Ed25519PublicKey { + x: x.0, + }) + } +} From 4ef0257b0777e04d4a351d9aa2d38d3ab7c44e7d Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 2 May 2024 17:11:17 +0200 Subject: [PATCH 2/4] Add CI workflow Fixes: https://github.com/trussed-dev/cosey/issues/6 --- .github/workflows/ci.yml | 58 ++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 2 +- 2 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..75d7167 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,58 @@ +name: CI + +on: [push, pull_request] + +jobs: + build: + name: Build library + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@master + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + override: true + - name: Check library + run: cargo check + + test: + name: Run tests + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@master + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + override: true + - name: Run tests + run: cargo test + + clippy: + name: Run clippy + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@master + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + override: true + components: "clippy" + - name: Run clippy + run: cargo clippy -- -D warnings + + fmt: + name: Run rustfmt + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@master + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + override: true + components: "rustfmt" + - name: Run rustfmt + run: cargo fmt -- --check diff --git a/src/lib.rs b/src/lib.rs index 5de79f2..d34c4b5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -43,8 +43,8 @@ } */ -pub use heapless_bytes::Bytes; use core::fmt::{self, Formatter}; +pub use heapless_bytes::Bytes; use serde::{ de::{Error as _, Expected, MapAccess, Unexpected}, Deserialize, Serialize, From 48d73a5ee0897f04e630889e8e0b99f2d544349d Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 2 May 2024 10:34:01 +0200 Subject: [PATCH 3/4] 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. Fixes: https://github.com/trussed-dev/cosey/issues/3 --- src/lib.rs | 7 +++--- tests/cose.rs | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index d34c4b5..08fa556 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -423,12 +423,13 @@ fn check_key_constants( crv: Option, ) -> 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 6d8837c..1e96473 100644 --- a/tests/cose.rs +++ b/tests/cose.rs @@ -55,6 +55,50 @@ fn test_de(s: &str, data: T) { assert_eq!(data, deserialized); } +fn test_de_alg( + data: T, + alg: Option, +) -> 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::(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(data: T) -> bool { let serialized_value = Value::serialized(&data).unwrap(); let canonical_fields = serialized_value.into_map().unwrap(); @@ -167,4 +211,24 @@ quickcheck::quickcheck! { x: x.0, }) } + + fn de_alg_p256(x: Input, y: Input, alg: Option) -> bool { + test_de_alg(P256PublicKey { + x: x.0, + y: y.0, + }, alg) + } + + fn de_alg_ecdh(x: Input, y: Input, alg: Option) -> bool { + test_de_alg(EcdhEsHkdf256PublicKey { + x: x.0, + y: y.0, + }, alg) + } + + fn de_alg_ed25519(x: Input, alg: Option) -> bool { + test_de_alg(Ed25519PublicKey { + x: x.0, + }, alg) + } } From 0fd493a7b9f24491a355c15c6e9cb29aace0a478 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 2 Jun 2024 12:15:07 +0200 Subject: [PATCH 4/4] Release v0.3.1 --- CHANGELOG.md | 35 +++++++++++++++++++++++++++++++++++ Cargo.toml | 2 +- 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..41997e7 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,35 @@ +# Changelog + +## Unreleased + +- + +[All Changes](https://github.com/trussed-dev/cosey/compare/v0.3.1...HEAD) + +## [v0.3.1](https://github.com/trussed-dev/cosey/releases/tag/v0.3.1) (2024-06-03) + +### Fixed + +- Accept keys without algorithm field ([#3](https://github.com/trussed-dev/cosey/issues/3)) + +[All Changes](https://github.com/trussed-dev/cosey/compare/v0.3.0...v0.3.1) + +## [v0.3.0](https://github.com/trussed-dev/cosey/releases/tag/v0.3.0) (2021-06-10) + +### Changed + +- Update `heapless-bytes` to v0.3.0 + +[All Changes](https://github.com/trussed-dev/cosey/compare/v0.2.0...v0.3.0) + +## [v0.2.0](https://github.com/trussed-dev/cosey/releases/tag/v0.2.0) (2021-02-26) + +### Changed + +- Update `heapless-bytes` to v0.2.0 + +[All Changes](https://github.com/trussed-dev/cosey/compare/v0.1.0...v0.2.0) + +## [v0.1.0](https://github.com/trussed-dev/cosey/releases/tag/v0.1.0) (2021-01-31) + +Initial release with support for EcdhEsHkdf256, Ed25519, P256, Totp and X255. diff --git a/Cargo.toml b/Cargo.toml index c93a6d2..5310399 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cosey" -version = "0.3.0" +version = "0.3.1" authors = ["Nicolas Stalder "] license = "Apache-2.0 OR MIT" description = "Data types and serde for public COSE_Keys"