Yubico / yubico-piv-tool

Command line tool for the YubiKey PIV application
https://developers.yubico.com/yubico-piv-tool
BSD 2-Clause "Simplified" License
298 stars 99 forks source link

SSH keygen errors (unable to sign keys) #234

Closed InnovativeInventor closed 4 years ago

InnovativeInventor commented 4 years ago

I can't seem to be able to sign my SSH keys with my certs.

Exact steps to reproduce:

  1. Generated keys and certs using ykman (straight from documentation)
  2. To convert to a proper SSH format for ssh-keygen:
    ssh-keygen -i -m PKCS8 -f ca.pem
  3. Started ssh-agent
    ssh-agent -c -d -P "/*"
  4. Switched to separate window and set the ssh agent manually to sign using ssh-keygen
    ssh-add
    ssh-add -s /usr/local/lib/libykcs11.dylib
    ssh-keygen -U -s ~/certs/ca.pub -n mbp.max.fan -vvv -V +1w -I mbp.max.fan ~/.ssh/id_rsa.pub

    Using macOS.

Logs: From ssh-agent:

setenv SSH_AUTH_SOCK /var/folders/n6/3v_jtjs90rv4s2b9_l0cr4kw0000gn/T//ssh-Sv6CHEyKEjU2/agent.76140;
echo Agent pid 76140;
debug2: fd 3 setting O_NONBLOCK
debug3: fd 4 is O_NONBLOCK
debug1: process_message: socket 1 (fd=4) type 17
debug3: fd 4 is O_NONBLOCK
debug1: process_message: socket 1 (fd=4) type 20
debug1: process_add_smartcard_key: add /usr/local/Cellar/yubico-piv-tool/2.0.0/lib/libykcs11.1.dylib
debug1: pkcs11_start_helper: starting /usr/local/Cellar/openssh/8.0p1_1/libexec/ssh-pkcs11-helper -vvv
debug1: process_add
debug1: provider /usr/local/Cellar/yubico-piv-tool/2.0.0/lib/libykcs11.1.dylib: manufacturerID <Yubico (www.yubico.com)> cryptokiVersion 2.40 libraryDescription <PKCS#11 PIV Library (SP-800-73)> libraryVersion 2.0
debug1: provider /usr/local/Cellar/yubico-piv-tool/2.0.0/lib/libykcs11.1.dylib slot 0: label <YubiKey PIV #10974650> manufacturerID <Yubico (www.yubico.com)> model <YubiKey YK5> serial <10974650> flags 0x40d
debug1: have 1 keys
debug1: have 2 keys
debug1: have 3 keys
debug1: pkcs11_k11_free: parent 0x7fb9d9e09ad0 ptr 0x7fb9d9e099c0 idx 0
debug1: pkcs11_provider_unref: 0x7fb9d9c0bed0 refcount 4
debug1: pkcs11_k11_free: parent 0x7fb9d9e09da0 ptr 0x7fb9d9e091a0 idx 0
debug1: pkcs11_provider_unref: 0x7fb9d9c0bed0 refcount 4
debug1: pkcs11_k11_free: parent 0x7fb9d9e09e90 ptr 0x7fb9d9e0a070 idx 0
debug1: pkcs11_provider_unref: 0x7fb9d9c0bed0 refcount 4
debug1: pkcs11_k11_free: parent 0x7fb9d9e0a5e0 ptr 0x7fb9d9e09540 idx 0
debug1: pkcs11_provider_unref: 0x7fb9d9c0bed0 refcount 4
debug1: pkcs11_k11_free: parent 0x7fb9d9e0a7a0 ptr 0x7fb9d9e0a180 idx 0
debug1: pkcs11_provider_unref: 0x7fb9d9c0bed0 refcount 4
debug3: fd 4 is O_NONBLOCK
debug1: process_message: socket 1 (fd=4) type 11
debug1: process_message: socket 1 (fd=4) type 13
debug1: process_sign
debug1: check 0x7fb9d9e09e50 /usr/local/Cellar/yubico-piv-tool/2.0.0/lib/libykcs11.1.dylib
debug1: check 0x7fb9d9e09e70 /usr/local/Cellar/yubico-piv-tool/2.0.0/lib/libykcs11.1.dylib
debug1: pkcs11_check_obj_bool_attrib: provider 0x7fb9d9c0bed0 slot 0 object 87: attrib 514 = 1
debug1: pkcs11_get_key: always-auth key
need pin entry
login failed for always-auth key
pkcs11_get_key failed
debug1: pkcs11_k11_free: parent 0x7fb9d9e08170 ptr 0x0 idx 0
process_sign_request2: sshkey_sign: error in libcrypto

