Skip to content

Commit

Permalink
Use references where possible
Browse files Browse the repository at this point in the history
  • Loading branch information
robin-nitrokey committed Jun 20, 2024
1 parent d172ccb commit 371d044
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 77 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Replace `cose` module with `cosey` dependency ([#36][])
- Mark `get_assertion::{ExtensionsInput, ExtensionsOutput}` and `make_credential::Extensions` as non-exhaustive and implement `Default`
- Mark CTAP2 request and response types as non-exhaustive where possible
- Use references where possible

[#8]: https://github.com/trussed-dev/ctap-types/pull/8
[#9]: https://github.com/solokeys/ctap-types/issues/9
Expand Down
2 changes: 1 addition & 1 deletion src/authenticator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub use ctap2::Authenticator as Ctap2Authenticator;
// - second is 10456 bytes
#[allow(clippy::large_enum_variant)]
pub enum Request<'a> {
Ctap1(ctap1::Request),
Ctap1(ctap1::Request<'a>),
Ctap2(ctap2::Request<'a>),
}

Expand Down
51 changes: 27 additions & 24 deletions src/ctap1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ pub mod authenticate {
use super::{Bytes, ControlByte};

#[derive(Clone, Debug, Eq, PartialEq)]
pub struct Request {
pub struct Request<'a> {
pub control_byte: ControlByte,
pub challenge: Bytes<32>,
pub app_id: Bytes<32>,
pub key_handle: Bytes<255>,
pub challenge: &'a [u8],
pub app_id: &'a [u8],
pub key_handle: &'a [u8],
}

#[derive(Clone, Debug, Eq, PartialEq)]
Expand All @@ -32,9 +32,9 @@ pub mod register {
use super::Bytes;

#[derive(Clone, Debug, Eq, PartialEq)]
pub struct Request {
pub challenge: Bytes<32>,
pub app_id: Bytes<32>,
pub struct Request<'a> {
pub challenge: &'a [u8],
pub app_id: &'a [u8],
}

#[derive(Clone, Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -112,9 +112,9 @@ impl TryFrom<u8> for ControlByte {
pub type Result<T> = core::result::Result<T, Error>;

/// Type alias for convenience.
pub type Register = register::Request;
pub type Register<'a> = register::Request<'a>;
/// Type alias for convenience.
pub type Authenticate = authenticate::Request;
pub type Authenticate<'a> = authenticate::Request<'a>;

/// Type alias for convenience.
pub type RegisterResponse = register::Response;
Expand All @@ -124,9 +124,9 @@ pub type AuthenticateResponse = authenticate::Response;
#[derive(Clone, Debug, Eq, PartialEq)]
#[allow(clippy::large_enum_variant)]
/// Enum of all CTAP1 requests.
pub enum Request {
Register(register::Request),
Authenticate(authenticate::Request),
pub enum Request<'a> {
Register(register::Request<'a>),
Authenticate(authenticate::Request<'a>),
Version,
}

Expand Down Expand Up @@ -165,10 +165,10 @@ impl Response {
}
}

impl<const S: usize> TryFrom<&iso7816::Command<S>> for Request {
impl<'a, const S: usize> TryFrom<&'a iso7816::Command<S>> for Request<'a> {
type Error = Error;
#[inline(never)]
fn try_from(apdu: &iso7816::Command<S>) -> Result<Request> {
fn try_from(apdu: &'a iso7816::Command<S>) -> Result<Request> {
let cla = apdu.class().into_inner();
let ins = match apdu.instruction() {
iso7816::Instruction::Unknown(ins) => ins,
Expand Down Expand Up @@ -196,8 +196,8 @@ impl<const S: usize> TryFrom<&iso7816::Command<S>> for Request {
return Err(Error::IncorrectDataParameter);
}
Ok(Request::Register(Register {
challenge: Bytes::from_slice(&request[..32]).unwrap(),
app_id: Bytes::from_slice(&request[32..]).unwrap(),
challenge: &request[..32],
app_id: &request[32..],
}))
}

Expand All @@ -213,9 +213,9 @@ impl<const S: usize> TryFrom<&iso7816::Command<S>> for Request {
}
Ok(Request::Authenticate(Authenticate {
control_byte,
challenge: Bytes::from_slice(&request[..32]).unwrap(),
app_id: Bytes::from_slice(&request[32..64]).unwrap(),
key_handle: Bytes::from_slice(&request[65..]).unwrap(),
challenge: &request[..32],
app_id: &request[32..64],
key_handle: &request[65..],
}))
}

Expand All @@ -233,16 +233,19 @@ impl<const S: usize> TryFrom<&iso7816::Command<S>> for Request {
/// [`Response`].
pub trait Authenticator {
/// Register a U2F credential.
fn register(&mut self, request: &register::Request) -> Result<register::Response>;
fn register(&mut self, request: &register::Request<'_>) -> Result<register::Response>;
/// Authenticate with a U2F credential.
fn authenticate(&mut self, request: &authenticate::Request) -> Result<authenticate::Response>;
fn authenticate(
&mut self,
request: &authenticate::Request<'_>,
) -> Result<authenticate::Response>;
/// Supported U2F version.
fn version() -> [u8; 6] {
*b"U2F_V2"
}

#[inline(never)]
fn call_ctap1(&mut self, request: &Request) -> Result<Response> {
fn call_ctap1(&mut self, request: &Request<'_>) -> Result<Response> {
match request {
Request::Register(reg) => {
debug_now!("CTAP1.REG");
Expand All @@ -257,9 +260,9 @@ pub trait Authenticator {
}
}

impl<A: Authenticator> crate::Rpc<Error, Request, Response> for A {
impl<A: Authenticator> crate::Rpc<Error, Request<'_>, Response> for A {
/// Dispatches the enum of possible requests into the appropriate trait method.
fn call(&mut self, request: &Request) -> Result<Response> {
fn call(&mut self, request: &Request<'_>) -> Result<Response> {
self.call_ctap1(request)
}
}
20 changes: 9 additions & 11 deletions src/ctap2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,12 +201,12 @@ bitflags! {
}

pub trait SerializeAttestedCredentialData {
fn serialize(&self) -> Bytes<ATTESTED_CREDENTIAL_DATA_LENGTH>;
fn serialize(&self, buffer: &mut SerializedAuthenticatorData) -> Result<()>;
}

#[derive(Clone, Debug, Eq, PartialEq)]
pub struct AuthenticatorData<A, E> {
pub rp_id_hash: Bytes<32>,
pub struct AuthenticatorData<'a, A, E> {
pub rp_id_hash: &'a [u8],
pub flags: AuthenticatorDataFlags,
pub sign_count: u32,
pub attested_credential_data: Option<A>,
Expand All @@ -217,13 +217,13 @@ pub type SerializedAuthenticatorData = Bytes<AUTHENTICATOR_DATA_LENGTH>;

// The reason for this non-use of CBOR is for compatibility with
// FIDO U2F authentication signatures.
impl<A: SerializeAttestedCredentialData, E: serde::Serialize> AuthenticatorData<A, E> {
impl<'a, A: SerializeAttestedCredentialData, E: serde::Serialize> AuthenticatorData<'a, A, E> {
#[inline(never)]
pub fn serialize(&self) -> SerializedAuthenticatorData {
pub fn serialize(&self) -> Result<SerializedAuthenticatorData> {
let mut bytes = SerializedAuthenticatorData::new();

// 32 bytes, the RP id's hash
bytes.extend_from_slice(&self.rp_id_hash).unwrap();
bytes.extend_from_slice(self.rp_id_hash).unwrap();
// flags
bytes.push(self.flags.bits()).unwrap();
// signature counts as 32-bit unsigned big-endian integer.
Expand All @@ -232,10 +232,8 @@ impl<A: SerializeAttestedCredentialData, E: serde::Serialize> AuthenticatorData<
.unwrap();

// the attested credential data
if let Some(ref attested_credential_data) = &self.attested_credential_data {
bytes
.extend_from_slice(&attested_credential_data.serialize())
.unwrap();
if let Some(attested_credential_data) = &self.attested_credential_data {
attested_credential_data.serialize(&mut bytes)?;
}

// the extensions data
Expand All @@ -245,7 +243,7 @@ impl<A: SerializeAttestedCredentialData, E: serde::Serialize> AuthenticatorData<
bytes.extend_from_slice(ser).unwrap();
}

bytes
Ok(bytes)
}
}

Expand Down
17 changes: 9 additions & 8 deletions src/ctap2/get_assertion.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use crate::{Bytes, String, Vec};
use crate::{Bytes, Vec};
use cosey::EcdhEsHkdf256PublicKey;
use serde::{Deserialize, Serialize};
use serde_indexed::{DeserializeIndexed, SerializeIndexed};

use super::AuthenticatorOptions;
use super::{AuthenticatorOptions, Result};
use crate::sizes::*;
use crate::webauthn::*;

Expand Down Expand Up @@ -40,24 +40,25 @@ pub struct ExtensionsOutput {
pub hmac_secret: Option<Bytes<80>>,
}

pub struct NoAttestedCredentialData(core::marker::PhantomData<()>);
pub struct NoAttestedCredentialData;

impl super::SerializeAttestedCredentialData for NoAttestedCredentialData {
fn serialize(&self) -> Bytes<ATTESTED_CREDENTIAL_DATA_LENGTH> {
Bytes::new()
fn serialize(&self, _buffer: &mut super::SerializedAuthenticatorData) -> Result<()> {
Ok(())
}
}

pub type AuthenticatorData = super::AuthenticatorData<NoAttestedCredentialData, ExtensionsOutput>;
pub type AuthenticatorData<'a> =
super::AuthenticatorData<'a, NoAttestedCredentialData, ExtensionsOutput>;

pub type AllowList<'a> = Vec<PublicKeyCredentialDescriptorRef<'a>, MAX_CREDENTIAL_COUNT_IN_LIST>;

#[derive(Clone, Debug, Eq, PartialEq, SerializeIndexed, DeserializeIndexed)]
#[non_exhaustive]
#[serde_indexed(offset = 1)]
pub struct Request<'a> {
pub rp_id: String<64>,
pub client_data_hash: Bytes<32>,
pub rp_id: &'a str,
pub client_data_hash: &'a serde_bytes::Bytes,
#[serde(skip_serializing_if = "Option::is_none")]
pub allow_list: Option<AllowList<'a>>,
#[serde(skip_serializing_if = "Option::is_none")]
Expand Down
54 changes: 25 additions & 29 deletions src/ctap2/make_credential.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::{Bytes, String, Vec};
use serde::{Deserialize, Serialize};
use serde_indexed::{DeserializeIndexed, SerializeIndexed};

use super::AuthenticatorOptions;
use super::{AuthenticatorOptions, Error};
use crate::ctap2::credential_management::CredentialProtectionPolicy;
use crate::sizes::*;
use crate::webauthn::*;
Expand Down Expand Up @@ -60,45 +60,41 @@ pub struct Request<'a> {

pub type AttestationObject = Response;

pub type AuthenticatorData = super::AuthenticatorData<AttestedCredentialData, Extensions>;
pub type AuthenticatorData<'a> =
super::AuthenticatorData<'a, AttestedCredentialData<'a>, Extensions>;

// NOTE: This is not CBOR, it has a custom encoding...
// https://www.w3.org/TR/webauthn/#sec-attested-credential-data
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct AttestedCredentialData {
pub aaguid: Bytes<16>,
pub struct AttestedCredentialData<'a> {
pub aaguid: &'a [u8],
// this is where "unlimited non-resident keys" get stored
// TODO: Model as actual credential ID, with ser/de to bytes (format is up to authenticator)
pub credential_id: Bytes<MAX_CREDENTIAL_ID_LENGTH>,
pub credential_public_key: Bytes<COSE_KEY_LENGTH>,
pub credential_id: &'a [u8],
pub credential_public_key: &'a [u8],
}

impl super::SerializeAttestedCredentialData for AttestedCredentialData {
fn serialize(&self) -> Bytes<ATTESTED_CREDENTIAL_DATA_LENGTH> {
let mut bytes = Vec::<u8, ATTESTED_CREDENTIAL_DATA_LENGTH>::new();
impl<'a> super::SerializeAttestedCredentialData for AttestedCredentialData<'a> {
fn serialize(&self, buffer: &mut super::SerializedAuthenticatorData) -> Result<(), Error> {
// TODO: validate lengths of credential ID and credential public key
// 16 bytes, the aaguid
bytes.extend_from_slice(&self.aaguid).unwrap();

buffer
.extend_from_slice(self.aaguid)
.map_err(|_| Error::Other)?;
// byte length of credential ID as 16-bit unsigned big-endian integer.
bytes
.extend_from_slice(&(self.credential_id.len() as u16).to_be_bytes())
.unwrap();
let credential_id_len =
u16::try_from(self.credential_id.len()).map_err(|_| Error::Other)?;
buffer
.extend_from_slice(&credential_id_len.to_be_bytes())
.map_err(|_| Error::Other)?;
// raw bytes of credential ID
bytes
.extend_from_slice(&self.credential_id[..self.credential_id.len()])
.unwrap();

// use existing `bytes` buffer
// let mut cbor_key = [0u8; 128];

// CHANGE this back if credential_public_key is not serialized again
// let l = crate::serde::cbor_serialize(&self.credential_public_key, &mut cbor_key).unwrap();
// bytes.extend_from_slice(&cbor_key[..l]).unwrap();
bytes
.extend_from_slice(&self.credential_public_key)
.unwrap();

Bytes::from(bytes)
buffer
.extend_from_slice(self.credential_id)
.map_err(|_| Error::Other)?;
buffer
.extend_from_slice(self.credential_public_key)
.map_err(|_| Error::Other)?;
Ok(())
}
}

Expand Down
4 changes: 0 additions & 4 deletions src/sizes.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
pub const ATTESTED_CREDENTIAL_DATA_LENGTH: usize = 612;
// // not sure why i can't use `::to_usize()` here?
// pub const ATTESTED_CREDENTIAL_DATA_LENGTH_BYTES: usize = 512;

pub const AUTHENTICATOR_DATA_LENGTH: usize = 676;
// pub const AUTHENTICATOR_DATA_LENGTH_BYTES: usize = 512;

Expand Down

0 comments on commit 371d044

Please sign in to comment.