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.

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

Improved Handling of CK_SLOT_ID values returned to C_GetSlotList #1947

Closed
wants to merge 1 commit into from

Conversation

dengert
Copy link
Member

@dengert dengert commented Feb 14, 2020

Partially Fixes #1945
Partially Fixes #1935

Firefox 72.0.2 still does not handle deleting of slots. See https://bugzilla.mozilla.org/show_bug.cgi?id=1613632

Previously the CK_SLOT_IDs where derived from the place on the
virtual_slot list. This means that if a reader was removed, the position
on the list could change. An application such as Firefox is also keeping
track of data based on the previous slot list. So the application
could get confused.

PKCS11 v2.30 code in in OpenSC to keep deleted slots on the list until the
application has see the slot once since itmiht have been deleted. But the
slot also looked like an available hotplug which might g reused before
the application ha seen it. This was fixed so it could not be reused until
the application had a chance to see it.

The CK_SLOT_ID now starts at 0, and increased by 1 each time a new slot
is created which occues when a reader is inserted. There is an upper limit
of 2^32 calls to create_slot. In practics this will never be reached but
the code has a "TODO" to add code to reuse slot IDs.

Pkcs11-tool.c has improved wording for listing slots.
On branch PKCS11-SLOTS
Changes to be committed:
modified: pkcs11-global.c
modified: sc-pkcs11.h
modified: slot.c
modified: ../tools/pkcs11-tool.c

Checklist
  • Documentation is added or updated
  • New files have a LGPL 2.1 license statement
  • PKCS#11 module is tested using FireFox
  • Windows minidriver is tested
  • macOS tokend is tested

Previously the CK_SLOT_IDs where derived from the place on the
virtual_slot list. This means that if a reader was removed, the position
on the list could change. An application such as Firefox is also keeping
track of data based on the previous slot list. So the application
could get confused.

PKCS11 v2.30 code in in OpenSC to keep deleted slots on the list until the
application has see the slot once since itmiht have been deleted. But the
slot also looked like an available hotplug which might g reused before
the application ha seen it. This was fixed so it could not be reused until
the application had a chance to see it.

The CK_SLOT_ID now starts at 0, and increased by 1 each time a new slot
is created which occues when a reader is inserted. There is an upper limit
of 2^32 calls to create_slot. In practics this will never be reached but
the code has a "TODO" to add code to reuse slot IDs.

Pkcs11-tool.c has improved wording for listing slots.
 On branch PKCS11-SLOTS
 Changes to be committed:
	modified:   pkcs11-global.c
	modified:   sc-pkcs11.h
	modified:   slot.c
	modified:   ../tools/pkcs11-tool.c
@frankmorgner
Copy link
Member

  • As far as I can see, the slot id doesn't need to be unique nor is it required to have static slot ids. Every meaningful operation is done with the session handle, not with a slot id.
  • preemptively changing OpenSC for working around Firefox's errors is not good. Not really solving the issue is even worse. We should wait for a fix (or even a response) on Firefox' side

@dengert
Copy link
Member Author

dengert commented Feb 18, 2020

I am still looking at this. I was able to get Firefox to not go into an infinite loop. It took a second cancel of the password prompt to end firefox. That was why I said partial fix.

The slot id needs to be persistent.

You can play around with current pkcs11-tool --test-hotplug -v with 2 or 3 devices and see the Slot indexs and slot_ids in hex. You can test both C_GetSlotList and if you type "x" it moves on to testing WaitForSlotEvent.

I expect Firefox will not do anything for a while. and only handle added devices.

@dengert
Copy link
Member Author

dengert commented Feb 18, 2020

Using single card and pkcs11-tool --test-hotplug -v

OpenSC 0.17.0 (default in my Ubuntu system) and current master both show:

Testing card detection using C_GetSlotList()
Please press return to continue, x to exit: 
Available slots:
Slot 0 (0x0): SCM Microsystems Inc. SCR 355 [CCID Interface] 00 00
  manufacturer:  SCM Microsystems Inc.
  hardware ver:  2.4
  firmware ver:  0.0
  flags:         token present, removable device, hardware slot
  token label        : PIV Card Holder pin (PIV_II)
  token manufacturer : piv_II
  token model        : PKCS#15 emulated
  token flags        : login required, rng, token initialized, PIN initialized
  hardware version   : 0.0
  firmware version   : 0.0
  serial num         : b8c3080027dbba7c
  pin min/max        : 4/8
Please press return to continue, x to exit:

But with 0.17.0 if I remove reader with card still in reader and hit enter it shows:

Available slots:
Slot 0 (0x0): Virtual hotplug slot
  manufacturer:  OpenSC Project
  hardware ver:  0.17
  firmware ver:  0.0
  flags:         removable device, hardware slot
Please press return to continue, x to exit: 

But Current master if I remove reader with card and hot enter it shows:

Available slots:
Please press return to continue, x to exit: 

With 0.17.0 and firefox, C_GetSlotList would show one slot, but as a hot plug slot.
With 0.20.0 and firefox , C_GetSlotList would show no slots, but firefox would ignore that there were no slots but then try and use the slot_id 0. SC_PKCS11_SLOT_FLAG_SEEN code may have changed this.

Firefox may fail in different ways in both cases when the card is removed. One causes a loop, the other does not.

@frankmorgner
Copy link
Member

I didn't find a reason why a virtual slot would make sense for itself (hotplugging is implemented by adding slots at runtime). We introduced a possible shrinking of the slot list in 7fb72cc, which also eliminates the virtual slot as placeholder for slots that the application has seen, but which aren't valid anymore (as seen in 0.17.0). My reading is that this is perfectly valid in terms of PKCS#11 and for users/applications it makes sense to remove this clutter.

If we don't find a proper workaround for all the related problems in Firefox, a partial fix doesn't make sense. Mozilla will have to change its code anyway, so why not implement the proper fixes in Firefox?

@dengert
Copy link
Member Author

dengert commented Feb 19, 2020 via email

@dengert
Copy link
Member Author

dengert commented Feb 22, 2020

OpenSSH mail list has related issue. from 2/21/2020,
Re-adding PKCS#11 key in ssh-agent produces "agent refused operation" error.

@dengert
Copy link
Member Author

dengert commented Feb 22, 2020

See #1956 for improved debugging of virtual_slots.
This PR #1947 does not resolve many of the issuer we have with with multiple tokens being inserted and removed while long running applications like Firefox are running.

@dengert dengert closed this Feb 22, 2020
@dengert
Copy link
Member Author

dengert commented Feb 22, 2020

Many of the comments in this PR may be helpful, but the PR itself is not the complete answer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants