From a111a1b41f7972debea66535b409ce451b4ae8e5 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Thu, 13 Feb 2025 12:17:09 +0100 Subject: [PATCH 1/6] cleanup imports --- src/lib_ykoath2/oath_credential.rs | 15 +-------------- src/lib_ykoath2/oath_credentialid.rs | 13 ++++--------- src/lib_ykoath2/transaction.rs | 7 ++----- 3 files changed, 7 insertions(+), 28 deletions(-) diff --git a/src/lib_ykoath2/oath_credential.rs b/src/lib_ykoath2/oath_credential.rs index 70afe8f..6cc0140 100644 --- a/src/lib_ykoath2/oath_credential.rs +++ b/src/lib_ykoath2/oath_credential.rs @@ -1,20 +1,7 @@ -#[crate_type = "lib"] -use crate::lib_ykoath2::*; -/// Utilities for interacting with YubiKey OATH/TOTP functionality -extern crate pcsc; -use pbkdf2::pbkdf2_hmac_array; -use regex::Regex; -use sha1::Sha1; - -use std::str::{self}; - -use base64::{engine::general_purpose, Engine as _}; -use hmac::{Hmac, Mac}; - use std::cmp::Ordering; use std::hash::{Hash, Hasher}; -use std::time::SystemTime; +use crate::lib_ykoath2::CredentialIDData; #[derive(Debug, Clone)] pub struct OathCredential { diff --git a/src/lib_ykoath2/oath_credentialid.rs b/src/lib_ykoath2/oath_credentialid.rs index df7ff2b..08191a1 100644 --- a/src/lib_ykoath2/oath_credentialid.rs +++ b/src/lib_ykoath2/oath_credentialid.rs @@ -1,13 +1,8 @@ -#![crate_type = "lib"] -use crate::lib_ykoath2::*; -/// Utilities for interacting with YubiKey OATH/TOTP functionality -extern crate pcsc; use regex::Regex; -use std::{ - fmt::Write, - str::{self}, -}; +use std::fmt::Display; + +use crate::lib_ykoath2::{OathType, Tag, DEFAULT_PERIOD}; #[derive(Debug, Eq, PartialEq, Clone)] pub struct CredentialIDData { @@ -48,7 +43,7 @@ impl CredentialIDData { // Function to parse the credential ID fn parse_cred_id(cred_id: &[u8], oath_type: OathType) -> (Option, String, u32) { - let data = match str::from_utf8(cred_id) { + let data = match std::str::from_utf8(cred_id) { Ok(d) => d, Err(_) => return (None, String::new(), 0), // Handle invalid UTF-8 }; diff --git a/src/lib_ykoath2/transaction.rs b/src/lib_ykoath2/transaction.rs index a2bdf45..a7817b1 100644 --- a/src/lib_ykoath2/transaction.rs +++ b/src/lib_ykoath2/transaction.rs @@ -1,12 +1,7 @@ -#[crate_type = "lib"] -use crate::lib_ykoath2::*; -/// Utilities for interacting with YubiKey OATH/TOTP functionality -extern crate pcsc; use iso7816_tlv::simple::{Tag as TlvTag, Tlv}; use ouroboros::self_referencing; use std::collections::HashMap; use std::fmt::Display; -use std::str::{self}; use apdu_core::{Command, Response}; @@ -14,6 +9,8 @@ use pcsc::{Card, Transaction}; use std::ffi::CString; +use crate::lib_ykoath2::{ErrorResponse, Instruction, SuccessResponse, Tag}; + #[derive(PartialEq, Eq, Debug)] pub enum FormattableErrorResponse { NoError, From dd7eb63afbbb0e1cd2fac368f2441a931ff3defe Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Thu, 13 Feb 2025 12:20:35 +0100 Subject: [PATCH 2/6] turn into a proper library with a separate example the example can be built with cargo run --example example --- Cargo.lock | 26 +++++++++++----------- Cargo.toml | 6 ++++- src/{lib_ykoath2 => }/constants.rs | 0 src/{main.rs => example.rs} | 2 -- src/{lib_ykoath2/mod.rs => lib.rs} | 0 src/{lib_ykoath2 => }/oath_credential.rs | 2 +- src/{lib_ykoath2 => }/oath_credentialid.rs | 2 +- src/{lib_ykoath2 => }/transaction.rs | 2 +- 8 files changed, 21 insertions(+), 19 deletions(-) rename src/{lib_ykoath2 => }/constants.rs (100%) rename src/{main.rs => example.rs} (99%) rename src/{lib_ykoath2/mod.rs => lib.rs} (100%) rename src/{lib_ykoath2 => }/oath_credential.rs (96%) rename src/{lib_ykoath2 => }/oath_credentialid.rs (97%) rename src/{lib_ykoath2 => }/transaction.rs (98%) diff --git a/Cargo.lock b/Cargo.lock index 3e80e4c..34a9edc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -121,19 +121,7 @@ dependencies = [ ] [[package]] -name = "libc" -version = "0.2.169" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b5aba8db14291edd000dfcc4d620c7ebfb122c613afb886ca8803fa4e128a20a" - -[[package]] -name = "memchr" -version = "2.7.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" - -[[package]] -name = "oath-rs-experiments" +name = "lib_ykoath2" version = "0.1.0" dependencies = [ "apdu-core", @@ -150,6 +138,18 @@ dependencies = [ "strum_macros", ] +[[package]] +name = "libc" +version = "0.2.169" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b5aba8db14291edd000dfcc4d620c7ebfb122c613afb886ca8803fa4e128a20a" + +[[package]] +name = "memchr" +version = "2.7.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" + [[package]] name = "ouroboros" version = "0.18.5" diff --git a/Cargo.toml b/Cargo.toml index 5d804f8..5fe1738 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,9 +13,13 @@ strum = { version = "0.27.0", features = [ ] } strum_macros = "0.27.0" [package] -name = "oath-rs-experiments" +name = "lib_ykoath2" version = "0.1.0" edition = "2021" authors = ["Grimmauld "] description = "experiments with smartcards" license-file = "LICENSE" + +[[example]] +name = "example" +path = "./src/example.rs" \ No newline at end of file diff --git a/src/lib_ykoath2/constants.rs b/src/constants.rs similarity index 100% rename from src/lib_ykoath2/constants.rs rename to src/constants.rs diff --git a/src/main.rs b/src/example.rs similarity index 99% rename from src/main.rs rename to src/example.rs index c6f19b9..1931d1c 100644 --- a/src/main.rs +++ b/src/example.rs @@ -1,7 +1,5 @@ // SPDX-License-Identifier: BSD-3-Clause -mod lib_ykoath2; - use core::time; use lib_ykoath2::OathSession; use pcsc; diff --git a/src/lib_ykoath2/mod.rs b/src/lib.rs similarity index 100% rename from src/lib_ykoath2/mod.rs rename to src/lib.rs diff --git a/src/lib_ykoath2/oath_credential.rs b/src/oath_credential.rs similarity index 96% rename from src/lib_ykoath2/oath_credential.rs rename to src/oath_credential.rs index 6cc0140..6344520 100644 --- a/src/lib_ykoath2/oath_credential.rs +++ b/src/oath_credential.rs @@ -1,7 +1,7 @@ use std::cmp::Ordering; use std::hash::{Hash, Hasher}; -use crate::lib_ykoath2::CredentialIDData; +use crate::CredentialIDData; #[derive(Debug, Clone)] pub struct OathCredential { diff --git a/src/lib_ykoath2/oath_credentialid.rs b/src/oath_credentialid.rs similarity index 97% rename from src/lib_ykoath2/oath_credentialid.rs rename to src/oath_credentialid.rs index 08191a1..63a7e56 100644 --- a/src/lib_ykoath2/oath_credentialid.rs +++ b/src/oath_credentialid.rs @@ -2,7 +2,7 @@ use regex::Regex; use std::fmt::Display; -use crate::lib_ykoath2::{OathType, Tag, DEFAULT_PERIOD}; +use crate::{OathType, Tag, DEFAULT_PERIOD}; #[derive(Debug, Eq, PartialEq, Clone)] pub struct CredentialIDData { diff --git a/src/lib_ykoath2/transaction.rs b/src/transaction.rs similarity index 98% rename from src/lib_ykoath2/transaction.rs rename to src/transaction.rs index a7817b1..002b07f 100644 --- a/src/lib_ykoath2/transaction.rs +++ b/src/transaction.rs @@ -9,7 +9,7 @@ use pcsc::{Card, Transaction}; use std::ffi::CString; -use crate::lib_ykoath2::{ErrorResponse, Instruction, SuccessResponse, Tag}; +use crate::{ErrorResponse, Instruction, SuccessResponse, Tag}; #[derive(PartialEq, Eq, Debug)] pub enum FormattableErrorResponse { From 7767b6e1714e9166a23531a793a75a7dc543bba2 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Thu, 13 Feb 2025 12:24:17 +0100 Subject: [PATCH 3/6] cleanup imports round 2 --- src/constants.rs | 21 ++++++++++----------- src/example.rs | 8 ++------ src/lib.rs | 14 +++----------- 3 files changed, 15 insertions(+), 28 deletions(-) diff --git a/src/constants.rs b/src/constants.rs index 27d8709..efdc6fb 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -1,8 +1,7 @@ use std::fmt::Display; use iso7816_tlv::simple::Tlv; -use sha1::{Digest, Sha1}; -use sha2::{Sha256, Sha512}; +use sha1::Digest; use strum::IntoEnumIterator; // 0.17.1 use strum_macros::EnumIter; // 0.17.1 pub const INS_SELECT: u8 = 0xa4; @@ -114,18 +113,18 @@ impl HashAlgo { // returns a function capable of hashing a byte array pub fn get_hash_fun(&self) -> impl Fn(&[u8]) -> Vec { match self { - HashAlgo::Sha1 => |m: &[u8]| { - let mut hasher = Sha1::new(); + Self::Sha1 => |m: &[u8]| { + let mut hasher = sha1::Sha1::new(); hasher.update(m); hasher.finalize().to_vec() }, - HashAlgo::Sha256 => |m: &[u8]| { - let mut hasher = Sha256::new(); + Self::Sha256 => |m: &[u8]| { + let mut hasher = sha2::Sha256::new(); hasher.update(m); hasher.finalize().to_vec() }, - HashAlgo::Sha512 => |m: &[u8]| { - let mut hasher = Sha512::new(); + Self::Sha512 => |m: &[u8]| { + let mut hasher = sha2::Sha512::new(); hasher.update(m); hasher.finalize().to_vec() }, @@ -135,9 +134,9 @@ impl HashAlgo { // returns digest output size in number of bytes pub fn digest_size(&self) -> usize { match self { - HashAlgo::Sha1 => 20, - HashAlgo::Sha256 => 32, - HashAlgo::Sha512 => 64, + Self::Sha1 => 20, + Self::Sha256 => 32, + Self::Sha512 => 64, } } } diff --git a/src/example.rs b/src/example.rs index 1931d1c..951c16d 100644 --- a/src/example.rs +++ b/src/example.rs @@ -1,10 +1,6 @@ // SPDX-License-Identifier: BSD-3-Clause -use core::time; use lib_ykoath2::OathSession; -use pcsc; -use std::process; -use std::thread; // use crate::args::Cli; // use clap::Parser; @@ -27,7 +23,7 @@ fn main() { // Show message if no YubiKey(s) if yubikeys.len() == 0 { println!("No yubikeys detected"); - process::exit(0); + std::process::exit(0); } // Print device info for all the YubiKeys we detected @@ -53,7 +49,7 @@ fn main() { println!("No credentials on device {}", device_label); } - thread::sleep(time::Duration::from_secs(5)); // show refresh is working + std::thread::sleep(std::time::Duration::from_secs(5)); // show refresh is working // Enumerate the OATH codes for oath in codes { diff --git a/src/lib.rs b/src/lib.rs index 4433066..40258ed 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,15 +7,7 @@ mod oath_credentialid; use oath_credential::*; use oath_credentialid::*; /// Utilities for interacting with YubiKey OATH/TOTP functionality -extern crate pcsc; -use pbkdf2::pbkdf2_hmac_array; -use sha1::Sha1; - -use std::{ - fmt::Display, - str::{self}, - time::Duration, -}; +use std::{fmt::Display, time::Duration}; use base64::{engine::general_purpose, Engine as _}; use hmac::{Hmac, Mac}; @@ -32,13 +24,13 @@ fn _get_device_id(salt: Vec) -> String { return general_purpose::URL_SAFE_NO_PAD.encode(hash_16_bytes); } fn _hmac_sha1(key: &[u8], message: &[u8]) -> Vec { - let mut mac = Hmac::::new_from_slice(key).expect("Invalid key length"); + let mut mac = Hmac::::new_from_slice(key).expect("Invalid key length"); mac.update(message); mac.finalize().into_bytes().to_vec() } fn _derive_key(salt: &[u8], passphrase: &str) -> Vec { - pbkdf2_hmac_array::(passphrase.as_bytes(), salt, 1000).to_vec() + pbkdf2::pbkdf2_hmac_array::(passphrase.as_bytes(), salt, 1000).to_vec() } fn _hmac_shorten_key(key: &[u8], algo: HashAlgo) -> Vec { From 821229af35d26896d69a63c75ed0f7d639b3b9d5 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Thu, 13 Feb 2025 12:25:57 +0100 Subject: [PATCH 4/6] enable some rustfmt nightly features to keep the grouping automatically --- .rustfmt.toml | 3 +++ src/lib.rs | 15 +++++++-------- src/oath_credential.rs | 6 ++++-- src/oath_credentialid.rs | 4 ++-- src/transaction.rs | 10 +++------- 5 files changed, 19 insertions(+), 19 deletions(-) create mode 100644 .rustfmt.toml diff --git a/.rustfmt.toml b/.rustfmt.toml new file mode 100644 index 0000000..6a75768 --- /dev/null +++ b/.rustfmt.toml @@ -0,0 +1,3 @@ +imports_granularity = "Crate" +format_code_in_doc_comments = true +group_imports = "StdExternalCrate" diff --git a/src/lib.rs b/src/lib.rs index 40258ed..aefd4d1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,15 +4,13 @@ mod transaction; use transaction::*; mod oath_credential; mod oath_credentialid; -use oath_credential::*; -use oath_credentialid::*; /// Utilities for interacting with YubiKey OATH/TOTP functionality -use std::{fmt::Display, time::Duration}; +use std::{fmt::Display, time::Duration, time::SystemTime}; use base64::{engine::general_purpose, Engine as _}; use hmac::{Hmac, Mac}; - -use std::time::SystemTime; +use oath_credential::*; +use oath_credentialid::*; fn _get_device_id(salt: Vec) -> String { let result = HashAlgo::Sha256.get_hash_fun()(salt.leak()); @@ -221,7 +219,8 @@ impl<'a> OathSession<'a> { )) } - /// Read the OATH codes from the device, calculate TOTP codes that don't need touch + /// Read the OATH codes from the device, calculate TOTP codes that don't + /// need touch pub fn calculate_oath_codes( &self, ) -> Result, FormattableErrorResponse> { @@ -242,8 +241,8 @@ impl<'a> OathSession<'a> { let id_data = CredentialIDData::from_tlv(cred_id.value(), meta.tag()); let code = OathCodeDisplay::from_tlv(meta); - /* println!("id bytes: {:?}", cred_id.value()); - println!("id recon: {:?}", id_data.format_cred_id()); */ + // println!("id bytes: {:?}", cred_id.value()); + // println!("id recon: {:?}", id_data.format_cred_id()); let cred = OathCredential { device_id: self.name.clone(), diff --git a/src/oath_credential.rs b/src/oath_credential.rs index 6344520..1712ef6 100644 --- a/src/oath_credential.rs +++ b/src/oath_credential.rs @@ -1,5 +1,7 @@ -use std::cmp::Ordering; -use std::hash::{Hash, Hasher}; +use std::{ + cmp::Ordering, + hash::{Hash, Hasher}, +}; use crate::CredentialIDData; diff --git a/src/oath_credentialid.rs b/src/oath_credentialid.rs index 63a7e56..ce98f48 100644 --- a/src/oath_credentialid.rs +++ b/src/oath_credentialid.rs @@ -1,7 +1,7 @@ -use regex::Regex; - use std::fmt::Display; +use regex::Regex; + use crate::{OathType, Tag, DEFAULT_PERIOD}; #[derive(Debug, Eq, PartialEq, Clone)] diff --git a/src/transaction.rs b/src/transaction.rs index 002b07f..41dffe4 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -1,14 +1,10 @@ -use iso7816_tlv::simple::{Tag as TlvTag, Tlv}; -use ouroboros::self_referencing; -use std::collections::HashMap; -use std::fmt::Display; +use std::{collections::HashMap, ffi::CString, fmt::Display}; use apdu_core::{Command, Response}; - +use iso7816_tlv::simple::{Tag as TlvTag, Tlv}; +use ouroboros::self_referencing; use pcsc::{Card, Transaction}; -use std::ffi::CString; - use crate::{ErrorResponse, Instruction, SuccessResponse, Tag}; #[derive(PartialEq, Eq, Debug)] From dfdc63d42460c1d0a4e5cbe4036ad7aed7c6bf80 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Thu, 13 Feb 2025 12:35:40 +0100 Subject: [PATCH 5/6] slightly better error handling by implementing the proper traits for error types and propagating error where possible --- src/constants.rs | 29 ++++++++++++---------- src/example.rs | 2 +- src/lib.rs | 8 +++---- src/transaction.rs | 60 ++++++++++++++++++++++------------------------ 4 files changed, 50 insertions(+), 49 deletions(-) diff --git a/src/constants.rs b/src/constants.rs index efdc6fb..e54d57d 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -24,19 +24,6 @@ pub enum ErrorResponse { } impl ErrorResponse { - pub fn as_string(self) -> String { - match self { - ErrorResponse::NoSpace => "No Space left on device", - ErrorResponse::CommandAborted => "Command aborted", - ErrorResponse::InvalidInstruction => "Invalid instruction", - ErrorResponse::AuthRequired => "Authentication required", - ErrorResponse::WrongSyntax => "Wrong syntax", - ErrorResponse::GenericError => "Generic Error", - ErrorResponse::NoSuchObject => "No such Object", - } - .to_string() - } - pub fn any_match(code: u16) -> Option { for resp in ErrorResponse::iter() { if code == resp as u16 { @@ -47,6 +34,22 @@ impl ErrorResponse { } } +impl std::fmt::Display for ErrorResponse { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::NoSpace => f.write_str("No Space left on device"), + Self::CommandAborted => f.write_str("Command aborted"), + Self::InvalidInstruction => f.write_str("Invalid instruction"), + Self::AuthRequired => f.write_str("Authentication required"), + Self::WrongSyntax => f.write_str("Wrong syntax"), + Self::GenericError => f.write_str("Generic Error"), + Self::NoSuchObject => f.write_str("No such Object"), + } + } +} + +impl std::error::Error for ErrorResponse {} + #[derive(Debug, EnumIter, Clone, Copy)] #[repr(u16)] pub enum SuccessResponse { diff --git a/src/example.rs b/src/example.rs index 951c16d..1775ceb 100644 --- a/src/example.rs +++ b/src/example.rs @@ -30,7 +30,7 @@ fn main() { for yubikey in yubikeys { let device_label: &str = yubikey; println!("Found device with label {}", device_label); - let session = OathSession::new(yubikey); + let session = OathSession::new(yubikey).unwrap(); println!("YubiKey version is {:?}", session.get_version()); for c in session.list_oath_codes().unwrap() { println!("{}", c); diff --git a/src/lib.rs b/src/lib.rs index aefd4d1..26a56bd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -133,8 +133,8 @@ impl<'a> RefreshableOathCredential<'a> { } impl<'a> OathSession<'a> { - pub fn new(name: &str) -> Self { - let transaction_context = TransactionContext::from_name(name); + pub fn new(name: &str) -> Result { + let transaction_context = TransactionContext::from_name(name)?; let info_buffer = transaction_context .apdu_read_all(0, INS_SELECT, 0x04, 0, Some(&OATH_AID)) .unwrap(); @@ -145,7 +145,7 @@ impl<'a> OathSession<'a> { println!("{:?}: {:?}", tag, data); } - OathSession { + Ok(Self { version: clone_with_lifetime( info_map.get(&(Tag::Version as u8)).unwrap_or(&vec![0u8; 0]), ) @@ -160,7 +160,7 @@ impl<'a> OathSession<'a> { .leak(), name: name.to_string(), transaction_context, - } + }) } pub fn get_version(&self) -> &[u8] { diff --git a/src/transaction.rs b/src/transaction.rs index 41dffe4..5dc1165 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -18,23 +18,23 @@ pub enum FormattableErrorResponse { } impl FormattableErrorResponse { - pub fn from_apdu_response(sw1: u8, sw2: u8) -> FormattableErrorResponse { + pub fn from_apdu_response(sw1: u8, sw2: u8) -> Self { let code: u16 = (sw1 as u16 | sw2 as u16) << 8; if let Some(e) = ErrorResponse::any_match(code) { - return FormattableErrorResponse::Protocol(e); + return Self::Protocol(e); } if SuccessResponse::any_match(code) .or(SuccessResponse::any_match(sw1.into())) .is_some() { - return FormattableErrorResponse::NoError; + return Self::NoError; } - FormattableErrorResponse::Unknown(String::from("Unknown error")) + Self::Unknown(String::from("Unknown error")) } pub fn is_ok(&self) -> bool { - *self == FormattableErrorResponse::NoError + *self == Self::NoError } - pub fn as_opt(self) -> Option { + pub fn as_opt(self) -> Option { if self.is_ok() { None } else { @@ -42,25 +42,27 @@ impl FormattableErrorResponse { } } - fn from_transmit(err: pcsc::Error) -> FormattableErrorResponse { - FormattableErrorResponse::PcscError(err) + fn from_transmit(err: pcsc::Error) -> Self { + Self::PcscError(err) } +} - fn as_string(&self) -> String { - match self { - FormattableErrorResponse::NoError => "ok".to_string(), - FormattableErrorResponse::Unknown(msg) => msg.to_owned(), - FormattableErrorResponse::Protocol(error_response) => error_response.as_string(), - FormattableErrorResponse::PcscError(error) => format!("{}", error), - FormattableErrorResponse::ParsingError(msg) => msg.to_owned(), - FormattableErrorResponse::DeviceMismatchError => "Devices do not match".to_string(), - } +impl From for FormattableErrorResponse { + fn from(value: pcsc::Error) -> Self { + Self::PcscError(value) } } impl Display for FormattableErrorResponse { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str(&self.as_string()) + match self { + Self::NoError => f.write_str("ok"), + Self::Unknown(msg) => f.write_str(msg), + Self::Protocol(error_response) => f.write_fmt(format_args!("{}", error_response)), + Self::PcscError(error) => f.write_fmt(format_args!("{}", error)), + Self::ParsingError(msg) => f.write_str(msg), + Self::DeviceMismatchError => f.write_str("Devices do not match"), + } } } @@ -139,26 +141,22 @@ pub struct TransactionContext { } impl TransactionContext { - pub fn from_name(name: &str) -> Self { - // FIXME: error handling here - + pub fn from_name(name: &str) -> Result { // Establish a PC/SC context - let ctx = pcsc::Context::establish(pcsc::Scope::User).unwrap(); + let ctx = pcsc::Context::establish(pcsc::Scope::User)?; // Connect to the card - let card = ctx - .connect( - &CString::new(name).unwrap(), - pcsc::ShareMode::Shared, - pcsc::Protocols::ANY, - ) - .unwrap(); + let card = ctx.connect( + &CString::new(name).unwrap(), + pcsc::ShareMode::Shared, + pcsc::Protocols::ANY, + )?; - TransactionContextBuilder { + Ok(TransactionContextBuilder { card, transaction_builder: |c| c.transaction().unwrap(), } - .build() + .build()) } pub fn apdu( From 11105377357d603d62c73bc98b782427f423f242 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Thu, 13 Feb 2025 12:45:19 +0100 Subject: [PATCH 6/6] turn FormattableErrorResponse into an actual Error type By removing the NoError variant as it is useless. --- src/lib.rs | 33 ++++++++++----------------- src/transaction.rs | 56 +++++++++++++--------------------------------- 2 files changed, 27 insertions(+), 62 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 26a56bd..d614a5b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -133,11 +133,10 @@ impl<'a> RefreshableOathCredential<'a> { } impl<'a> OathSession<'a> { - pub fn new(name: &str) -> Result { + pub fn new(name: &str) -> Result { let transaction_context = TransactionContext::from_name(name)?; - let info_buffer = transaction_context - .apdu_read_all(0, INS_SELECT, 0x04, 0, Some(&OATH_AID)) - .unwrap(); + let info_buffer = + transaction_context.apdu_read_all(0, INS_SELECT, 0x04, 0, Some(&OATH_AID))?; let info_map = tlv_to_map(info_buffer); for (tag, data) in &info_map { @@ -167,10 +166,7 @@ impl<'a> OathSession<'a> { self.version } - pub fn delete_code( - &self, - cred: OathCredential, - ) -> Result { + pub fn delete_code(&self, cred: OathCredential) -> Result { self.transaction_context.apdu( 0, Instruction::Delete as u8, @@ -184,9 +180,9 @@ impl<'a> OathSession<'a> { &self, cred: OathCredential, timestamp_sys: Option, - ) -> Result { + ) -> Result { if self.name != cred.device_id { - return Err(FormattableErrorResponse::DeviceMismatchError); + return Err(Error::DeviceMismatchError); } let timestamp = time_to_u64(timestamp_sys.unwrap_or_else(SystemTime::now)); @@ -207,23 +203,18 @@ impl<'a> OathSession<'a> { Some(&data), ); - let meta = - TlvIter::from_vec(resp?) - .next() - .ok_or(FormattableErrorResponse::ParsingError( - "No credentials to unpack found in response".to_string(), - ))?; + let meta = TlvIter::from_vec(resp?).next().ok_or(Error::ParsingError( + "No credentials to unpack found in response".to_string(), + ))?; - OathCodeDisplay::from_tlv(meta).ok_or(FormattableErrorResponse::ParsingError( + OathCodeDisplay::from_tlv(meta).ok_or(Error::ParsingError( "error parsing calculation response".to_string(), )) } /// Read the OATH codes from the device, calculate TOTP codes that don't /// need touch - pub fn calculate_oath_codes( - &self, - ) -> Result, FormattableErrorResponse> { + pub fn calculate_oath_codes(&self) -> Result, Error> { let timestamp = SystemTime::now(); // Request OATH codes from device let response = self.transaction_context.apdu_read_all( @@ -258,7 +249,7 @@ impl<'a> OathSession<'a> { return Ok(key_buffer); } - pub fn list_oath_codes(&self) -> Result, FormattableErrorResponse> { + pub fn list_oath_codes(&self) -> Result, Error> { // Request OATH codes from device let response = self.transaction_context diff --git a/src/transaction.rs b/src/transaction.rs index 5dc1165..0b7484f 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -8,8 +8,7 @@ use pcsc::{Card, Transaction}; use crate::{ErrorResponse, Instruction, SuccessResponse, Tag}; #[derive(PartialEq, Eq, Debug)] -pub enum FormattableErrorResponse { - NoError, +pub enum Error { Unknown(String), Protocol(ErrorResponse), PcscError(pcsc::Error), @@ -17,46 +16,31 @@ pub enum FormattableErrorResponse { DeviceMismatchError, } -impl FormattableErrorResponse { - pub fn from_apdu_response(sw1: u8, sw2: u8) -> Self { +impl Error { + fn from_apdu_response(sw1: u8, sw2: u8) -> Result<(), Self> { let code: u16 = (sw1 as u16 | sw2 as u16) << 8; if let Some(e) = ErrorResponse::any_match(code) { - return Self::Protocol(e); + return Err(Self::Protocol(e)); } if SuccessResponse::any_match(code) .or(SuccessResponse::any_match(sw1.into())) .is_some() { - return Self::NoError; + return Ok(()); } - Self::Unknown(String::from("Unknown error")) - } - pub fn is_ok(&self) -> bool { - *self == Self::NoError - } - pub fn as_opt(self) -> Option { - if self.is_ok() { - None - } else { - Some(self) - } - } - - fn from_transmit(err: pcsc::Error) -> Self { - Self::PcscError(err) + Err(Self::Unknown(String::from("Unknown error"))) } } -impl From for FormattableErrorResponse { +impl From for Error { fn from(value: pcsc::Error) -> Self { Self::PcscError(value) } } -impl Display for FormattableErrorResponse { +impl Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::NoError => f.write_str("ok"), Self::Unknown(msg) => f.write_str(msg), Self::Protocol(error_response) => f.write_fmt(format_args!("{}", error_response)), Self::PcscError(error) => f.write_fmt(format_args!("{}", error)), @@ -80,7 +64,7 @@ fn apdu( parameter1: u8, parameter2: u8, data: Option<&[u8]>, -) -> Result { +) -> Result { let command = if let Some(data) = data { Command::new_with_payload(class, instruction, parameter1, parameter2, data) } else { @@ -93,19 +77,9 @@ fn apdu( let mut rx_buf = [0; pcsc::MAX_BUFFER_SIZE]; // Write the payload to the device and error if there is a problem - let rx_buf = match tx.transmit(&tx_buf, &mut rx_buf) { - Ok(slice) => slice, - // Err(err) => return Err(format!("{}", err)), - Err(err) => return Err(FormattableErrorResponse::from_transmit(err)), - }; - + let rx_buf = tx.transmit(&tx_buf, &mut rx_buf)?; let resp = Response::from(rx_buf); - let error_context = - FormattableErrorResponse::from_apdu_response(resp.trailer.0, resp.trailer.1); - - if !error_context.is_ok() { - return Err(error_context); - } + Error::from_apdu_response(resp.trailer.0, resp.trailer.1)?; Ok(ApduResponse { buf: resp.payload.to_vec(), @@ -121,7 +95,7 @@ fn apdu_read_all( parameter1: u8, parameter2: u8, data: Option<&[u8]>, -) -> Result, FormattableErrorResponse> { +) -> Result, Error> { let mut response_buf = Vec::new(); let mut resp = apdu(tx, class, instruction, parameter1, parameter2, data)?; response_buf.extend(resp.buf); @@ -141,7 +115,7 @@ pub struct TransactionContext { } impl TransactionContext { - pub fn from_name(name: &str) -> Result { + pub fn from_name(name: &str) -> Result { // Establish a PC/SC context let ctx = pcsc::Context::establish(pcsc::Scope::User)?; @@ -166,7 +140,7 @@ impl TransactionContext { parameter1: u8, parameter2: u8, data: Option<&[u8]>, - ) -> Result { + ) -> Result { apdu( self.borrow_transaction(), class, @@ -184,7 +158,7 @@ impl TransactionContext { parameter1: u8, parameter2: u8, data: Option<&[u8]>, - ) -> Result, FormattableErrorResponse> { + ) -> Result, Error> { apdu_read_all( self.borrow_transaction(), class,