Skip to content

Commit

Permalink
Ignore Trussed CCID devices, document ignored_reader better, and impr…
Browse files Browse the repository at this point in the history
…ove its tests
  • Loading branch information
micolous committed Oct 25, 2023
1 parent 377bdc6 commit 414f5ae
Showing 1 changed file with 35 additions and 15 deletions.
50 changes: 35 additions & 15 deletions webauthn-authenticator-rs/src/nfc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,16 +99,29 @@ pub const APPLET_DF: [u8; 8] = [
/// indicate we should ignore it.
///
/// **See:** [`ignored_reader()`]
const IGNORED_READERS: [&str; 2] = [
// Nitrokey 3 exposes a CCID interface, which we can select the FIDO applet
// on, but it doesn't actually work.
"Nitrokey",
const IGNORED_READERS: [&str; 3] = [
// Trussed (used by Nitrokey 3 and SoloKeys Solo 2) expose a USB CCID
// interface which allows U2F applet selection, but then returns 0x6985
// (conditions of use not satisfied) to every command sent thereafter:
// https://github.com/trussed-dev/fido-authenticator/blob/7bd0c3bc5105a122fa11d9b354457746f391c4fb/src/dispatch/apdu.rs#L44-L48
// https://github.com/trussed-dev/fido-authenticator/issues/38
"Nitrokey", "SoloKey",
// YubiKey exposes a CCID interface when OpenGPG or PIV support is enabled,
// and this interface doesn't support FIDO.
"YubiKey",
];

/// Is the PC/SC card reader one which should be ignored?
///
/// Some USB security keys expose a USB CCID interface, which either don't
/// expose a FIDO applet (as recommended by [non-public FIDO documentation][0])
/// or is unusable.
///
/// Don't waste time enumerating these!
///
/// If `reader_name` does not contain valid UTF-8, this returns `false`.
///
/// [0]: https://github.com/w3c/webauthn/issues/1835#issuecomment-1382660217
fn ignored_reader(reader_name: &CStr) -> bool {
let reader_name = match reader_name.as_ref().to_str() {
Ok(r) => r,
Expand Down Expand Up @@ -670,31 +683,38 @@ mod test {
let _ = tracing_subscriber::fmt().try_init();

// CCID interfaces on tokens
const IGNORED: [&str; 3] = [
"Nitrokey Nitrokey 3",
"Nitrokey Nitrokey 3 [CCID/ICCD Interface] 00 00",
"Yubico YubiKey FIDO+CCID",
const IGNORED: [&[u8]; 4] = [
b"Nitrokey Nitrokey 3",
b"Nitrokey Nitrokey 3 [CCID/ICCD Interface] 00 00",
b"SoloKeys Solo 2",
b"Yubico YubiKey FIDO+CCID",
];

// Smartcard readers
const ALLOWED: [&str; 4] = [
"ACS ACR122U 00 00",
"ACS ACR122U 01 00",
"ACS ACR122U PICC Interface",
"ACS ACR123 3S Reader [ACR123U-PICC] (1.00.xx) 00 00",
const ALLOWED: [&[u8]; 6] = [
b"ACS ACR122U 00 00",
b"ACS ACR122U 01 00",
b"ACS ACR122U PICC Interface",
b"ACS ACR123 3S Reader [ACR123U-PICC] (1.00.xx) 00 00",
// Invalid UTF-8 should be allowed, even if it contains ignored
// reader names.
b"\xFF\xFF",
b"\xFFYubico YubiKey FIDO+CCID\xFF",
];

for n in IGNORED {
let e = String::from_utf8(n.to_vec()).unwrap_or_else(|_| hex::encode(n));
assert!(
ignored_reader(&CString::new(n)?),
"expected {n} to be ignored"
"expected {e} to be ignored"
);
}

for n in ALLOWED {
let e = String::from_utf8(n.to_vec()).unwrap_or_else(|_| hex::encode(n));
assert!(
!ignored_reader(&CString::new(n)?),
"expected {n} to be allowed"
"expected {e} to be allowed"
);
}

Expand Down

0 comments on commit 414f5ae

Please sign in to comment.