Skip to content
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.

Sign up for GitHub

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

Jump to bottom

opensc-notify endless loops #1903

Closed
Jakuje opened this issue Jan 10, 2020 · 5 comments
Closed

opensc-notify endless loops #1903

Jakuje opened this issue Jan 10, 2020 · 5 comments
Projects
Release 0.21.0

Comments

@Jakuje
Copy link
Member

Jakuje commented Jan 10, 2020

Problem Description

While playing with the opensc-notify, I noticed it ends up in infinite loop in case I plug out my yubikey and start the daemon (first log).

Similarly, there is an issue when the daemon is started while yubikey connected. The disconnect of yubikey causes the opensc-notify deamon to exit (the SCardGetStatusChange returns SCARD_E_UNKNOWN_READER). The reader is not removed from the reader list when the removal is detected and in the next round it is failing as it does not know this reader.

Proposed Resolution

The inifinite loop should be probably solved with the following patch:

diff --git a/src/libopensc/reader-pcsc.c b/src/libopensc/reader-pcsc.c
index c2dac30c..96524656 100644
--- a/src/libopensc/reader-pcsc.c
+++ b/src/libopensc/reader-pcsc.c
@@ -1553,7 +1553,7 @@ static int pcsc_wait_for_event(sc_context_t *ctx, unsigned int event_mask, sc_re
 		}
 #ifndef __APPLE__
 	   	/* OS X 10.6.2 - 10.12.6 do not support PnP notification */
