feat(nfc): dedupe USB-HID authenticators (#270)

Some security keys appear twice when we go looking for authenticators.
The same key shows up once as a USB device over HID, and again as a
PC/SC reader for its built-in smart-card interface. Both entries are the
same physical key, but until now nothing connected them, so a single key
plugged into USB also looked like a separate NFC authenticator waiting
to be used.

We now recognize the two entries as one device by comparing the USB
address behind each. When a reader resolves to the same physical USB
device as a key we already see over HID, we drop it from the NFC list. A
key plugged in over USB is no longer offered as a separate NFC device.

Reading real NFC keys is unchanged. A key held to a contactless reader
has no matching USB device, so it stays in the list and is read exactly
as before.

Relates to #182

Author: AlfioEmanueleFresta

libwebauthn/examples/ceremony/webauthn_nfc.rs

@@ -4,8 +4,8 @@ use libwebauthn::ops::webauthn::{
     GetAssertionRequest, JsonFormat, MakeCredentialRequest, OriginValidation, RelatedOrigins,
     RequestOrigin, RequestSettings, SystemPublicSuffixList, WebAuthnIDLResponse as _,
 };
-use libwebauthn::transport::nfc::{get_nfc_device, is_nfc_available};
-use libwebauthn::transport::{Channel as _, ChannelSettings, Device};
+use libwebauthn::transport::nfc::NfcDeviceSliceExt;
+use libwebauthn::transport::{hid, nfc, Channel as _, ChannelSettings, Device};
 use libwebauthn::webauthn::WebAuthn;
 
 #[path = "../common/mod.rs"]
@@ -15,16 +15,38 @@ mod common;
 pub async fn main() -> Result<(), Box<dyn Error>> {
     common::setup_logging();
 
-    if !is_nfc_available() {
+    if !nfc::is_nfc_available() {
         println!("No NFC-Reader found. NFC is not available on your system.");
         return Err("NFC not available".into());
     }
 
-    let Some(mut device) = get_nfc_device().await? else {
+    // A USB key's CCID interface also shows up as a PC/SC reader, so drop the
+    // NFC entries that duplicate a connected HID key.
+    let hid_devices = hid::list_devices().await.unwrap_or_default();
+    let nfc_devices = nfc::list_devices().await;
+    let discovered = nfc_devices.len();
+    let nfc_devices = nfc_devices.without_hid_duplicates(&hid_devices);
+    println!(
+        "Discovered {discovered} NFC device(s); dropped {} that duplicate a connected HID key.",
+        discovered - nfc_devices.len()
+    );
+
+    // Unfiltered, so keep the first survivor that opens a FIDO channel.
+    let mut selected = None;
+    for mut device in nfc_devices {
+        match device.channel(ChannelSettings::default()).await {
+            Ok(channel) => {
+                println!("Selected NFC authenticator: {device}");
+                selected = Some(channel);
+                break;
+            }
+            Err(error) => println!("Skipping NFC reader (no FIDO applet): {error}"),
+        }
+    }
+    let Some(mut channel) = selected else {
+        println!("No FIDO NFC authenticator found after de-duplication.");
         return Ok(());
     };
-    println!("Selected NFC authenticator: {}", device);
-    let mut channel = device.channel(ChannelSettings::default()).await?;
 
     let request_origin: RequestOrigin = "https://example.org".try_into().expect("Invalid origin");
     let psl = SystemPublicSuffixList::auto().expect(

libwebauthn/src/transport/hid/device.rs

@@ -12,6 +12,7 @@ use tracing::{debug, info, instrument};
 #[cfg(feature = "virt")]
 use super::framing::HidMessage;
 use crate::transport::error::TransportError;
+use crate::transport::usb::{usb_id_from_hidraw, UsbDeviceId};
 use crate::transport::{ChannelSettings, Device};
 use crate::webauthn::error::Error;
 
@@ -41,6 +42,17 @@ impl From<&DeviceInfo> for HidDevice {
     }
 }
 
+impl HidDevice {
+    /// The USB (bus, address) backing this key, when it can be resolved.
+    pub fn usb_device_id(&self) -> Option<UsbDeviceId> {
+        match &self.backend {
+            HidBackendDevice::HidApiDevice(info) => usb_id_from_hidraw(info.path()),
+            #[cfg(feature = "virt")]
+            HidBackendDevice::VirtualDevice(_) => None,
+        }
+    }
+}
+
 impl fmt::Display for HidDevice {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match &self.backend {

libwebauthn/src/transport/mod.rs

@@ -30,6 +30,7 @@ pub mod nfc {
         false
     }
 }
+pub mod usb;
 
 mod channel;
 #[allow(clippy::module_inception)]
@@ -43,3 +44,4 @@ pub use channel::ChannelStatus;
 
 pub use device::Device;
 pub use transport::Transport;
+pub use usb::UsbDeviceId;

libwebauthn/src/transport/nfc/device.rs

@@ -1,10 +1,11 @@
 use async_trait::async_trait;
+use std::collections::HashSet;
 use std::fmt;
 #[allow(unused_imports)]
 use tracing::{debug, info, instrument, trace};
 
 use crate::{
-    transport::{device::Device, Channel, ChannelSettings},
+    transport::{device::Device, hid::HidDevice, Channel, ChannelSettings, UsbDeviceId},
     webauthn::Error,
 };
 
@@ -60,6 +61,17 @@ impl NfcDevice {
         }
     }
 
+    /// The USB (bus, address) backing this device, when it can be resolved.
+    /// Performs blocking PC/SC I/O (a `Direct`-mode connect).
+    pub fn usb_device_id(&self) -> Option<UsbDeviceId> {
+        match &self.info {
+            #[cfg(feature = "nfc-backend-pcsc")]
+            DeviceInfo::Pcsc(info) => info.usb_device_id(),
+            #[cfg(feature = "nfc-backend-libnfc")]
+            DeviceInfo::LibNfc(_) => None,
+        }
+    }
+
     async fn channel_sync(&self, settings: ChannelSettings) -> Result<NfcChannel<Context>, Error> {
         trace!("nfc channel {:?}", self);
         let mut channel: NfcChannel<Context> = match &self.info {
@@ -135,3 +147,65 @@ pub fn is_nfc_available() -> bool {
 
     available
 }
+
+/// Lists all NFC devices from the compiled backends, unfiltered. A failing
+/// backend is skipped. Cross-backend duplicates are not removed.
+#[instrument]
+pub async fn list_devices() -> Vec<NfcDevice> {
+    #[allow(unused_mut)]
+    let mut devices = Vec::new();
+    #[cfg(feature = "nfc-backend-libnfc")]
+    if let Ok(found) = libnfc::list_devices() {
+        devices.extend(found);
+    }
+    #[cfg(feature = "nfc-backend-pcsc")]
+    if let Ok(found) = pcsc::list_devices() {
+        devices.extend(found);
+    }
+    devices
+}
+
+/// Drops NFC devices that are the CCID face of a USB key already seen over HID,
+/// matched by USB (bus, address). Does blocking PC/SC I/O per reader.
+pub trait NfcDeviceSliceExt {
+    fn without_hid_duplicates(&self, hid: &[HidDevice]) -> Vec<NfcDevice>;
+}
+
+impl NfcDeviceSliceExt for [NfcDevice] {
+    fn without_hid_duplicates(&self, hid: &[HidDevice]) -> Vec<NfcDevice> {
+        let hid_ids: HashSet<UsbDeviceId> =
+            hid.iter().filter_map(HidDevice::usb_device_id).collect();
+        let nfc_ids: Vec<Option<UsbDeviceId>> = self.iter().map(NfcDevice::usb_device_id).collect();
+        self.iter()
+            .zip(dedup_keep_mask(&nfc_ids, &hid_ids))
+            .filter(|&(_, keep)| keep)
+            .map(|(device, _)| device.clone())
+            .collect()
+    }
+}
+
+/// Pure dedup core, unit-testable without hardware.
+fn dedup_keep_mask(nfc_ids: &[Option<UsbDeviceId>], hid_ids: &HashSet<UsbDeviceId>) -> Vec<bool> {
+    nfc_ids
+        .iter()
+        .map(|id| match id {
+            Some(id) => !hid_ids.contains(id),
+            None => true,
+        })
+        .collect()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn dedup_keeps_non_duplicates_and_unknown() {
+        let a = UsbDeviceId { bus: 1, address: 8 };
+        let b = UsbDeviceId { bus: 1, address: 9 };
+        let nfc_ids = [Some(a), Some(b), None];
+        let hid_ids: HashSet<UsbDeviceId> = [a].into_iter().collect();
+
+        assert_eq!(dedup_keep_mask(&nfc_ids, &hid_ids), vec![false, true, true]);
+    }
+}

libwebauthn/src/transport/nfc/mod.rs

@@ -8,7 +8,7 @@ pub mod libnfc;
 #[cfg(feature = "nfc-backend-pcsc")]
 pub mod pcsc;
 
-pub use device::{get_nfc_device, is_nfc_available};
+pub use device::{get_nfc_device, is_nfc_available, list_devices, NfcDevice, NfcDeviceSliceExt};
 
 use super::Transport;
 

libwebauthn/src/transport/nfc/pcsc/mod.rs

@@ -2,6 +2,7 @@ use super::channel::{HandlerInCtx, NfcBackend, NfcChannel};
 use super::device::NfcDevice;
 use super::Context;
 use crate::transport::error::TransportError;
+use crate::transport::usb::UsbDeviceId;
 use crate::transport::ChannelSettings;
 use crate::webauthn::Error;
 use apdu::core::HandleError;
@@ -93,6 +94,33 @@ impl Info {
         let channel = NfcChannel::new(Box::new(chan), ctx, settings);
         Ok(channel)
     }
+
+    pub(crate) fn usb_device_id(&self) -> Option<UsbDeviceId> {
+        usb_id_from_reader(&self.name)
+    }
+}
+
+/// Reads the reader's `SCARD_ATTR_CHANNEL_ID` to get its USB (bus, address).
+/// Connects in `Direct` mode so no card is required and none is reset.
+pub(crate) fn usb_id_from_reader(name: &CStr) -> Option<UsbDeviceId> {
+    let context = pcsc::Context::establish(pcsc::Scope::User).ok()?;
+    let card = context
+        .connect(name, pcsc::ShareMode::Direct, pcsc::Protocols::UNDEFINED)
+        .ok()?;
+
+    let mut buf = [0u8; 8];
+    let id = card
+        .get_attribute(pcsc::Attribute::ChannelId, &mut buf)
+        .ok()
+        .and_then(|attr| attr.get(..4)?.try_into().ok())
+        .and_then(UsbDeviceId::from_channel_id_bytes);
+
+    // If disconnect fails, forget the returned Card so its Drop cannot reset an
+    // inserted card. The context release below frees the handle.
+    if let Err((card, _)) = card.disconnect(pcsc::Disposition::LeaveCard) {
+        std::mem::forget(card);
+    }
+    id
 }
 
 impl Channel {

libwebauthn/src/transport/usb.rs

@@ -0,0 +1,91 @@
+//! Identifies a physical USB device by its (bus, address), which is shared by
+//! all of its interfaces. Lets us recognise one key seen over both HID and PC/SC.
+
+/// A physical USB device, identified by its (bus, address) pair.
+#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
+pub struct UsbDeviceId {
+    pub bus: u8,
+    pub address: u8,
+}
+
+impl UsbDeviceId {
+    /// Decodes a `SCARD_ATTR_CHANNEL_ID` DWORD (USB marker `0x0020`, then `bus << 8 | address`).
+    #[cfg_attr(not(feature = "nfc-backend-pcsc"), allow(dead_code))]
+    pub(crate) fn from_channel_id(dword: u32) -> Option<Self> {
+        if (dword >> 16) != 0x0020 {
+            return None;
+        }
+        Some(Self {
+            bus: ((dword & 0xffff) >> 8) as u8,
+            address: (dword & 0xff) as u8,
+        })
+    }
+
+    /// Decodes the 4 channel-id bytes, trying both byte orders.
+    #[cfg_attr(not(feature = "nfc-backend-pcsc"), allow(dead_code))]
+    pub(crate) fn from_channel_id_bytes(bytes: [u8; 4]) -> Option<Self> {
+        let dword = u32::from_ne_bytes(bytes);
+        Self::from_channel_id(dword).or_else(|| Self::from_channel_id(dword.swap_bytes()))
+    }
+}
+
+/// Resolves the USB (bus, address) behind a hidraw node via sysfs.
+pub(crate) fn usb_id_from_hidraw(path: &std::ffi::CStr) -> Option<UsbDeviceId> {
+    use std::ffi::OsStr;
+    use std::os::unix::ffi::OsStrExt;
+    use std::path::{Path, PathBuf};
+
+    let name = Path::new(OsStr::from_bytes(path.to_bytes())).file_name()?;
+    let mut dir: PathBuf =
+        std::fs::canonicalize(Path::new("/sys/class/hidraw").join(name).join("device")).ok()?;
+
+    loop {
+        let busnum = dir.join("busnum");
+        let devnum = dir.join("devnum");
+        if busnum.is_file() && devnum.is_file() {
+            return Some(UsbDeviceId {
+                bus: read_sysfs_u8(&busnum)?,
+                address: read_sysfs_u8(&devnum)?,
+            });
+        }
+        if !dir.pop() {
+            return None;
+        }
+    }
+}
+
+fn read_sysfs_u8(path: &std::path::Path) -> Option<u8> {
+    // >255 yields None: keep the device rather than risk a false match.
+    let value: u32 = std::fs::read_to_string(path).ok()?.trim().parse().ok()?;
+    u8::try_from(value).ok()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn from_channel_id_decodes_usb() {
+        let id = UsbDeviceId::from_channel_id(0x0020_0108).expect("USB marker");
+        assert_eq!(id.bus, 1);
+        assert_eq!(id.address, 8);
+    }
+
+    #[test]
+    fn from_channel_id_rejects_non_usb() {
+        assert!(UsbDeviceId::from_channel_id(0x0010_0108).is_none());
+    }
+
+    #[test]
+    fn from_channel_id_bytes_decodes_either_byte_order() {
+        let want = UsbDeviceId { bus: 1, address: 8 };
+        assert_eq!(
+            UsbDeviceId::from_channel_id_bytes([0x08, 0x01, 0x20, 0x00]),
+            Some(want)
+        );
+        assert_eq!(
+            UsbDeviceId::from_channel_id_bytes([0x00, 0x20, 0x01, 0x08]),
+            Some(want)
+        );
+    }
+}