Merge pull request #2 from bilelmoussaoui/bilelmoussaoui/cleanup

follow up of the previous one

.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

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"

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"

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"

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<ErrorResponse> {
-        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<SuccessResponse> {
-        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
     }
 }
 
@@ -178,14 +188,6 @@ impl OathCodeDisplay {
             None
         }
     }
-    pub fn from_bytes(tlv: Tlv) -> Option<Self> {
-        if Into::<u8>::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 {

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);
         }
 

src/lib.rs

@@ -1,3 +1,4 @@
+#![allow(unused)]
 mod constants;
 use constants::*;
 mod transaction;
@@ -19,7 +20,7 @@ fn _get_device_id(salt: Vec<u8>) -> 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<u8> {
     let mut mac = Hmac::<sha1::Sha1>::new_from_slice(key).expect("Invalid key length");
@@ -40,7 +41,7 @@ fn _hmac_shorten_key(key: &[u8], algo: HashAlgo) -> Vec<u8> {
 }
 
 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 {
@@ -57,7 +58,7 @@ pub struct OathSession<'a> {
     pub name: String,
 }
 
-fn clone_with_lifetime<'a>(data: &'a [u8]) -> Vec<u8> {
+fn clone_with_lifetime(data: &[u8]) -> Vec<u8> {
     // Clone the slice into a new Vec<u8>
     data.to_vec() // `to_vec()` will return a Vec<u8> that has its own ownership
 }
@@ -70,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))
@@ -94,7 +95,7 @@ impl<'a> RefreshableOathCredential<'a> {
     pub fn force_update(&mut self, code: Option<OathCodeDisplay>, 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) {
@@ -110,7 +111,7 @@ impl<'a> RefreshableOathCredential<'a> {
         if !self.is_valid() {
             self.refresh();
         }
-        return self;
+        self
     }
 
     pub fn is_valid(&self) -> bool {
@@ -132,7 +133,7 @@ impl<'a> RefreshableOathCredential<'a> {
     }
 }
 
-impl<'a> OathSession<'a> {
+impl OathSession<'_> {
     pub fn new(name: &str) -> Result<Self, Error> {
         let transaction_context = TransactionContext::from_name(name)?;
         let info_buffer =
@@ -198,7 +199,7 @@ impl<'a> OathSession<'a> {
         timestamp_sys: Option<SystemTime>,
     ) -> Result<OathCodeDisplay, Error> {
         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));
@@ -219,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(),
         ))
     }
@@ -263,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<Vec<CredentialIDData>, Error> {
         // Request OATH codes from device
@@ -276,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)
     }
 }
 

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<H: Hasher>(&self, state: &mut H) {
         self.device_id.hash(state);
         self.id_data.format_cred_id().hash(state);

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::<u8>::into(oath_type_tag));
+        CredentialIDData::from_bytes(id_bytes, Into::<u8>::into(oath_type_tag))
     }
 
     pub fn as_tlv(&self) -> Vec<u8> {
-        return to_tlv(Tag::Name, &self.format_cred_id());
+        to_tlv(Tag::Name, &self.format_cred_id())
     }
 
     pub fn format_cred_id(&self) -> Vec<u8> {
@@ -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::<u32>().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,
-        };
+        }
     }
 }

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<pcsc::Error> for Error {
     fn from(value: pcsc::Error) -> Self {
-        Self::PcscError(value)
+        Self::Pcsc(value)
     }
 }
 
@@ -43,13 +43,15 @@ 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"),
         }
     }
 }
 
+impl std::error::Error for Error {}
+
 pub struct ApduResponse {
     pub buf: Vec<u8>,
     pub sw1: u8,
@@ -181,7 +183,7 @@ pub fn tlv_to_map(data: Vec<u8>) -> HashMap<u8, Vec<u8>> {
     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> {
@@ -205,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<Self::Item> {
-        return Some((self.iter.next()?, self.iter.next()?));
+        Some((self.iter.next()?, self.iter.next()?))
     }
 }
 
@@ -226,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<Self::Item> {
@@ -235,7 +237,7 @@ impl<'a> Iterator for TlvIter<'a> {
         }
         let (r, remaining) = Tlv::parse(self.buf);
         self.buf = remaining;
-        return r.ok();
+        r.ok()
     }
 }
 
@@ -244,8 +246,8 @@ pub fn tlv_to_lists(data: Vec<u8>) -> HashMap<u8, Vec<Vec<u8>>> {
     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
 }