feat: refactor hal module architecture and add tests in fido module

2026-07-28 08:01:19 -07:00 · 2026-07-07 01:23:26 +05:30
parent f8fc40c6cc
commit b293e294f5
16 changed files with 1371 additions and 76 deletions
@@ -0,0 +1,112 @@
+#![allow(dead_code)]
+use std::fmt;
+
+#[repr(i32)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum CoseAlgorithm {
+    ES256 = -7,
+    EdDSA = -8,
+    ESP256 = -9,
+    Ed25519 = -19,
+    EcdhEsHkdf256 = -25,
+    ES384 = -35,
+    ES512 = -36,
+    ES256K = -47,
+    ESP384 = -51,
+    ESP512 = -52,
+    Ed448 = -53,
+    RS256 = -257,
+    RS384 = -258,
+    RS512 = -259,
+    ESB256 = -265,
+    ESB384 = -267,
+    ESB512 = -268,
+    MLDSA44 = -48,
+    MLDSA65 = -49,
+    MLDSA87 = -50,
+}
+
+impl CoseAlgorithm {
+    pub fn from_i128(val: i128) -> Option<Self> {
+        match val as i32 {
+            -7 => Some(Self::ES256),
+            -8 => Some(Self::EdDSA),
+            -9 => Some(Self::ESP256),
+            -19 => Some(Self::Ed25519),
+            -25 => Some(Self::EcdhEsHkdf256),
+            -35 => Some(Self::ES384),
+            -36 => Some(Self::ES512),
+            -47 => Some(Self::ES256K),
+            -51 => Some(Self::ESP384),
+            -52 => Some(Self::ESP512),
+            -53 => Some(Self::Ed448),
+            -257 => Some(Self::RS256),
+            -258 => Some(Self::RS384),
+            -259 => Some(Self::RS512),
+            -265 => Some(Self::ESB256),
+            -267 => Some(Self::ESB384),
+            -268 => Some(Self::ESB512),
+            -48 => Some(Self::MLDSA44),
+            -49 => Some(Self::MLDSA65),
+            -50 => Some(Self::MLDSA87),
+            _ => None,
+        }
+    }
+}
+
+impl fmt::Display for CoseAlgorithm {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::ES256 => write!(f, "ES256"),
+            Self::EdDSA => write!(f, "EdDSA"),
+            Self::ESP256 => write!(f, "ESP256"),
+            Self::Ed25519 => write!(f, "Ed25519"),
+            Self::EcdhEsHkdf256 => write!(f, "ECDH-ES-HKDF-256"),
+            Self::ES384 => write!(f, "ES384"),
+            Self::ES512 => write!(f, "ES512"),
+            Self::ES256K => write!(f, "ES256K"),
+            Self::ESP384 => write!(f, "ESP384"),
+            Self::ESP512 => write!(f, "ESP512"),
+            Self::Ed448 => write!(f, "Ed448"),
+            Self::RS256 => write!(f, "RS256"),
+            Self::RS384 => write!(f, "RS384"),
+            Self::RS512 => write!(f, "RS512"),
+            Self::ESB256 => write!(f, "ESB256"),
+            Self::ESB384 => write!(f, "ESB384"),
+            Self::ESB512 => write!(f, "ESB512"),
+            Self::MLDSA44 => write!(f, "ML-DSA-44"),
+            Self::MLDSA65 => write!(f, "ML-DSA-65"),
+            Self::MLDSA87 => write!(f, "ML-DSA-87"),
+        }
+    }
+}
+
+#[repr(u8)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum CoseCurve {
+    P256 = 1,
+    P384 = 2,
+    P521 = 3,
+    X25519 = 4,
+    X448 = 5,
+    Ed25519 = 6,
+    Ed448 = 7,
+    P256K1 = 8,
+    BP256R1 = 9,
+    BP384R1 = 10,
+    BP512R1 = 11,
+}
+
+#[repr(i32)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum CoseKeyParam {
+    Kty = 1,
+    Kid = 2,
+    Alg = 3,
+    KeyOps = 4,
+    BaseIV = 5,
+    Crv = -1,
+    X = -2,
+    Y = -3,
+    D = -4,
+}
@@ -0,0 +1,4 @@
+pub mod cose;
+pub mod version;
+
+pub use version::FirmwareVersion;
@@ -0,0 +1,170 @@
+#![allow(dead_code)]
+use std::fmt;
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct FirmwareVersion {
+    pub major: u16,
+    pub minor: u16,
+    pub patch: u16,
+    pub raw: String,
+}
+
+impl FirmwareVersion {
+    pub fn parse(version: &str) -> Option<Self> {
+        let parts: Vec<&str> = version.split('.').collect();
+        let major = parts.first()?.parse().ok()?;
+        let minor = parts.get(1)?.parse().ok()?;
+        let patch = parts.get(2).and_then(|s| s.parse().ok()).unwrap_or(0);
+        Some(Self {
+            major,
+            minor,
+            patch,
+            raw: version.to_string(),
+        })
+    }
+
+    pub fn is_at_least(&self, major: u16, minor: u16) -> bool {
+        self.major > major || (self.major == major && self.minor >= minor)
+    }
+
+    pub fn is_between(&self, lo_major: u16, lo_minor: u16, hi_major: u16, hi_minor: u16) -> bool {
+        self.is_at_least(lo_major, lo_minor)
+            && (self.major < hi_major || (self.major == hi_major && self.minor <= hi_minor))
+    }
+}
+
+impl fmt::Display for FirmwareVersion {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.raw)
+    }
+}
+
+impl Default for FirmwareVersion {
+    fn default() -> Self {
+        Self {
+            major: 0,
+            minor: 0,
+            patch: 0,
+            raw: "0.0".into(),
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_parse_two_part_version() {
+        let v = FirmwareVersion::parse("7.6").unwrap();
+        assert_eq!(v.major, 7);
+        assert_eq!(v.minor, 6);
+        assert_eq!(v.patch, 0);
+        assert_eq!(v.raw, "7.6");
+    }
+
+    #[test]
+    fn test_parse_three_part_version() {
+        let v = FirmwareVersion::parse("5.7.4").unwrap();
+        assert_eq!(v.major, 5);
+        assert_eq!(v.minor, 7);
+        assert_eq!(v.patch, 4);
+        assert_eq!(v.raw, "5.7.4");
+    }
+
+    #[test]
+    fn test_parse_single_part_fails() {
+        assert!(FirmwareVersion::parse("7").is_none());
+    }
+
+    #[test]
+    fn test_parse_non_numeric_fails() {
+        assert!(FirmwareVersion::parse("a.b").is_none());
+        assert!(FirmwareVersion::parse("7.x").is_none());
+    }
+
+    #[test]
+    fn test_parse_empty_fails() {
+        assert!(FirmwareVersion::parse("").is_none());
+    }
+
+    #[test]
+    fn test_is_at_least_exact_match() {
+        let v = FirmwareVersion::parse("7.2").unwrap();
+        assert!(v.is_at_least(7, 2));
+    }
+
+    #[test]
+    fn test_is_at_least_above() {
+        let v = FirmwareVersion::parse("7.6").unwrap();
+        assert!(v.is_at_least(7, 2));
+        assert!(v.is_at_least(6, 0));
+        assert!(v.is_at_least(7, 6));
+    }
+
+    #[test]
+    fn test_is_at_least_below() {
+        let v = FirmwareVersion::parse("7.0").unwrap();
+        assert!(!v.is_at_least(7, 2));
+        assert!(!v.is_at_least(8, 0));
+    }
+
+    #[test]
+    fn test_is_between_inclusive_range() {
+        let v = FirmwareVersion::parse("7.2").unwrap();
+        assert!(v.is_between(6, 0, 8, 0));
+        assert!(v.is_between(7, 0, 7, 2));
+        assert!(v.is_between(7, 2, 7, 2));
+    }
+
+    #[test]
+    fn test_is_between_outside_range() {
+        let v = FirmwareVersion::parse("7.6").unwrap();
+        assert!(!v.is_between(6, 0, 7, 2));
+        assert!(!v.is_between(8, 0, 9, 0));
+    }
+
+    #[test]
+    fn test_default_version() {
+        let v = FirmwareVersion::default();
+        assert_eq!(v.major, 0);
+        assert_eq!(v.minor, 0);
+        assert_eq!(v.patch, 0);
+        assert_eq!(v.raw, "0.0");
+    }
+
+    #[test]
+    fn test_display() {
+        let v = FirmwareVersion::parse("7.6.1").unwrap();
+        assert_eq!(v.to_string(), "7.6.1");
+    }
+
+    #[test]
+    fn test_parse_with_patch_zero() {
+        let v = FirmwareVersion::parse("7.6.0").unwrap();
+        assert_eq!(v.major, 7);
+        assert_eq!(v.minor, 6);
+        assert_eq!(v.patch, 0);
+    }
+
+    #[test]
+    fn test_legacy_fido_config_boundaries() {
+        // <= 7.2 supports legacy FIDO hardware config
+        assert!(FirmwareVersion::parse("7.2").unwrap().is_at_least(0, 0));
+        assert!(
+            !FirmwareVersion::parse("7.3")
+                .unwrap()
+                .is_between(0, 0, 7, 2)
+        );
+        assert!(
+            FirmwareVersion::parse("7.2")
+                .unwrap()
+                .is_between(0, 0, 7, 2)
+        );
+        assert!(
+            FirmwareVersion::parse("6.6")
+                .unwrap()
+                .is_between(0, 0, 7, 2)
+        );
+    }
+}
@@ -156,6 +156,7 @@ const HID_TOTAL_TIMEOUT_MS: i32 = 5000;
 ///
 /// Created via [`HidTransport::open`], which scans for a device with the FIDO
 /// HID Usage Page (0xF1D0) and performs the INIT handshake to obtain a Channel ID.
+#[derive(Debug)]
 pub struct HidTransport {
    device: hidapi::HidDevice,
    cid: u32,
@@ -59,9 +59,12 @@ pub mod hid;

 use crate::{
    error::PFError,
-    hal::types::{
-        AppConfig, AppConfigInput, DeviceInfo, DeviceMethod, FidoDeviceInfo, FirmwareType,
-        FullDeviceStatus, PICOFIDO_AAGUID, RSKEY_AAGUID, StoredCredential,
+    hal::{
+        firmwares::AnyFirmware,
+        types::{
+            AppConfig, AppConfigInput, DeviceInfo, DeviceMethod, FidoDeviceInfo, FirmwareType,
+            FullDeviceStatus, PICOFIDO_AAGUID, RSKEY_AAGUID, StoredCredential,
+        },
    },
 };
 use base64::{Engine as _, engine::general_purpose};
@@ -374,18 +377,11 @@ fn parse_get_info_extension_list(
 }

 pub(crate) fn firmware_supports_legacy_fido_hardware_config(version: &str) -> bool {
-    let Some((major, minor)) = parse_firmware_version(version) else {
+    let ver = crate::hal::common::FirmwareVersion::parse(version);
+    let Some(ref ver) = ver else {
        return false;
    };
-
-    major < 7 || (major == 7 && minor <= 2)
-}
-
-fn parse_firmware_version(version: &str) -> Option<(u16, u16)> {
-    let mut parts = version.split('.');
-    let major = parts.next()?.parse().ok()?;
-    let minor = parts.next()?.parse().ok()?;
-    Some((major, minor))
+    ver.major < 7 || (ver.major == 7 && ver.minor <= 2)
 }

 pub(crate) fn change_fido_pin(
@@ -595,8 +591,15 @@ pub fn read_device_details() -> Result<FullDeviceStatus, PFError> {
        fido_info.firmware_version
    );

-    let supports_legacy_hardware_config =
-        firmware_supports_legacy_fido_hardware_config(&fido_info.firmware_version);
+    let firmware_type = if fido_info.aaguid == RSKEY_AAGUID {
+        FirmwareType::RSKey
+    } else if fido_info.aaguid == PICOFIDO_AAGUID {
+        FirmwareType::PicoFido
+    } else {
+        FirmwareType::Unknown
+    };
+    let firmware = AnyFirmware::new(firmware_type, &fido_info.firmware_version);
+    let supports_legacy_hardware_config = firmware.supports_legacy_fido_hardware_config();
    let management = read_management_info(&transport);
    let config = AppConfig {
        vid: format!("{:04X}", transport.vid),
@@ -629,14 +632,6 @@ pub fn read_device_details() -> Result<FullDeviceStatus, PFError> {
            .unwrap_or_else(|| "Unknown".to_string())
    };

-    let firmware_type = if fido_info.aaguid == RSKEY_AAGUID {
-        FirmwareType::RSKey
-    } else if fido_info.aaguid == PICOFIDO_AAGUID {
-        FirmwareType::PicoFido
-    } else {
-        FirmwareType::Unknown
-    };
-
    Ok(FullDeviceStatus {
        info: DeviceInfo {
            serial: management
@@ -650,7 +645,7 @@ pub fn read_device_details() -> Result<FullDeviceStatus, PFError> {
        secure_boot: false,
        secure_lock: false,
        method: DeviceMethod::Fido,
-        firmware_type,
+        firmware_type: firmware.firmware_type(),
    })
 }

@@ -830,8 +825,15 @@ pub fn write_config(config: AppConfigInput, pin: Option<String>) -> Result<Strin
        PFError::Device(format!("Could not open HID transport: {}", e))
    })?;
    let fido_info = read_device_info(&transport)?;
-    let supports_legacy_hardware_config =
-        firmware_supports_legacy_fido_hardware_config(&fido_info.firmware_version);
+    let firmware_type = if fido_info.aaguid == RSKEY_AAGUID {
+        FirmwareType::RSKey
+    } else if fido_info.aaguid == PICOFIDO_AAGUID {
+        FirmwareType::PicoFido
+    } else {
+        FirmwareType::Unknown
+    };
+    let firmware = AnyFirmware::new(firmware_type, &fido_info.firmware_version);
+    let supports_legacy_hardware_config = firmware.supports_legacy_fido_hardware_config();

    validate_fido_config_changes(&config, supports_legacy_hardware_config)?;

@@ -1129,6 +1131,7 @@ mod tests {
            raw_curves_mask: None,
            led_order: None,
            enabled_usb_itf: None,
+            led_num: None,
        }
    }

@@ -1328,4 +1331,146 @@ mod tests {

        assert!(err.contains("VID and PID"));
    }
+
+    #[test]
+    fn test_parse_get_info_rskey_style() {
+        // RS-Key GetInfo has a different AAGUID, may include PQC algorithms,
+        // and reports firmware at a different key.
+        let mut map = BTreeMap::new();
+        map.insert(
+            Value::Integer(0x01),
+            Value::Array(vec![
+                Value::Text("U2F_V2".into()),
+                Value::Text("FIDO_2_0".into()),
+                Value::Text("FIDO_2_1".into()),
+            ]),
+        );
+        // RS-Key AAGUID
+        map.insert(
+            Value::Integer(0x03),
+            Value::Bytes(vec![
+                0x24, 0x79, 0xC7, 0xBF, 0x6B, 0x30, 0x56, 0x83, 0x9E, 0xC8, 0x0E, 0x81, 0x71, 0xA9,
+                0x18, 0xB7,
+            ]),
+        );
+        map.insert(Value::Integer(0x05), Value::Integer(1200));
+        map.insert(
+            Value::Integer(0x06),
+            Value::Array(vec![Value::Integer(1), Value::Integer(2)]),
+        );
+        map.insert(Value::Integer(0x0D), Value::Integer(4));
+
+        // RS-Key firmware version (5.7.4 encoded as (5<<8)|7 = 0x0507)
+        map.insert(Value::Integer(0x0E), Value::Integer(0x0507));
+
+        // Algorithms list including PQC
+        let es256 = BTreeMap::from([(Value::Text("alg".into()), Value::Integer(-7))]);
+        let eddsa = BTreeMap::from([(Value::Text("alg".into()), Value::Integer(-8))]);
+        let pqc = BTreeMap::from([(Value::Text("alg".into()), Value::Integer(-48))]);
+        map.insert(
+            Value::Integer(0x0A),
+            Value::Array(vec![Value::Map(es256), Value::Map(eddsa), Value::Map(pqc)]),
+        );
+
+        // RS-Key options
+        let mut opts = BTreeMap::new();
+        opts.insert(Value::Text("rk".into()), Value::Bool(true));
+        opts.insert(Value::Text("up".into()), Value::Bool(true));
+        opts.insert(Value::Text("clientPin".into()), Value::Bool(false));
+        opts.insert(Value::Text("credMgmt".into()), Value::Bool(true));
+        opts.insert(Value::Text("authnrCfg".into()), Value::Bool(true));
+        map.insert(Value::Integer(0x04), Value::Map(opts));
+
+        let info = parse_fido_get_info(&Value::Map(map)).unwrap();
+
+        assert_eq!(info.aaguid, "2479C7BF6B3056839EC80E8171A918B7");
+        assert_eq!(info.firmware_version, "5.7");
+        assert_eq!(info.versions, vec!["U2F_V2", "FIDO_2_0", "FIDO_2_1"]);
+        assert_eq!(info.algorithms, vec!["ES256", "EdDSA", "ML-DSA-44"]);
+        assert_eq!(info.min_pin_length, 4);
+        assert!(info.options.get("rk") == Some(&true));
+        assert!(info.options.get("credMgmt") == Some(&true));
+        assert!(info.options.get("clientPin") == Some(&false));
+    }
+
+    #[test]
+    fn test_parse_get_info_empty_returns_default() {
+        let result = parse_fido_get_info(&Value::Map(BTreeMap::new()));
+        assert!(result.is_ok());
+        assert!(result.unwrap().versions.is_empty());
+    }
+
+    #[test]
+    fn test_parse_get_info_skips_unknown_keys() {
+        let mut map = BTreeMap::new();
+        map.insert(
+            Value::Integer(0x01),
+            Value::Array(vec![Value::Text("FIDO_2_1".into())]),
+        );
+        map.insert(Value::Integer(0x03), Value::Bytes(vec![0x89; 16]));
+        map.insert(Value::Integer(0x05), Value::Integer(1024));
+
+        // Unknown keys should be silently skipped
+        map.insert(Value::Integer(0x10), Value::Integer(999));
+        map.insert(Value::Integer(0x11), Value::Integer(888));
+        map.insert(Value::Integer(0x12), Value::Integer(777));
+
+        let info = parse_fido_get_info(&Value::Map(map)).unwrap();
+        assert_eq!(info.versions, vec!["FIDO_2_1"]);
+        assert_eq!(info.max_msg_size, 1024);
+    }
+
+    #[test]
+    fn test_parse_get_info_minimal_response() {
+        let mut map = BTreeMap::new();
+        map.insert(
+            Value::Integer(0x01),
+            Value::Array(vec![Value::Text("FIDO_2_0".into())]),
+        );
+
+        let info = parse_fido_get_info(&Value::Map(map)).unwrap();
+        assert_eq!(info.versions, vec!["FIDO_2_0"]);
+        assert_eq!(info.max_msg_size, 0);
+    }
+
+    #[test]
+    fn test_parse_get_info_certification_map_unknown_id_becomes_hex() {
+        let mut cert_map = BTreeMap::new();
+        cert_map.insert(Value::Text("0xDEADBEEFCAFEBABE".into()), Value::Bool(true));
+
+        let mut map = BTreeMap::new();
+        map.insert(Value::Integer(0x15), Value::Map(cert_map));
+
+        let info = parse_fido_get_info(&Value::Map(map)).unwrap();
+        assert_eq!(info.certifications.get("0xDEADBEEFCAFEBABE"), Some(&true));
+    }
+
+    #[test]
+    fn test_parse_get_info_management_info_version_fallback() {
+        // When firmware version in GetInfo is 0.0, management info version
+        // should be used instead - this is tested via the management info
+        // parsing, but verify the GetInfo parser handles the edge case.
+        let mut map = BTreeMap::new();
+        map.insert(
+            Value::Integer(0x01),
+            Value::Array(vec![Value::Text("FIDO_2_1".into())]),
+        );
+        map.insert(Value::Integer(0x03), Value::Bytes(vec![0x89; 16]));
+        // firmware_version = 0x0000 -> would become "0.0"
+        map.insert(Value::Integer(0x0E), Value::Integer(0x0000));
+
+        let info = parse_fido_get_info(&Value::Map(map)).unwrap();
+        assert_eq!(info.firmware_version, "0.0");
+    }
+
+    #[test]
+    fn test_is_empty_config_input_works() {
+        assert!(is_empty_config_input(&empty_config_input()));
+        let mut c = empty_config_input();
+        c.led_gpio = Some(25);
+        assert!(!is_empty_config_input(&c));
+        let mut c = empty_config_input();
+        c.vid = Some("FEFF".to_string());
+        assert!(!is_empty_config_input(&c));
+    }
 }
@@ -0,0 +1,93 @@
+#![allow(dead_code)]
+pub mod picofido;
+pub mod rskey;
+
+pub use picofido::*;
+pub use rskey::*;
+
+use crate::hal::common::FirmwareVersion;
+use crate::hal::types::FirmwareType;
+
+#[derive(Debug, Clone)]
+pub enum AnyFirmware {
+    PicoFido(PicoFidoFirmware),
+    RSKey(RSKeyFirmware),
+}
+
+pub trait FirmwareTrait {
+    fn firmware_type(&self) -> FirmwareType;
+    fn version(&self) -> &FirmwareVersion;
+    fn major_minor(&self) -> (u16, u16) {
+        (self.version().major, self.version().minor)
+    }
+    fn version_str(&self) -> &str {
+        &self.version().raw
+    }
+
+    fn supports_legacy_fido_hardware_config(&self) -> bool;
+    fn supports_rs_key_vendor_command(&self) -> bool;
+    fn supports_rescue_channel(&self) -> bool;
+}
+
+impl AnyFirmware {
+    pub fn detect_by_aaguid(aaguid: &str) -> FirmwareType {
+        if aaguid == crate::hal::types::RSKEY_AAGUID {
+            FirmwareType::RSKey
+        } else if aaguid == crate::hal::types::PICOFIDO_AAGUID {
+            FirmwareType::PicoFido
+        } else {
+            FirmwareType::Unknown
+        }
+    }
+
+    pub fn new(fw_type: FirmwareType, version: &str) -> Self {
+        let ver = FirmwareVersion::parse(version).unwrap_or_default();
+        match fw_type {
+            FirmwareType::PicoFido => Self::PicoFido(PicoFidoFirmware::new(ver)),
+            FirmwareType::RSKey => Self::RSKey(RSKeyFirmware::new(ver)),
+            FirmwareType::Unknown => Self::PicoFido(PicoFidoFirmware::new(ver)),
+        }
+    }
+
+    pub fn version(&self) -> &FirmwareVersion {
+        match self {
+            Self::PicoFido(fw) => fw.version(),
+            Self::RSKey(fw) => fw.version(),
+        }
+    }
+
+    pub fn firmware_type(&self) -> FirmwareType {
+        match self {
+            Self::PicoFido(_) => FirmwareType::PicoFido,
+            Self::RSKey(_) => FirmwareType::RSKey,
+        }
+    }
+
+    pub fn supports_legacy_fido_hardware_config(&self) -> bool {
+        match self {
+            Self::PicoFido(fw) => fw.supports_legacy_fido_hardware_config(),
+            Self::RSKey(fw) => fw.supports_legacy_fido_hardware_config(),
+        }
+    }
+
+    pub fn supports_new_fido_hardware_config(&self) -> bool {
+        match self {
+            Self::PicoFido(fw) => !fw.supports_legacy_fido_hardware_config(),
+            Self::RSKey(_) => false,
+        }
+    }
+
+    pub fn supports_rs_key_vendor_command(&self) -> bool {
+        match self {
+            Self::PicoFido(_) => false,
+            Self::RSKey(fw) => fw.supports_rs_key_vendor_command(),
+        }
+    }
+
+    pub fn supports_rescue_channel(&self) -> bool {
+        match self {
+            Self::PicoFido(_) => true,
+            Self::RSKey(_) => true,
+        }
+    }
+}
@@ -0,0 +1,36 @@
+use crate::hal::common::FirmwareVersion;
+use crate::hal::firmwares::FirmwareTrait;
+use crate::hal::types::FirmwareType;
+
+#[derive(Debug, Clone)]
+pub struct PicoFidoFirmware {
+    version: FirmwareVersion,
+}
+
+impl PicoFidoFirmware {
+    pub fn new(version: FirmwareVersion) -> Self {
+        Self { version }
+    }
+}
+
+impl FirmwareTrait for PicoFidoFirmware {
+    fn firmware_type(&self) -> FirmwareType {
+        FirmwareType::PicoFido
+    }
+
+    fn version(&self) -> &FirmwareVersion {
+        &self.version
+    }
+
+    fn supports_legacy_fido_hardware_config(&self) -> bool {
+        self.version.major < 7 || (self.version.major == 7 && self.version.minor <= 2)
+    }
+
+    fn supports_rs_key_vendor_command(&self) -> bool {
+        false
+    }
+
+    fn supports_rescue_channel(&self) -> bool {
+        true
+    }
+}
@@ -0,0 +1,36 @@
+use crate::hal::common::FirmwareVersion;
+use crate::hal::firmwares::FirmwareTrait;
+use crate::hal::types::FirmwareType;
+
+#[derive(Debug, Clone)]
+pub struct RSKeyFirmware {
+    version: FirmwareVersion,
+}
+
+impl RSKeyFirmware {
+    pub fn new(version: FirmwareVersion) -> Self {
+        Self { version }
+    }
+}
+
+impl FirmwareTrait for RSKeyFirmware {
+    fn firmware_type(&self) -> FirmwareType {
+        FirmwareType::RSKey
+    }
+
+    fn version(&self) -> &FirmwareVersion {
+        &self.version
+    }
+
+    fn supports_legacy_fido_hardware_config(&self) -> bool {
+        false
+    }
+
+    fn supports_rs_key_vendor_command(&self) -> bool {
+        self.version.is_at_least(0, 1)
+    }
+
+    fn supports_rescue_channel(&self) -> bool {
+        true
+    }
+}
@@ -1,31 +1,103 @@
-//! Device I/O layer bridging rescue (pcsc) and FIDO2 protocols.
-//!
-//! High-level entry points for reading/writing device configuration,
-//! managing credentials, and controlling LED/boot behavior.
-//!
-//! Functions are grouped by the protocol they use:
-//! - Functions that use both rescue and FIDO (fallback/dispatch logic)
-//! - Functions that communicate exclusively over the rescue (PC/SC) channel
-//! - Functions that communicate exclusively over the FIDO2 channel
+use crate::{
+    error::PFError,
+    hal::{fido, rescue, types::*},
+};

-#![allow(unused)]
-
-use crate::{error::PFError, hal::fido, hal::rescue, hal::types::*};
-
-// ── Shared: functions that use both rescue and FIDO ─────────────────────────
-
-/// Read full device status. Tries rescue first, falls back to FIDO on failure.
 pub fn read_device_details() -> Result<FullDeviceStatus, PFError> {
+    let mut fido_status: Option<FullDeviceStatus> = None;
+    let mut rescue_status: Option<FullDeviceStatus> = None;
+
+    match fido::read_device_details() {
+        Ok(status) => {
+            log::info!("FIDO device details read successfully");
+            fido_status = Some(status);
+        }
+        Err(e) => log::warn!("FIDO read_device_details failed: {}", e),
+    }
+
    match rescue::read_device_details() {
-        Ok(status) => Ok(status),
-        Err(e) => {
-            log::warn!("Rescue method failed: {}. Falling back to FIDO...", e);
-            fido::read_device_details()
+        Ok(status) => {
+            log::info!("Rescue device details read successfully");
+            rescue_status = Some(status);
+        }
+        Err(e) => log::warn!("Rescue read_device_details failed: {}", e),
+    }
+
+    match (fido_status, rescue_status) {
+        (Some(fido), Some(rescue)) => {
+            log::info!("Merging FIDO and Rescue device details");
+            Ok(FullDeviceStatus {
+                info: DeviceInfo {
+                    serial: rescue.info.serial,
+                    flash_used: rescue.info.flash_used,
+                    flash_total: rescue.info.flash_total,
+                    firmware_version: fido.info.firmware_version,
+                },
+                config: AppConfig {
+                    vid: if !rescue.config.vid.is_empty() {
+                        rescue.config.vid
+                    } else {
+                        fido.config.vid
+                    },
+                    pid: if !rescue.config.pid.is_empty() {
+                        rescue.config.pid
+                    } else {
+                        fido.config.pid
+                    },
+                    led_gpio: rescue.config.led_gpio,
+                    led_brightness: rescue.config.led_brightness,
+                    led_dimmable: rescue.config.led_dimmable,
+                    power_cycle_on_reset: rescue.config.power_cycle_on_reset,
+                    led_steady: rescue.config.led_steady,
+                    enable_secp256k1: rescue.config.enable_secp256k1,
+                    led_driver: rescue.config.led_driver.or_else(|| {
+                        if fido.config.led_driver.is_some() {
+                            fido.config.led_driver
+                        } else {
+                            None
+                        }
+                    }),
+                    product_name: rescue.config.product_name,
+                    touch_timeout: rescue.config.touch_timeout,
+                    raw_curves_mask: rescue.config.raw_curves_mask,
+                    led_order: rescue.config.led_order,
+                    enabled_usb_itf: rescue.config.enabled_usb_itf,
+                    led_num: rescue.config.led_num,
+                },
+                secure_boot: rescue.secure_boot,
+                secure_lock: rescue.secure_lock,
+                method: DeviceMethod::Fido,
+                firmware_type: fido.firmware_type,
+            })
+        }
+        (Some(fido), None) => {
+            log::info!("Using FIDO-only device details");
+            Ok(FullDeviceStatus {
+                firmware_type: fido.firmware_type,
+                ..fido
+            })
+        }
+        (None, Some(rescue)) => {
+            log::info!("Using Rescue-only device details");
+            Ok(rescue)
+        }
+        (None, None) => {
+            log::error!("Failed to read device details via both FIDO and Rescue");
+            Err(PFError::NoDevice)
        }
    }
 }

-/// Write app config. Dispatches to rescue or FIDO based on `method`.
+#[allow(dead_code)]
+pub fn enable_secure_boot(lock: bool) -> Result<String, PFError> {
+    rescue::enable_secure_boot(lock)
+}
+
+#[allow(dead_code)]
+pub fn reboot(to_bootsel: bool) -> Result<String, PFError> {
+    rescue::reboot_device(to_bootsel)
+}
+
 pub fn write_config(
    config: AppConfigInput,
    method: DeviceMethod,
@@ -38,24 +110,10 @@ pub fn write_config(
    }
 }

-// ── Rescue protocol (PC/SC) ─────────────────────────────────────────────────
-
-/// Lock or unlock secure boot via rescue.
-pub fn enable_secure_boot(lock: bool) -> Result<String, PFError> {
-    rescue::enable_secure_boot(lock)
-}
-
-/// Reboot the device. Pass `true` to enter BOOTSEL mode.
-pub fn reboot(to_bootsel: bool) -> Result<String, PFError> {
-    rescue::reboot_device(to_bootsel)
-}
-
-/// Read current LED status config via rescue.
 pub fn read_led_config() -> Result<LedStatusConfig, PFError> {
    rescue::read_led_config()
 }

-/// Write LED status (on/off, color, brightness, steady/blinking).
 pub fn write_led_status(
    status: u8,
    color: u8,
@@ -65,24 +123,18 @@ pub fn write_led_status(
    rescue::write_led_status(status, color, brightness, steady)
 }

-/// Read management app config via rescue.
 pub fn read_management_config() -> Result<ManagementAppConfig, PFError> {
    rescue::read_management_config()
 }

-/// Write management app enabled-mask via rescue.
 pub fn write_management_config(enabled_mask: u16) -> Result<String, PFError> {
    rescue::write_management_config(enabled_mask)
 }

-// ── FIDO2 protocol ──────────────────────────────────────────────────────────
-
-/// Query basic FIDO device info (AAGUID, version, etc.).
 pub(crate) fn get_fido_info() -> Result<FidoDeviceInfo, String> {
    fido::get_fido_info()
 }

-/// Change the FIDO user PIN.
 pub(crate) fn change_fido_pin(
    current_pin: Option<String>,
    new_pin: String,
@@ -90,7 +142,6 @@ pub(crate) fn change_fido_pin(
    fido::change_fido_pin(current_pin, new_pin)
 }

-/// Set the minimum PIN length requirement.
 pub(crate) fn set_min_pin_length(
    current_pin: String,
    min_pin_length: u8,
@@ -98,32 +149,26 @@ pub(crate) fn set_min_pin_length(
    fido::set_min_pin_length(current_pin, min_pin_length)
 }

-/// List stored credentials for the given PIN.
 pub fn get_credentials(pin: String) -> Result<Vec<StoredCredential>, String> {
    fido::get_credentials(pin)
 }

-/// Delete a single credential by its ID.
 pub fn delete_credential(pin: String, credential_id: String) -> Result<String, String> {
    fido::delete_credential(pin, credential_id)
 }

-/// Factory-reset the device, wiping all credentials and settings.
 pub fn reset_device() -> Result<String, String> {
    fido::reset_device()
 }

-/// Enable enterprise attestation for the device.
 pub fn enable_enterprise_attestation(pin: String) -> Result<String, String> {
    fido::enable_enterprise_attestation(pin)
 }

-/// Retrieve the enterprise attestation CSR.
 pub fn get_enterprise_attestation_csr() -> Result<String, String> {
    fido::get_enterprise_attestation_csr()
 }

-/// Upload a signed enterprise attestation certificate.
 pub fn upload_enterprise_attestation_cert(
    pin: String,
    cert_path: String,
@@ -39,7 +39,10 @@
 //!    and converts errors to the caller's expected type.
 //! 4. Wire the wrapper into a gpui-component view or action handler.

+pub mod common;
 pub mod fido;
+pub mod firmwares;
 pub mod io;
 pub mod rescue;
+pub mod transport;
 pub mod types;
@@ -124,10 +124,13 @@ pub enum RescueInstruction {
    /// Data field contains the tag value to write.
    Write = 0x1C,

-    /// Lock or unlock device access.
+    /// Lock or unlock device access (pico-fido only, not RS-Key).
    ///
    /// P2 parameter determines lock state (0x00=Unlock, 0x01=Lock).
    /// When locked, PHY configuration commands are rejected.
+    ///
+    /// **Note**: This instruction is only available on pico-fido firmware
+    /// (RP2350/ESP32). RS-Key uses `OtpLock = 0x1B` instead.
    Secure = 0x1D,

    /// Read hardware configuration from flash memory.
@@ -241,7 +244,8 @@ pub const P2_UNUSED: u8 = 0x00;
 /// commands to access hardware configuration.
 ///
 /// PHY configuration is shared between pico-fido and RS-Key, with RS-Key adding
-/// additional tags like `LedOrder` for RGB LED support.
+/// additional tags like `LedOrder` for RGB LED support and `LedNum` for
+/// multi-LED count.
 ///
 /// References:
 /// - [pico-fido](https://github.com/polhenarejos/pico-fido) `src/fs/phy.h`
@@ -308,6 +312,13 @@ pub enum PhyTag {
    /// Data format: `[ORDER]` (1 byte).
    /// RS-Key specific tag for configuring LED color channel order.
    LedOrder = 0x0D,
+
+    /// Number of LEDs on the device (RS-Key extension).
+    ///
+    /// Data format: `[COUNT]` (1 byte).
+    /// RS-Key specific tag specifying how many individual LEDs
+    /// are present (e.g., 1 for single, 3 for RGB).
+    LedNum = 0x0E,
 }

 impl PhyTag {
@@ -327,6 +338,7 @@ impl PhyTag {
            0x0B => Some(Self::EnabledUsbItf),
            0x0C => Some(Self::LedDriver),
            0x0D => Some(Self::LedOrder),
+            0x0E => Some(Self::LedNum),
            _ => None,
        }
    }
@@ -344,6 +356,13 @@ impl PhyTag {
 /// - [RS-Key](https://github.com/TheMaxMur/RS-Key) `crates/rsk-rescue/src/phy.rs`
 bitflags::bitflags! {
    pub struct RescueOptions: u16 {
+        /// Windows Compatible ID (WCID) support.
+        ///
+        /// When set, the device advertises WCID descriptors for
+        /// automatic driver installation on Windows without
+        /// requiring a custom .inf file.
+        const WCID = 0x01;
+
        /// LED supports dimming (PWM control).
        ///
        /// When set, the LED brightness can be adjusted. When clear,
@@ -418,6 +418,11 @@ pub fn read_device_details() -> Result<FullDeviceStatus, PFError> {
                        config.led_order = Some(val[0]);
                    }
                }
+                PhyTag::LedNum => {
+                    if !val.is_empty() {
+                        config.led_num = Some(val[0]);
+                    }
+                }
                PhyTag::EnabledUsbItf => {
                    if !val.is_empty() {
                        config.enabled_usb_itf = Some(val[0]);
@@ -633,6 +638,7 @@ pub fn write_config(config: AppConfigInput) -> Result<String, PFError> {
 ///
 /// # Errors
 /// - `PFError::Device` if the APDU fails or returns a non-success status
+#[allow(dead_code)]
 pub fn reboot_device(to_bootsel: bool) -> Result<String, PFError> {
    let (card, _, _) = connect_and_select()?;

@@ -681,6 +687,7 @@ pub fn reboot_device(to_bootsel: bool) -> Result<String, PFError> {
 /// # Warning
 /// This function is unstable and may change. Locking secure boot can permanently
 /// prevent firmware downgrades. Use with caution.
+#[allow(dead_code)]
 pub fn enable_secure_boot(lock: bool) -> Result<String, PFError> {
    let (card, _, _) = connect_and_select()?;

@@ -0,0 +1,89 @@
+#![allow(dead_code)]
+use std::fmt;
+
+use crate::error::PFError;
+use crate::hal::fido::hid::HidTransport;
+use crate::hal::types::FirmwareType;
+
+pub enum DeviceHandle {
+    Fido(HidTransport),
+    Rescue(pcsc::Card, FirmwareType),
+}
+
+impl fmt::Debug for DeviceHandle {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::Fido(t) => f.debug_tuple("Fido").field(t).finish(),
+            Self::Rescue(_, ft) => f.debug_tuple("Rescue").field(ft).finish(),
+        }
+    }
+}
+
+#[derive(Debug)]
+pub struct DeviceIdentity {
+    pub vid: u16,
+    pub pid: u16,
+    pub product_name: String,
+    pub firmware_type: FirmwareType,
+}
+
+impl DeviceHandle {
+    pub fn discover() -> Result<(Self, DeviceIdentity), PFError> {
+        match Self::try_fido() {
+            Ok(Some((handle, identity))) => {
+                log::info!("Device discovered via FIDO HID transport");
+                return Ok((handle, identity));
+            }
+            Ok(None) => log::info!("No FIDO HID device found"),
+            Err(e) => log::warn!("FIDO HID discovery error: {}", e),
+        }
+
+        match Self::try_rescue() {
+            Ok(Some((handle, identity))) => {
+                log::info!("Device discovered via Rescue PC/SC transport");
+                return Ok((handle, identity));
+            }
+            Ok(None) => log::info!("No Rescue PC/SC device found"),
+            Err(e) => log::warn!("Rescue PC/SC discovery error: {}", e),
+        }
+
+        Err(PFError::NoDevice)
+    }
+
+    fn try_fido() -> Result<Option<(Self, DeviceIdentity)>, PFError> {
+        let transport = HidTransport::open()?;
+        let identity = DeviceIdentity {
+            vid: transport.vid,
+            pid: transport.pid,
+            product_name: transport.product_name.clone(),
+            firmware_type: FirmwareType::Unknown,
+        };
+        Ok(Some((Self::Fido(transport), identity)))
+    }
+
+    fn try_rescue() -> Result<Option<(Self, DeviceIdentity)>, PFError> {
+        let ctx = pcsc::Context::establish(pcsc::Scope::User).map_err(PFError::Pcsc)?;
+        let mut readers_buf = [0; 2048];
+        let mut readers = ctx.list_readers(&mut readers_buf).map_err(PFError::Pcsc)?;
+        let reader = match readers.next() {
+            Some(r) => r,
+            None => return Ok(None),
+        };
+        let reader_name = reader.to_string_lossy();
+        let fw_type = if reader_name.contains("RS-Key") || reader_name.contains("RSK") {
+            FirmwareType::RSKey
+        } else {
+            FirmwareType::Unknown
+        };
+        let card = ctx
+            .connect(reader, pcsc::ShareMode::Shared, pcsc::Protocols::ANY)
+            .map_err(PFError::Pcsc)?;
+        let identity = DeviceIdentity {
+            vid: 0,
+            pid: 0,
+            product_name: reader_name.to_string(),
+            firmware_type: fw_type.clone(),
+        };
+        Ok(Some((Self::Rescue(card, fw_type), identity)))
+    }
+}
@@ -49,6 +49,8 @@ pub struct AppConfig {
    pub led_order: Option<u8>,
    #[serde(skip_serializing_if = "Option::is_none")]
    pub enabled_usb_itf: Option<u8>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub led_num: Option<u8>,
 }

 /// Partial config update; `None` fields are left unchanged on the device.
@@ -69,6 +71,7 @@ pub struct AppConfigInput {
    pub raw_curves_mask: Option<u32>,
    pub led_order: Option<u8>,
    pub enabled_usb_itf: Option<u8>,
+    pub led_num: Option<u8>,
 }

 /// Aggregated snapshot of device info, config, and security state.
@@ -677,6 +677,7 @@ impl ConfigViewModel {
            raw_curves_mask,
            led_order,
            enabled_usb_itf: final_enabled_usb_itf,
+            led_num: None,
        };

        if method == DeviceMethod::Fido {