latchset / pkcs11-provider

A pkcs#11 provider for OpenSSL 3.0+
Other
65 stars 39 forks source link

Provider requests PIN to access unprotected/readable objects on tokens #188

Closed mouse07410 closed 1 year ago

mouse07410 commented 1 year ago

Describe the bug Provider requires a login to access anything on token.

All the other software, open source (OpenSC, libp11, ShowToken.app) and commercial (Smart Card Utility.app) require login only to access or perform operation on private keys and such.

To Reproduce Steps to reproduce the behavior:

  1. Install and configure pkcs11-provider
  2. Insert a physical PIV token (any - I use Yubikey and DoD CAC), though for this problem SoftHSM or NSS soft-token should do as well
  3. Try to, e.g., verify an existing good digital signature with a command like
    openssl dgst -verify "pkcs11:id=%02;object=SIGN%20pubkey;type=public" -sha384 -sigopt rsa_padding_mode:pss -sigopt rsa_pss_saltlen:-1 -signature /tmp/derive.291.text.sig  /tmp/derive.291.text

    correct the file names for your case.

Expected behavior Only ask for a PIN when

Operating environment (please complete the following information):

Token and application used (please complete the following information):

Additional context In general, it would be great if pkcs11-provider behavior could match that of libp11 wrt. as many operations on physical tokens as possible, ideally - all of them.

simo5 commented 1 year ago

As mentioned in #185 this is more of an application level issue.

If the application does not pass in callbacks when opening the store, no prompting will happen and pkcs11-provider will still go ahead and try to open the store w/o authentication, and return whatever key handle is can find that way.

For example using the trick of passing in an empty password work to avoid prompting.

Example with prompt:

openssl pkey -in pkcs11:object="Generated RSA Public Key" -pubin -pubout -out testrsa.pub

Example without prompt:

openssl pkey -in pkcs11:object="Generated RSA Public Key" -pubin -pubout -out testrsa.pub -passin pass:""
mouse07410 commented 1 year ago

I would still prefer that there's no prompt in such cases, but https://github.com/latchset/pkcs11-provider/issues/188#issuecomment-1424929160 gives an effective workaround, which I can implement in my scripts to side-step the issue. Thank you!

Jakuje commented 1 year ago

It looks like I got hit by the same issue now while trying to make the pkcs11 provider with libssh working again.

AFAIK passing the "empty" pin/pass is not the same as passing no pin at all. This sounds easy for one-shot applications such as openssl commandline tool, but for any more complicated applications, this requires some different solution.

For example in ssh, you are expected that you can list public keys without the prompt for PIN and you log in only when you want to provide the signature during the authentication.

I traced this back to #173 where we revamped the pin prompting, but I probably missed this detail when reviewing that one.

The problem is that this is under-specified in PKCS#11 2.4 as the CKF_LOGIN_REQUIRED means the login is required for "some operations" (discussed previously). This is solved with PKCS#11 3.0 with the profile objects (CKP_PUBLIC_CERTIFICATES_TOKEN), but we should probably try to keep some sane behavior. AFAIK the only application that requires PIN with CKF_LOGIN_REQUIRED is still NSS.

I was playing with this a bit today and it looks like (even when accessing the store through the OSSL_STORE_open() API as in https://gitlab.com/libssh/libssh-mirror/-/merge_requests/334/ without any password callbacks I am aware, the OpenSSL adds its own ones. Though, I am not sure if we will get rid of the pw callback here, if the provider will be able to ask for pin later.

manison commented 1 year ago

Couldn't the store provider use the hint of OSSL_STORE_PARAM_EXPECT when available to possibly eliminate the login attempt?

E.g. in the case of openssl dgst -verify the OpenSSL passes OSSL_STORE_INFO_PUBKEY to OSSL_STORE_expect.

Of course this will only work as long as

simo5 commented 1 year ago

Be really nice if that worked, but if I look at the OpenSSL code, OSSL_STORE_expect is called only in two places:

simo5 commented 1 year ago
  • Unfortunately as far as I can tell is is gated by an int called "expect" initialized at -1 and never set by this function, rendering the whole piece of cose that calls OSSL_STORE_expect() dead apparently...

I take this back, of course it is set via some macro, so this looks promising, odd I did not notice this earlier...

The annoyance here is that once again this is set after opening the store, rendering this proposed PR https://github.com/openssl/openssl/pull/20131 incomplete ...

simo5 commented 1 year ago

@manison thanks a ton for driving my attention to OSSL_STORE_expect(), the funny thing is I already saved the value in the store ctx ... only to then completely ignore it ... duh!

simo5 commented 1 year ago

PR #190 makes things seemingly working as wanted for me.

@mouse07410 @Jakuje can you both test respectively with your token and libssh and that PR and let me know if this works as expected now?

mouse07410 commented 1 year ago

Please merge #190 - I see great improvement with it. Thanks!

Jakuje commented 1 year ago

AFAIK this is fixed with #190. Closing.