From 11533edc88c1b9811c4df3b2583382c43ba470ba Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Thu, 13 Feb 2025 12:45:19 +0100 Subject: [PATCH 1/6] Add missing Error trait impl --- src/transaction.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/transaction.rs b/src/transaction.rs index 0b7484f..11b330a 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -50,6 +50,8 @@ impl Display for Error { } } +impl std::error::Error for Error {} + pub struct ApduResponse { pub buf: Vec, pub sw1: u8, From 26fc4f1aae57eff096d75451c41b5277ec4579b7 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Thu, 13 Feb 2025 13:11:03 +0100 Subject: [PATCH 2/6] get rid of strum dependency the enums are small enough, could find a better to rewrite the code later on as well --- Cargo.lock | 35 +---------------------------------- Cargo.toml | 2 -- src/constants.rs | 38 ++++++++++++++++++++++++-------------- 3 files changed, 25 insertions(+), 50 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 34a9edc..8776d4e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -96,12 +96,6 @@ version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "95505c38b4572b2d910cecb0281560f54b440a19336cbbcb27bf6ce6adc6f5a8" -[[package]] -name = "heck" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" - [[package]] name = "hmac" version = "0.12.1" @@ -134,8 +128,6 @@ dependencies = [ "regex", "sha1", "sha2", - "strum", - "strum_macros", ] [[package]] @@ -167,7 +159,7 @@ version = "0.18.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3c7028bdd3d43083f6d8d4d5187680d0d3560d54df4cc9d752005268b41e64d0" dependencies = [ - "heck 0.4.1", + "heck", "proc-macro2", "proc-macro2-diagnostics", "quote", @@ -270,12 +262,6 @@ version = "0.8.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2b15c43186be67a4fd63bee50d0303afffcef381492ebe2c5d87f324e1b8815c" -[[package]] -name = "rustversion" -version = "1.0.19" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f7c45b9784283f1b2e7fb61b42047c2fd678ef0960d4f6f1eba131594cc369d4" - [[package]] name = "sha1" version = "0.10.6" @@ -304,25 +290,6 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" -[[package]] -name = "strum" -version = "0.27.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ce1475c515a4f03a8a7129bb5228b81a781a86cb0b3fbbc19e1c556d491a401f" - -[[package]] -name = "strum_macros" -version = "0.27.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9688894b43459159c82bfa5a5fa0435c19cbe3c9b427fa1dd7b1ce0c279b18a7" -dependencies = [ - "heck 0.5.0", - "proc-macro2", - "quote", - "rustversion", - "syn", -] - [[package]] name = "subtle" version = "2.6.1" diff --git a/Cargo.toml b/Cargo.toml index 5fe1738..b474735 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,8 +9,6 @@ pcsc = "2.9.0" regex = "1.11.1" sha1 = "0.10.6" sha2 = "0.10.8" -strum = { version = "0.27.0", features = [ ] } -strum_macros = "0.27.0" [package] name = "lib_ykoath2" diff --git a/src/constants.rs b/src/constants.rs index e54d57d..4e71334 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -2,8 +2,6 @@ use std::fmt::Display; use iso7816_tlv::simple::Tlv; use sha1::Digest; -use strum::IntoEnumIterator; // 0.17.1 -use strum_macros::EnumIter; // 0.17.1 pub const INS_SELECT: u8 = 0xa4; pub const OATH_AID: [u8; 7] = [0xa0, 0x00, 0x00, 0x05, 0x27, 0x21, 0x01]; @@ -11,7 +9,7 @@ pub const DEFAULT_PERIOD: u32 = 30; pub const DEFAULT_DIGITS: OathDigits = OathDigits::Six; pub const DEFAULT_IMF: u32 = 0; -#[derive(Debug, EnumIter, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] #[repr(u16)] pub enum ErrorResponse { NoSpace = 0x6a84, @@ -25,12 +23,23 @@ pub enum ErrorResponse { impl ErrorResponse { pub fn any_match(code: u16) -> Option { - for resp in ErrorResponse::iter() { - if code == resp as u16 { - return Some(resp); - } + if code == ErrorResponse::NoSpace as u16 { + Some(ErrorResponse::NoSpace) + } else if code == ErrorResponse::CommandAborted as u16 { + Some(ErrorResponse::CommandAborted) + } else if code == ErrorResponse::InvalidInstruction as u16 { + Some(ErrorResponse::InvalidInstruction) + } else if code == ErrorResponse::AuthRequired as u16 { + Some(ErrorResponse::AuthRequired) + } else if code == ErrorResponse::WrongSyntax as u16 { + Some(ErrorResponse::WrongSyntax) + } else if code == ErrorResponse::GenericError as u16 { + Some(ErrorResponse::GenericError) + } else if code == ErrorResponse::NoSuchObject as u16 { + Some(ErrorResponse::NoSuchObject) + } else { + None } - None } } @@ -50,7 +59,7 @@ impl std::fmt::Display for ErrorResponse { impl std::error::Error for ErrorResponse {} -#[derive(Debug, EnumIter, Clone, Copy)] +#[derive(Debug, Clone, Copy)] #[repr(u16)] pub enum SuccessResponse { MoreData = 0x61, @@ -59,12 +68,13 @@ pub enum SuccessResponse { impl SuccessResponse { pub fn any_match(code: u16) -> Option { - for resp in SuccessResponse::iter() { - if code == resp as u16 { - return Some(resp); - } + if code == SuccessResponse::MoreData as u16 { + Some(SuccessResponse::MoreData) + } else if code == SuccessResponse::Okay as u16 { + Some(SuccessResponse::Okay) + } else { + None } - None } } From c83a9c6739206454bee89f7bda4fbf72bbe96b28 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Thu, 13 Feb 2025 13:12:15 +0100 Subject: [PATCH 3/6] drop duplicated function --- src/constants.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/constants.rs b/src/constants.rs index 4e71334..0bb88cb 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -188,14 +188,6 @@ impl OathCodeDisplay { None } } - pub fn from_bytes(tlv: Tlv) -> Option { - if Into::::into(tlv.tag()) == (Tag::TruncatedResponse as u8) && tlv.value().len() == 5 { - let display = OathCodeDisplay::new(tlv.value()[..].try_into().unwrap()); - Some(display) - } else { - None - } - } pub fn new(bytes: &[u8; 5]) -> Self { Self { From 36b15e87b2636b32227b74f7ce2343e34bdced05 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Thu, 13 Feb 2025 13:26:14 +0100 Subject: [PATCH 4/6] silence unused warning for now so we can run clippy on CI, we can drop it later once the api is complete --- src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib.rs b/src/lib.rs index f29afe7..fc8c929 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,4 @@ +#![allow(unused)] mod constants; use constants::*; mod transaction; From a3b5ae98398a3cdf24e28951f75f66b47f2eeb88 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Thu, 13 Feb 2025 13:28:48 +0100 Subject: [PATCH 5/6] fix clippy warnings --- src/example.rs | 4 ++-- src/lib.rs | 26 +++++++++++++------------- src/oath_credential.rs | 4 ++-- src/oath_credentialid.rs | 22 +++++++++++----------- src/transaction.rs | 28 ++++++++++++++-------------- 5 files changed, 42 insertions(+), 42 deletions(-) diff --git a/src/example.rs b/src/example.rs index 1775ceb..dd4e68a 100644 --- a/src/example.rs +++ b/src/example.rs @@ -21,7 +21,7 @@ fn main() { } // Show message if no YubiKey(s) - if yubikeys.len() == 0 { + if yubikeys.is_empty() { println!("No yubikeys detected"); std::process::exit(0); } @@ -45,7 +45,7 @@ fn main() { }; // Show message is node codes found - if codes.len() == 0 { + if codes.is_empty() { println!("No credentials on device {}", device_label); } diff --git a/src/lib.rs b/src/lib.rs index fc8c929..ce35b0e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,7 +20,7 @@ fn _get_device_id(salt: Vec) -> String { let hash_16_bytes = &result[..16]; // Base64 encode the result and remove padding ('=') - return general_purpose::URL_SAFE_NO_PAD.encode(hash_16_bytes); + 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"); @@ -41,7 +41,7 @@ fn _hmac_shorten_key(key: &[u8], algo: HashAlgo) -> Vec { } fn _get_challenge(timestamp: u64, period: u64) -> [u8; 8] { - return ((timestamp / period) as u64).to_be_bytes(); + (timestamp / period).to_be_bytes() } fn time_to_u64(timestamp: SystemTime) -> u64 { @@ -58,7 +58,7 @@ pub struct OathSession<'a> { pub name: String, } -fn clone_with_lifetime<'a>(data: &'a [u8]) -> Vec { +fn clone_with_lifetime(data: &[u8]) -> Vec { // Clone the slice into a new Vec data.to_vec() // `to_vec()` will return a Vec that has its own ownership } @@ -71,7 +71,7 @@ pub struct RefreshableOathCredential<'a> { refresh_provider: &'a OathSession<'a>, } -impl<'a> Display for RefreshableOathCredential<'a> { +impl Display for RefreshableOathCredential<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { if let Some(c) = self.code { f.write_fmt(format_args!("{}: {}", self.cred.id_data, c)) @@ -95,7 +95,7 @@ impl<'a> RefreshableOathCredential<'a> { pub fn force_update(&mut self, code: Option, timestamp: SystemTime) { self.code = code; (self.valid_from, self.valid_to) = - RefreshableOathCredential::format_validity_time_frame(&self, timestamp); + RefreshableOathCredential::format_validity_time_frame(self, timestamp); } pub fn refresh(&mut self) { @@ -111,7 +111,7 @@ impl<'a> RefreshableOathCredential<'a> { if !self.is_valid() { self.refresh(); } - return self; + self } pub fn is_valid(&self) -> bool { @@ -133,7 +133,7 @@ impl<'a> RefreshableOathCredential<'a> { } } -impl<'a> OathSession<'a> { +impl OathSession<'_> { pub fn new(name: &str) -> Result { let transaction_context = TransactionContext::from_name(name)?; let info_buffer = @@ -199,7 +199,7 @@ impl<'a> OathSession<'a> { timestamp_sys: Option, ) -> Result { if self.name != cred.device_id { - return Err(Error::DeviceMismatchError); + return Err(Error::DeviceMismatch); } let timestamp = time_to_u64(timestamp_sys.unwrap_or_else(SystemTime::now)); @@ -220,11 +220,11 @@ impl<'a> OathSession<'a> { Some(&data), ); - let meta = TlvIter::from_vec(resp?).next().ok_or(Error::ParsingError( + let meta = TlvIter::from_vec(resp?).next().ok_or(Error::Parsing( "No credentials to unpack found in response".to_string(), ))?; - OathCodeDisplay::from_tlv(meta).ok_or(Error::ParsingError( + OathCodeDisplay::from_tlv(meta).ok_or(Error::Parsing( "error parsing calculation response".to_string(), )) } @@ -264,7 +264,7 @@ impl<'a> OathSession<'a> { key_buffer.push(refreshable_cred); } - return Ok(key_buffer); + Ok(key_buffer) } pub fn list_oath_codes(&self) -> Result, Error> { // Request OATH codes from device @@ -277,12 +277,12 @@ impl<'a> OathSession<'a> { for cred_id in TlvIter::from_vec(response?) { let id_data = CredentialIDData::from_bytes( &cred_id.value()[1..], - *cred_id.value().get(0).unwrap_or(&0u8) & 0xf0, + *cred_id.value().first().unwrap_or(&0u8) & 0xf0, ); key_buffer.push(id_data); } - return Ok(key_buffer); + Ok(key_buffer) } } diff --git a/src/oath_credential.rs b/src/oath_credential.rs index 1712ef6..d4b3324 100644 --- a/src/oath_credential.rs +++ b/src/oath_credential.rs @@ -35,13 +35,13 @@ impl PartialOrd for OathCredential { } } -impl<'a> PartialEq for OathCredential { +impl PartialEq for OathCredential { fn eq(&self, other: &Self) -> bool { self.device_id == other.device_id && self.id_data == other.id_data } } -impl<'a> Hash for OathCredential { +impl Hash for OathCredential { fn hash(&self, state: &mut H) { self.device_id.hash(state); self.id_data.format_cred_id().hash(state); diff --git a/src/oath_credentialid.rs b/src/oath_credentialid.rs index f7fe075..fded71c 100644 --- a/src/oath_credentialid.rs +++ b/src/oath_credentialid.rs @@ -23,11 +23,11 @@ impl Display for CredentialIDData { impl CredentialIDData { pub fn from_tlv(id_bytes: &[u8], oath_type_tag: iso7816_tlv::simple::Tag) -> Self { - return CredentialIDData::from_bytes(id_bytes, Into::::into(oath_type_tag)); + CredentialIDData::from_bytes(id_bytes, Into::::into(oath_type_tag)) } pub fn as_tlv(&self) -> Vec { - return to_tlv(Tag::Name, &self.format_cred_id()); + to_tlv(Tag::Name, &self.format_cred_id()) } pub fn format_cred_id(&self) -> Vec { @@ -42,7 +42,7 @@ impl CredentialIDData { } cred_id.push_str(self.name.as_str()); - return cred_id.into_bytes(); // Convert the string to bytes + cred_id.into_bytes() // Convert the string to bytes } // Function to parse the credential ID @@ -55,19 +55,19 @@ impl CredentialIDData { if oath_type == OathType::Totp { Regex::new(r"^((\d+)/)?(([^:]+):)?(.+)$") .ok() - .and_then(|r| r.captures(&data)) + .and_then(|r| r.captures(data)) .map_or((None, data.to_string(), DEFAULT_PERIOD), |caps| { - let period = (&caps.get(2)) + let period = caps + .get(2) .and_then(|s| s.as_str().parse::().ok()) .unwrap_or(DEFAULT_PERIOD); - return (Some(caps[4].to_string()), caps[5].to_string(), period); + (Some(caps[4].to_string()), caps[5].to_string(), period) }) } else { - return data - .split_once(':') + data.split_once(':') .map_or((None, data.to_string(), 0), |(i, n)| { (Some(i.to_string()), n.to_string(), 0) - }); + }) } } @@ -78,11 +78,11 @@ impl CredentialIDData { OathType::Totp }; let (issuer, name, period) = CredentialIDData::parse_cred_id(id_bytes, oath_type); - return CredentialIDData { + CredentialIDData { issuer, name, period, oath_type, - }; + } } } diff --git a/src/transaction.rs b/src/transaction.rs index 11b330a..1e894a8 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -11,9 +11,9 @@ use crate::{ErrorResponse, Instruction, SuccessResponse, Tag}; pub enum Error { Unknown(String), Protocol(ErrorResponse), - PcscError(pcsc::Error), - ParsingError(String), - DeviceMismatchError, + Pcsc(pcsc::Error), + Parsing(String), + DeviceMismatch, } impl Error { @@ -34,7 +34,7 @@ impl Error { impl From for Error { fn from(value: pcsc::Error) -> Self { - Self::PcscError(value) + Self::Pcsc(value) } } @@ -43,9 +43,9 @@ impl Display for Error { match self { 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"), + Self::Pcsc(error) => f.write_fmt(format_args!("{}", error)), + Self::Parsing(msg) => f.write_str(msg), + Self::DeviceMismatch => f.write_str("Devices do not match"), } } } @@ -183,7 +183,7 @@ pub fn tlv_to_map(data: Vec) -> HashMap> { for res in TlvIter::from_vec(data) { parsed_manual.insert(res.tag().into(), res.value().to_vec()); } - return parsed_manual; + parsed_manual } pub struct TlvZipIter<'a> { @@ -207,10 +207,10 @@ impl<'a> TlvZipIter<'a> { } } -impl<'a> Iterator for TlvZipIter<'a> { +impl Iterator for TlvZipIter<'_> { type Item = (Tlv, Tlv); fn next(&mut self) -> Option { - return Some((self.iter.next()?, self.iter.next()?)); + Some((self.iter.next()?, self.iter.next()?)) } } @@ -228,7 +228,7 @@ impl<'a> TlvIter<'a> { } } -impl<'a> Iterator for TlvIter<'a> { +impl Iterator for TlvIter<'_> { type Item = Tlv; fn next(&mut self) -> Option { @@ -237,7 +237,7 @@ impl<'a> Iterator for TlvIter<'a> { } let (r, remaining) = Tlv::parse(self.buf); self.buf = remaining; - return r.ok(); + r.ok() } } @@ -246,8 +246,8 @@ pub fn tlv_to_lists(data: Vec) -> HashMap>> { for res in TlvIter::from_vec(data) { parsed_manual .entry(res.tag().into()) - .or_insert_with(Vec::new) + .or_default() .push(res.value().to_vec()); } - return parsed_manual; + parsed_manual } From 01d98dc807c541f87bc3f5ec4c686b997cdaefd1 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Thu, 13 Feb 2025 13:33:49 +0100 Subject: [PATCH 6/6] setup ci along with cargo deny --- .github/workflows/CI.yml | 77 ++++++++++++++++++++++++++++++++++++++++ deny.toml | 18 ++++++++++ 2 files changed, 95 insertions(+) create mode 100644 .github/workflows/CI.yml create mode 100644 deny.toml diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml new file mode 100644 index 0000000..9163538 --- /dev/null +++ b/.github/workflows/CI.yml @@ -0,0 +1,77 @@ +on: + push: + branches: [main] + pull_request: + +name: CI + +jobs: + check: + name: Check + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + override: true + - run: sudo apt install libpcsclite-dev -y + - uses: actions-rs/cargo@v1 + with: + command: check + + test: + name: Test Suite + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + override: true + - run: sudo apt install libpcsclite-dev -y + - uses: actions-rs/cargo@v1 + with: + command: test + + fmt: + name: Rustfmt + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + override: true + - run: sudo apt install libpcsclite-dev -y + - run: rustup component add rustfmt + - uses: actions-rs/cargo@v1 + with: + command: fmt + args: --all -- --check + + clippy: + name: Clippy + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + override: true + - run: sudo apt install libpcsclite-dev -y + - run: rustup component add clippy + - uses: actions-rs/cargo@v1 + with: + command: clippy + args: -- -D warnings + + cargo-deny: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: EmbarkStudios/cargo-deny-action@v1 \ No newline at end of file diff --git a/deny.toml b/deny.toml new file mode 100644 index 0000000..ee8c461 --- /dev/null +++ b/deny.toml @@ -0,0 +1,18 @@ + +[advisories] +db-path = "~/.cargo/advisory-db" +db-urls = ["https://github.com/rustsec/advisory-db"] +ignore = [] + +[licenses] +allow = [ + "MIT", # from ouroboros + "Unicode-3.0", + "ISC", # from iso7816-tlv + "BSD-3-Clause" +] + +[bans] +multiple-versions = "deny" +wildcards = "deny" +highlight = "all" \ No newline at end of file