From 2eb54de22960288ca858e3c311cde273da2cf88b Mon Sep 17 00:00:00 2001
From: chrysn <chrysn@fsfe.org>
Date: Wed, 27 Nov 2024 16:27:17 +0100
Subject: [PATCH] parse_ccs: Be strict

---
 shared/src/cred.rs | 119 ++++++++++++++++++++++++++++++---------------
 1 file changed, 81 insertions(+), 38 deletions(-)

diff --git a/shared/src/cred.rs b/shared/src/cred.rs
index 12329042..098b6908 100644
--- a/shared/src/cred.rs
+++ b/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.