From f4954439d3afe23d8609113016909c3445ba0d69 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sat, 22 Jun 2024 15:01:08 +0200 Subject: [PATCH] Use enums instead of string constants Previously, we used String<_> to represent string constants in some responses. This has multiple drawbacks: - It is error-prone because the value is not validated. - Construction is fallible because of the fixed length of the string. - The length needs to be bumped if longer values are added. This patch introduces enums to replace these constants. As cbor_smol serializes enums using the variant index instead of the string, we need to manually implement the string conversion. --- CHANGELOG.md | 6 +- src/ctap2/get_info.rs | 124 +++++++++++++++++++++++++++++++++-- src/ctap2/make_credential.rs | 34 ++++++++-- src/lib.rs | 13 ++++ 4 files changed, 167 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8160139..d23955d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [Unreleased]: https://github.com/trussed-dev/ctap-types/compare/0.2.0...HEAD -- +### Breaking Changes + +- Use enums instead of string constants + - Introduce `Version`, `Extension` and `Transport` enums and use them in `ctap2::get_info` + - Fix serialization of the `AttestationStatementFormat` enum and use it in `ctap2::make_credential` ## [0.2.0] - 2024-06-21 diff --git a/src/ctap2/get_info.rs b/src/ctap2/get_info.rs index 2b999f4..d966096 100644 --- a/src/ctap2/get_info.rs +++ b/src/ctap2/get_info.rs @@ -1,5 +1,5 @@ use crate::webauthn::FilteredPublicKeyCredentialParameters; -use crate::{Bytes, String, Vec}; +use crate::{Bytes, TryFromStrError, Vec}; use serde::{Deserialize, Serialize}; use serde_indexed::{DeserializeIndexed, SerializeIndexed}; @@ -10,11 +10,11 @@ pub type AuthenticatorInfo = Response; #[serde_indexed(offset = 1)] pub struct Response { // 0x01 - pub versions: Vec, 4>, + pub versions: Vec, // 0x02 #[serde(skip_serializing_if = "Option::is_none")] - pub extensions: Option, 4>>, + pub extensions: Option>, // 0x03 pub aaguid: Bytes<16>, @@ -44,7 +44,7 @@ pub struct Response { // 0x09 // FIDO_2_1 #[serde(skip_serializing_if = "Option::is_none")] - pub transports: Option, 4>>, + pub transports: Option>, // 0x0A // FIDO_2_1 @@ -135,7 +135,7 @@ impl Default for Response { #[derive(Debug)] pub struct ResponseBuilder { - pub versions: Vec, 4>, + pub versions: Vec, pub aaguid: Bytes<16>, } @@ -178,6 +178,120 @@ impl ResponseBuilder { } } +#[derive(Copy, Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[non_exhaustive] +#[serde(into = "&str", try_from = "&str")] +pub enum Version { + Fido2_0, + Fido2_1, + Fido2_1Pre, + U2fV2, +} + +impl Version { + const FIDO_2_0: &'static str = "FIDO_2_0"; + const FIDO_2_1: &'static str = "FIDO_2_1"; + const FIDO_2_1_PRE: &'static str = "FIDO_2_1_PRE"; + const U2F_V2: &'static str = "U2F_V2"; +} + +impl From for &str { + fn from(version: Version) -> Self { + match version { + Version::Fido2_0 => Version::FIDO_2_0, + Version::Fido2_1 => Version::FIDO_2_1, + Version::Fido2_1Pre => Version::FIDO_2_1_PRE, + Version::U2fV2 => Version::U2F_V2, + } + } +} + +impl TryFrom<&str> for Version { + type Error = TryFromStrError; + + fn try_from(s: &str) -> Result { + match s { + Self::FIDO_2_0 => Ok(Self::Fido2_0), + Self::FIDO_2_1 => Ok(Self::Fido2_1), + Self::FIDO_2_1_PRE => Ok(Self::Fido2_1Pre), + Self::U2F_V2 => Ok(Self::U2fV2), + _ => Err(TryFromStrError), + } + } +} + +#[derive(Copy, Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[non_exhaustive] +#[serde(into = "&str", try_from = "&str")] +pub enum Extension { + CredProtect, + HmacSecret, + LargeBlobKey, +} + +impl Extension { + const CRED_PROTECT: &'static str = "credProtect"; + const HMAC_SECRET: &'static str = "hmac-secret"; + const LARGE_BLOB_KEY: &'static str = "largeBlobKey"; +} + +impl From for &str { + fn from(extension: Extension) -> Self { + match extension { + Extension::CredProtect => Extension::CRED_PROTECT, + Extension::HmacSecret => Extension::HMAC_SECRET, + Extension::LargeBlobKey => Extension::LARGE_BLOB_KEY, + } + } +} + +impl TryFrom<&str> for Extension { + type Error = TryFromStrError; + + fn try_from(s: &str) -> Result { + match s { + Self::CRED_PROTECT => Ok(Self::CredProtect), + Self::HMAC_SECRET => Ok(Self::HmacSecret), + Self::LARGE_BLOB_KEY => Ok(Self::LargeBlobKey), + _ => Err(TryFromStrError), + } + } +} + +#[derive(Copy, Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[non_exhaustive] +#[serde(into = "&str", try_from = "&str")] +pub enum Transport { + Nfc, + Usb, +} + +impl Transport { + const NFC: &'static str = "nfc"; + const USB: &'static str = "usb"; +} + +impl From for &str { + fn from(transport: Transport) -> Self { + match transport { + Transport::Nfc => Transport::NFC, + Transport::Usb => Transport::USB, + } + } +} + +impl TryFrom<&str> for Transport { + type Error = TryFromStrError; + + fn try_from(s: &str) -> Result { + match s { + Self::NFC => Ok(Self::Nfc), + Self::USB => Ok(Self::Usb), + _ => Err(TryFromStrError), + } + } +} + #[derive(Copy, Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] #[non_exhaustive] #[serde(rename_all = "camelCase")] diff --git a/src/ctap2/make_credential.rs b/src/ctap2/make_credential.rs index 7ba3ad8..e2082cd 100644 --- a/src/ctap2/make_credential.rs +++ b/src/ctap2/make_credential.rs @@ -1,4 +1,4 @@ -use crate::{Bytes, String, Vec}; +use crate::{Bytes, TryFromStrError, Vec}; use serde::{Deserialize, Serialize}; use serde_bytes::ByteArray; @@ -103,7 +103,7 @@ impl<'a> super::SerializeAttestedCredentialData for AttestedCredentialData<'a> { #[non_exhaustive] #[serde_indexed(offset = 1)] pub struct Response { - pub fmt: String<32>, + pub fmt: AttestationStatementFormat, pub auth_data: super::SerializedAuthenticatorData, #[serde(skip_serializing_if = "Option::is_none")] pub att_stmt: Option, @@ -115,7 +115,7 @@ pub struct Response { #[derive(Debug)] pub struct ResponseBuilder { - pub fmt: String<32>, + pub fmt: AttestationStatementFormat, pub auth_data: super::SerializedAuthenticatorData, } @@ -143,12 +143,38 @@ pub enum AttestationStatement { #[derive(Clone, Debug, Eq, PartialEq, Serialize)] #[non_exhaustive] -#[serde(untagged)] +#[serde(into = "&str", try_from = "&str")] pub enum AttestationStatementFormat { None, Packed, } +impl AttestationStatementFormat { + const NONE: &'static str = "none"; + const PACKED: &'static str = "packed"; +} + +impl From for &str { + fn from(version: AttestationStatementFormat) -> Self { + match version { + AttestationStatementFormat::None => AttestationStatementFormat::NONE, + AttestationStatementFormat::Packed => AttestationStatementFormat::PACKED, + } + } +} + +impl TryFrom<&str> for AttestationStatementFormat { + type Error = TryFromStrError; + + fn try_from(s: &str) -> Result { + match s { + Self::NONE => Ok(Self::None), + Self::PACKED => Ok(Self::Packed), + _ => Err(TryFromStrError), + } + } +} + #[derive(Clone, Debug, Eq, PartialEq, Serialize)] pub struct NoneAttestationStatement {} diff --git a/src/lib.rs b/src/lib.rs index 6763da2..feb2e1f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -35,6 +35,19 @@ pub mod webauthn; pub use ctap2::{Error, Result}; +use core::fmt::{self, Display, Formatter}; + +/// An error returned by the `TryFrom<&str>` implementation for enums if an invalid value is +/// provided. +#[derive(Debug)] +pub struct TryFromStrError; + +impl Display for TryFromStrError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + "invalid enum value".fmt(f) + } +} + #[cfg(test)] mod tests {}