From ccb85cefdee4a47e3ca592e7424398cff34037f1 Mon Sep 17 00:00:00 2001 From: Hugo Hakim Damer Date: Sun, 28 Jul 2024 16:34:46 +0200 Subject: [PATCH 1/4] header: fix authentication when protected header is zero-length map COSE allows an empty protected header to be encoded as a zero-length map, even though the standard encourages encoding empty protected headers as a zero-length string (RFC 2119 SHOULD according to RFC 9052, Section 3). However, according to RFC 9052, Section 4.4, 5.3 and 6.3, even if the header is encoded as a zero-length map, the structure used for authentication should not include the empty map if the protected header is empty ("if there are no protected attributes, a zero-length byte string is used"). This commit ensures that this behavior is implemented in coset, which previously did include the zero length map (encoded as h'a0') in signature calculation. This previously caused signature verification failures, e.g. when verifying the CoseSign1 object provided in https://github.com/cose-wg/Examples/blob/master/sign1-tests/sign-pass-03.json using coset. --- src/encrypt/mod.rs | 2 +- src/header/mod.rs | 13 +++++++++++++ src/mac/mod.rs | 2 +- src/sign/mod.rs | 9 ++++++--- 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/encrypt/mod.rs b/src/encrypt/mod.rs index db38de5..16f715b 100644 --- a/src/encrypt/mod.rs +++ b/src/encrypt/mod.rs @@ -496,7 +496,7 @@ pub fn enc_structure_data( ) -> Vec { let arr = vec![ Value::Text(context.text().to_owned()), - protected.cbor_bstr().expect("failed to serialize header"), // safe: always serializable + protected.to_be_authenticated().expect("failed to serialize header"), // safe: always serializable Value::Bytes(external_aad.to_vec()), ]; diff --git a/src/header/mod.rs b/src/header/mod.rs index c19835f..6eabcc3 100644 --- a/src/header/mod.rs +++ b/src/header/mod.rs @@ -385,6 +385,19 @@ impl ProtectedHeader { )) } + pub fn to_be_authenticated(self) -> Result { + let result = self.cbor_bstr()?; + // protected header might have been encoded as a zero length map, only containing + // the byte 0xA0 (see RFC 9052, Section 3). + // However, this byte should not be included during authentication (RFC 9052, Section 4.4, + // 5.3 and 6.3), so we need to return an empty byte string here instead. + if result.eq(&Value::Bytes(vec![0xA0])) { + Ok(Value::Bytes(vec![])) + } else { + Ok(result) + } + } + /// Indicate whether the `ProtectedHeader` is empty. pub fn is_empty(&self) -> bool { self.header.is_empty() diff --git a/src/mac/mod.rs b/src/mac/mod.rs index a16bfa3..f340a2e 100644 --- a/src/mac/mod.rs +++ b/src/mac/mod.rs @@ -336,7 +336,7 @@ pub fn mac_structure_data( ) -> Vec { let arr = vec![ Value::Text(context.text().to_owned()), - protected.cbor_bstr().expect("failed to serialize header"), // safe: always serializable + protected.to_be_authenticated().expect("failed to serialize header"), // safe: always serializable Value::Bytes(external_aad.to_vec()), Value::Bytes(payload.to_vec()), ]; diff --git a/src/sign/mod.rs b/src/sign/mod.rs index 141df2a..67f44d2 100644 --- a/src/sign/mod.rs +++ b/src/sign/mod.rs @@ -518,13 +518,16 @@ pub fn sig_structure_data( aad: &[u8], payload: &[u8], ) -> Vec { + let mut arr = vec![ Value::Text(context.text().to_owned()), - body.cbor_bstr().expect("failed to serialize header"), // safe: always serializable + body.to_be_authenticated().expect("failed to serialize header"), // safe: always serializable ]; if let Some(sign) = sign { - arr.push(sign.cbor_bstr().expect("failed to serialize header")); // safe: always - // serializable + arr.push( + sign.to_be_authenticated() + .expect("failed to serialize header") // safe: always serializable + ); } arr.push(Value::Bytes(aad.to_vec())); arr.push(Value::Bytes(payload.to_vec())); From d97cb143f1087cf97e7efc1315ac3582b5750e2f Mon Sep 17 00:00:00 2001 From: Hugo Hakim Damer Date: Wed, 31 Jul 2024 22:55:38 +0200 Subject: [PATCH 2/4] fixup! header: fix authentication when protected header is zero-length map --- src/encrypt/mod.rs | 4 +++- src/header/tests.rs | 2 +- src/mac/mod.rs | 4 +++- src/sign/mod.rs | 5 +++-- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/encrypt/mod.rs b/src/encrypt/mod.rs index 16f715b..b5f5a98 100644 --- a/src/encrypt/mod.rs +++ b/src/encrypt/mod.rs @@ -496,7 +496,9 @@ pub fn enc_structure_data( ) -> Vec { let arr = vec![ Value::Text(context.text().to_owned()), - protected.to_be_authenticated().expect("failed to serialize header"), // safe: always serializable + protected + .to_be_authenticated() + .expect("failed to serialize header"), // safe: always serializable Value::Bytes(external_aad.to_vec()), ]; diff --git a/src/header/tests.rs b/src/header/tests.rs index 9dff115..51e26f8 100644 --- a/src/header/tests.rs +++ b/src/header/tests.rs @@ -184,7 +184,7 @@ fn test_header_encode() { assert_eq!(*header, got.header); assert_eq!( *header_data, - hex::encode(&got.original_data.expect("missing original data")) + hex::encode(got.original_data.expect("missing original data")) ); } } diff --git a/src/mac/mod.rs b/src/mac/mod.rs index f340a2e..ada131a 100644 --- a/src/mac/mod.rs +++ b/src/mac/mod.rs @@ -336,7 +336,9 @@ pub fn mac_structure_data( ) -> Vec { let arr = vec![ Value::Text(context.text().to_owned()), - protected.to_be_authenticated().expect("failed to serialize header"), // safe: always serializable + protected + .to_be_authenticated() + .expect("failed to serialize header"), // safe: always serializable Value::Bytes(external_aad.to_vec()), Value::Bytes(payload.to_vec()), ]; diff --git a/src/sign/mod.rs b/src/sign/mod.rs index 67f44d2..b8281da 100644 --- a/src/sign/mod.rs +++ b/src/sign/mod.rs @@ -521,12 +521,13 @@ pub fn sig_structure_data( let mut arr = vec![ Value::Text(context.text().to_owned()), - body.to_be_authenticated().expect("failed to serialize header"), // safe: always serializable + body.to_be_authenticated() + .expect("failed to serialize header"), // safe: always serializable ]; if let Some(sign) = sign { arr.push( sign.to_be_authenticated() - .expect("failed to serialize header") // safe: always serializable + .expect("failed to serialize header"), // safe: always serializable ); } arr.push(Value::Bytes(aad.to_vec())); From 818c958ab789bf1d5be7a766ea53a995ba418bef Mon Sep 17 00:00:00 2001 From: Hugo Hakim Damer Date: Wed, 31 Jul 2024 22:57:36 +0200 Subject: [PATCH 3/4] fixup! header: fix authentication when protected header is zero-length map --- src/sign/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/sign/mod.rs b/src/sign/mod.rs index b8281da..e664072 100644 --- a/src/sign/mod.rs +++ b/src/sign/mod.rs @@ -518,7 +518,6 @@ pub fn sig_structure_data( aad: &[u8], payload: &[u8], ) -> Vec { - let mut arr = vec![ Value::Text(context.text().to_owned()), body.to_be_authenticated() @@ -527,7 +526,7 @@ pub fn sig_structure_data( if let Some(sign) = sign { arr.push( sign.to_be_authenticated() - .expect("failed to serialize header"), // safe: always serializable + .expect("failed to serialize header"), // safe: always serializable ); } arr.push(Value::Bytes(aad.to_vec())); From b501ff8d96582b49dd28129c477b9bb2665b62e9 Mon Sep 17 00:00:00 2001 From: Hugo Hakim Damer Date: Wed, 31 Jul 2024 23:01:39 +0200 Subject: [PATCH 4/4] fixup! header: fix authentication when protected header is zero-length map --- src/header/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/header/mod.rs b/src/header/mod.rs index 6eabcc3..12df533 100644 --- a/src/header/mod.rs +++ b/src/header/mod.rs @@ -385,6 +385,8 @@ impl ProtectedHeader { )) } + /// Convert this protected header into a [`Value`] that represents the binary data used for + /// authentication (i.e. for inclusion in `Sig_structure`, `Enc_structure` or `MAC_structure`). pub fn to_be_authenticated(self) -> Result { let result = self.cbor_bstr()?; // protected header might have been encoded as a zero length map, only containing