Merge pull request #327 from chrysn-pull-requests/parce-ccs-fixes

parse_ccs fixes

shared/src/cred.rs

@@ -217,50 +217,93 @@ impl Credential {
     /// If the given value matches the shape lakers expects of a CCS, i.e. credentials from RFC9529,
     /// its public key and key ID are extracted into a full credential.
     pub fn parse_ccs(value: &[u8]) -> Result<Self, EDHOCError> {
-        const CCS_PREFIX_LEN: usize = 3;
-        const CNF_AND_COSE_KEY_PREFIX_LEN: usize = 8;
-        const COSE_KEY_FIRST_ITEMS_LEN: usize = 6;
+        let mut decoder = CBORDecoder::new(value);
+        if decoder.map()? != 2 {
+            // eg. no subject present
+            return Err(EDHOCError::ParsingError);
+        }
 
-        if value.len()
-            < 3 + CCS_PREFIX_LEN
-                + 1
-                + CNF_AND_COSE_KEY_PREFIX_LEN
-                + COSE_KEY_FIRST_ITEMS_LEN
-                + P256_ELEM_LEN
-        {
-            Err(EDHOCError::ParsingError)
-        } else {
-            let subject_len = CBORDecoder::info_of(value[2]) as usize;
+        if decoder.u8()? != 2 {
+            // expected 2 (subject)
+            return Err(EDHOCError::ParsingError);
+        }
 
-            let id_cred_offset: usize = CCS_PREFIX_LEN
-                .checked_add(subject_len)
-                .and_then(|x| x.checked_add(CNF_AND_COSE_KEY_PREFIX_LEN))
-                .ok_or(EDHOCError::ParsingError)?;
+        let _subject = decoder.str()?;
 
-            let g_a_x_offset: usize = id_cred_offset
-                .checked_add(COSE_KEY_FIRST_ITEMS_LEN)
-                .ok_or(EDHOCError::ParsingError)?;
+        if decoder.u8()? != 8 {
+            // expected 8 (cnf)
+            return Err(EDHOCError::ParsingError);
+        }
 
-            if g_a_x_offset
-                .checked_add(P256_ELEM_LEN)
-                .map_or(false, |end| end <= value.len())
-            {
-                let public_key: BytesKeyEC2 = value[g_a_x_offset..g_a_x_offset + P256_ELEM_LEN]
-                    .try_into()
-                    .expect("Wrong key length");
-                let kid = value[id_cred_offset];
+        if decoder.map()? != 1 {
+            // cnf is always single-item'd
+            return Err(EDHOCError::ParsingError);
+        }
 
-                Ok(Self {
-                    bytes: BufferCred::new_from_slice(value)
-                        .map_err(|_| EDHOCError::ParsingError)?,
-                    key: CredentialKey::EC2Compact(public_key),
-                    kid: Some(BufferKid::new_from_slice(&[kid]).unwrap()),
-                    cred_type: CredentialType::CCS,
-                })
-            } else {
-                Err(EDHOCError::ParsingError)
-            }
+        if decoder.u8()? != 1 {
+            // Unexpected cnf
+            return Err(EDHOCError::ParsingError);
+        }
+
+        if decoder.map()? != 5 {
+            // Right now we're *very* strict and expect exactly 1/kty=/ec2, 2/kid, -1/crv, -2/x, -3/y.
+            return Err(EDHOCError::ParsingError);
+        }
+
+        // kty: EC2
+        if decoder.u8()? != 1 {
+            return Err(EDHOCError::ParsingError);
+        }
+        if decoder.u8()? != 2 {
+            return Err(EDHOCError::ParsingError);
+        }
+
+        // kid: bytes. Note that this is always a byte string, even if in other places it's used
+        // with integer compression.
+        if decoder.u8()? != 2 {
+            return Err(EDHOCError::ParsingError);
+        }
+        let kid = decoder.bytes()?;
+        let kid = BufferKid::new_from_slice(kid)
+            // Could be too long
+            .map_err(|_| EDHOCError::ParsingError)?;
+
+        // crv: p-256
+        if decoder.i8()? != -1 {
+            return Err(EDHOCError::ParsingError);
+        }
+        if decoder.u8()? != 1 {
+            return Err(EDHOCError::ParsingError);
         }
+
+        // x
+        if decoder.i8()? != -2 {
+            return Err(EDHOCError::ParsingError);
+        }
+        let x = decoder.bytes()?;
+        let x = CredentialKey::EC2Compact(
+            x
+                // Wrong length
+                .try_into()
+                .map_err(|_| EDHOCError::ParsingError)?,
+        );
+
+        // y
+        if decoder.i8()? != -3 {
+            return Err(EDHOCError::ParsingError);
+        }
+        let y = decoder.bytes()?;
+
+        if !decoder.finished() {
+            return Err(EDHOCError::ParsingError);
+        }
+
+        Ok(Self {
+            bytes: BufferCred::new_from_slice(value).map_err(|_| EDHOCError::ParsingError)?,
+            key: x,
+            kid: Some(kid),
+            cred_type: CredentialType::CCS,
+        })
     }
 
     /// Parse a CCS style credential, but the key is a symmetric key.
@@ -409,6 +452,17 @@ mod test {
         );
         assert_eq!(cred.kid.unwrap().as_slice(), KID_VALUE_TV);
         assert_eq!(cred.cred_type, CredentialType::CCS);
+
+        // A CCS without a subject. It's OK if this starts working in future, but then its
+        // public key needs to start with F5AEBA08B599754 (it'd be clearly wrong if this produced
+        // an Ok value with a different public key).
+        let cred_no_sub = hex!("a108a101a401022001215820f5aeba08b599754ba16f5db80feafdf91e90a5a7ccb2e83178adb51b8c68ea9522582097e7a3fdd70a3a7c0a5f9578c6e4e96d8bc55f6edd0ff64f1caeaac19d37b67d");
+        Credential::parse_ccs(&cred_no_sub).unwrap_err();
+        // A CCS without a KID. It's OK if this starts working in future, but then its
+        // public key needs to start with F5AEBA08B599754 (it'd be clearly wrong if this produced
+        // an Ok value with a different public key).
+        let cred_no_kid = hex!("a20263666f6f08a101a401022001215820f5aeba08b599754ba16f5db80feafdf91e90a5a7ccb2e83178adb51b8c68ea9522582097e7a3fdd70a3a7c0a5f9578c6e4e96d8bc55f6edd0ff64f1caeaac19d37b67d");
+        Credential::parse_ccs(&cred_no_kid).unwrap_err();
     }
 
     #[rstest]