-
-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
USB resets leave the reader connection in a broken state #154
Comments
Can you indicate in the log when the computer goes to sleep please. I also see a "USB Device removed" event. Have you removed the token while the computer was awake? |
No problem, I'll have to capture another log.
Yes, as I said in the original post, I unplug and replug to key ("cycle") to show that this fixes it. I can leave that part out in the new log. |
I've added a second, annotated log to the same gist link, scroll down to find it. |
The first command after the resume fails. The reader returns the error 0xFC "Overrun error"
My problem is that I do not see any notification that the USB device has been removed and inserted again. To be sure you can run the command
And compare with the output of
Do you have differences? Do you also have the problem when you do a sleep/wake up without removing the USB device? |
Indeed, however I do see the USB reset event in dmesg. I haven't tried, but I suspect that triggering a reset from software might have a similar effect. Maybe that would be helpful?
Yes, I see the following two lines in the log when I unplug the device:
These are the only events relating to the yubikey in the udev log. I've posted both logs to the gist in case you want to have a look.
No, in that case the device remains usable after resuming from sleep, and the ongoing transaction survives (unless I disable "always on USB" in the system firmware, which causes VBUS to be powered down during sleep, and thus has the same effect as unplugging the token). |
Hi, I think that this bug might be caused the order in which piv-go is closing the connection and transaction. I've reproduced the issue locally, and confirmed that this patch fixes it for me. @9ary could you check if it works for you? If so I'll send a PR to piv-go. I guess that the transaction should be closed before the connection. Is that correct @LudovicRousseau ? |
pcscd is looking for "add" & "remove" events, not "unbind". It is strange that the kernel does not re-enumerate the USB devices after cutting the power of the USB devices. |
Following this SE answer, I've issued a reset to the device. dmesg:
pcscd log:
So far, this is everything I can see after resuming when I've unplugged the device, however the transaction survives and I don't have to enter my PIN again (and it seems yubikey-agent is none the wiser).
I'm not entirely sure this is correct. Your patch is closing the PCSC context then the card transaction. I think what it's really fixing is that maybe we shouldn't attempt to close the transaction at all when we receive an error? Maybe closing only the context would do all the necessary cleanup and we can drop |
That might be true, but the |
I think you misunderstood me. I am pretty sure that calling
What I am unsure about is whether this will cleanly shut down the card connection. My intuition says yes, but I would still be cautious. Even then, why would closing the context without disconnecting from the card first solve our problem? MSDN is slightly different here (https://learn.microsoft.com/en-us/windows/win32/api/winscard/nf-winscard-scardreleasecontext):
I'm still trying to understand what's going on here. pcsclite may not be at fault, however it should probably at least handle LIBUSB_TRANSFER_NO_DEVICE to get rid of that kernel warning about using an interface without claiming it. |
Calling The CCID driver does not do anything with the libusb error What is very strange is that the next libusb call works fine with no error:
I would expect the kernel/libusb to also return |
https://libusb.sourceforge.io/api-1.0/libusb_caveats.html If I understand the libusb docs correctly, this might be expected behavior. The kernel considers this event a reset because the descriptor is identical (even across different tokens, because they set the serial number to 0). On top of that, there's basically no way to get notified of a bus reset from userspace. As far as I understand, a bus reset actually involves re-enumeration, but the kernel simply does not expose it the same way as unplugging and replugging the device. Yubikeys seem happy to continue working after a bus reset, and in fact it seems if they haven't lost power then the "card's" state remains intact so clients don't even notice that anything happened. I have no idea how other readers would behave. I think we have two potential bugs then:
|
Good news, I am able to reproduce the problem on my side. But I am not yet sure how to handle the issue. Maybe the good answer is to report a card reset. This is, in fact, what happens when the computer shuts down the power of the USB reader/token. You can use reset_card.py to generate a card reset and see what happens for your application. |
If I run this while yubikey-agent is holding the card connection, I get a sharing violation error.
If I make yk-agent release the card first, then run that script, it works fine:
After that, attempting to use the agent again results in another pin prompt and things work fine. For the record, although I believe it's shown in the logs: before any operation, yubikey-agent tries to get the attestation certificate from the card; if that fails, it disconnects, releases the context, and tries to connect again from scratch. For some reason, it's this last part that fails. At this point, other clients are also unable to connect until I kill or restart yubikey-agent. |
Thanks for the comment. Another idea is to report a card removal/insertion. It is always possible to remove a card, even in the middle of an exclusive transaction. This behaviour can be done inside libccid itself without modifying pcsc-lite, so this is a good point. But maybe that a yubikey application will be surprised to see a "card" removal event for a USB token if the token itself has not been removed. |
For what it's worth, the application doesn't listen for hotplug events. It simply enumerates readers anytime it wants to connect. I've been considering writing my own replacement for it which would use hotplug events and support multiple cards. In that case, a removal and insertion event would be useful in order to re-enumerate the certificates on the card. I think properly detecting the resets is going to be difficult though, as far as I can tell there's no way to do that from userspace unless there's an open connection to the card. |
@9ary I have a question.
In my case on a Lenovo laptop I have "always-on USB" enabled but I can see that the smart card reader is powered off (and also the smart card of course) on sleep. |
Yes indeed, my laptop is a Lenovo as well, and I am seeing the same thing:
What I don't get is that some/all state is preserved in my case, are you sure this isn't just USB suspend? Maybe the reader you're testing with powers off the card in that state. |
Ah yes of course, thank you.
I was thinking that maybe some invalid state inside the context is unaware of the card reset caused by unplugging the card, which has severed the connection to the card without a call to
Okay, thanks for confirming. And yes, I see this with my patch. In that case, the fact that my patch works at all would seem to indicate that the bug can be triggered by calling |
I think the missing piece is attempting to use the card even though the connection wasn't initialized. If you send sighup to yk-agent immediately after wakeup, then things work. |
I think I got to the bottom of this problem. I did a google search for "Linux unplug usb during suspend" and this was the first result: https://www.kernel.org/doc/html/v4.13/driver-api/usb/persist.html.
Well, that explains a lot. Sure enough:
After setting this to 0, I can now see a new device in dmesg after wakeup, and yubikey-agent simply prompts me for my pin again, as if I had unplugged the token while the system was awake. So this is the root cause. Userspace is doing nothing wrong, it's a kernel feature that's messing things up. A udev rule (or a kernel-side quirk?) should take care of this. Edit: despite what that documentation page seems to imply, USB persist has been on by default for a while. https://github.com/torvalds/linux/blob/de4664485abbc0529b1eec44d0061bbfe58a28fb/drivers/usb/core/Kconfig#L20-L33 |
I came up with the following udev rule. It matches any USB device with a smartcard class interface and disables persistence.
@LudovicRousseau if this looks good to you, I can send a PR to systemd to add this to the standard configuration. Or maybe it's better for pcsclite to ship this rule instead? That said, I find it shocking that this standards non-compliant hack is enabled by default in the kernel, and more importantly, for every device rather than just those it's trying to "fix" (i.e., mass storage devices). I wonder if changing this would be acceptable or if it'd be considered breaking userspace. |
Thanks @9ary for your research about Before I saw your comments I worked on a patch for the CCID driver: diff --git a/src/ccid_usb.c b/src/ccid_usb.c
index 92b8982..1e76aab 100644
--- a/src/ccid_usb.c
+++ b/src/ccid_usb.c
@@ -1540,6 +1540,11 @@ int InterruptRead(int reader_index, int timeout /* in ms */)
case LIBUSB_TRANSFER_TIMED_OUT:
break;
+ case LIBUSB_TRANSFER_NO_DEVICE:
+ DEBUG_COMM("Simulate card removal");
+ get_ccid_slot(reader_index)->bPowerFlags = MASK_POWERFLAGS_PUP;
+ break;
+
default:
/* if libusb_interrupt_transfer() times out we get EILSEQ or EAGAIN */
DEBUG_COMM4("InterruptRead (%d/%d): %s", You can try it to see if it solves the problem. |
I am not sure I updated the CCID udev rule file (installed in diff --git a/src/92_pcscd_ccid.rules b/src/92_pcscd_ccid.rules
index 0078017..79ca431 100644
--- a/src/92_pcscd_ccid.rules
+++ b/src/92_pcscd_ccid.rules
@@ -11,6 +11,10 @@ ENV{DEVTYPE}!="usb_device", GOTO="pcscd_ccid_rules_end"
# Kobil mIDentity
ATTRS{idVendor}=="0d46", ATTRS{idProduct}=="4081", RUN+="/usr/sbin/Kobil_mIDentity_switch"
+# disable USB persist on suspend
+# https://www.kernel.org/doc/html/v4.13/driver-api/usb/persist.html
+ENV{ID_USB_INTERFACES}==":0b0000:", TEST=="power/persist", ATTR{power/persist}="0"
+
# Keep USB autosuspend off for the C3PO LTC31 v1 SmartCard Reader
ATTR{idVendor}=="0783", ATTR{idProduct}=="0003", GOTO="pcscd_ccid_rules_end" Now I have:
But the USB reader is not disconnected/reconnected. It has the same device number (
The device number changes if, and only if, I unplug/replug the reader while the laptop is suspended. Do you have the same beaviour on your side? |
Ah good, I was looking for this. It looks like the best place to put this rule indeed.
Yes, it's the same for me. I believe this is working as intended, the kernel's USB persistence mechanism only kicks in when the device has been unplugged (in their terms, when the "power session" has been interrupted). Is your reader losing state or powering the card off during suspend? In that case, I think it might be a buggy reader. The yubikey doesn't appear to be losing state in sleep mode, the connection/transaction survives sleep cycles no problem (I don't have to re-enter my PIN). In fact, I've gone back and disabled always-on USB again, and it doesn't appear to be losing power during sleep, so I guess it was just me imagining things on that point. The problem only occurs when the token is unplugged, which is already annoying enough - unplugging it after closing the lid is a bit of a habit. |
I was looking at this again and I realized you removed the wildcards. This fails to match yubikeys because they are composite devices: in addition to the smartcard/CCID interface, they also have HID interfaces for the FIDO2 and "OTP" functions. |
FWIW, this rule fixes the problem for me:
Suspend, unplug, replug, resume now works correctly. |
@9ary do you confirm that the udev rule also fixes all your problems? The patch I proposed in #154 (comment) will not work with a yubikey. But I may use it for a reader with a real smart card. |
The rule I proposed does, the one @smlx posted should also work fine, though I haven't tested it.
That's fair enough. If I only cared about yubikeys, I probably would've matched on the VID, but I wanted to make it generic since this problem most likely affects all CCID devices. |
The Linux kernel tries to keep the USB connection during suspend/resume cycles. This is a bad idea if the reader/token is unpluged while the computer is suspended. More documentation is available at https://docs.kernel.org/driver-api/usb/persist.html If a CCID reader (including a composite one) is disconnected and reconnected while the computer is suspended then the USB device will be considered removed and connected again. PC/SC applications will have to reconnect. Closes: LudovicRousseau/PCSC#154 "USB resets leave the reader connection in a broken state #154"
The Linux kernel tries to keep the USB connection during suspend/resume cycles. This is a bad idea if the reader/token is unpluged while the computer is suspended. More documentation is available at https://docs.kernel.org/driver-api/usb/persist.html If a CCID reader (including a composite one) is disconnected and reconnected while the computer is suspended then the USB device will be considered removed and connected again. PC/SC applications will have to reconnect. Closes: LudovicRousseau/PCSC#154 "USB resets leave the reader connection in a broken state #154"
Versions
/usr/sbin/pcscd --version
Platform
Issue
Originally reported at FiloSottile/yubikey-agent#142.
I initially ran into this because I've disabled always-on USB on my laptop.
Steps to reproduce:
At this point, yubikey-agent fails to reconnect to the key. I see log messages such as
could not reach YubiKey: selecting piv applet: command failed: transmitting request: an attempt was made to end a non-existent transaction
andcould not reach YubiKey: connecting to smart card: the smart card cannot be accessed because of other connections outstanding
.Looking at https://github.com/go-piv/piv-go/blob/master/piv/pcsc_errors, these match SCARD_E_NOT_TRANSACTED and SCARD_E_SHARING_VIOLATION.
Also looks like the kernel only sees a device reset, rather than a new device. I see these messages in dmesg during wakeup:
and
I can fix it by unplugging the key and plugging it back in again, or restarting either yubikey-agent or pcscd.
Moving the key to a different USB port or waiting for the computer to be awake before reinserting it doesn't trigger the problem.
The monitor example from yubikey.rs does not see any events. I suppose pcsc_scan would yield the same result.
Log
Here's a log of me plugging the key in, using it successfully once, reproducing the broken state, trying to use the key again, then cycling it and using it successfully once again: https://gist.github.com/9ary/ec325e4370e0fd97cc7b831e9baa1fd4.
The text was updated successfully, but these errors were encountered: