fix: oath type parsing mismatches

doc: oath credential id

src/constants.rs

@@ -168,8 +168,8 @@ impl HashAlgo {
 #[derive(Debug, PartialEq, Copy, Clone, Eq, Hash)]
 #[repr(u8)]
 pub enum OathType {
-    Totp = 0x10,
-    Hotp = 0x20,
+    Hotp = 0x10,
+    Totp = 0x20,
 }
 
 /// describes display information of a code, keeping track of the code and number of digits

src/lib.rs

@@ -40,13 +40,13 @@ fn hmac_shorten_key(key: &[u8], algo: HashAlgo) -> Vec<u8> {
     }
 }
 
-fn time_challenge(timestamp: Option<SystemTime>, period: Option<Duration>) -> [u8; 8] {
+fn time_challenge(timestamp: Option<SystemTime>, period: Duration) -> [u8; 8] {
     (timestamp
         .unwrap_or_else(SystemTime::now)
         .duration_since(SystemTime::UNIX_EPOCH)
         .as_ref()
         .map_or(0, Duration::as_secs)
-        / period.unwrap_or(DEFAULT_PERIOD).as_secs())
+        / period.as_secs())
     .to_be_bytes()
 }
 
@@ -272,7 +272,7 @@ impl OathSession {
         if cred.id_data.oath_type == OathType::Totp {
             data.extend(to_tlv(
                 Tag::Challenge,
-                &time_challenge(Some(timestamp), Some(cred.id_data.period)),
+                &time_challenge(Some(timestamp), cred.id_data.get_period()),
             ));
         }
 
@@ -305,7 +305,7 @@ impl OathSession {
             0x01,
             Some(&to_tlv(
                 Tag::Challenge,
-                &time_challenge(Some(timestamp), None),
+                &time_challenge(Some(timestamp), DEFAULT_PERIOD),
             )),
         );
 
@@ -339,10 +339,13 @@ impl OathSession {
         let mut key_buffer = Vec::new();
 
         for cred_id in TlvIter::from_vec(response?) {
-            let id_data = CredentialIDData::from_bytes(
-                &cred_id.value()[1..],
-                *cred_id.value().first().unwrap_or(&0u8) & 0xf0,
-            );
+            let oath_type = if (cred_id.value()[0] & 0xf0) == (Tag::Hotp as u8) {
+                OathType::Hotp
+            } else {
+                OathType::Totp
+            };
+
+            let id_data = CredentialIDData::from_bytes(&cred_id.value()[1..], oath_type);
             key_buffer.push(id_data);
         }
 

src/oath_credential.rs

@@ -44,6 +44,6 @@ impl PartialEq 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);
+        self.id_data.hash(state);
     }
 }

src/oath_credential_id.rs

@@ -1,15 +1,31 @@
-use std::{fmt::Display, time::Duration};
+use std::{
+    fmt::Display,
+    hash::{Hash, Hasher},
+    time::Duration,
+};
 
 use regex::Regex;
 
 use crate::{to_tlv, OathType, Tag, DEFAULT_PERIOD};
 
