-
Notifications
You must be signed in to change notification settings - Fork 711
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
Fix for OpenPGP applet selection #1232
Conversation
I did some basic testing with |
@frankmorgner I don't see any code in OpenSC OpenPGP driver to recover from the wrong AID being selected between transactions. (Please correct me if I am wrong.) Its not easy to test but it is one of things we should support. Basically as part of any begin transaction on a card with multiple AIDs the first command should be to reselect the AID. The PIV driver tries to do this partly as it can run using disconnect = leave. See #1230 (comment) The openPGP driver needs to do something like this too. Same goes for any multiple applet card, they must expect that the active AID may have been changed since they ended a transaction. |
partially implements OpenSC#1215 Refactored OpenPGP code so that future versions of the card will be accessed using the logic for OpenPGP V2. We hope that backward compatibility of the standard will keep the new versions functional.
I refactored the changes so that future releases of OpenPGP card should also work (with V2 commands). |
Yes, the driver does not protect against resetting the card or deselecting the applet (and so do some other drivers in OpenSC).
I disagree, this problem is not a priority for me. If you're using only OpenSC the worst that can happen is that the applet gets selected twice. The same holds for scdaemon if it uses the card non-exclusively (in exclusive use the card would not be available for OpenSC in the first place). If you're using a different Middlware or program, then, indeed, it can get complicated. Do you some other middleware that people are using together with OpenSC? I don't; and until today, we didn't get any issue reports about such a situation. We may eventually do something against resetting the card, but deselecting cannot be easily detected nor can it be easily prevented. Hence, we would have a gigantic workload for a problem that's virtually non-existent (and I don't even mention the possibility of introducing new bugs with the new complexity). Anyway, if you think that more discussion is required, let's put it into #962. |
Here is a first cut to make sure the correct AID is selected even if some other driver has selected a different AID on the same card. It uses the the Thus the first command after a sBeginTransaction is a SELECT_AID. Only minimal testing was done with a Yubikey 4 with a PIV and OpenPGP applets but no data in either. I note that card-openpgp.c does not call sc_lock itself. Thus some commands that could all be done as part of a transaction are treated as individual transaction. I would really like to see a opensc-debug.log of a signature operation. I also note that openpgp does not do any special processing of a SC_PIN_CMD_GET_INFO. If it did the logged_in state could have been reset in This patch is built on top of this PR, and requires at least commit 005cd48 in this PR. |
Sorry, forgot the patch: |
@dengert please open a seperate PR for your work in progress. |
@frankmorgner Opened new PR, based on your branch. If you commit your branch or at lease 59963e0 @Jakuje @mouse07410 |
@dengert I'd be happy to test it, as now I got a good opportunity for that. Could you please clarify what exactly I should try:
|
@mouse07410 You better ask @frankmorgner. My PR #1232 was to Frank's pgp33 branch because I needed at least 59963e0 Frank has since added more commits to that branch, but not mine,. Frank wrote #1243 which appears to replace my #1232, and also modified many other card drivers to handle the reset issues. I consider that a good thing. But that still leaves cards with multiple card applications need to SELECT_AID at the start of every sCardBeginTranaaction to make sure the correct AID is selected. @frankmorgner and I disagree on the overhead. I think it is small, as a card driver should batch multiple APDUs under a single transaction. Which I dont think is very much overhead. So I am holding off on the SELECT_AID at the start of every sCardBeginTranaaction until @frankmorgner commits his current changes to master. |
I'm confident with both, #1232 and #1243, and think they are ready to be integrated. I'm still thinking of a more elegant way of handling the card context in case of no reset; I believe that https://github.com/OpenSC/OpenSC/files/1626835/openpgp-aid-per-sesion.txt is not quite ready. |
Yes then please merge #1232 #1243 I agree that #1239 may not be ready. But was ready for testing. Note:
then it will do the normal select file. So without some way to know for sure the AID was not changed, the best solution is to avoid interference in the first place, by doing a select_aid as the first the card command after a sCardBeginTransaction. And the place to do it is in the But changing the AID may also lose the login state, ot it may also need to be tested. The PIV specs do talk about the PIV application PIN vs the Global PIN. The Discovery object says which PIN should be used. card-piv.c honers this, but needs some needs some work use this fact in To avoid additional overhead and interference at unexpected times:
|
I wonder whether it makes sense to worry about additional overhead, considering that smart cards in general are pretty slow, so a user isn't likely to notice that overhead anyway. |
I've done preliminary testing of #1232 and #1243 (applied both) with the master, and it appears to work. I'm not sure what a good OpenSC-based test of the OpenPGP applet would be, but If you can think of a better test that you'd like me to try - please let me know. |
I believe this is as a baseline for other work should be merged. @mouse07410 Ideal would be some concurrent test -- load the pkcs11 module into Firefox, verify the functionality here, then try some signatures with the openPGP applet from pkcs11-tool and than again the Firefox (without loading and unloading). I just ran simple
|
thanks for testing, @mouse07410. I additionally verified the changes and everything looks good. |
@frankmorgner This PR is merged, and seems to be working OK - but I have one weird issue. My card has three subkeys on it: Signature, Encryption, and Authentication. OpenSC appears to recognize only two of them - Encryption and Authentication. What's happening to the first subkey, and how to make it visible/usable?
|
FYI, I have similar issue -- I generated only two subkeys if I remember well and I saw only one of them through the pkcs11, but I was accounting it to be bad knowledge of GnuPG, while I was setting that thing up some months ago. |
No, GnuPG is perfect in this respect. Here's an example:
You can correlate the key IDs used by GnuPG, and see that indeed it used the Signature subkey to sign/verify, and the Encryption subkey to encrypt/decrypt. Leaving Authentication subkey alone, as it doesn't belong in this operation. So the problem is solely with OpenSC. Perhaps I should copy this to a new issue... |
Sounds interesting, could be a problem that may also be present for OpenPGP before version 3. Please open a seperate issue to track the status (copying the comments should be enough) |
IMHO this has nothing todo with OpenPGP Card 3. As seen above the Yubikey is using version 2.1 anyway. And I see the exact same behaviour on a Nitrokey Pro with OPC 2 here. |
@alex-nitrokey please note different the commit description. Not all of them are are version 3 specific. Yes, some of them apply to all versions of OpenPGP tokens. |
Maybe I misunderstood.
This seemed to be in question, therefore I thought of pointing out, that it occurs no matter which OpenPGP Card is used... |
Checklist
opensc-tool -n