-		if (event_mask & SC_EVENT_READER_ATTACHED) {
+		if (event_mask & (SC_EVENT_READER_ATTACHED | SC_EVENT_CARD_INSERTED)) {
 			rgReaderStates[i].szReader = "\\\\?PnP?\\Notification";
 			rgReaderStates[i].dwCurrentState = SCARD_STATE_UNAWARE;
 			rgReaderStates[i].dwEventState = SCARD_STATE_UNAWARE;

but I am not sure about the other issue. I tried to put something together, but it did not work so far.

Steps to reproduce

  1. Remove any readers from the system and run opensc-notify daemon. It will end in infinite loop
  2. Connect a reader to the system start opensc-notify and then disconnect the reader. It will exit with errors.

Logs

P:1843207; T:0x140071392825600 10:42:49.915 [opensc-notify] ctx.c:720:process_config_file: Used configuration file '/etc/opensc.conf'
P:1843207; T:0x140071392825600 10:42:49.915 [opensc-notify] ctx.c:851:sc_context_create: ===================================
P:1843207; T:0x140071392825600 10:42:49.915 [opensc-notify] ctx.c:852:sc_context_create: opensc version: 0.20.0
P:1843207; T:0x140071392825600 10:42:49.915 [opensc-notify] reader-pcsc.c:858:pcsc_init: PC/SC options: connect_exclusive=0 disconnect_action=0 transaction_end_action=0 reconnect_action=0 enable_pinpad=0 enable_pace=1
P:1843207; T:0x140071392825600 10:42:49.916 [opensc-notify] reader-pcsc.c:1347:pcsc_detect_readers: called
P:1843207; T:0x140071392825600 10:42:49.916 [opensc-notify] reader-pcsc.c:1360:pcsc_detect_readers: Probing PC/SC readers
P:1843207; T:0x140071392825600 10:42:49.916 [opensc-notify] reader-pcsc.c:1411:pcsc_detect_readers: Establish PC/SC context
P:1843207; T:0x140071392825600 10:42:49.952 [opensc-notify] reader-pcsc.c:1406:pcsc_detect_readers: SCardListReaders failed: 0x8010002e
P:1843207; T:0x140071392825600 10:42:49.952 [opensc-notify] reader-pcsc.c:1515:pcsc_detect_readers: returning with: -1101 (No readers found)
P:1843207; T:0x140071392825600 10:42:49.952 [opensc-notify] ctx.c:912:sc_wait_for_event: called
P:1843207; T:0x140071392825600 10:42:49.952 [opensc-notify] reader-pcsc.c:1532:pcsc_wait_for_event: called
P:1843207; T:0x140071392825600 10:42:49.952 [opensc-notify] reader-pcsc.c:1548:pcsc_wait_for_event: Trying to watch 0 readers
P:1843207; T:0x140071392825600 10:42:49.970 [opensc-notify] reader-pcsc.c:1610:pcsc_wait_for_event: Looping...
P:1843207; T:0x140071392825600 10:42:49.970 [opensc-notify] reader-pcsc.c:1610:pcsc_wait_for_event: Looping...
P:1843207; T:0x140071392825600 10:42:49.970 [opensc-notify] reader-pcsc.c:1610:pcsc_wait_for_event: Looping...
P:1843207; T:0x140071392825600 10:42:49.970 [opensc-notify] reader-pcsc.c:1610:pcsc_wait_for_event: Looping...
P:1843207; T:0x140071392825600 10:42:49.970 [opensc-notify] reader-pcsc.c:1610:pcsc_wait_for_event: Looping...
...

Removal:

P:57288; T:0x140430039920896 11:36:52.939 [opensc-notify] reader-pcsc.c:1617:pcsc_wait_for_event: 'Yubico YubiKey OTP+FIDO+CCID 00 00' before=0x00000120 now=0x0000000E
P:57288; T:0x140430039920896 11:36:52.939 [opensc-notify] reader-pcsc.c:1639:pcsc_wait_for_event: card removed event
P:57288; T:0x140430039920896 11:36:52.939 [opensc-notify] reader-pcsc.c:1644:pcsc_wait_for_event: reader detached event
P:57288; T:0x140430039920896 11:36:52.939 [opensc-notify] reader-pcsc.c:1654:pcsc_wait_for_event: Matching event 0x0A in reader Yubico YubiKey OTP+FIDO+CCID 00 00
P:57288; T:0x140430039920896 11:36:52.940 [opensc-notify] reader-pcsc.c:1707:pcsc_wait_for_event: returning with: 0 (Success)
P:57288; T:0x140430039920896 11:36:52.940 [opensc-notify] card.c:1147:match_atr_table: ATR     : 3b:f8:13:00:00:81:31:fe:15:59:75:62:69:6b:65:79:34:d4
P:57288; T:0x140430039920896 11:36:52.940 [opensc-notify] card.c:1158:match_atr_table: ATR try : 3b:f5:00:00:02:10:80:4f:73:45:49:44
P:57288; T:0x140430039920896 11:36:52.941 [opensc-notify] card.c:1161:match_atr_table: ignored - wrong length
P:57288; T:0x140430039920896 11:36:52.941 [opensc-notify] card.c:1147:match_atr_table: ATR     : 3b:f8:13:00:00:81:31:fe:15:59:75:62:69:6b:65:79:34:d4
P:57288; T:0x140430039920896 11:36:52.941 [opensc-notify] card.c:1158:match_atr_table: ATR try : 3b:f5:00:00:02:10:80:4f:73:45:49:44
P:57288; T:0x140430039920896 11:36:52.941 [opensc-notify] card.c:1161:match_atr_table: ignored - wrong length
P:57288; T:0x140430039920896 11:36:52.942 [opensc-notify] ctx.c:912:sc_wait_for_event: called
P:57288; T:0x140430039920896 11:36:52.942 [opensc-notify] reader-pcsc.c:1532:pcsc_wait_for_event: called
P:57288; T:0x140430039920896 11:36:52.942 [opensc-notify] reader-pcsc.c:1567:pcsc_wait_for_event: re-use reader 'Yubico YubiKey OTP+FIDO+CCID 00 00'
P:57288; T:0x140430039920896 11:36:52.943 [opensc-notify] reader-pcsc.c:1567:pcsc_wait_for_event: re-use reader '\\?PnP?\Notification'
P:57288; T:0x140430039920896 11:36:52.943 [opensc-notify] reader-pcsc.c:1600:pcsc_wait_for_event: SCardGetStatusChange(1) failed: 0x80100009
P:57288; T:0x140430039920896 11:36:52.943 [opensc-notify] reader-pcsc.c:1707:pcsc_wait_for_event: returning with: -1900 (Unknown error)
P:57288; T:0x140430039920896 11:36:52.944 [opensc-notify] ctx.c:912:sc_wait_for_event: called
...
@frankmorgner
Copy link
Member

Hmm, I see the problem. Basically, the current logic is too simple to realize the functionality that's promised with pcsc_wait_for_event's interface.

pcsc_wait_for_event should run SCardGetStatusChange with all known readers (including known states) and "\\?PnP?\Notification". If the PnP notification detects a new reader, SCardGetStatusChange should be run again to detect the new readers smart card status.

Anyway, that's quite a complex change, that needs some time to implement. I'm not sure when I'll have time to do that...

@frankmorgner frankmorgner mentioned this issue Jan 11, 2020
Closed
@Jakuje
Copy link
Member Author

Jakuje commented Jan 12, 2020

No problem. I do not think it is very urgent, but I wanted to keep track of this issue here. Especially if we want to make the notify more useful. From my experience it somehow works for me in my Ubuntu, but I had hard time to get even some notifications in Fedora, especially from opensc tools for some reason.

For now, we are building opensc without notify support in Fedora (it started as it was missing some dependencies in past releases, but when I tried to enable it, we hit few issues including this one so we left it disabled).

Other nice option would be some possibility to make a decision about notifications at run time either through configuration or simply by presence of the notification binary, where the notifications would always go through the separate binary (possibly configured through some environment variable?). The use case would be to have the notification support split in separate subpackage, which would depend on the gio/gnome/desktop things, while the opensc itself would be headless without notification dependencies.

@frankmorgner
Copy link
Member

Improving the notifications in general would be a seperate issue. For one, I think that the runtime/building requirements are already quite minimal; it just needs libgio (which just implements IPC with dbus...). Anyway, there's sure room for improvement.

frankmorgner added a commit to frankmorgner/OpenSC that referenced this issue Jan 13, 2020
@frankmorgner
reader-pcsc: fixed handling of detatching events
4ff4b84 
- allows re-attatching a reader to an existing reader object by
resetting the SC_READER_REMOVED flag
- readers that are flagged with SC_READER_REMOVED are not used for
SCardGetStatusChange to avoid SCARD_E_UNKNOWN_READER

fixes OpenSC#1903
@frankmorgner
Copy link
Member

I've drafted a change here, which works in PKCS#11 and Linux. However, macOS is still buggy and Windows is untested...

@Jakuje
Copy link
Member Author

Jakuje commented Jan 16, 2020

From my fast re-read, it looks good. It would be great if you could create a PR and somebody with Mac/Windows could test it.

@frankmorgner frankmorgner added this to In progress in Release 0.21.0 Jan 26, 2020
frankmorgner added a commit to frankmorgner/OpenSC that referenced this issue Mar 3, 2020
@frankmorgner
reader-pcsc: fixed handling of detatching events
61a2c6a 
- allows re-attatching a reader to an existing reader object by
resetting the SC_READER_REMOVED flag
- readers that are flagged with SC_READER_REMOVED are not used for
SCardGetStatusChange to avoid SCARD_E_UNKNOWN_READER

fixes OpenSC#1903
Release 0.21.0 automation moved this from In progress to Done Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
No open projects
Release 0.21.0
  
Done
Milestone
No milestone
Development

No branches or pull requests
2 participants
@frankmorgner @Jakuje