iqlusioninc / yubikey.rs

Pure Rust YubiKey host-side driver for PIV-based RSA/ECC key storage + signing/encryption support
BSD 2-Clause "Simplified" License
218 stars 27 forks source link

Crate cannot communicate with a Yubikey 4 device #465

Closed djmoch closed 1 year ago

djmoch commented 1 year ago

See str4d/age-plugin-yubikey#84 for context.

yubikey.rs 0.5.0 fails to communicate with a Yubikey 4 device. Later versions of the crate appear to have the same issue as I've built age-plugin-yubikey with v0.7.0 of yubikey.rs as well.

What happened

$ ./age-plugin-yubikey
✨ Let's get your YubiKey set up for age! ✨

This tool can create a new age identity in a free slot of your YubiKey.
It will generate an identity file that you can use with an age client,
along with the corresponding recipient. You can also do this directly
with:
    age-plugin-yubikey --generate

If you are already using a YubiKey with age, you can select an existing
slot to recreate its corresponding identity file and recipient.

When asked below to select an option, use the up/down arrow keys to
make your choice, or press [Esc] or [q] to quit.

thread 'main' panicked at 'range end index 4 out of range for slice of length 0', /home/djmoch/.cargo/registry/src/github.com-1ecc6299db9ec823/yubikey-0.5.0/src/transaction.rs:160:9

Here's the device info:

$ ykman info
Device type: YubiKey 4
Firmware version: 4.3.3
Enabled USB interfaces: OTP, FIDO, CCID

Applications
FIDO2           Not available   
OTP             Enabled         
FIDO U2F        Enabled         
OATH            Enabled         
YubiHSM Auth    Not available   
OpenPGP         Enabled         
PIV             Enabled         

Attempts to do the same thing have worked with a Yubikey 5C NFC and a Yubikey 4C FIPS. The firmware version of the latter is 4.4.5, leading me to believe either the individual device is defective (although it works in every other context), or there's something unique about the 4.3.3 firmware.

tony-iqlusion commented 1 year ago

Well, that's definitely a bug.

If you'd like to help debug, it'd be helpful if you could add dbg!(&response); on line 158 of transaction.rs so we can see how long the serial number is.

It's possible there are serial numbers which are shorter than 4-bytes.

tony-iqlusion commented 1 year ago

@djmoch if possible, could you try the branch from #466 and see if it fixes the problem?

djmoch commented 1 year ago

This might be a wonky device, as running ykman list --serials with it returns nothing at all.

I'm not a Rust dev, but I was able to backport your change onto v0.5.0 so it would build with age-plugin-yubikey. The communication path here appears to work as I can select the Yubikey and so on.

~This crate might be reporting the device management key features incorrectly, since age-plugin-yubikey seems to think mine isn't using TDES, but I'll take that up with @str4d elsewhere to confirm.~

EDIT: I didn't read the plugin's error message closely enough. The error message the plugin was reporting was perfectly accurate. Protecting the management key resolved the issue.

tony-iqlusion commented 1 year ago

Going to consider this resolved by #466