+/// holds data on one credential.
+/// Acts as a handle to credentials when requesting codes from the YubiKey.
 #[derive(Debug, Eq, PartialEq, Clone)]
 pub struct CredentialIDData {
+    /// name of a credential.
+    /// Typically specifies an account.
     pub name: String,
+
+    /// One of `OathType::Totp` or `OathType::Hotp`.
+    /// Specifies the type of OTP used represented by this credential.
     pub oath_type: OathType,
+
+    /// issuer of the credential.
+    /// Typically specifies the platform.
     pub issuer: Option<String>,
-    pub period: Duration,
+
+    /// validity period of each generated code.
+    period: Option<Duration>,
 }
 
 impl Display for CredentialIDData {
@@ -22,19 +38,35 @@ impl Display for CredentialIDData {
 }
 
 impl CredentialIDData {
+    /// reads id data from tlv data
+    /// `id_bytes` refers to the byte buffer containing issuer, name and period
+    /// `oath_type_tag` refers to the tlv tag containing the oath type information
     pub fn from_tlv(id_bytes: &[u8], oath_type_tag: iso7816_tlv::simple::Tag) -> Self {
-        CredentialIDData::from_bytes(id_bytes, Into::<u8>::into(oath_type_tag))
+        let oath_type = if Into::<u8>::into(oath_type_tag) == Tag::Hotp as u8 {
+            OathType::Hotp
+        } else {
+            OathType::Totp
+        };
+        CredentialIDData::from_bytes(id_bytes, oath_type)
     }
 
+    /// Reconstructs the tlv data to refer to this credential on the YubiKey
     pub fn as_tlv(&self) -> Vec<u8> {
         to_tlv(Tag::Name, &self.format_cred_id())
     }
 
-    pub fn format_cred_id(&self) -> Vec<u8> {
+    /// Returns the defined period or default
+    pub fn get_period(&self) -> Duration {
+        self.period.unwrap_or(DEFAULT_PERIOD)
+    }
+
+    fn format_cred_id(&self) -> Vec<u8> {
         let mut cred_id = String::new();
 
-        if self.oath_type == OathType::Totp && self.period != DEFAULT_PERIOD {
-            cred_id.push_str(&format!("{}/", self.period.as_secs()));
+        if self.oath_type == OathType::Totp {
+            if let Some(p) = self.period {
+                cred_id.push_str(&format!("{}/", p.as_secs()));
+            }
         }
 
         if let Some(issuer) = self.issuer.as_deref() {
@@ -45,18 +77,20 @@ impl CredentialIDData {
         cred_id.into_bytes() // Convert the string to bytes
     }
 
-    // Function to parse the credential ID
-    fn parse_cred_id(cred_id: &[u8], oath_type: OathType) -> (Option<String>, String, Duration) {
+    fn parse_cred_id(
+        cred_id: &[u8],
+        oath_type: OathType,
+    ) -> (Option<String>, String, Option<Duration>) {
         let data = match std::str::from_utf8(cred_id) {
             Ok(d) => d,
-            Err(_) => return (None, String::new(), Duration::ZERO), // Handle invalid UTF-8
+            Err(_) => return (None, String::new(), Some(Duration::ZERO)), // Handle invalid UTF-8
         };
 
         if oath_type == OathType::Totp {
             Regex::new(r"^((\d+)/)?(([^:]+):)?(.+)$")
                 .ok()
                 .and_then(|r| r.captures(data))
-                .map_or((None, data.to_string(), DEFAULT_PERIOD), |caps| {
+                .map_or((None, data.to_string(), None), |caps| {
                     let period = caps
                         .get(2)
                         .and_then(|s| s.as_str().parse::<u32>().ok())
@@ -64,22 +98,19 @@ impl CredentialIDData {
                         .unwrap_or(DEFAULT_PERIOD);
                     let issuer = caps.get(4).map(|m| m.as_str().to_string());
                     let cred_name = caps.get(5).map_or(data, |m| m.as_str());
-                    (issuer, cred_name.to_string(), period)
+                    (issuer, cred_name.to_string(), Some(period))
                 })
         } else {
             data.split_once(':')
-                .map_or((None, data.to_string(), Duration::ZERO), |(i, n)| {
-                    (Some(i.to_string()), n.to_string(), Duration::ZERO)
+                .map_or((None, data.to_string(), None), |(i, n)| {
+                    (Some(i.to_string()), n.to_string(), None)
                 })
         }
     }
 
-    pub(crate) fn from_bytes(id_bytes: &[u8], tag: u8) -> CredentialIDData {
-        let oath_type = if tag == (Tag::Hotp as u8) {
-            OathType::Hotp
-        } else {
-            OathType::Totp
-        };
+    /// parses a credential id from byte buffers
+    /// `id_bytes` contains information about issuer, name and duration
+    pub fn from_bytes(id_bytes: &[u8], oath_type: OathType) -> CredentialIDData {
         let (issuer, name, period) = CredentialIDData::parse_cred_id(id_bytes, oath_type);
         CredentialIDData {
             issuer,
@@ -89,3 +120,9 @@ impl CredentialIDData {
         }
     }
 }
+
+impl Hash for CredentialIDData {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.format_cred_id().hash(state);
+    }
+}

src/refreshable_oath_credential.rs

@@ -66,11 +66,18 @@ impl<'a> RefreshableOathCredential<'a> {
                     .duration_since(SystemTime::UNIX_EPOCH)
                     .as_ref()
                     .map_or(0, Duration::as_secs);
-                let time_step = timestamp_seconds / (self.cred.id_data.period.as_secs());
+                let time_step = timestamp_seconds / (self.cred.id_data.get_period().as_secs());
                 let valid_from = SystemTime::UNIX_EPOCH
-                    .checked_add(self.cred.id_data.period.saturating_mul(time_step as u32))
+                    .checked_add(
+                        self.cred
+                            .id_data
+                            .get_period()
+                            .saturating_mul(time_step as u32),
+                    )
+                    .unwrap();
+                let valid_to = valid_from
+                    .checked_add(self.cred.id_data.get_period())
                     .unwrap();
-                let valid_to = valid_from.checked_add(self.cred.id_data.period).unwrap();
                 valid_from..valid_to
             }
             OathType::Hotp => {