Skip to content

Commit

Permalink
Uses CtapResult instead of explicit Result (#696)
Browse files Browse the repository at this point in the history 
Generated with

```
find . -type f -name "*.rs" -exec sed -i 's/Result<\([^,]*\), Ctap2StatusCode>/CtapResult<\1>/g' {} \;
cd libraries/opensk
find . -type f -name "*.rs" -exec grep -q 'CtapResult' {} \; -exec sed -i '15 i\
use crate::ctap::status_code::CtapResult;
' {} +
```

Then we fix the last few compiler errors and run `cargo fmt`.

Next step is to move away from custom error types in the API to only use
CtapResult everywhere.
  • Loading branch information
@kaczmarczyck
kaczmarczyck authored Jul 25, 2024
1 parent d624558 commit 4c23e61
Show file tree
Hide file tree
Showing 13 changed files with 182 additions and 216 deletions.
17 changes: 7 additions & 10 deletions libraries/opensk/src/api/private_key.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::api::crypto::ecdsa::{SecretKey as _, Signature};
use crate::ctap::crypto_wrapper::{aes256_cbc_decrypt, aes256_cbc_encrypt};
use crate::ctap::data_formats::{extract_array, extract_byte_string, CoseKey, SignatureAlgorithm};
use crate::ctap::secret::Secret;
use crate::ctap::status_code::Ctap2StatusCode;
use crate::ctap::status_code::{Ctap2StatusCode, CtapResult};
use crate::env::{AesKey, EcdsaSk, Env};
use alloc::vec;
use alloc::vec::Vec;
Expand Down Expand Up @@ -89,7 +89,7 @@ impl PrivateKey {
}

/// Returns the ECDSA private key.
pub fn ecdsa_key<E: Env>(&self) -> Result<EcdsaSk<E>, Ctap2StatusCode> {
pub fn ecdsa_key<E: Env>(&self) -> CtapResult<EcdsaSk<E>> {
match self {
PrivateKey::Ecdsa(bytes) => ecdsa_key_from_bytes::<E>(bytes),
#[allow(unreachable_patterns)]
Expand All @@ -98,7 +98,7 @@ impl PrivateKey {
}

/// Returns the corresponding public key.
pub fn get_pub_key<E: Env>(&self) -> Result<CoseKey, Ctap2StatusCode> {
pub fn get_pub_key<E: Env>(&self) -> CtapResult<CoseKey> {
Ok(match self {
PrivateKey::Ecdsa(bytes) => {
CoseKey::from_ecdsa_public_key(ecdsa_key_from_bytes::<E>(bytes)?.public_key())
Expand All @@ -109,7 +109,7 @@ impl PrivateKey {
}

/// Returns the encoded signature for a given message.
pub fn sign_and_encode<E: Env>(&self, message: &[u8]) -> Result<Vec<u8>, Ctap2StatusCode> {
pub fn sign_and_encode<E: Env>(&self, message: &[u8]) -> CtapResult<Vec<u8>> {
Ok(match self {
PrivateKey::Ecdsa(bytes) => ecdsa_key_from_bytes::<E>(bytes)?.sign(message).to_der(),
#[cfg(feature = "ed25519")]
Expand Down Expand Up @@ -141,7 +141,7 @@ impl PrivateKey {
&self,
rng: &mut E::Rng,
wrap_key: &AesKey<E>,
) -> Result<cbor::Value, Ctap2StatusCode> {
) -> CtapResult<cbor::Value> {
let bytes = self.to_bytes();
let wrapped_bytes = aes256_cbc_encrypt::<E>(rng, wrap_key, &bytes, true)?;
Ok(cbor_array![
Expand All @@ -150,10 +150,7 @@ impl PrivateKey {
])
}

pub fn from_cbor<E: Env>(
wrap_key: &AesKey<E>,
cbor_value: cbor::Value,
) -> Result<Self, Ctap2StatusCode> {
pub fn from_cbor<E: Env>(wrap_key: &AesKey<E>, cbor_value: cbor::Value) -> CtapResult<Self> {
let mut array = extract_array(cbor_value)?;
if array.len() != 2 {
return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR);
Expand All @@ -171,7 +168,7 @@ impl PrivateKey {
}
}

fn ecdsa_key_from_bytes<E: Env>(bytes: &[u8; 32]) -> Result<EcdsaSk<E>, Ctap2StatusCode> {
fn ecdsa_key_from_bytes<E: Env>(bytes: &[u8; 32]) -> CtapResult<EcdsaSk<E>> {
EcdsaSk::<E>::from_slice(bytes).ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)
}

Expand Down
51 changes: 23 additions & 28 deletions libraries/opensk/src/ctap/client_pin.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use crate::api::crypto::sha256::Sha256;
use crate::api::customization::Customization;
use crate::api::key_store::KeyStore;
use crate::api::persist::Persist;
use crate::ctap::status_code::CtapResult;
use crate::ctap::storage;
#[cfg(test)]
use crate::env::EcdhSk;
Expand Down Expand Up @@ -65,7 +66,7 @@ const PIN_PADDED_LENGTH: usize = 64;
fn decrypt_pin<E: Env>(
shared_secret: &SharedSecret<E>,
new_pin_enc: Vec<u8>,
) -> Result<Secret<[u8]>, Ctap2StatusCode> {
) -> CtapResult<Secret<[u8]>> {
let decrypted_pin = shared_secret.decrypt(&new_pin_enc)?;
if decrypted_pin.len() != PIN_PADDED_LENGTH {
return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER);
Expand All @@ -91,7 +92,7 @@ fn check_and_store_new_pin<E: Env>(
env: &mut E,
shared_secret: &SharedSecret<E>,
new_pin_enc: Vec<u8>,
) -> Result<(), Ctap2StatusCode> {
) -> CtapResult<()> {
let pin = decrypt_pin(shared_secret, new_pin_enc)?;
let min_pin_length = storage::min_pin_length(env)? as usize;
let pin_length = str::from_utf8(&pin).unwrap_or("").chars().count();
Expand Down Expand Up @@ -168,7 +169,7 @@ impl<E: Env> ClientPin<E> {
&self,
pin_uv_auth_protocol: PinUvAuthProtocol,
key_agreement: CoseKey,
) -> Result<SharedSecret<E>, Ctap2StatusCode> {
) -> CtapResult<SharedSecret<E>> {
self.get_pin_protocol(pin_uv_auth_protocol)
.decapsulate(key_agreement, pin_uv_auth_protocol)
}
Expand All @@ -184,7 +185,7 @@ impl<E: Env> ClientPin<E> {
pin_uv_auth_protocol: PinUvAuthProtocol,
shared_secret: &SharedSecret<E>,
pin_hash_enc: Vec<u8>,
) -> Result<(), Ctap2StatusCode> {
) -> CtapResult<()> {
match env.persist().pin_hash()? {
Some(pin_hash) => {
if self.consecutive_pin_mismatches >= 3 {
Expand Down Expand Up @@ -217,10 +218,7 @@ impl<E: Env> ClientPin<E> {
Ok(())
}

fn process_get_pin_retries(
&self,
env: &mut E,
) -> Result<AuthenticatorClientPinResponse, Ctap2StatusCode> {
fn process_get_pin_retries(&self, env: &mut E) -> CtapResult<AuthenticatorClientPinResponse> {
Ok(AuthenticatorClientPinResponse {
key_agreement: None,
pin_uv_auth_token: None,
Expand All @@ -232,7 +230,7 @@ impl<E: Env> ClientPin<E> {
fn process_get_key_agreement(
&self,
client_pin_params: AuthenticatorClientPinParameters,
) -> Result<AuthenticatorClientPinResponse, Ctap2StatusCode> {
) -> CtapResult<AuthenticatorClientPinResponse> {
let key_agreement = Some(
self.get_pin_protocol(client_pin_params.pin_uv_auth_protocol)
.get_public_key(),
Expand All @@ -249,7 +247,7 @@ impl<E: Env> ClientPin<E> {
&mut self,
env: &mut E,
client_pin_params: AuthenticatorClientPinParameters,
) -> Result<(), Ctap2StatusCode> {
) -> CtapResult<()> {
let AuthenticatorClientPinParameters {
pin_uv_auth_protocol,
key_agreement,
Expand All @@ -276,7 +274,7 @@ impl<E: Env> ClientPin<E> {
&mut self,
env: &mut E,
client_pin_params: AuthenticatorClientPinParameters,
) -> Result<(), Ctap2StatusCode> {
) -> CtapResult<()> {
let AuthenticatorClientPinParameters {
pin_uv_auth_protocol,
key_agreement,
Expand Down Expand Up @@ -309,7 +307,7 @@ impl<E: Env> ClientPin<E> {
&mut self,
env: &mut E,
client_pin_params: AuthenticatorClientPinParameters,
) -> Result<AuthenticatorClientPinResponse, Ctap2StatusCode> {
) -> CtapResult<AuthenticatorClientPinResponse> {
let AuthenticatorClientPinParameters {
pin_uv_auth_protocol,
key_agreement,
Expand Down Expand Up @@ -357,12 +355,12 @@ impl<E: Env> ClientPin<E> {
// If you want to support local user verification, implement this function.
// Lacking a fingerprint reader, this subcommand is currently unsupported.
_client_pin_params: AuthenticatorClientPinParameters,
) -> Result<AuthenticatorClientPinResponse, Ctap2StatusCode> {
) -> CtapResult<AuthenticatorClientPinResponse> {
// User verification is only supported through PIN currently.
Err(Ctap2StatusCode::CTAP2_ERR_INVALID_SUBCOMMAND)
}

fn process_get_uv_retries(&self) -> Result<AuthenticatorClientPinResponse, Ctap2StatusCode> {
fn process_get_uv_retries(&self) -> CtapResult<AuthenticatorClientPinResponse> {
// User verification is only supported through PIN currently.
Err(Ctap2StatusCode::CTAP2_ERR_INVALID_SUBCOMMAND)
}
Expand All @@ -371,7 +369,7 @@ impl<E: Env> ClientPin<E> {
&mut self,
env: &mut E,
mut client_pin_params: AuthenticatorClientPinParameters,
) -> Result<AuthenticatorClientPinResponse, Ctap2StatusCode> {
) -> CtapResult<AuthenticatorClientPinResponse> {
// Mutating client_pin_params is just an optimization to move it into
// process_get_pin_token, without cloning permissions_rp_id here.
// getPinToken requires permissions* to be None.
Expand Down Expand Up @@ -399,7 +397,7 @@ impl<E: Env> ClientPin<E> {
&mut self,
env: &mut E,
client_pin_params: AuthenticatorClientPinParameters,
) -> Result<ResponseData, Ctap2StatusCode> {
) -> CtapResult<ResponseData> {
if !env.customization().allows_pin_protocol_v1()
&& client_pin_params.pin_uv_auth_protocol == PinUvAuthProtocol::V1
{
Expand Down Expand Up @@ -441,7 +439,7 @@ impl<E: Env> ClientPin<E> {
hmac_contents: &[u8],
pin_uv_auth_param: &[u8],
pin_uv_auth_protocol: PinUvAuthProtocol,
) -> Result<(), Ctap2StatusCode> {
) -> CtapResult<()> {
if !self.pin_uv_auth_token_state.is_in_use() {
return Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID);
}
Expand Down Expand Up @@ -477,7 +475,7 @@ impl<E: Env> ClientPin<E> {
env: &mut E,
hmac_secret_input: GetAssertionHmacSecretInput,
cred_random: &[u8; 32],
) -> Result<Vec<u8>, Ctap2StatusCode> {
) -> CtapResult<Vec<u8>> {
let GetAssertionHmacSecretInput {
key_agreement,
salt_enc,
Expand Down Expand Up @@ -523,7 +521,7 @@ impl<E: Env> ClientPin<E> {
}

/// Checks if user verification is cached for use of the pinUvAuthToken.
pub fn check_user_verified_flag(&mut self) -> Result<(), Ctap2StatusCode> {
pub fn check_user_verified_flag(&mut self) -> CtapResult<()> {
if self.pin_uv_auth_token_state.get_user_verified_flag_value() {
Ok(())
} else {
Expand All @@ -532,27 +530,24 @@ impl<E: Env> ClientPin<E> {
}

/// Check if the required command's token permission is granted.
pub fn has_permission(&self, permission: PinPermission) -> Result<(), Ctap2StatusCode> {
pub fn has_permission(&self, permission: PinPermission) -> CtapResult<()> {
self.pin_uv_auth_token_state.has_permission(permission)
}

/// Check if no RP ID is associated with the token permission.
pub fn has_no_rp_id_permission(&self) -> Result<(), Ctap2StatusCode> {
pub fn has_no_rp_id_permission(&self) -> CtapResult<()> {
self.pin_uv_auth_token_state.has_no_permissions_rp_id()
}

/// Check if no or the passed RP ID is associated with the token permission.
pub fn has_no_or_rp_id_permission(&mut self, rp_id: &str) -> Result<(), Ctap2StatusCode> {
pub fn has_no_or_rp_id_permission(&mut self, rp_id: &str) -> CtapResult<()> {
self.pin_uv_auth_token_state
.has_no_permissions_rp_id()
.or_else(|_| self.pin_uv_auth_token_state.has_permissions_rp_id(rp_id))
}

/// Check if no RP ID is associated with the token permission, or it matches the hash.
pub fn has_no_or_rp_id_hash_permission(
&self,
rp_id_hash: &[u8],
) -> Result<(), Ctap2StatusCode> {
pub fn has_no_or_rp_id_hash_permission(&self, rp_id_hash: &[u8]) -> CtapResult<()> {
self.pin_uv_auth_token_state
.has_no_permissions_rp_id()
.or_else(|_| {
Expand All @@ -564,7 +559,7 @@ impl<E: Env> ClientPin<E> {
/// Check if the passed RP ID is associated with the token permission.
///
/// If no RP ID is associated, associate the passed RP ID as a side effect.
pub fn ensure_rp_id_permission(&mut self, rp_id: &str) -> Result<(), Ctap2StatusCode> {
pub fn ensure_rp_id_permission(&mut self, rp_id: &str) -> CtapResult<()> {
if self
.pin_uv_auth_token_state
.has_no_permissions_rp_id()
Expand Down Expand Up @@ -1277,7 +1272,7 @@ mod test {
pin_uv_auth_protocol: PinUvAuthProtocol,
cred_random: &[u8; 32],
salt: Vec<u8>,
) -> Result<Vec<u8>, Ctap2StatusCode> {
) -> CtapResult<Vec<u8>> {
let mut env = TestEnv::default();
let (client_pin, shared_secret) = create_client_pin_and_shared_secret(pin_uv_auth_protocol);

Expand Down
21 changes: 11 additions & 10 deletions libraries/opensk/src/ctap/command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use super::data_formats::{
#[cfg(feature = "config_command")]
use super::data_formats::{ConfigSubCommand, ConfigSubCommandParams, SetMinPinLengthParams};
use super::status_code::Ctap2StatusCode;
use crate::ctap::status_code::CtapResult;
use alloc::string::String;
use alloc::vec::Vec;
#[cfg(feature = "fuzz")]
Expand Down Expand Up @@ -70,7 +71,7 @@ impl Command {
const AUTHENTICATOR_VENDOR_CREDENTIAL_MANAGEMENT: u8 = 0x41;
const _AUTHENTICATOR_VENDOR_LAST: u8 = 0xBF;

pub fn deserialize(bytes: &[u8]) -> Result<Command, Ctap2StatusCode> {
pub fn deserialize(bytes: &[u8]) -> CtapResult<Command> {
if bytes.is_empty() {
// The error to return is not specified, missing parameter seems to fit best.
return Err(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER);
Expand Down Expand Up @@ -157,7 +158,7 @@ pub struct AuthenticatorMakeCredentialParameters {
impl TryFrom<cbor::Value> for AuthenticatorMakeCredentialParameters {
type Error = Ctap2StatusCode;

fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
fn try_from(cbor_value: cbor::Value) -> CtapResult<Self> {
destructure_cbor_map! {
let {
0x01 => client_data_hash,
Expand All @@ -181,15 +182,15 @@ impl TryFrom<cbor::Value> for AuthenticatorMakeCredentialParameters {
let pub_key_cred_params = cred_param_vec
.into_iter()
.map(PublicKeyCredentialParameter::try_from)
.collect::<Result<Vec<PublicKeyCredentialParameter>, Ctap2StatusCode>>()?;
.collect::<CtapResult<Vec<PublicKeyCredentialParameter>>>()?;

let exclude_list = match exclude_list {
Some(entry) => {
let exclude_list_vec = extract_array(entry)?;
let exclude_list = exclude_list_vec
.into_iter()
.map(PublicKeyCredentialDescriptor::try_from)
.collect::<Result<Vec<PublicKeyCredentialDescriptor>, Ctap2StatusCode>>()?;
.collect::<CtapResult<Vec<PublicKeyCredentialDescriptor>>>()?;
Some(exclude_list)
}
None => None,
Expand Down Expand Up @@ -244,7 +245,7 @@ pub struct AuthenticatorGetAssertionParameters {
impl TryFrom<cbor::Value> for AuthenticatorGetAssertionParameters {
type Error = Ctap2StatusCode;

fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
fn try_from(cbor_value: cbor::Value) -> CtapResult<Self> {
destructure_cbor_map! {
let {
0x01 => rp_id,
Expand All @@ -266,7 +267,7 @@ impl TryFrom<cbor::Value> for AuthenticatorGetAssertionParameters {
let allow_list = allow_list_vec
.into_iter()
.map(PublicKeyCredentialDescriptor::try_from)
.collect::<Result<Vec<PublicKeyCredentialDescriptor>, Ctap2StatusCode>>()?;
.collect::<CtapResult<Vec<PublicKeyCredentialDescriptor>>>()?;
Some(allow_list)
}
None => None,
Expand Down Expand Up @@ -315,7 +316,7 @@ pub struct AuthenticatorClientPinParameters {
impl TryFrom<cbor::Value> for AuthenticatorClientPinParameters {
type Error = Ctap2StatusCode;

fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
fn try_from(cbor_value: cbor::Value) -> CtapResult<Self> {
destructure_cbor_map! {
let {
0x01 => pin_uv_auth_protocol,
Expand Down Expand Up @@ -370,7 +371,7 @@ pub struct AuthenticatorLargeBlobsParameters {
impl TryFrom<cbor::Value> for AuthenticatorLargeBlobsParameters {
type Error = Ctap2StatusCode;

fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
fn try_from(cbor_value: cbor::Value) -> CtapResult<Self> {
destructure_cbor_map! {
let {
0x01 => get,
Expand Down Expand Up @@ -432,7 +433,7 @@ pub struct AuthenticatorConfigParameters {
impl TryFrom<cbor::Value> for AuthenticatorConfigParameters {
type Error = Ctap2StatusCode;

fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
fn try_from(cbor_value: cbor::Value) -> CtapResult<Self> {
destructure_cbor_map! {
let {
0x01 => sub_command,
Expand Down Expand Up @@ -474,7 +475,7 @@ pub struct AuthenticatorCredentialManagementParameters {
impl TryFrom<cbor::Value> for AuthenticatorCredentialManagementParameters {
type Error = Ctap2StatusCode;

fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
fn try_from(cbor_value: cbor::Value) -> CtapResult<Self> {
destructure_cbor_map! {
let {
0x01 => sub_command,
Expand Down
Loading

0 comments on commit 4c23e61

Please sign in to comment.