From ssh-keygen:

debug3: add_flag_option: permit-X11-forwarding
debug3: add_flag_option: permit-agent-forwarding
debug3: add_flag_option: permit-port-forwarding
debug3: add_flag_option: permit-pty
debug3: add_flag_option: permit-user-rc
Couldn't certify key /Users/max/.ssh/id_rsa.pub via agent: agent refused operation

I suspect this may be a problem with touch – not sure though. You guys certainly have more experience than me.

mouse07410 commented 4 years ago

Is your Yubikey configured to enforce touch? I believe it's off by default. Anyway, if it is on - the LED on the Yubikey touch-pad should blink slowly, and that would be your cue to touch it.

I suspect the problem is with how the software deals with the ALWAYS_AUTHENTICATE attribute. Some time ago OpenSC made a questionable decision to play police and enforce this attribute by prompting user for the PIN whenever the access to such a key is requested. The correct way is to attempt the operation, and prompt for the PIN only if the card responded with USER NOT LOGGED IN. It matters because some software (Java, for example) does not expect that and just fails if the underlying middleware wants the user to re-enter the PIN.

Why I explained this: it appears that YKCS11 follows this erroneous OpenSC behavior, and wants the PIN re-entered - which ssh-keygen clearly does not expect.

As a workaround, you can try PKCS11 library from my fork of OpenSC at https://GitHub.com/mouse07410/OpenSC.git - and use opensc-pkcs11.dylib instead of libykcs11.dylib.

qpernil commented 4 years ago

In regard to always-auth, the only thing that ykcs11 does in that regard is to return the flag from static information compiled-in into the ykcs11 library, based on the default PIN policy of the Yubikey. That means that slot 9c returns always-authenticate, the rest not, regardless of how you configure the YubiKey. You can only specify PIN policy when a key is created, and you can't do it at all through ykcs11. If you want non-default policy you must create the key using yubico-piv-tool (or write your own application). Touch policy is not visible to the software, as it is handled by the YubiKey itself. If you don't touch it within the timeout it will eventually time out, but otherwise it is transparent.

qpernil commented 4 years ago

Finally, if a key has a pin policy that means it requires PIN every time (always-auth in ykkcs11) this means the PIN must literally be verified just before performing a sign command, every time. Any command performed after that de-authenticates that PIN (including the sign operation). This behavior is per the PIV spec. According to the PKCS11 spec you are not allowed to call C_Login in the logged in state, unless you use the CONTEXT_SPECIFIC user type. With YKCS11 this user type is only allowed when you have an ongoing sign or decrypt operation (You have called C_SignInit or C_DecryptInit). Ykcs11 enforces these rules. To enable this with non-default PIN policy configured the implementation does not check that the key has the always_auth flag set. If the key doesn't actually require always auth it will still succeed (providing the PIN is correct).

qpernil commented 4 years ago

Please see this issue as well: https://github.com/Yubico/yubico-piv-tool/issues/118 From the logs it doesn't appear you are running into the bug regarding -P in the built-in ssh-agent that I have seen reports about. Or maybe you are using a different ssh-agent.

mouse07410 commented 4 years ago

In regard to always-auth, the only thing that ykcs11 does in that regard is to return the flag from static information compiled-in into the ykcs11 library, based on the default PIN policy of the Yubikey.

I disagree. Based on the evidence, it does more than just telling the caller what flags the key had set.

What's happening is that your library prompts for the PIN twice for a single operation. This breaks several applications, jarsigner and SSH among them.

Finally, if a key has a pin policy that means it requires PIN every time (always-auth in ykcs11) this means the PIN must literally be verified just before performing a sign command, every time. Any command performed after that de-authenticates that PIN (including the sign operation). This behavior is per the PIV spec.

I think you misunderstand the specs. The spec requires that there is no crypto operation performed by the card between authentication and, e.g., this sign command. And this spec is for the card, not for the middleware.

Ykcs11 enforces these rules

