Detached reader macOS #2418
Detached reader macOS #2418
Conversation
In opensc.h the documentation states that the function detect_card_presence should return 0 if no card is present, which is reasonable. This was not the case before since more flags could be set even if SC_READER_CARD_PRESENT is not. This was probably missed before since more flags has been added since this function was first written. Signed-off-by: Frida Flodin <[email protected]>
refresh_attributes returns SC_SUCCESS if we can't detect the reader. The same should happen if the reader is unknown/detached. This was found when detaching reader on macOS. Then we don't get SCARD_E_UNKNOWN_READER from SCardGetStatusChange. We notice it later from that the state of the reader is SCARD_STATE_UNKNOWN. This resulted in C_WaitForSlotEvent not noticing that the reader, and thus the card, was removed. Signed-off-by: Frida Flodin <[email protected]>
Looking in opensc.h the flag SC_READER_CARD_CHANGED should be set if the card was exchanged. In other words if a card is present but it is not the same card as before. It looks like SC_READER_CARD_CHANGED was misinterpreted as a flag for when the card was removed and thus that the status has changed. Signed-off-by: Frida Flodin <[email protected]>
The return value from detect_card_presence should be 0 if no card is present. Therefore the flag SC_READER_CARD_PRESENT is not allowed to be 0 if detect_card_presence has non-zero return value. Signed-off-by: Frida Flodin <[email protected]>
Jakuje
left a comment
Jakuje
left a comment
There was a problem hiding this comment.
the handling of the pcsc is hard to get right, especially with different pcsc versions and implementations. I did not manage to test all of them, but they look sensible.
To avoid regressions in the future, I would love to see some tests written for this. But unfortunately as far as I know, the vpcd does not support removing of a reader (Frank, correct me if I am wrong), so the manual testing is probably most we could do. We have some testcases described in https://github.com/OpenSC/OpenSC/wiki/Smart-Card-Release-Testing#test-cases. Would you mind writing a section with couple of points how this can be tested so we can run the tests at least before releases to avoid regressions?
|
Indeed, vpcd doesn't support removal of the reader. But it would be a nice idea to accept multiple card connections on a single port by automagically creating (and destroying) additional reader "slots"... |
Right now, there are 4 static readers created from VPCD, which sometimes causes issues if applications looks for just first reader or does not support more than X readers. Should I create an issue in https://github.com/frankmorgner/vsmartcard/issues for this or do you already have some idea how this could work? |
|
I have now added a section in the release testing for detaching and attaching reader with the card in it. I did the instructions using the ThinLinc client because that's what I used when finding this issue. It might be possible to test with another application that lists certificates in real time in a non-blocking way, but I have not investigated this. |
3 participants
This fixes an issue where a detached reader wasn't noticed when running C_WaitForSlotEvent(CKF_DONT_BLOCK) on macOS 11.6. This was found when trying the work around mentioned in issue #2415:
(We're going to see if we can work around this by doing a more complex tango with OpenSC:
C_WaitForSlotEvent(CKF_DONT_BLOCK)C_GetSlotList(NULL)C_WaitForSlotEvent(CKF_DONT_BLOCK)Originally posted by @CendioOssman in #2415 (comment))
After my changes, I have tested that the following events are observed with the work around mentioned above:
✓ Remove card
✓ Detach reader with card in
✓ Insert card
✓ Attach reader with card in
✓ Change to another card in the reader
Tested on both Fedora 34 and macOS 11.6. The last point about changing to another card did not work as expected on macOS. I have tested this before my changes and it didn't work then either.