From 385f270ca66eb0a9659d347cafd891fe3ca66bee Mon Sep 17 00:00:00 2001 From: Zac Shenker Date: Sun, 18 Oct 2026 21:54:30 -0600 Subject: [PATCH 1/2] Reject COSE structure tags that disagree with the algorithm on decode to_bytes writes COSE_Mac0 (17) for MAC algorithms and COSE_Sign1 (18) for signature algorithms, but from_bytes discarded the tag, so a token tagged COSE_Mac0 carrying ES256 (or COSE_Sign1 carrying HMAC) decoded and verified. from_bytes now records any 17/18 tag among the leading tags and rejects it with InvalidFormat if it doesn't match alg.class(). Untagged and CWT-only (61) tokens are unchanged. Resolves the FOLLOWUPS item. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 +++ FOLLOWUPS.md | 20 ----------- src/lib.rs | 3 ++ src/tests_cose.rs | 88 +++++++++++++++++++++++++++++++++++++++++++++++ src/token.rs | 40 +++++++++++++++------ 5 files changed, 125 insertions(+), 30 deletions(-) create mode 100644 src/tests_cose.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e5cfd4..b876a1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 weak to be secure — a 512-bit modulus is factorable on commodity hardware — and neither the DER decoders nor PSS verification impose a floor, so without this an undersized key would sign and verify normally. +- `Token::from_bytes` rejects tokens whose COSE structure tag disagrees with + the protected header's algorithm. A COSE_Mac0 (17) tag with ES256 or PS256, + or a COSE_Sign1 (18) tag with HMAC, now fails with `Error::InvalidFormat`. + Untagged and CWT-tag-only tokens are accepted as before. ## [0.2.7] - 2025-11-05 diff --git a/FOLLOWUPS.md b/FOLLOWUPS.md index a39c6a3..fcf1003 100644 --- a/FOLLOWUPS.md +++ b/FOLLOWUPS.md @@ -30,26 +30,6 @@ classic JWT `alg`-confusion attack. independently and soon. Reviewed and consciously deferred (PR #5). - Location: `src/token.rs` — `Token::verify`. -### COSE structure tag and `alg` are not cross-checked on decode - -`from_bytes` decodes and discards the COSE structure tag (it skips up to two -tags as noise), so the tag and the protected-header `alg` are never compared. -`to_bytes` carefully selects the wire tag from the algorithm's class (COSE_Mac0 -= 17 for MAC, COSE_Sign1 = 18 for signature), but the inverse path never -enforces that invariant: a token tagged COSE_Mac0 (17) carrying `alg = -7` -(ES256), or tagged COSE_Sign1 (18) carrying `alg = 5` (HMAC), decodes and -verifies with no complaint. The class → tag invariant the encoder writes is -write-only. - -- **Fix direction:** when a recognized COSE structure tag (17/18) is present, - reject it if it disagrees with `alg.class()`. Tolerate the untagged / unknown - CWT-tag cases as today. -- **Why deferred:** this is the same "harden what the decoder trusts" policy as - the algorithm-confusion item above (what to reject vs. tolerate should be - designed once), and it pairs naturally with that PR. Low impact on its own — - only a malformed/contradictory producer is affected. -- Location: `src/token.rs` — `Token::from_bytes`. - ## Data model (separate PR — breaking) ### Array-valued `aud` is not readable through the typed API diff --git a/src/lib.rs b/src/lib.rs index 802af76..5d69a54 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -155,5 +155,8 @@ pub use utils::current_timestamp; /// Re-export minicbor for users of this crate pub use minicbor; +#[cfg(test)] +mod tests_cose; + #[cfg(test)] mod tests; diff --git a/src/tests_cose.rs b/src/tests_cose.rs new file mode 100644 index 0000000..cd024ba --- /dev/null +++ b/src/tests_cose.rs @@ -0,0 +1,88 @@ +//! Tests for COSE structure tag handling on decode + +use crate::{ + claims::RegisteredClaims, + header::{Algorithm, KeyId}, + token::{Token, TokenBuilder}, + Error, +}; +use ct_codecs::{Base64, Decoder}; + +const CWT: u8 = 0x3d; // tag 61, encoded as 0xd8 0x3d +const MAC0: u8 = 0xd1; // tag 17 +const SIGN1: u8 = 0xd2; // tag 18 + +// Same demo ES256 key as `src/tests.rs`; DO NOT USE IN PRODUCTION. +const ES256_PRIVATE_KEY_B64: &str = "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQg7BOlgwBOMKscTUCaG3RmlSCgUznDdxMn+9Pvoqp4pUOhRANCAARWMcvR3DnF1U15IvgcOyAxr3pJPfOHcF7ESuY+H+ya3LCH03PC1d99/XgN1ldF+wmMxVhY0w9iop10N6tNZDTg"; + +/// Sign a token with `alg` and return its encoding: tag 61, COSE tag, array. +fn token_bytes(alg: Algorithm) -> Vec { + let key = match alg { + Algorithm::HmacSha256 => b"cose-tag-test-key".to_vec(), + _ => Base64::decode_to_vec(ES256_PRIVATE_KEY_B64, None).unwrap(), + }; + let bytes = TokenBuilder::new() + .algorithm(alg) + .protected_key_id(KeyId::string("k")) + .registered_claims(RegisteredClaims::new().with_issuer("issuer")) + .sign(&key) + .unwrap() + .to_bytes() + .unwrap(); + assert_eq!(&bytes[..2], &[0xd8, CWT]); + bytes +} + +fn with_structure_tag(mut bytes: Vec, tag: u8) -> Vec { + bytes[2] = tag; + bytes +} + +#[test] +fn test_matching_structure_tag_is_accepted() { + assert!(Token::from_bytes(&token_bytes(Algorithm::HmacSha256)).is_ok()); + assert!(Token::from_bytes(&token_bytes(Algorithm::Es256)).is_ok()); +} + +#[test] +fn test_mac0_tag_with_signature_alg_is_rejected() { + let bytes = with_structure_tag(token_bytes(Algorithm::Es256), MAC0); + assert!(matches!( + Token::from_bytes(&bytes), + Err(Error::InvalidFormat(_)) + )); +} + +#[test] +fn test_sign1_tag_with_mac_alg_is_rejected() { + let bytes = with_structure_tag(token_bytes(Algorithm::HmacSha256), SIGN1); + assert!(matches!( + Token::from_bytes(&bytes), + Err(Error::InvalidFormat(_)) + )); +} + +#[test] +fn test_mismatch_is_rejected_without_cwt_tag() { + // COSE tag only, no leading CWT tag + let bytes = with_structure_tag(token_bytes(Algorithm::HmacSha256), SIGN1); + assert!(matches!( + Token::from_bytes(&bytes[2..]), + Err(Error::InvalidFormat(_)) + )); + let bytes = token_bytes(Algorithm::HmacSha256); + assert!(Token::from_bytes(&bytes[2..]).is_ok()); +} + +#[test] +fn test_untagged_and_cwt_only_tokens_are_still_accepted() { + for alg in [Algorithm::HmacSha256, Algorithm::Es256] { + let bytes = token_bytes(alg); + // Untagged: drop tag 61 and the COSE tag + assert!(Token::from_bytes(&bytes[3..]).is_ok()); + // CWT tag only: tag 61 followed directly by the array + let mut cwt_only = bytes[..2].to_vec(); + cwt_only.extend_from_slice(&bytes[3..]); + assert!(Token::from_bytes(&cwt_only).is_ok()); + } +} diff --git a/src/token.rs b/src/token.rs index 024da7b..3bc3d81 100644 --- a/src/token.rs +++ b/src/token.rs @@ -114,19 +114,25 @@ impl Token { /// Decode a token from CBOR bytes /// /// This function supports both COSE_Sign1 (tag 18) and COSE_Mac0 (tag 17) structures, - /// as well as custom tags. It will automatically skip any tags and process the underlying - /// CBOR array. + /// optionally preceded by the CWT tag (61), as well as untagged input. If a + /// COSE_Mac0 or COSE_Sign1 tag is present, it must match the class of the + /// protected header's algorithm (MAC or signature); a mismatch is rejected + /// with [`Error::InvalidFormat`]. pub fn from_bytes(bytes: &[u8]) -> Result { let mut dec = Decoder::new(bytes); - // Check if the token starts with a tag (COSE_Sign1 tag = 18, COSE_Mac0 tag = 17, or custom tag = 61) - if dec.datatype()? == minicbor::data::Type::Tag { - // Skip the tag - let _ = dec.tag()?; - - // Check for a second tag - if dec.datatype()? == minicbor::data::Type::Tag { - let _ = dec.tag()?; + // Accept up to two leading tags (e.g. CWT = 61 followed by COSE_Mac0 = 17 + // or COSE_Sign1 = 18), remembering any COSE structure tag so it can be + // checked against the algorithm below. + let mut structure_tag = None; + for _ in 0..2 { + if dec.datatype()? != minicbor::data::Type::Tag { + break; + } + let tag = dec.tag()?.as_u64(); + if tag == AlgorithmClass::Mac.cbor_tag() || tag == AlgorithmClass::Signature.cbor_tag() + { + structure_tag = Some(tag); } } @@ -152,6 +158,20 @@ impl Token { unprotected, }; + // The COSE structure tag must agree with the algorithm's class, the + // inverse of the invariant `to_bytes` writes: a COSE_Mac0 token cannot + // carry a signature algorithm and vice versa. Untagged tokens, and + // tokens with an unknown or missing algorithm (which `verify` rejects), + // are left as before. + if let (Some(tag), Some(alg)) = (structure_tag, header.algorithm()) { + if tag != alg.class().cbor_tag() { + return Err(Error::InvalidFormat(format!( + "COSE structure tag {tag} does not match algorithm {alg:?} (expected tag {})", + alg.class().cbor_tag() + ))); + } + } + // 3. Payload let claims_bytes = dec.bytes()?; let claims_map = decode_map(claims_bytes)?; From f1cc4184aa3da2ce82a3ed92ee21e4cfb381fb93 Mon Sep 17 00:00:00 2001 From: Zac Shenker Date: Mon, 19 Oct 2026 21:41:12 -0600 Subject: [PATCH 2/2] Report COSE tag/algorithm mismatches with a typed error Replace InvalidFormat(String) with Error::StructureTagMismatch { tag, algorithm, expected }, so callers can distinguish a structure that contradicts its declared algorithm from other malformed input without matching on message text. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 ++- src/error.rs | 14 ++++++++++++++ src/tests_cose.rs | 10 +++++++--- src/token.rs | 11 ++++++----- 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b876a1b..5979c1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,7 +50,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 this an undersized key would sign and verify normally. - `Token::from_bytes` rejects tokens whose COSE structure tag disagrees with the protected header's algorithm. A COSE_Mac0 (17) tag with ES256 or PS256, - or a COSE_Sign1 (18) tag with HMAC, now fails with `Error::InvalidFormat`. + or a COSE_Sign1 (18) tag with HMAC, now fails with the new + `Error::StructureTagMismatch { tag, algorithm, expected }`. Untagged and CWT-tag-only tokens are accepted as before. ## [0.2.7] - 2025-11-05 diff --git a/src/error.rs b/src/error.rs index 637babd..bf6400c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -39,6 +39,20 @@ pub enum Error { #[error("Signature verification failed. The token's signature does not match the expected signature")] SignatureVerification, + /// The COSE structure tag disagrees with the protected header's algorithm, + /// e.g. a COSE_Mac0 (17) token declaring a signature algorithm + #[error( + "COSE structure tag {tag} does not match algorithm {algorithm:?} (expected tag {expected})" + )] + StructureTagMismatch { + /// The COSE structure tag on the token (17 or 18) + tag: u64, + /// The algorithm declared in the protected header + algorithm: crate::header::Algorithm, + /// The tag that algorithm requires + expected: u64, + }, + /// Missing required claim #[error("Missing required claim: {0}. The token does not contain a required claim")] MissingClaim(String), diff --git a/src/tests_cose.rs b/src/tests_cose.rs index cd024ba..595522f 100644 --- a/src/tests_cose.rs +++ b/src/tests_cose.rs @@ -49,7 +49,11 @@ fn test_mac0_tag_with_signature_alg_is_rejected() { let bytes = with_structure_tag(token_bytes(Algorithm::Es256), MAC0); assert!(matches!( Token::from_bytes(&bytes), - Err(Error::InvalidFormat(_)) + Err(Error::StructureTagMismatch { + tag: 17, + algorithm: Algorithm::Es256, + expected: 18, + }) )); } @@ -58,7 +62,7 @@ fn test_sign1_tag_with_mac_alg_is_rejected() { let bytes = with_structure_tag(token_bytes(Algorithm::HmacSha256), SIGN1); assert!(matches!( Token::from_bytes(&bytes), - Err(Error::InvalidFormat(_)) + Err(Error::StructureTagMismatch { .. }) )); } @@ -68,7 +72,7 @@ fn test_mismatch_is_rejected_without_cwt_tag() { let bytes = with_structure_tag(token_bytes(Algorithm::HmacSha256), SIGN1); assert!(matches!( Token::from_bytes(&bytes[2..]), - Err(Error::InvalidFormat(_)) + Err(Error::StructureTagMismatch { .. }) )); let bytes = token_bytes(Algorithm::HmacSha256); assert!(Token::from_bytes(&bytes[2..]).is_ok()); diff --git a/src/token.rs b/src/token.rs index 3bc3d81..000b030 100644 --- a/src/token.rs +++ b/src/token.rs @@ -117,7 +117,7 @@ impl Token { /// optionally preceded by the CWT tag (61), as well as untagged input. If a /// COSE_Mac0 or COSE_Sign1 tag is present, it must match the class of the /// protected header's algorithm (MAC or signature); a mismatch is rejected - /// with [`Error::InvalidFormat`]. + /// with [`Error::StructureTagMismatch`]. pub fn from_bytes(bytes: &[u8]) -> Result { let mut dec = Decoder::new(bytes); @@ -165,10 +165,11 @@ impl Token { // are left as before. if let (Some(tag), Some(alg)) = (structure_tag, header.algorithm()) { if tag != alg.class().cbor_tag() { - return Err(Error::InvalidFormat(format!( - "COSE structure tag {tag} does not match algorithm {alg:?} (expected tag {})", - alg.class().cbor_tag() - ))); + return Err(Error::StructureTagMismatch { + tag, + algorithm: alg, + expected: alg.class().cbor_tag(), + }); } }