Precisely! It chooses to play a cop, enforces your misunderstanding of the rules, and breaks software that expects to perform only one operation per invocation - like jarsigner. That software does not expect being asked to authenticate more than once (because, as I said many times, it performs only one signature between start and exit), so it fails.

For N crypto operations with a key that has ALWAYS_AUTHENTICATE flag, one expects to enter the PIN exactly N times. You force it to be N+1 (and for no good reason). For a single operation you force entering the PIN twice in a row. As I already explained plenty of times (apparently, "repeticio est mater studiorum" isn't always true), this unnatural behavior is not only stupid user-unfriendly, but breaks existing widely-used applications, e.g., Java.

I must add that in a relatively short time YKCS11 progressed from a practically unusable piece of crap code to a fairly decent and mostly-usable library. Kudos to the maintainers. But sharp edges like this one still prevent it's deployment in production.

qpernil commented 4 years ago

Im not sure what you mean when you say the library prompts for the PIN code ? The library doesn't do anything except return the flags when requested, and possibly fail commands if the PIV application is not properly authenticated.

Anyway, re-reading the PIV spec just now I still read it as I stated. And, this is how the Yubico PIV application works : The PIV command before a GENERAL AUTHENTICATE must be a PIN verification for keys with such PIN policy (9c by default). This is enforced by the PIV application. ykcs11 doesn't do anything special in this regard, except it tries hard to avoid sending commands to the PIV application other than at C_OpenSession time, and in response to other calls that explicitly requires it to send commands. Specifically, it reads all it needs from PIV to able to perform searches without touching PIV. This was explicitly implemented to support use cases where for instance pkcs11-tool opens a second session and does some searching on it after having verified PIN on the first session, but before actually signing on the first session.

What the ykcs11 library enforces is that you are not allowed to call C_Login with the CK_USER usertype if you are in a logged in state (rw or ro). You can however perform an additional PIN verification by calling C_Login with CK_CONTEXT_SPECIFIC user type after having called C_SignInit but before calling C_Sign (or C_SignUpdate etc). That user type is only allowed in this specific case. Effectively it is a C_Login with CKU_USER that skips the check, but is only allowed between C_SignInit (or C_DecryptInit) and the following C_Sign / C_Decrypt etc. This was done to comply with the PKCS#11 spec.

So ykcs11 does not enforce double entry of PIN, at least not by itself. If you compile 2.0.0 with debug tracing, or use the code in master, which allows you to turn on debug tracing using an environment variable maybe you could see what calls are being made to ykcs11. Feel free to share with me and I will assist as best I can. Using PKCS11 Spy would achieve the same.

qpernil commented 4 years ago

I'll include the wording here from NIST.SP.800-73-4

"In other words, the PIN or OCC data must be submitted and verified every time immediately before a digital signature key operation. This ensures cardholder participation every time the private key is used for digital signature generation."

mouse07410 commented 4 years ago

Im not sure what you mean when you say the library prompts for the PIN code ?

I mean that the library gives the caller an error aka request for some action instead of proceeding with operation.

So ykcs11 does not enforce double entry of PIN, at least not by itself.

Correct - not by itself. But it makes the calling application do so or fail if the app did not expect that response, and interpreted the return code as an ordinary failure.

...maybe you could see what calls are being made to ykcs11. Feel free to share with me and I will assist as best I can. Using PKCS11 Spy would achieve the same.

Thank you. Offer to help is appreciated. Perhaps. As I said, my forks of OpenSC and libp11 already do the right thing - they let the token itself tell them whether re-auth is necessary. So, my needs are satisfied for both Java- and OpenSSL-based apps, removing urgency from this effort. I'll see what kind of a simple show-case I can cobble together to illustrate the problem in YKCS11.

qpernil commented 4 years ago

I have now tried the above steps with a debug build of ykcs11, and according to my results this would seem like a bug in ssh-agent, or ssh-keygen. Whats seems to happen is this:

// First it performs a search to find that it wants object 0x87, then:

debug: ykcs11.c:2732 (C_SignInit): In debug: ykcs11.c:2802 (C_SignInit): Out debug: ykcs11.c:1721 (C_GetAttributeValue): In debug: objects.c:519 (get_proa): For private key object 87, get debug: objects.c:710 (get_proa): ALWAYS AUTHENTICATE debug: ykcs11.c:1766 (C_GetAttributeValue): Out debug1: pkcs11_check_obj_bool_attrib: provider 0x7fef53d04af0 slot 0 object 87: attrib 514 = 1 debug1: pkcs11_get_key: always-auth key need pin entry login failed for always-auth key pkcs11_get_key failed debug1: pkcs11_k11_free: parent 0x7fef550055c0 ptr 0x0 idx 0 process_sign_request2: sshkey_sign: error in libcrypto

Here ykcs11 is compiled with debug output enabled, which means it logs the entry and exit of all functions in ykcs11. And there are none after the call to C_GetAttributeValue. The way I interpret this is that the agent gives up without calling any more functions in ykcs11 when it detects that it is an ALWAUS_AUTH=true key. It works fine if I do the same with a key that isn't ALWAYS_AUTH. Does your implementation set the ALWAYS_AUTH flag ?

Caveat: This was done using code on master, not the 2.0.0 release. But I don't think there are any changes that would impact this.

PS It seems the agent never calls C_SignFinal either, which means ykcs11 thinks the sign operation is still ongoing. So if one tries this again if fails because of that:

debug: ykcs11.c:2732 (C_SignInit): In debug: ykcs11.c:2750 (C_SignInit): Other operation in process debug: ykcs11.c:2802 (C_SignInit): Out C_SignInit failed: 144 pkcs11_get_key failed debug1: pkcs11_k11_free: parent 0x7fde0c904150 ptr 0x0 idx 0 process_sign_request2: sshkey_sign: error in libcrypto

qpernil commented 4 years ago

Additional note: Assuming CKR_USER_NOT_LOGGED_IN is the relevant return code that would trigger applications to prompt for PIN, there are two occasions we return that code: In any sign or decrypt call where the ykcs11 state is YKCS11_PUBLIC, meaning C_Login hasn't been called (or C_Logout was called after C_Login) on that session, or when we get YKPIV_AUTHENTICATION_ERROR from piv library. This happens in C_DecryptFinal or C_SignFinal if we get SW_ERR_SECURITY_STATUS (0x6982) from the PIV application on the YubiKey. That seems reasonable enough to me.

One possible problem could be if the application only expects this return code in some earlier call (C_SignInit, C_Sign, C_SignUpdate..). This would be a little problematic, as we actually avoid doing any 'policing' in the code beyond what is required by the PKCS11 spec. In particular we don't do it based on whether keys are ALWAYS_AUTH or not, because we don't currently keep track of that in the library. (We just return the flags based on default values, which might not be accurate). So we leave it to the PIV application to report this status, and that happens in the C_SignFinal and C_DecryptFinal call.

The other thing that could cause a problem is if an application uses multiple sessions and does something on a different session than the one it did the C_Login call on that causes a command to be sent to the device, which would 'de-authenticate' always-auth keys in PIV. As I've said before we have taken steps to avoid this for searches and retrieving attributes, but other calls are likely to trigger this. This would also be difficult to fix.

mouse07410 commented 4 years ago

The way I interpret this is that the agent gives up without calling any more functions in ykcs11 when it detects that it is an ALWAUS_AUTH=true key. It works fine if I do the same with a key that isn't ALWAYS_AUTH.

This is what I've been talking about all this time. For an application that knows it performs only one crypto operation they're should be no difference whether the ALWAYS_AUTH flag is set the the relevant key or not. So, such an app does not usually expect to have to re-authenticate, so it usually has no mechanisms to cope with it.

debug: ykcs11.c:1766 (C_GetAttributeValue): Out debug1: pkcs11_check_obj_bool_attrib: provider 0x7fef53d04af0 slot 0 object 87: attrib 514 = 1 debug1: pkcs11_get_key: always-auth key need pin entry login failed for always-auth key pkcs11_get_key failed

This "need pin entry" shouldn't have happened. The token cannot indicate that it wants it, because there shouldn't've been any crypto operation yet between the successful C_Login and now. And that's how/why the apps failing on your library with ALWAYS_AUTH, work correctly with "normal" keys even through your library.

Do you see now what should be fixed?

One possible problem could be if the application only expects this return code in some earlier call...

That would indeed be the likely case, based on my experience.

The other thing that could cause a problem is if an application uses multiple sessions and does something on a different session than the one it did the C_Login call on that causes a command to be sent to the device, which would 'de-authenticate' always-auth keys in PIV.

Based on my experience, unlikely.

qpernil commented 4 years ago

According to my understanding of the PKCS#11 spec the CKA_ALWAYS_AUTHENTICATE flag is not something that should dynamically change, rather it should reflect the PIN policy of that private key. It is up to the application to handle this information in the correct way, re-verifying the PIN only as needed (ykcs11 does not require re-authentication if you haven't performed other calls that cause commands to be sent to PIV. As I explained before ykcs11 ensures searching and retrieving attributes does not). The application should either keep track of the fact that it might have verified the PIN but not performed any other operations, or just simply re-verify every time. The application doesn't need to prompt the user more than one time in any case. The application could also just react to the CKR_USER_NOT_LOGGED_IN error code. It should only re-verify if the CKA_ALWAYS_AUTHENTICATE flags is true to comply with the spec, but with ykcs11 it would always work. (Reacting to error codes is what you mentioned in an earlier post)

In this particular case I think the fact that the application explicitly checks for the flag indicates that it is aware if this logic, but is somehow still failing. It doesn't make sense that it would check this just to deliberately fail if it is true. If this wasn't a bug it would probably have called C_SignFinal as well.

mouse07410 commented 4 years ago

I have a feeling that we're talking past each other.

the CKA_ALWAYS_AUTHENTICATE flag is not something that should dynamically change...

??? What's this about?!

...should reflect the PIN policy of that private key...

Of course! It's a mechanism by which

ykcs11 does not require re-authentication if you haven't performed other calls that cause commands to be sent to PIV. As I explained before ykcs11 ensures searching and retrieving attributes does not...

We might differ here on what exactly constitutes a command that triggers "re-verify" condition.

It should only re-verify if the CKA_ALWAYS_AUTHENTICATE flags is true to comply with the spec, but with ykcs11 it would always work...

Wrong, I think. That flag is for the card to know how to behave, not for the app.

The app (a PKCS#11 library in this case, because that's who has visibility into communications with the card) should only re-verify if the error code on that specific point in the process is USER NOT LOGGED IN (indicating that re-verify is required).

I don't think that's what YKCS11 does.

In this particular case I think the fact that the application explicitly checks for the flag indicates that it is aware if this logic, but is somehow still failing...

First, what makes you think that this app (ssh-keygen) explicitly checks for this flag?

Second, that's the behavior/problem that I first encountered with Java. So, SSH is by no means the only app exhibiting this behavior.

Finally, in the above trace, what commands in your opinion invalidated the PIV state and required re-auth?

qpernil commented 4 years ago

I agree we seem to be talking past each other. To clarify, I use the word application to mean the client of the PKCS#11 API, and not ykcs11 itself or ykcs11 and the application (client) taken together. In my view there are three 'actors', the client (application), ykcs11, and the PIV application on the YubiKey. It would help if you clearly state which actor you mean should be doing what.

I'll reiterate a summary of what ykcs11 is doing:

It always reports CKA_ALWAYS_AUTHENTICATE TRUE for key 9c, FALSE for all other keys. It returns CKR_USER_NOT_LOGGED_ON for two reasons (only talking of sign/decrypt here): The ykcs11 session object's state is YKCS11_PUBLIC, meaning the client has not called C_Login The PIV application returns SW_ERR_SECURITY_STATUS. This can only happen in C_SignFinal / C_DecryptFinal

ykcs11 does literally nothing differently for key 9c, besides the flag value as mentioned above. However, the client application will notice two differences : The flag (if it asks for it) and the fact that C_SignFinal / C_DecryptFinal can now return CKR_USER_NOT_LOGGED_ON even though the application has called C_Login (successfully) on that session. But this is caused by the PIV application behaving differently for key 9c (and/or other keys if so configured) and not ykcs11.

In order to move forward with this issue I would ask that you state in some more detail what you think is wrong, what behaviour the applications you have mentioned exhibits, and what ykcs11 is doing versus what you think it should be doing.

I'll admit the possibility that ykcs11 has some bug and doesn't behave as I said above, but based on the logs above I don't see what that bug would be. So please let me know if you think the above summary is in error, or if you believe it is correct but that isn't what ykcs11 actually does.

qpernil commented 4 years ago

Regarding CKA_ALWAYS_AUTHENTICATE - The logs show the application explicitly asking ykcs11 for that flag by calling C_GetAttributeValue with attribute CKA_ALWAYS_AUTHENTICATE (514) on the private key object. And I would argue that a well-written app should ask any pkcs11 implementation for that flag so that it can avoid having to try, and fail, signature calls. Of course it could learn after the first failure to always re-verify, but if it's going to do that it might as well use the flag to avoid failing even once. Of course, it should also keep track of the fact that if it starts off with a C_Login then it doesn't need to call it again until it has 'used up' that authentication.

Regarding commands that invalidate the key authentication - there are none in this case. And the reason for that is that we have gone to pains in ykcs11 to minimize de-auths by pre-reading the objects in C_OpenSession, thus allowing us to perform searches and return attributes without sending any commands to the PIV application.

Incidentally, the logic in ykcs11 also supports the somewhat troublesome scenario in PIV where you want to create and use a key in one go. This works by calling C_Login to log in as the SO (performs a management key authentication) and then doing a C_Login as CONTEXT_SPECIFIC to verify the PIN. As always, the CONTEXT_SPECIFIC C_Login must be called after C_SignInit.

Finally, you say you have seen multiple applications exhibiting this behavior - could you elaborate on what exactly you mean, in terms of what calls they make to the PKCS11 api, and what goes wrong ? Some sort of call flow diagram would be best...

mouse07410 commented 4 years ago

It always reports CKA_ALWAYS_AUTHENTICATE TRUE for key 9c, FALSE for all other keys.

This is fine.

In the following arguments, it is not clear to me when the discussed actions refer to the user app (software) that invokes YKCS11, when - to the YKCS11 middleware itself (that acts as a PKCS#11 bridge between the user app and the token), and when - to the physical token (of the PIV applet that it is running - hardware from my point of view).

It returns CKR_USER_NOT_LOGGED_ON for two reasons (only talking of sign/decrypt here):

a. The ykcs11 session object's state is YKCS11_PUBLIC, meaning the client has not called C_Login b. The PIV application returns SW_ERR_SECURITY_STATUS. This can only happen in C_SignFinal / C_DecryptFinal

Please explain the (a) case. If I understood you correctly, and YKCS11 middleware tries to keep track by itself when the C_Login command was issued (and whether it immediately preceded C_Sign or such) - this behavior is wrong.

I do not understand (b).

Regarding commands that invalidate the key authentication - there are none in this case. And the reason for that is that we have gone to pains in ykcs11 to minimize de-auths by pre-reading the objects in C_OpenSession, thus allowing us to perform searches and return attributes without sending any commands to the PIV application.

I've just tested YKCS11 with OpenSSL (via my fork of libp11), and it behaved correctly:

$ ykcs11-rsa-pss-sign-demo2 
This is not a CAC
Generating ephemeral file /tmp/derive.37486.text to test RSA-PSS signature...

openssl rand -engine rdrand -hex -out /tmp/derive.37486.text 5120
engine "rdrand" set.

Signing file /tmp/derive.37486.text...
openssl dgst -engine pkcs11 -keyform engine -sign "pkcs11:manufacturer=Yubico%20%28www.yubico.com%29;id=%02;object=Private%20key%20for%20Digital%20Signature;type=private" -sha384 -sigopt rsa_padding_mode:pss -sigopt rsa_pss_saltlen:-1 -out /tmp/derive.37486.text.sig /tmp/derive.37486.text
engine "pkcs11" set.
Enter PKCS#11 token PIN for YubiKey PIV #7444666:
Signature for /tmp/derive.37486.text is stored in /tmp/derive.37486.text.sig

Verifying signature:
openssl dgst -engine pkcs11 -keyform engine -verify "pkcs11:manufacturer=Yubico%20%28www.yubico.com%29;id=%02;object=Public%20key%20for%20Digital%20Signature;type=public" -sha384 -sigopt rsa_padding_mode:pss -sigopt rsa_pss_saltlen:-1 -signature /tmp/derive.37486.text.sig  /tmp/derive.37486.text
engine "pkcs11" set.
Verified OK

And I would argue that a well-written app should ask any pkcs11 implementation for that flag so that it can avoid having to try, and fail, signature calls

And I say - absolutely not. Let the token (the device, the PIV app running on it) be the judge here - and don't anticipate what it should do. Just react to what it is actually doing - aka, whether it requests re-auth by returning USER_NOT_LOGGED_IN or not.

Of course it could learn after the first failure to always re-verify, but if it's going to do that it might as well use the flag to avoid failing even once.

Apparently, some apps cannot "as well use the flag" - because using the flag means either keeping track of what took place during the entire session (whether any performed command should've invalidated the login state), or taking the "easy" way out. The majority that I know of (like the upstream libp11) took the "easy" way out - if the key has ALWAYS_AUTHENTICATE, then just prompt for the PIN before the operation no matter what happened before - and if user gets prompted twice for the same operation (first time to login to the token, and the 2nd time - to actually perform the signing operation), so be it. Except that, as I explained, some apps (e.g., jarsigner) simply fail in this case.

Of course, it should also keep track of the fact that if it starts off with a C_Login then it doesn't need to call it again until it has 'used up' that authentication.

The cost and the extra (unnecessary, mind you!) complexity of keeping track of the state doesn't seem worth it to me.

Simply repeating the operation works always, is fool-proof, and as simple as they get.

As always, the CONTEXT_SPECIFIC C_Login must be called after C_SignInit

No. One should call it only if the initial login has been "used up" on another crypto operation. The best way to ensure that is to attempt the operation and observe the response.

The key point here is - it's up to the token to keep track of it's state, and to inform the middleware when the user should re-authenticate by returning the appropriate error code (USER_NOT_LOGGED_IN). It makes no sense for the middleware to duplicate this functionality, trying to "foresee" when the token may want to re-auth, and proactively pushing it.

qpernil commented 4 years ago

HI again,

Case a refers to the fact that the ykcs11 middleware does keep track of whether a C_Login call has been made in order to comply with the pkcs11 specification, which does not map exactly to how PIV works. One example is that we filter private key objects based on this, so that they are only returned on a logged in session, while in PIV we can find private keys with or without PIN verification (to the extent that you can find them at all in PIV. The YubiKey has extensions that allows this though, depending on the version of the firmware. When they are not available we infer a private key for any certificate we find). The state is actually kept track of per slot, per the pkcs11 specification. However, this tracking does not take into account CKA_ALWAYS_AUTH, and once you have called C_Login on that slot the ykcs11 middleware will only return USER_NOT_LOGGED_IN if the YubiKey returns the corresponding status code.

In case b I'm just stating that the actual command to PIV is not sent until the external application calls C_SignFinal or C_DecryptFinal, and thus you won't get the error until then.

Regarding CONTEXT_SPECIFIC logins I was just emphasizing that such logins can only be made after C_SignInit has been called, as per the pkcs11 spec, which we enforce in the ykcs11 middleware.

Regarding de-auth: Yubico PIV requires PIN Verification immediately before any crypto operation on a key with such policy, which was Yubico's interpretation of the spec at the time. This behavior is dictated by the firmware, and not the ykcs11 middleware.

Bottom line is that an application is free to behave as you describe, and ykcs11 will not interfere. It does work exactly as you say it should, in fact, barring any bugs of course, and the fine details of exactly when you can get the error.

The original problem this issue was about seems to be caused by the application bailing out when it gets the CKA_ALWAYS_AUTHENTICATE TRUE. It is clear from the logs it is asking for that flag, and then it doesn't call ykcs11 any more. To me it seems that ssh-agent is aware it needs a PIN to perform the operation, but it is somehow not able to ask for one.

qpernil commented 4 years ago

After digging through the source code for ssh-agent I found the reason for this error. In short, ssh-agent doesn't support CKA_ALWAYS_AUTH keys.

This is a snippet from ssh-pkcs11.c:

if (!pkcs11_interactive) {
    error("need pin entry%s",
        (si->token.flags & CKF_PROTECTED_AUTHENTICATION_PATH) ?
        " on reader keypad" : "");
    return (-1);
}

the variable pkcs11_interactive is set in the method pkcs11_init(int interactive), which get called in ssh-agent.c:

ifdef ENABLE_PKCS11

pkcs11_init(0);

endif

The debug output from the top of this issue shows that this is what happens.

Since I believe the original issue is explained by this, and there is nothing we can do about it in ykcs11 I am closing this issue now. The suggested workaround would be to use a key other than slot 9c.

Please feel free to continue the discussion about how ykcs11 should behave by opening a new issue, if desired.

mouse07410 commented 4 years ago

FYI, I've done a few tests with jarsigner just now. To my surprise, now it works.

This issue can stay closed.