Hi,
I have discovered what appears to be a small oversight on how you handle FIDO2 resets through the Reset-YubiKeyFIDO2 cmdlet:
The problem:
While the cmdlet works correctly on YubiKey models that have a serial number (YubiKey 5 Series, BIO, FIPS...) the command fails to detect YubiKey models without one (Security Key series). This is due to how your event handlers are built, as you solely rely on serial numbers to detect if the (same) YK has been removed and reinserted:
|
private void YubiKeyRemoved(object? sender, YubiKeyDeviceEventArgs e) |
|
{ |
|
if (YubiKeyModule._yubikey!.SerialNumber == e.Device.SerialNumber) |
|
{ |
|
_yubiKeyRemoved = true; |
|
} |
|
} |
|
|
|
private void YubiKeyArrived(object? sender, YubiKeyDeviceEventArgs e) |
|
{ |
|
if (YubiKeyModule._yubikey!.SerialNumber == e.Device.SerialNumber) |
|
{ |
|
_yubiKeyArrived = true; |
|
} |
|
} |
This results in the removal/reinsertion detection mechanism not working correctly, which will ultimately trigger this block:
|
case ResponseStatus.ConditionsNotSatisfied: |
|
// This should not happen anymore after the forcing of yubikey reinsertion above. |
|
throw new Exception("Failed to reset, YubiKey needs to be reinserted within 5 seconds."); |
(By the way, isn't the Exception outdated? It mentions 5 seconds instead of 10. The documentation also appears to be outdated, and slightly confusing:
https://github.com/virot/powershellYK/blob/master/Docs/Commands/Reset-YubikeyFIDO2.md#description)
Don't get me wrong, this is a clever approach to uniquely identify devices by relying on a unique metadata, however this obviously fails for devices that do not have a serial number.
Suggested fix:
We may be able to get around this problem by relying on a more robust way to uniquely verify the identity of a YK, for example by doing something akin to:
- Submit a small FIDO2-HMAC Challenge to the YK before removal
- Save the issued challenge and the YK response in their own respective variables
- Wait 10 seconds for the YK to be removed
- After the timeout, check if the YK has been reinserted (if not, handle it as usual), and reissue the same challenge we saved during phase 1 to the YK
- If the response matches what we saved during phase 2, we can safely assume this is the same YK, as the response should theoretically be the same, and unique to that YK
- If the response does not match, it means a different YK was inserted, which should make the reset fail
- If all is well, we can proceed with the actual reset as usual
While this is obviously overkill (and I haven't tested it) it is the only way I found to potentially achieve a more universal, and "bulletproof" approach.
I apologize for not submitting a PR, but I haven't cloned the repo and couldn't test/fix this myself.
Final notes:
Thank you so much for the wonderful work around this module. It is amazing!
Hi,
I have discovered what appears to be a small oversight on how you handle FIDO2 resets through the
Reset-YubiKeyFIDO2cmdlet:The problem:
While the cmdlet works correctly on YubiKey models that have a serial number (YubiKey 5 Series, BIO, FIPS...) the command fails to detect YubiKey models without one (Security Key series). This is due to how your event handlers are built, as you solely rely on serial numbers to detect if the (same) YK has been removed and reinserted:
powershellYK/Module/Cmdlets/FIDO2/ResetYubikeyFIDO2.cs
Lines 125 to 139 in 9fd1a98
This results in the removal/reinsertion detection mechanism not working correctly, which will ultimately trigger this block:
powershellYK/Module/Cmdlets/FIDO2/ResetYubikeyFIDO2.cs
Lines 111 to 113 in 9fd1a98
Don't get me wrong, this is a clever approach to uniquely identify devices by relying on a unique metadata, however this obviously fails for devices that do not have a serial number.
Suggested fix:
We may be able to get around this problem by relying on a more robust way to uniquely verify the identity of a YK, for example by doing something akin to:
While this is obviously overkill (and I haven't tested it) it is the only way I found to potentially achieve a more universal, and "bulletproof" approach.
I apologize for not submitting a PR, but I haven't cloned the repo and couldn't test/fix this myself.
Final notes:
Thank you so much for the wonderful work around this module. It is amazing!