From 43dcec36259a35c4336105579a5aa2dd69227dbb Mon Sep 17 00:00:00 2001 From: Grimmauld Date: Sun, 16 Feb 2025 13:33:00 +0100 Subject: [PATCH] fix: oath type parsing mismatches doc: oath credential id --- src/constants.rs | 4 +- src/lib.rs | 19 ++++---- src/oath_credential.rs | 2 +- src/oath_credential_id.rs | 75 ++++++++++++++++++++++-------- src/refreshable_oath_credential.rs | 13 ++++-- 5 files changed, 80 insertions(+), 33 deletions(-) diff --git a/src/constants.rs b/src/constants.rs index 199a891..56bc241 100644 --- a/src/constants.rs +++ b/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 diff --git a/src/lib.rs b/src/lib.rs index 42242ba..871d188 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -40,13 +40,13 @@ fn hmac_shorten_key(key: &[u8], algo: HashAlgo) -> Vec { } } -fn time_challenge(timestamp: Option, period: Option) -> [u8; 8] { +fn time_challenge(timestamp: Option, 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); } diff --git a/src/oath_credential.rs b/src/oath_credential.rs index d4b3324..e5a6d88 100644 --- a/src/oath_credential.rs +++ b/src/oath_credential.rs @@ -44,6 +44,6 @@ impl PartialEq 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); + self.id_data.hash(state); } } diff --git a/src/oath_credential_id.rs b/src/oath_credential_id.rs index 4c75aaa..27ec4c5 100644 --- a/src/oath_credential_id.rs +++ b/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, - pub period: Duration, + + /// validity period of each generated code. + period: Option, } 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::::into(oath_type_tag)) + let oath_type = if Into::::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 { to_tlv(Tag::Name, &self.format_cred_id()) } - pub fn format_cred_id(&self) -> Vec { + /// Returns the defined period or default + pub fn get_period(&self) -> Duration { + self.period.unwrap_or(DEFAULT_PERIOD) + } + + fn format_cred_id(&self) -> Vec { 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, Duration) { + fn parse_cred_id( + cred_id: &[u8], + oath_type: OathType, + ) -> (Option, String, Option) { 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::().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(&self, state: &mut H) { + self.format_cred_id().hash(state); + } +} diff --git a/src/refreshable_oath_credential.rs b/src/refreshable_oath_credential.rs index 3cfe33c..0c6ed57 100644 --- a/src/refreshable_oath_credential.rs +++ b/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 => {