Skip to content

refactor(yubikey): dynamically load libusb at runtime#348

Merged
jdx merged 6 commits intomainfrom
feat/yubikey-dynamic-libusb
Mar 13, 2026
Merged

refactor(yubikey): dynamically load libusb at runtime#348
jdx merged 6 commits intomainfrom
feat/yubikey-dynamic-libusb

Conversation

@jdx
Copy link
Copy Markdown
Owner

@jdx jdx commented Mar 10, 2026

Summary

  • Replace yubico_manager crate with a minimal reimplementation in yubikey_usb.rs that loads libusb-1.0 at runtime via libloading (dlopen)
  • Eliminates the hard link-time dependency on libusb — the binary starts normally on systems without libusb installed
  • When a user actually tries to use the YubiKey provider without libusb, they get a clear error with platform-specific install instructions

Fixes #346

Details

The yubico_manager crate depends on rusblibusb1-sys, which links libusb at build time. On macOS, this hardcodes the Homebrew libusb path into the binary, causing a dyld crash at startup for users without Homebrew libusb — even if they never use the YubiKey provider.

The new yubikey_usb module:

  • Loads libusb dynamically via libloading::Library::new(), trying platform-appropriate paths
  • Implements just enough of the libusb API (init, device enumeration, control transfers) and YubiKey OTP frame protocol (CRC16, HID feature reports, challenge-response) to support HMAC-SHA1 operations
  • Is a drop-in replacement — the yubikey.rs provider and setup code work identically

The FIDO2 provider (ctap-hid-fido2hidapi) was already fine — hidapi uses IOKit on macOS natively, no libusb needed.

Test plan

  • cargo build — no warnings
  • otool -L target/debug/fnox | grep libusb — no matches (no link-time dep)
  • cargo test — all 118 unit tests pass
  • mise run test:bats — all 605 integration tests pass
  • Manual: test YubiKey challenge-response with libusb installed
  • Manual: verify clean error message when libusb is not installed

🤖 Generated with Claude Code


Note

High Risk
Introduces a new unsafe, FFI-based libusb implementation for YubiKey HMAC-SHA1 operations and removes the previous dependency stack; bugs here could break YubiKey encryption/decryption or cause platform-specific USB/runtime failures.

Overview
Reworks the YubiKey provider to avoid a link-time libusb dependency. The PR removes the yubico_manager-based flow and replaces it with a new yubikey_usb module that dynamically loads libusb via libloading and performs the HID feature-report challenge/response directly.

This updates yubikey runtime and setup paths to call yubikey_usb::challenge_response_hmac, adds libloading to dependencies, and updates the lockfile accordingly so the binary can start even when libusb is not installed (failing only when the YubiKey provider is used).

Written by Cursor Bugbot for commit 714764d. This will update automatically on new commits. Configure here.

Replace the yubico_manager crate (which hard-links libusb via rusb →
libusb1-sys) with a minimal reimplementation that loads libusb-1.0 at
runtime using libloading. This eliminates the link-time dependency on
libusb, so the binary starts and runs normally on systems without
libusb installed. Users only need libusb when they actually use the
YubiKey provider — at which point a clear install hint is shown if
the library is missing.

Closes #346

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread src/providers/yubikey_usb.rs Outdated
Comment thread src/providers/yubikey_usb.rs
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Mar 10, 2026

Greptile Summary

This PR replaces the compile-time yubico_manager/rusb/libusb1-sys dependency chain with a minimal yubikey_usb module that loads libusb-1.0 at runtime via libloading. The result is a binary that starts cleanly on systems without libusb and surfaces a clear, platform-specific install hint only when the YubiKey provider is actually used.

Key changes:

  • yubikey_usb.rs (new): dynamically loads libusb function pointers, implements the YubiKey OTP HID frame protocol (CRC16, 7-byte chunk framing, HMAC-SHA1 challenge-response), and manages kernel driver detach/reattach on Linux.
  • yubikey.rs: drop-in update — swaps yubico_manager::hmac call for yubikey_usb::challenge_response_hmac.
  • Cargo.toml: removes yubico_manager, adds libloading = "0.8" (unsorted).
  • Cargo.lock: removes several previously-duplicated crates (bitflags 1.x, cipher 0.3, block-buffer 0.9, crypto-mac 0.11, etc.) as a welcome side-effect.

Issues addressed from previous review rounds (all fixed): CRC coverage of the command byte, wait_for timeout, libusb_claim_interface on Linux, kernel driver reattach in Drop, TOCTOU device enumeration, OTP product-ID allowlist, Windows library discovery fallback paths, and short-read error surfacing.

