Skip to content

Commit

Permalink
Fix incorrect ATR Y(n) byte parsing
Browse files Browse the repository at this point in the history
  • Loading branch information
@micolous
micolous committed Oct 25, 2023
1 parent 377bdc6 commit fa03756
Showing 1 changed file with 51 additions and 15 deletions.
66 changes: 51 additions & 15 deletions webauthn-authenticator-rs/src/nfc/atr.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
//! ISO/IEC 7816-3 _Answer-to-Reset_ and 7816-4 _Historical Bytes_ parser.
use std::collections::{HashSet, VecDeque};

#[cfg(feature = "nfc")]
use pcsc::MAX_ATR_SIZE;

Expand Down Expand Up @@ -32,7 +34,7 @@ use super::tlv::*;
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Atr {
/// Supported protocols (`T=`), specified in ISO/IEC 7816-3:2006 §8.2.3.
pub protocols: Vec<u8>,
pub protocols: HashSet<u8>,

/// Historical bytes (T<sub>1</sub> .. T<sub>k</sub>), as specified in
/// ISO/IEC 7816-4:2005 §8.1.1.
Expand Down Expand Up @@ -113,7 +115,7 @@ impl TryFrom<&[u8]> for Atr {
return Err(WebauthnCError::MessageTooShort);
}

let mut nibbles = Vec::with_capacity(MAX_ATR_SIZE);
let mut nibbles = VecDeque::with_capacity(MAX_ATR_SIZE);
// Byte 0 intentionally skipped

// Calculate checksum (TCK), present unless the only protocol is T=0:
Expand All @@ -129,30 +131,32 @@ impl TryFrom<&[u8]> for Atr {
let mut i: usize = 1;
loop {
let y = atr[i] >> 4;
nibbles.push(atr[i] & 0x0f);
nibbles.push_back(atr[i] & 0x0f);
i += 1;

// skip Ta, Tb, Tc fields
i += (y & 0x7) as usize;
i += (y & 0x7).count_ones() as usize;
if y & 0x8 == 0 {
/* Td = 0 */
break;
}
}

let t1_len = nibbles[0] as usize;
let protocols = if nibbles.len() > 1 {
&nibbles[1..]
let t1_len = nibbles.pop_front().unwrap_or_default() as usize;

let protocols = if nibbles.len() >= 1 {
HashSet::from_iter(nibbles.into_iter())
} else {
// If TD1 is absent, the only offer is T=0.
&PROTOCOL_T0
HashSet::from(PROTOCOL_T0)
};

let mut storage_card = false;
let mut command_chaining = None;
let mut extended_lc = None;
let mut card_issuers_data = None;
if i + t1_len > atr.len() {
error!(?i, ?t1_len, "atr.len = {}", atr.len());
return Err(WebauthnCError::MessageTooShort);
}
let t1 = &atr[i..i + t1_len];
Expand Down Expand Up @@ -199,7 +203,7 @@ impl TryFrom<&[u8]> for Atr {
}

Ok(Atr {
protocols: protocols.to_vec(),
protocols,
t1: t1.to_vec(),
storage_card,
command_chaining,
Expand Down Expand Up @@ -227,12 +231,13 @@ mod tests {

#[test]
fn yubikey_5_nfc() {
let _ = tracing_subscriber::fmt().try_init();
let input = [
0x3b, 0x8d, 0x80, 0x01, 0x80, 0x73, 0xc0, 0x21, 0xc0, 0x57, 0x59, 0x75, 0x62, 0x69,
0x4b, 0x65, 0xff, 0x7f,
];
let expected = Atr {
protocols: [0, 1].to_vec(),
protocols: HashSet::from([0, 1]),
t1: [
0x80, 0x73, 0xc0, 0x21, 0xc0, 0x57, 0x59, 0x75, 0x62, 0x69, 0x4b, 0x65, 0xff,
]
Expand All @@ -251,12 +256,13 @@ mod tests {

#[test]
fn yubico_security_key_c_nfc() {
let _ = tracing_subscriber::fmt().try_init();
let input = [
0x3b, 0x8d, 0x80, 0x01, 0x80, 0x73, 0xc0, 0x21, 0xc0, 0x57, 0x59, 0x75, 0x62, 0x69,
0x4b, 0x65, 0x79, 0xf9,
];
let expected = Atr {
protocols: [0, 1].to_vec(),
protocols: HashSet::from([0, 1]),
t1: [
0x80, 0x73, 0xc0, 0x21, 0xc0, 0x57, 0x59, 0x75, 0x62, 0x69, 0x4b, 0x65, 0x79,
]
Expand All @@ -273,11 +279,37 @@ mod tests {
assert_eq!("YubiKey", actual.card_issuers_data_str().unwrap());
}

#[test]
fn yubico_yubikey_5c_usb_macos() {
let _ = tracing_subscriber::fmt().try_init();
let input = [
0x3b, 0xfd, 0x13, 0x00, 0x00, 0x81, 0x31, 0xfe, 0x15, 0x80, 0x73, 0xc0, 0x21, 0xc0,
0x57, 0x59, 0x75, 0x62, 0x69, 0x4b, 0x65, 0x79, 0x40,
];
let expected = Atr {
// T=1 repeated twice
protocols: HashSet::from([1]),
t1: [
0x80, 0x73, 0xc0, 0x21, 0xc0, 0x57, 0x59, 0x75, 0x62, 0x69, 0x4b, 0x65, 0x79,
]
.to_vec(),
storage_card: false,
// "YubiKey"
card_issuers_data: Some([0x59, 0x75, 0x62, 0x69, 0x4b, 0x65, 0x79].to_vec()),
command_chaining: Some(true),
extended_lc: Some(true),
};

let actual = Atr::try_from(&input[..]).expect("yubico_yubikey_5c_usb_macos ATR");
assert_eq!(expected, actual);
assert_eq!("YubiKey", actual.card_issuers_data_str().unwrap());
}

#[test]
fn desfire_storage_card() {
let input = [0x3b, 0x81, 0x80, 0x01, 0x80, 0x80];
let expected = Atr {
protocols: [0, 1].to_vec(),
protocols: HashSet::from([0, 1]),
t1: [0x80].to_vec(),
storage_card: false,
card_issuers_data: None,
Expand All @@ -292,12 +324,13 @@ mod tests {

#[test]
fn felica_storage_card() {
let _ = tracing_subscriber::fmt().try_init();
let input = [
0x3b, 0x8f, 0x80, 0x01, 0x80, 0x4f, 0x0c, 0xa0, 0x00, 0x00, 0x03, 0x06, 0x11, 0x00,
0x3b, 0x00, 0x00, 0x00, 0x00, 0x42,
];
let expected = Atr {
protocols: [0, 1].to_vec(),
protocols: HashSet::from([0, 1]),
t1: [
0x80, 0x4f, 0x0c, 0xa0, 0x00, 0x00, 0x03, 0x06, 0x11, 0x00, 0x3b, 0x00, 0x00, 0x00,
0x00,
Expand All @@ -316,10 +349,11 @@ mod tests {

#[test]
fn short_capabilities() {
let _ = tracing_subscriber::fmt().try_init();
// These have a 1 and 2 byte tag 0x7X, so command chaining and extended
// lc support isn't available.
let i1 = [0x3b, 0x83, 0x80, 0x01, 0x80, 0x71, 0xc0, 0x33];
let expected_protocols = [0, 1].to_vec();
let expected_protocols = HashSet::from([0, 1]);
let a1 = Atr::try_from(&i1[..]).expect("short caps atr1");

assert_eq!(expected_protocols, a1.protocols);
Expand All @@ -338,8 +372,9 @@ mod tests {

#[test]
fn edge_cases() {
let _ = tracing_subscriber::fmt().try_init();
let expected = Atr {
protocols: [0].to_vec(),
protocols: HashSet::from([0]),
t1: [].to_vec(),
storage_card: false,
card_issuers_data: None,
Expand All @@ -358,6 +393,7 @@ mod tests {

#[test]
fn error_cases() {
let _ = tracing_subscriber::fmt().try_init();
let i1 = [0x3b];
let a1 = Atr::try_from(&i1[..]);
assert_eq!(a1, Err(WebauthnCError::MessageTooShort));
Expand Down

0 comments on commit fa03756

Please sign in to comment.