From 7e41e0fa566b3a1e55bbc953f4f7891267cf875e Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Fri, 18 Oct 2024 18:16:51 +0200 Subject: [PATCH 1/2] Remove unused TryFrom implementation for YkGetHmac --- src/command.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/command.rs b/src/command.rs index 3fdb0d99..368367c8 100644 --- a/src/command.rs +++ b/src/command.rs @@ -70,16 +70,6 @@ pub struct YkGetHmac<'l> { pub slot_cmd: Option, } -impl<'l, const C: usize> TryFrom<&'l Data> for YkGetHmac<'l> { - type Error = Status; - fn try_from(data: &'l Data) -> Result { - Ok(Self { - challenge: data, - slot_cmd: None, - }) - } -} - impl<'l> YkGetHmac<'l> { pub fn get_credential_label(&self) -> Result<&[u8], Status> { Ok(match self.slot_cmd.ok_or(Status::IncorrectDataParameter)? { From 690e9d1ffb84ed8a1b74db1370eaecc31a6f6560 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Oct 2024 23:19:31 +0200 Subject: [PATCH 2/2] Use apdu-app instead of apdu-dispatch --- Cargo.toml | 6 +-- fuzz/corpus-viewer/corpus-viewer.rs | 4 +- fuzz/fuzz_targets/fuzz_target_1.rs | 15 ++----- fuzz/fuzz_targets/virt.rs | 10 ++--- fuzz/fuzz_targets/virt/dispatch.rs | 3 +- src/authenticator.rs | 14 +++--- src/command.rs | 68 ++++++++++++++--------------- src/ctaphid.rs | 19 ++++---- 8 files changed, 66 insertions(+), 73 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2ab256a0..40bd65dd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ license = "Apache-2.0 OR MIT" description = "Secrets App - a Trussed app to manage OTP and Password Safe features of Nitrokey 3" [dependencies] -apdu-dispatch = { version = "0.1.2", optional = true } +apdu-app = { version = "0.1", optional = true } ctaphid-dispatch = { version = "0.1", optional = true } cbor-smol = "0.4" delog = "0.1.6" @@ -16,7 +16,7 @@ flexiber = { version = "0.1.0", features = ["derive", "heapless"] } heapless = "0.7" heapless-bytes = "0.3" hex-literal = "0.3" -iso7816 = "0.1" +iso7816 = "0.1.3" serde = { version = "1", default-features = false } trussed = { version = "0.1", features = ["clients-3"] } encrypted_container = { path = "components/encrypted_container" } @@ -29,6 +29,7 @@ trussed-auth = "0.3.0" [features] default = ["apdu-dispatch"] devel = ["apdu-dispatch", "log-all", "delog/std-log", "devel-counters"] +apdu-dispatch = ["dep:apdu-app"] # Count accesses to the read-only and read-write persistence storage devel-counters = [] @@ -50,7 +51,6 @@ log-warn = [] log-error = [] [patch.crates-io] -apdu-dispatch = { git = "https://github.com/trussed-dev/apdu-dispatch.git", rev = "915fc237103fcecc29d0f0b73391f19abf6576de" } ctaphid-dispatch = { git = "https://github.com/Nitrokey/ctaphid-dispatch", tag = "v0.1.1-nitrokey.2" } flexiber = { git = "https://github.com/Nitrokey/flexiber", tag = "0.1.1.nitrokey" } littlefs2 = { git = "https://github.com/trussed-dev/littlefs2.git", rev = "ebd27e49ca321089d01d8c9b169c4aeb58ceeeca" } diff --git a/fuzz/corpus-viewer/corpus-viewer.rs b/fuzz/corpus-viewer/corpus-viewer.rs index 849eda91..db277cd8 100644 --- a/fuzz/corpus-viewer/corpus-viewer.rs +++ b/fuzz/corpus-viewer/corpus-viewer.rs @@ -57,8 +57,8 @@ fn main() -> Result<(), ()> { let commands = parse(data.as_ref()); for data in commands { - if let Ok(command) = iso7816::Command::<{ 10 * 255 }>::try_from(data) { - if let Ok(cmd) = secrets_app::Command::try_from(&command) { + if let Ok(command) = iso7816::command::CommandView::try_from(data) { + if let Ok(cmd) = secrets_app::Command::try_from(command) { println!(">>> {:?}", cmd); } else { println!(">>> (unparsed) {:?}", command); diff --git a/fuzz/fuzz_targets/fuzz_target_1.rs b/fuzz/fuzz_targets/fuzz_target_1.rs index 1e308bbc..aa9faabc 100644 --- a/fuzz/fuzz_targets/fuzz_target_1.rs +++ b/fuzz/fuzz_targets/fuzz_target_1.rs @@ -42,25 +42,18 @@ use trussed::types::Location; mod virt; fuzz_target!(|data: &[u8]| { - virt::with_ram_client("secrets", move |client| { - - let options = secrets_app::Options::new( - Location::Internal, - 0, - 1, - [0x42, 0x42, 0x42, 0x42], - u16::MAX, - ); + let options = + secrets_app::Options::new(Location::Internal, 0, 1, [0x42, 0x42, 0x42, 0x42], u16::MAX); let mut secrets = secrets_app::Authenticator::new(client, options); let mut response = heapless::Vec::::new(); let commands = parse(data); for data in commands { - if let Ok(command) = iso7816::Command::<{ 10 * 255 }>::try_from(data) { + if let Ok(command) = iso7816::command::CommandView::try_from(data) { response.clear(); - secrets.respond(&command, &mut response).ok(); + secrets.respond(command, &mut response).ok(); } } }) diff --git a/fuzz/fuzz_targets/virt.rs b/fuzz/fuzz_targets/virt.rs index 403eaf06..e45c8a8a 100644 --- a/fuzz/fuzz_targets/virt.rs +++ b/fuzz/fuzz_targets/virt.rs @@ -13,9 +13,9 @@ pub type VirtClient = Client; /// Run a client using a provided store pub fn with_client(store: S, client_id: &str, f: F) -> R - where - F: FnOnce(VirtClient) -> R, - S: StoreProvider, +where + F: FnOnce(VirtClient) -> R, + S: StoreProvider, { #[allow(clippy::unwrap_used)] virt::with_platform(store, |platform| { @@ -31,8 +31,8 @@ pub fn with_client(store: S, client_id: &str, f: F) -> R /// Run the backend with the extensions required /// using a RAM file storage pub fn with_ram_client(client_id: &str, f: F) -> R - where - F: FnOnce(VirtClient) -> R, +where + F: FnOnce(VirtClient) -> R, { with_client(Ram::default(), client_id, f) } diff --git a/fuzz/fuzz_targets/virt/dispatch.rs b/fuzz/fuzz_targets/virt/dispatch.rs index 443e867a..0ad530fb 100644 --- a/fuzz/fuzz_targets/virt/dispatch.rs +++ b/fuzz/fuzz_targets/virt/dispatch.rs @@ -9,8 +9,7 @@ use trussed::{ }; use trussed_auth::{AuthBackend, AuthContext, AuthExtension, MAX_HW_KEY_LEN}; -pub const BACKENDS: &[BackendId] = - &[BackendId::Custom(Backend::Auth), BackendId::Core]; +pub const BACKENDS: &[BackendId] = &[BackendId::Custom(Backend::Auth), BackendId::Core]; pub enum Backend { Auth, diff --git a/src/authenticator.rs b/src/authenticator.rs index 3ef48ade..42a523cc 100644 --- a/src/authenticator.rs +++ b/src/authenticator.rs @@ -232,9 +232,9 @@ where } /// Respond to the iso7816 encoded request - pub fn respond( + pub fn respond( &mut self, - command: &iso7816::Command, + command: iso7816::command::CommandView<'_>, reply: &mut Data, ) -> Result { let client_authorized_before = self.state.runtime.client_authorized; @@ -258,9 +258,9 @@ where result } - fn inner_respond( + fn inner_respond( &mut self, - command: &iso7816::Command, + command: iso7816::command::CommandView<'_>, reply: &mut Data, ) -> Result { let class = command.class(); @@ -1355,7 +1355,7 @@ impl iso7816::App for Authenticator { } #[cfg(feature = "apdu-dispatch")] -impl apdu_dispatch::app::App for Authenticator +impl apdu_app::App for Authenticator where T: client::Client + client::HmacSha1 @@ -1367,7 +1367,7 @@ where fn select( &mut self, _interface: iso7816::Interface, - apdu: &iso7816::Command, + apdu: iso7816::command::CommandView<'_>, reply: &mut Data, ) -> Result { self.respond(apdu, reply) @@ -1379,7 +1379,7 @@ where fn call( &mut self, _: iso7816::Interface, - apdu: &iso7816::Command, + apdu: iso7816::command::CommandView<'_>, reply: &mut Data, ) -> Result { self.respond(apdu, reply) diff --git a/src/command.rs b/src/command.rs index 368367c8..82e006ed 100644 --- a/src/command.rs +++ b/src/command.rs @@ -9,7 +9,7 @@ use flexiber::{Encodable, SimpleTag, TagLike}; use serde::{Deserialize, Serialize}; use iso7816::command::class::Class; -use iso7816::{Data, Instruction, Status}; +use iso7816::{Instruction, Status}; use YkCommand::GetSerial; use crate::oath::{Tag, YkCommand}; @@ -142,9 +142,9 @@ pub struct SetPassword<'l> { pub response: &'l [u8], } -impl<'l, const C: usize> TryFrom<&'l Data> for SetPassword<'l> { +impl<'l> TryFrom<&'l [u8]> for SetPassword<'l> { type Error = Status; - fn try_from(data: &'l Data) -> Result { + fn try_from(data: &'l [u8]) -> Result { // key = self.derive_key(password) // keydata = bytearray([OATH_TYPE.TOTP | ALGO.SHA1]) + key // challenge = os.urandom(8) @@ -205,9 +205,9 @@ pub struct Validate<'l> { pub challenge: &'l [u8], } -impl<'l, const C: usize> TryFrom<&'l Data> for Validate<'l> { +impl<'l> TryFrom<&'l [u8]> for Validate<'l> { type Error = Status; - fn try_from(data: &'l Data) -> Result { + fn try_from(data: &'l [u8]) -> Result { use flexiber::TaggedSlice; let mut decoder = flexiber::Decoder::new(data); @@ -238,9 +238,9 @@ pub struct VerifyCode<'l> { pub response: u32, } -impl<'l, const C: usize> TryFrom<&'l Data> for VerifyCode<'l> { +impl<'l> TryFrom<&'l [u8]> for VerifyCode<'l> { type Error = Status; - fn try_from(data: &'l Data) -> Result { + fn try_from(data: &'l [u8]) -> Result { use flexiber::TaggedSlice; let mut decoder = flexiber::Decoder::new(data); @@ -272,9 +272,9 @@ pub struct SetPin<'l> { pub password: &'l [u8], } -impl<'l, const C: usize> TryFrom<&'l Data> for SetPin<'l> { +impl<'l> TryFrom<&'l [u8]> for SetPin<'l> { type Error = Status; - fn try_from(data: &'l Data) -> Result { + fn try_from(data: &'l [u8]) -> Result { use flexiber::TaggedSlice; let mut decoder = flexiber::Decoder::new(data); @@ -295,9 +295,9 @@ pub struct RenameCredential<'l> { pub new_label: &'l [u8], } -impl<'l, const C: usize> TryFrom<&'l Data> for RenameCredential<'l> { +impl<'l> TryFrom<&'l [u8]> for RenameCredential<'l> { type Error = Status; - fn try_from(data: &'l Data) -> Result { + fn try_from(data: &'l [u8]) -> Result { use flexiber::TaggedSlice; let mut decoder = flexiber::Decoder::new(data); @@ -324,9 +324,9 @@ pub struct GetCredential<'l> { pub label: &'l [u8], } -impl<'l, const C: usize> TryFrom<&'l Data> for GetCredential<'l> { +impl<'l> TryFrom<&'l [u8]> for GetCredential<'l> { type Error = Status; - fn try_from(data: &'l Data) -> Result { + fn try_from(data: &'l [u8]) -> Result { use flexiber::TaggedSlice; let mut decoder = flexiber::Decoder::new(data); @@ -347,9 +347,9 @@ pub struct ChangePin<'l> { pub new_password: &'l [u8], } -impl<'l, const C: usize> TryFrom<&'l Data> for ChangePin<'l> { +impl<'l> TryFrom<&'l [u8]> for ChangePin<'l> { type Error = Status; - fn try_from(data: &'l Data) -> Result { + fn try_from(data: &'l [u8]) -> Result { use flexiber::TaggedSlice; let mut decoder = flexiber::Decoder::new(data); @@ -379,9 +379,9 @@ pub struct VerifyPin<'l> { pub password: &'l [u8], } -impl<'l, const C: usize> TryFrom<&'l Data> for VerifyPin<'l> { +impl<'l> TryFrom<&'l [u8]> for VerifyPin<'l> { type Error = Status; - fn try_from(data: &'l Data) -> Result { + fn try_from(data: &'l [u8]) -> Result { use flexiber::TaggedSlice; let mut decoder = flexiber::Decoder::new(data); @@ -402,9 +402,9 @@ pub struct Calculate<'l> { pub challenge: &'l [u8], } -impl<'l, const C: usize> TryFrom<&'l Data> for Calculate<'l> { +impl<'l> TryFrom<&'l [u8]> for Calculate<'l> { type Error = Status; - fn try_from(data: &'l Data) -> Result { + fn try_from(data: &'l [u8]) -> Result { use flexiber::TaggedSlice; let mut decoder = flexiber::Decoder::new(data); @@ -431,9 +431,9 @@ pub struct CalculateAll<'l> { pub challenge: &'l [u8], } -impl<'l, const C: usize> TryFrom<&'l Data> for CalculateAll<'l> { +impl<'l> TryFrom<&'l [u8]> for CalculateAll<'l> { type Error = Status; - fn try_from(data: &'l Data) -> Result { + fn try_from(data: &'l [u8]) -> Result { use flexiber::TaggedSlice; let mut decoder = flexiber::Decoder::new(data); @@ -464,9 +464,9 @@ impl core::fmt::Debug for Delete<'_> { } } -impl<'l, const C: usize> TryFrom<&'l Data> for Delete<'l> { +impl<'l> TryFrom<&'l [u8]> for Delete<'l> { type Error = Status; - fn try_from(data: &'l Data) -> Result { + fn try_from(data: &'l [u8]) -> Result { use flexiber::TaggedSlice; let mut decoder = flexiber::Decoder::new(data); @@ -481,9 +481,9 @@ impl<'l, const C: usize> TryFrom<&'l Data> for Delete<'l> { } } -impl<'l, const C: usize> TryFrom<&'l Data> for ListCredentials { +impl<'l> TryFrom<&'l [u8]> for ListCredentials { type Error = Status; - fn try_from(data: &'l Data) -> Result { + fn try_from(data: &'l [u8]) -> Result { let v = if !data.is_empty() { data[0] } else { 0 }; Ok(ListCredentials { version: v }) } @@ -516,9 +516,9 @@ pub struct UpdateCredential<'l> { pub password_safe: Option>, } -impl<'l, const C: usize> TryFrom<&'l Data> for UpdateCredential<'l> { +impl<'l> TryFrom<&'l [u8]> for UpdateCredential<'l> { type Error = Status; - fn try_from(data: &'l Data) -> Result { + fn try_from(data: &'l [u8]) -> Result { use flexiber::TaggedSlice; let mut decoder = flexiber::Decoder::new(data); @@ -697,10 +697,10 @@ impl TryFrom for Tag { } } -impl<'l, const C: usize> TryFrom<&'l Data> for Register<'l> { +impl<'l> TryFrom<&'l [u8]> for Register<'l> { type Error = Status; - fn try_from(data: &'l Data) -> Result { + fn try_from(data: &'l [u8]) -> Result { // All fields of the OTP Credential are obligatory // The PWS entries are optional use flexiber::Decodable; @@ -878,7 +878,7 @@ impl<'l> Command<'l> { } } } -impl<'l, const C: usize> TryFrom<&'l iso7816::Command> for Command<'l> { +impl<'l> TryFrom> for Command<'l> { type Error = Status; /// The first layer of unraveling the iso7816::Command onion. /// @@ -886,7 +886,7 @@ impl<'l, const C: usize> TryFrom<&'l iso7816::Command> for Command<'l> { /// in the "Command Syntax" boxes of NIST SP 800-73-4, and return early errors. /// /// The individual piv::Command TryFroms then further interpret these validated parameters. - fn try_from(command: &'l iso7816::Command) -> Result { + fn try_from(command: iso7816::command::CommandView<'l>) -> Result { let (class, instruction, p1, p2) = ( command.class(), command.instruction(), @@ -956,11 +956,11 @@ impl<'l, const C: usize> TryFrom<&'l iso7816::Command> for Command<'l> { } } -impl<'l, const C: usize> TryFrom<&'l Data> for Select<'l> { +impl<'l> TryFrom<&'l [u8]> for Select<'l> { type Error = Status; - fn try_from(data: &'l Data) -> Result { + fn try_from(data: &'l [u8]) -> Result { // info_now!("comparing {} against {}", hex_str!(data.as_slice()), hex_str!(crate::YUBICO_OATH_AID)); - Ok(match data.as_slice() { + Ok(match data { crate::YUBICO_OATH_AID => Self { aid: data }, _ => return Err(Status::NotFound), }) diff --git a/src/ctaphid.rs b/src/ctaphid.rs index ff20d0eb..26f250f7 100644 --- a/src/ctaphid.rs +++ b/src/ctaphid.rs @@ -3,7 +3,7 @@ // // SPDX-License-Identifier: Apache-2.0 OR MIT -use crate::{Authenticator, CTAPHID_MESSAGE_SIZE_LIMIT}; +use crate::Authenticator; use ctaphid_dispatch::app::{self, Command as HidCommand, Message}; use ctaphid_dispatch::command::VendorCommand; use iso7816::Status; @@ -29,18 +29,19 @@ where input_data: &Message, response: &mut Message, ) -> app::AppResult { - const MAX_COMMAND_LENGTH: usize = CTAPHID_MESSAGE_SIZE_LIMIT; match command { HidCommand::Vendor(OTP_CCID) => { let arr: [u8; 2] = Status::Success.into(); response.extend(arr); - let ctap_to_iso7816_command = - iso7816::Command::::try_from(input_data).map_err(|_e| { - response.clear(); - info_now!("ISO conversion error: {:?}", _e); - app::Error::InvalidLength - })?; - let res = self.respond(&ctap_to_iso7816_command, response); + let ctap_to_iso7816_command = iso7816::command::CommandView::try_from( + input_data.as_slice(), + ) + .map_err(|_e| { + response.clear(); + info_now!("ISO conversion error: {:?}", _e); + app::Error::InvalidLength + })?; + let res = self.respond(ctap_to_iso7816_command, response); match res { Ok(_) => return Ok(()),