Remaining minor items:

  • write_packet reports "wrote -3, expected 8" when control_transfer returns a negative libusb error code — the message reads as a short-write rather than a device error.
  • DeviceHandle::drop calls release_interface unconditionally even on macOS where claim_interface returned LIBUSB_ERROR_NOT_SUPPORTED and no interface was actually claimed.
  • read_response returns r0 (the start offset of the last written chunk) rather than a true byte count — the caller discards the value, so no bug today, but the -> Result<usize> signature is a future trap.

Confidence Score: 3/5

  • Safe to merge for non-Linux users; Linux correctness of the kernel-driver lifecycle and release_interface tracking should be manually validated before shipping.
  • All critical protocol bugs from previous review rounds (CRC, timeout, Linux interface claiming, driver reattach, TOCTOU) have been addressed. The remaining issues are style/usability nits. The module contains substantial hand-rolled FFI and USB HID protocol code that has not yet been exercised on real hardware across all three supported OSes; the test plan explicitly notes manual YubiKey testing is still outstanding.
  • src/providers/yubikey_usb.rs — the DeviceHandle drop path and write_packet error handling warrant a final read before merging.

Important Files Changed

Filename Overview
src/providers/yubikey_usb.rs Core new module: loads libusb at runtime, implements YubiKey HID frame protocol. Previous review issues (CRC coverage, timeout, kernel driver management, TOCTOU, Linux claim_interface, Windows paths, drop cleanup) are all addressed. Two remaining issues: write_packet error message is misleading for negative libusb error codes, and DeviceHandle::drop calls release_interface unconditionally even when the interface was never claimed (returns silently-ignored error on macOS).
src/providers/yubikey.rs High-level YubiKey provider: drop-in replacement using new yubikey_usb module. Mutex-guarded cache prevents concurrent HID access. Clean implementation.
src/providers/mod.rs Adds pub mod yubikey_usb declaration. No functional changes to provider framework.
Cargo.toml Adds libloading = "0.8" (appended out of alphabetical order at line 90) and removes yubico_manager. Otherwise clean.
Cargo.lock Lock file correctly reflects removal of rusb/libusb1-sys/yubico_manager transitive deps and addition of libloading. Several duplicate dep versions (bitflags 1.x, cipher 0.3, etc.) are cleaned up as a side effect.

Sequence Diagram

sequenceDiagram
    participant App as yubikey.rs
    participant USB as yubikey_usb.rs
    participant Lib as libusb (dlopen)
    participant HW as YubiKey hardware

    App->>USB: challenge_response_hmac(challenge, slot)
    USB->>Lib: libloading::Library::new(libusb-1.0)
    Lib-->>USB: function pointers (LibUsb)
    USB->>Lib: libusb_init()
    USB->>Lib: libusb_get_device_list()
    Lib-->>USB: device list
    USB->>USB: filter by VENDOR_ID + OTP_PRODUCT_IDS
    USB->>Lib: libusb_open(device)
    note over USB,Lib: Linux only
    USB->>Lib: libusb_kernel_driver_active(iface 0)
    USB->>Lib: libusb_detach_kernel_driver(iface 0)
    USB->>Lib: libusb_claim_interface(iface 0)
    USB->>Lib: libusb_free_device_list()
    USB->>HW: wait_for SLOT_WRITE_FLAG==0 via control_transfer GET_REPORT
    USB->>USB: build_frame(payload, command) - CRC16 over 65 bytes
    loop 10 chunks, skip all-zero intermediate
        USB->>HW: control_transfer SET_REPORT 7-byte chunk + seq
    end
    USB->>HW: wait_for RESP_PENDING_FLAG, poll up to 15s
    loop until RESP_PENDING clear or seq wraps to 0
        USB->>HW: control_transfer GET_REPORT
    end
    USB->>HW: write_reset packet 0x8F
    USB->>USB: crc16(response 0..22) == CRC_RESIDUAL_OK
    USB-->>App: 20-byte HMAC response
    note over USB,Lib: DeviceHandle drop
    USB->>Lib: libusb_release_interface(iface 0)
    USB->>Lib: libusb_attach_kernel_driver(iface 0) if detached
    USB->>Lib: libusb_close(handle)
    USB->>Lib: libusb_exit(ctx)
Loading

