Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uses CtapResult instead of explicit Result #696

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Uses CtapResult instead of explicit Result #696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions libraries/opensk/src/api/private_key.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading