Merge branch 'msirringhaus-drop_nom' into ctap2-2021

jschanck · jschanck · commit 9f1b3acdbb2c · 2023-06-01T12:26:47.000-07:00
diff --git a/Cargo.toml b/Cargo.toml
@@ -65,7 +65,6 @@ serde_cbor = "0.11"
 serde_json = "1.0"
 bytes = { version = "0.5", optional = true, features = ["serde"] }
 base64 = "^0.13"
-nom = { version = "^7.1.1", features = ["std"], default-features = false}
 sha2 = "^0.10.0"
 cfg-if = "1.0"
 # Crypto backends
diff --git a/src/ctap2/attestation.rs b/src/ctap2/attestation.rs
@@ -1,15 +1,9 @@
-use super::utils::from_slice_stream;
+use super::utils::{from_slice_stream, read_be_u16, read_be_u32, read_byte};
 use crate::crypto::COSEAlgorithm;
 use crate::ctap2::commands::CommandError;
 use crate::ctap2::server::RpIdHash;
+use crate::ctap2::utils::serde_parse_err;
 use crate::{crypto::COSEKey, errors::AuthenticatorError};
-use nom::{
-    bytes::complete::take,
-    combinator::{cond, map},
-    error::Error as NomError,
-    number::complete::{be_u16, be_u32, be_u8},
-    Err as NomErr, IResult,
-};
 use serde::ser::{Error as SerError, SerializeMap, Serializer};
 use serde::{
     de::{Error as SerdeError, MapAccess, Visitor},
@@ -18,6 +12,7 @@ use serde::{
 use serde_bytes::ByteBuf;
 use serde_cbor;
 use std::fmt;
+use std::io::{Cursor, Read};
 
 #[derive(Debug, PartialEq, Eq)]
 pub enum HmacSecretResponse {
@@ -86,10 +81,6 @@ impl Extension {
     }
 }
 
-fn parse_extensions(input: &[u8]) -> IResult<&[u8], Extension, NomError<&[u8]>> {
-    serde_to_nom(input)
-}
-
 #[derive(Serialize, PartialEq, Default, Eq, Clone)]
 pub struct AAGuid(pub [u8; 16]);
 
@@ -172,34 +163,23 @@ pub struct AttestedCredentialData {
     pub credential_public_key: COSEKey,
 }
 
-fn serde_to_nom<'a, Output>(input: &'a [u8]) -> IResult<&'a [u8], Output>
-where
-    Output: Deserialize<'a>,
-{
-    from_slice_stream(input)
-        .map_err(|_e| nom::Err::Error(nom::error::make_error(input, nom::error::ErrorKind::NoneOf)))
-    // can't use custom errorkind because of error type mismatch in parse_attested_cred_data
-    //.map_err(|e| NomErr::Error(Context::Code(input, ErrorKind::Custom(e))))
-    //         .map_err(|_| NomErr::Error(Context::Code(input, ErrorKind::Custom(42))))
-}
-
-fn parse_attested_cred_data(
-    input: &[u8],
-) -> IResult<&[u8], AttestedCredentialData, NomError<&[u8]>> {
-    let (rest, aaguid_res) = map(take(16u8), AAGuid::from)(input)?;
-    // // We can unwrap here, since we _know_ the input will be 16 bytes error out before calling from()
-    let aaguid = aaguid_res.unwrap();
-    let (rest, cred_len) = be_u16(rest)?;
-    let (rest, credential_id) = map(take(cred_len), Vec::from)(rest)?;
-    let (rest, credential_public_key) = serde_to_nom(rest)?;
-    Ok((
-        rest,
-        (AttestedCredentialData {
-            aaguid,
-            credential_id,
-            credential_public_key,
-        }),
-    ))
+fn parse_attested_cred_data<R: Read, E: SerdeError>(
+    data: &mut R,
+) -> Result<AttestedCredentialData, E> {
+    let mut aaguid_raw = [0u8; 16];
+    data.read_exact(&mut aaguid_raw)
+        .map_err(|_| serde_parse_err("AAGuid"))?;
+    let aaguid = AAGuid(aaguid_raw);
+    let cred_len = read_be_u16(data)?;
+    let mut credential_id = vec![0u8; cred_len as usize];
+    data.read_exact(&mut credential_id)
+        .map_err(|_| serde_parse_err("CredentialId"))?;
+    let credential_public_key = from_slice_stream(data)?;
+    Ok(AttestedCredentialData {
+        aaguid,
+        credential_id,
+        credential_public_key,
+    })
 }
 
 bitflags! {
@@ -226,36 +206,6 @@ pub struct AuthenticatorData {
     pub extensions: Extension,
 }
 
-fn parse_ad(input: &[u8]) -> IResult<&[u8], AuthenticatorData, NomError<&[u8]>> {
-    let (rest, rp_id_hash_res) = map(take(32u8), RpIdHash::from)(input)?;
-    // We can unwrap here, since we _know_ the input to from() will be 32 bytes or error out before calling from()
-    let rp_id_hash = rp_id_hash_res.unwrap();
-    // preserve the flags, even if some reserved values are set.
-    let (rest, flags) = map(be_u8, AuthenticatorDataFlags::from_bits_truncate)(rest)?;
-    let (rest, counter) = be_u32(rest)?;
-    let (rest, credential_data) = cond(
-        flags.contains(AuthenticatorDataFlags::ATTESTED),
-        parse_attested_cred_data,
-    )(rest)?;
-    let (rest, extensions) = cond(
-        flags.contains(AuthenticatorDataFlags::EXTENSION_DATA),
-        parse_extensions,
-    )(rest)?;
-    // TODO(baloo): we should check for end of buffer and raise a parse
-    //              parse error if data is still in the buffer
-    //eof!() >>
-    Ok((
-        rest,
-        AuthenticatorData {
-            rp_id_hash,
-            flags,
-            counter,
-            credential_data,
-            extensions: extensions.unwrap_or_default(),
-        },
-    ))
-}
-
 impl<'de> Deserialize<'de> for AuthenticatorData {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
@@ -270,23 +220,40 @@ impl<'de> Deserialize<'de> for AuthenticatorData {
                 formatter.write_str("a byte array")
             }
 
-            fn visit_bytes<E>(self, v: &[u8]) -> Result<Self::Value, E>
+            fn visit_bytes<E>(self, input: &[u8]) -> Result<Self::Value, E>
             where
                 E: SerdeError,
             {
-                parse_ad(v)
-                    .map(|(_input, value)| value)
-                    .map_err(|e| match e {
-                        NomErr::Incomplete(nom::Needed::Size(len)) => {
-                            E::invalid_length(v.len(), &format!("{}", v.len() + len.get()).as_ref())
-                        }
-                        NomErr::Incomplete(nom::Needed::Unknown) => {
-                            E::invalid_length(v.len(), &"unknown") // We don't know the expected value
-                        }
-                        // TODO(baloo): is that enough? should we be more
-                        //              specific on the error type?
-                        e => E::custom(e.to_string()),
-                    })
+                let mut cursor = Cursor::new(input);
+                let mut rp_id_hash_raw = [0u8; 32];
+                cursor
+                    .read_exact(&mut rp_id_hash_raw)
+                    .map_err(|_| serde_parse_err("32 bytes"))?;
+                let rp_id_hash = RpIdHash(rp_id_hash_raw);
+
+                // preserve the flags, even if some reserved values are set.
+                let flags = AuthenticatorDataFlags::from_bits_truncate(read_byte(&mut cursor)?);
+                let counter = read_be_u32(&mut cursor)?;
+                let mut credential_data = None;
+                if flags.contains(AuthenticatorDataFlags::ATTESTED) {
+                    credential_data = Some(parse_attested_cred_data(&mut cursor)?);
+                }
+
+                let extensions = if flags.contains(AuthenticatorDataFlags::EXTENSION_DATA) {
+                    from_slice_stream(&mut cursor)?
+                } else {
+                    Default::default()
+                };
+
+                // TODO(baloo): we should check for end of buffer and raise a parse
+                //              parse error if data is still in the buffer
+                Ok(AuthenticatorData {
+                    rp_id_hash,
+                    flags,
+                    counter,
+                    credential_data,
+                    extensions,
+                })
             }
         }
 
@@ -805,12 +772,21 @@ mod test {
         let v: Vec<u8> = vec![
             0x66, 0x66, 0x6f, 0x6f, 0x62, 0x61, 0x72, 0x66, 0x66, 0x6f, 0x6f, 0x62, 0x61, 0x72,
         ];
-        let (rest, value): (&[u8], String) = from_slice_stream(&v).unwrap();
+        let mut data = Cursor::new(v);
+        let value: String = from_slice_stream::<_, _, serde_cbor::Error>(&mut data).unwrap();
         assert_eq!(value, "foobar");
-        assert_eq!(rest, &[0x66, 0x66, 0x6f, 0x6f, 0x62, 0x61, 0x72]);
-        let (rest, value): (&[u8], String) = from_slice_stream(rest).unwrap();
+        let mut remaining = Vec::new();
+        data.read_to_end(&mut remaining).unwrap();
+        assert_eq!(
+            remaining.as_slice(),
+            &[0x66, 0x66, 0x6f, 0x6f, 0x62, 0x61, 0x72]
+        );
+        let mut data = Cursor::new(remaining);
+        let value: String = from_slice_stream::<_, _, serde_cbor::Error>(&mut data).unwrap();
         assert_eq!(value, "foobar");
-        assert!(rest.is_empty());
+        let mut remaining = Vec::new();
+        data.read_to_end(&mut remaining).unwrap();
+        assert!(remaining.is_empty());
     }
 
     #[test]
diff --git a/src/ctap2/commands/get_assertion.rs b/src/ctap2/commands/get_assertion.rs
@@ -16,15 +16,11 @@ use crate::ctap2::commands::make_credentials::UserVerification;
 use crate::ctap2::server::{
     PublicKeyCredentialDescriptor, RelyingPartyWrapper, RpIdHash, User, UserVerificationRequirement,
 };
+use crate::ctap2::utils::{read_be_u32, read_byte};
 use crate::errors::AuthenticatorError;
 use crate::transport::errors::{ApduErrorStatus, HIDError};
 use crate::transport::FidoDevice;
 use crate::u2ftypes::CTAP1RequestAPDU;
-use nom::{
-    error::VerboseError,
-    number::complete::{be_u32, be_u8},
-    sequence::tuple,
-};
 use serde::{
     de::{Error as DesError, MapAccess, Visitor},
     ser::{Error as SerError, SerializeMap},
@@ -33,6 +29,7 @@ use serde::{
 use serde_bytes::ByteBuf;
 use serde_cbor::{de::from_slice, ser, Value};
 use std::fmt;
+use std::io::Cursor;
 
 #[derive(Clone, Copy, Debug, Serialize)]
 #[cfg_attr(test, derive(Deserialize))]
@@ -452,17 +449,11 @@ impl GetAssertionResult {
         rp_id_hash: &RpIdHash,
         key_handle: &PublicKeyCredentialDescriptor,
     ) -> Result<GetAssertionResult, CommandError> {
-        let parse_authentication = |input| {
-            // Parsing an u8, then a u32, and the rest is the signature
-            let (rest, (user_presence, counter)) = tuple((be_u8, be_u32))(input)?;
-            let signature = Vec::from(rest);
-            Ok((user_presence, counter, signature))
-        };
-        let (user_presence, counter, signature) =
-            parse_authentication(input).map_err(|e: nom::Err<VerboseError<_>>| {
-                error!("error while parsing authentication: {:?}", e);
-                CommandError::Deserializing(DesError::custom("unable to parse authentication"))
-            })?;
+        let mut data = Cursor::new(input);
+        let user_presence = read_byte(&mut data).map_err(CommandError::Deserializing)?;
+        let counter = read_be_u32(&mut data).map_err(CommandError::Deserializing)?;
+        // Remaining data is signature (Note: `data.remaining_slice()` is not yet stabilized)
+        let signature = Vec::from(&data.get_ref()[data.position() as usize..]);
 
         // Step 5 of Section 10.3 of CTAP2.1: "Copy bits 0 (the UP bit) and bit 1 from the
         // CTAP2/U2F response user presence byte to bits 0 and 1 of the CTAP2 flags, respectively.
diff --git a/src/ctap2/commands/make_credentials.rs b/src/ctap2/commands/make_credentials.rs
@@ -18,15 +18,11 @@ use crate::ctap2::server::{
     PublicKeyCredentialDescriptor, PublicKeyCredentialParameters, RelyingParty,
     RelyingPartyWrapper, RpIdHash, User, UserVerificationRequirement,
 };
+use crate::ctap2::utils::{read_byte, serde_parse_err};
 use crate::errors::AuthenticatorError;
 use crate::transport::errors::{ApduErrorStatus, HIDError};
 use crate::transport::FidoDevice;
 use crate::u2ftypes::CTAP1RequestAPDU;
-use nom::{
-    bytes::complete::{tag, take},
-    error::VerboseError,
-    number::complete::be_u8,
-};
 #[cfg(test)]
 use serde::Deserialize;
 use serde::{
@@ -35,6 +31,7 @@ use serde::{
     Serialize, Serializer,
 };
 use serde_cbor::{self, de::from_slice, ser, Value};
+use std::io::{Cursor, Read};
 
 #[derive(Debug)]
 pub struct MakeCredentialsResult(pub AttestationObject);
@@ -44,30 +41,35 @@ impl MakeCredentialsResult {
         input: &[u8],
         rp_id_hash: &RpIdHash,
     ) -> Result<MakeCredentialsResult, CommandError> {
-        let parse_register = |input| {
-            let (rest, _) = tag(&[0x05])(input)?;
-            let (rest, public_key) = take(65u8)(rest)?;
-            let (rest, key_handle_len) = be_u8(rest)?;
-            let (rest, key_handle) = take(key_handle_len)(rest)?;
-            Ok((rest, public_key, key_handle))
-        };
+        let mut data = Cursor::new(input);
+        let magic_num = read_byte(&mut data).map_err(CommandError::Deserializing)?;
+        if magic_num != 0x05 {
+            error!("error while parsing registration: magic header not 0x05, but {magic_num}");
+            return Err(CommandError::Deserializing(DesError::invalid_value(
+                serde::de::Unexpected::Unsigned(magic_num as u64),
+                &"0x05",
+            )));
+        }
+        let mut public_key = [0u8; 65];
+        data.read_exact(&mut public_key)
+            .map_err(|_| CommandError::Deserializing(serde_parse_err("PublicKey")))?;
 
-        let (rest, public_key, key_handle) =
-            parse_register(input).map_err(|e: nom::Err<VerboseError<_>>| {
-                error!("error while parsing registration: {:?}", e);
-                CommandError::Deserializing(DesError::custom("unable to parse registration"))
-            })?;
+        let credential_id_len = read_byte(&mut data).map_err(CommandError::Deserializing)?;
+        let mut credential_id = vec![0u8; credential_id_len as usize];
+        data.read_exact(&mut credential_id)
+            .map_err(|_| CommandError::Deserializing(serde_parse_err("CredentialId")))?;
 
-        let cert_and_sig = parse_u2f_der_certificate(rest).map_err(|e| {
-            error!("error while parsing registration: {:?}", e);
-            CommandError::Deserializing(DesError::custom("unable to parse registration"))
+        let cert_and_sig = parse_u2f_der_certificate(&data.get_ref()[data.position() as usize..])
+            .map_err(|err| {
+            CommandError::Deserializing(serde_parse_err(&format!(
+                "Certificate and Signature: {err:?}",
+            )))
         })?;
 
-        let credential_ec2_key = COSEEC2Key::from_sec1_uncompressed(Curve::SECP256R1, public_key)
-            .map_err(|e| {
-            error!("error while parsing registration: {:?}", e);
-            CommandError::Deserializing(DesError::custom("unable to parse registration"))
-        })?;
+        let credential_ec2_key = COSEEC2Key::from_sec1_uncompressed(Curve::SECP256R1, &public_key)
+            .map_err(|err| {
+                CommandError::Deserializing(serde_parse_err(&format!("EC2 Key: {err:?}",)))
+            })?;
 
         let credential_public_key = COSEKey {
             alg: COSEAlgorithm::ES256,
@@ -84,7 +86,7 @@ impl MakeCredentialsResult {
             counter: 0,
             credential_data: Some(AttestedCredentialData {
                 aaguid: AAGuid::default(),
-                credential_id: Vec::from(key_handle),
+                credential_id,
                 credential_public_key,
             }),
             extensions: Default::default(),
diff --git a/src/ctap2/utils.rs b/src/ctap2/utils.rs
@@ -1,14 +1,39 @@
 use serde::de;
-use serde_cbor::error::Result;
 use serde_cbor::Deserializer;
+use std::io::Read;
 
-pub fn from_slice_stream<'a, T>(slice: &'a [u8]) -> Result<(&'a [u8], T)>
+pub fn serde_parse_err<E: de::Error>(s: &str) -> E {
+    E::custom(format!("Failed to parse {s}"))
+}
+
+pub fn from_slice_stream<'a, T, R: Read, E: de::Error>(data: &mut R) -> Result<T, E>
 where
     T: de::Deserialize<'a>,
 {
-    let mut deserializer = Deserializer::from_slice(slice);
-    let value = de::Deserialize::deserialize(&mut deserializer)?;
-    let rest = &slice[deserializer.byte_offset()..];
+    let mut deserializer = Deserializer::from_reader(data);
+    de::Deserialize::deserialize(&mut deserializer)
+        .map_err(|x| serde_parse_err(&format!("{}: {}", stringify!(T), &x.to_string())))
+}
+
+// Parsing routines
+
+pub fn read_be_u32<R: Read, E: de::Error>(data: &mut R) -> Result<u32, E> {
+    let mut buf = [0; 4];
+    data.read_exact(&mut buf)
+        .map_err(|_| serde_parse_err("u32"))?;
+    Ok(u32::from_be_bytes(buf))
+}
+
+pub fn read_be_u16<R: Read, E: de::Error>(data: &mut R) -> Result<u16, E> {
+    let mut buf = [0; 2];
+    data.read_exact(&mut buf)
+        .map_err(|_| serde_parse_err("u16"))?;
+    Ok(u16::from_be_bytes(buf))
+}
 
-    Ok((rest, value))
+pub fn read_byte<R: Read, E: de::Error>(data: &mut R) -> Result<u8, E> {
+    match data.bytes().next() {
+        Some(Ok(s)) => Ok(s),
+        _ => Err(serde_parse_err("u8")),
+    }
 }