Comments Outside Diff (3)

  1. src/providers/yubikey_usb.rs, line 398-401 (link)

    write_packet error message conflates libusb error codes with short-write counts

    control_transfer returns either a non-negative byte count on success or a negative libusb error code (e.g., -3 = LIBUSB_ERROR_ACCESS, -6 = LIBUSB_ERROR_BUSY) on failure. The current condition rc != 8 correctly catches both cases, but the message "wrote -3, expected 8" reads as if the device returned a 3-byte payload rather than surfacing the underlying error code. This makes it harder to diagnose access/permission issues on Linux (e.g., udev rules missing) vs. genuine HID write failures.

    Distinguishing the two cases makes the error actionable:

  2. src/providers/yubikey_usb.rs, line 496-505 (link)

    release_interface called unconditionally even when interface was never claimed

    find_and_open passes through the claim_interface call returning LIBUSB_ERROR_NOT_SUPPORTED (-12) without error, which on macOS means the IOKit HID subsystem owns the device and no interface was claimed. DeviceHandle::drop then calls release_interface on an interface that was never claimed, which libusb returns an error for (silently ignored here). While harmless in practice, it triggers a spurious kernel error log entry on some versions of macOS and will confuse future readers auditing the cleanup path.

    The standard fix is to mirror the driver_was_detached pattern with an interface_claimed field:

    struct DeviceHandle<'a> {
        lib: &'a LibUsb,
        handle: *mut LibusbDeviceHandle,
        driver_was_detached: bool,
        interface_claimed: bool,   // true when claim_interface returned 0
    }

    Then in drop:

    if self.interface_claimed {
        (self.lib.release_interface)(self.handle, 0);
    }
    if self.driver_was_detached {
        (self.lib.attach_kernel_driver)(self.handle, 0);
    }
    (self.lib.close)(self.handle);
  3. src/providers/yubikey_usb.rs, line 452-492 (link)

    read_response return value does not represent bytes read

    The function signature -> Result<usize> implies a byte count, but the returned value is r0 — the start offset of the last written chunk, not the number of response bytes in the buffer. For a 20-byte HMAC + 2-byte CRC response, r0 ends up at 21 (the position of the fourth chunk's start) rather than 22 (the bytes actually populated). The caller in challenge_response_hmac discards this value (handle.read_response(&mut response)?;), so there is no current bug, but the signature is a future trap.

    Consider returning a meaningful constant (22 for the data + CRC length) or changing the return type to Result<()> if the count is not needed.

Fix All in Claude Code

Last reviewed commit: 714764d

Comment thread src/providers/yubikey_usb.rs Outdated
Comment thread src/providers/yubikey_usb.rs
Comment thread src/providers/yubikey_usb.rs
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the YubiKey integration to remove a problematic hard dependency on libusb. Previously, the application would crash on startup if libusb was not present, even if YubiKey functionality was not used. By dynamically loading libusb at runtime, the application can now start without libusb and provides a user-friendly error message with installation guidance if YubiKey functionality is attempted without the library. This change significantly improves the application's robustness and user experience on systems where libusb might not be universally installed.

Highlights

  • Dynamic libusb Loading: Replaced the yubico_manager crate with a custom yubikey_usb module that dynamically loads libusb at runtime using libloading.
  • Eliminated Hard Dependency: Removed the hard link-time dependency on libusb, allowing the application to start normally even if libusb is not installed.
  • Improved Error Handling: When the YubiKey provider is used without libusb installed, users now receive a clear error message with platform-specific installation instructions.
  • Minimal Reimplementation: The new yubikey_usb module provides a minimal reimplementation of the necessary libusb API (init, device enumeration, control transfers) and YubiKey OTP frame protocol (CRC16, HID feature reports, challenge-response) to support HMAC-SHA1 operations.
  • Dependency Refactor: Removed yubico_manager, rusb, libusb1-sys, and several other transitive dependencies, while adding libloading and updating many other dependency version specifications in Cargo.lock.
Changelog
  • Cargo.lock
    • Removed numerous transitive dependencies associated with yubico_manager, rusb, and libusb1-sys.
    • Added libloading as a new dependency.
    • Generalized many other dependency version specifications.
  • Cargo.toml
    • Replaced the yubico_manager dependency with libloading.
  • src/providers/mod.rs
    • Added a new module declaration for yubikey_usb.
  • src/providers/yubikey.rs
    • Refactored YubiKey operations to use the new yubikey_usb module instead of yubico_manager.
  • src/providers/yubikey_usb.rs
    • Introduced a new module containing the custom implementation for dynamic libusb loading, device enumeration, control transfers, and YubiKey HMAC-SHA1 challenge-response logic.
Activity
  • The pull request summary and initial commit were generated by "Claude Code" and "Cursor Bugbot", indicating automated assistance in its creation.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the YubiKey provider to dynamically load libusb at runtime, which is a great improvement for portability by removing the hard link-time dependency. The visible changes in src/providers/yubikey.rs are positive, simplifying the code and improving error formatting. However, I've noted a subtle behavioral change in how YubiKey slots are handled; the previous implementation was more lenient with invalid slot numbers, whereas the new code passes them on directly. I've left comments suggesting that this should be handled gracefully to avoid confusing errors for the user. My primary concern is that the new yubikey_usb.rs file, which contains a new unsafe FFI implementation, was not available for review. This is the most critical and high-risk part of the change, and its correctness, security, and error handling (especially for missing libusb) are paramount. Assuming that part is thoroughly tested, this PR is a solid step forward.

Comment thread src/providers/yubikey.rs Outdated
Comment on lines +63 to +64
let hmac_result = yubikey_usb::challenge_response_hmac(&self.challenge, &device, self.slot)
.map_err(|e| FnoxError::Provider(format!("YubiKey HMAC-SHA1 challenge failed: {e}")))?;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The previous implementation coerced any slot value other than 1 to slot 2. By passing self.slot directly, this behavior is changed. If a user configures an invalid slot (e.g., 3), it will now be passed to the hardware, which might produce a cryptic error. Please ensure that yubikey_usb::challenge_response_hmac handles invalid slot numbers gracefully and provides a clear error message, or consider adding validation for self.slot at this call site to ensure it's either 1 or 2.

Comment thread src/providers/yubikey.rs Outdated
Comment on lines +134 to +138
yubikey_usb::challenge_response_hmac(&challenge, &device, slot_num).map_err(|e| {
FnoxError::Provider(format!(
"YubiKey HMAC-SHA1 challenge failed: {e}. Make sure HMAC-SHA1 is configured on slot {slot_num}.",
))
})?;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to my other comment, removing the match statement for the slot number changes behavior. The old code would treat any input other than '1' as slot 2. Now, slot_num is passed directly. If a user enters an invalid number at the prompt, it could lead to a confusing error from the underlying USB communication. It would be more user-friendly to validate slot_num immediately after the user provides it to ensure it is either 1 or 2.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread src/providers/yubikey_usb.rs
Comment thread src/providers/yubikey_usb.rs Outdated
Comment thread src/providers/yubikey_usb.rs
Comment thread src/providers/yubikey_usb.rs
- Fix CRC range to cover payload + command byte (65 bytes, not 64)
- Add 15s timeout to wait_for() to prevent infinite hangs
- Load and call libusb_kernel_driver_active/detach/claim_interface for Linux
- Filter find_yubikey() by OTP-capable product IDs, not just vendor ID
- Return error on short HID reads instead of silently discarding
- Guard read_response against buffer overflow when r0 >= 36
- Validate slot is 1 or 2 in challenge_response_hmac
- Document zero-chunk skip optimization in write_frame

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread src/providers/yubikey_usb.rs
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment thread src/providers/yubikey_usb.rs
Comment thread src/providers/yubikey_usb.rs Outdated
After detaching the Linux kernel HID driver to perform USB control
transfers, the DeviceHandle now properly releases the interface and
re-attaches the kernel driver when dropped. Without this, other apps
(browsers for WebAuthn, ykman, gpg-agent, etc.) cannot access the
YubiKey until it is physically replugged.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread src/providers/yubikey_usb.rs Outdated
Comment thread src/providers/yubikey_usb.rs
jdx and others added 2 commits March 13, 2026 14:38
…e pass

Merge find_yubikey() and open_device() into a single find_and_open()
that enumerates USB devices once, finds the first OTP-capable YubiKey,
and opens it — all within one libusb context. This removes the
redundant second enumeration and the time-of-check-time-of-use gap
where the device could be unplugged between discovery and open.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove 0x0112 (NEO CCID-only, no OTP interface) from the allowlist
- Add per-PID comments sourced from yubikey-manager (ykman)
- Add common Windows install paths for libusb-1.0.dll (LibUSB-Win32,
  libusb.info installer, vcpkg) to match the macOS fallback pattern

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jdx jdx merged commit ed7fd86 into main Mar 13, 2026
16 checks passed
@jdx jdx deleted the feat/yubikey-dynamic-libusb branch March 13, 2026 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant