| Summary: | Honor PKCS#11 CKA_ALWAYS_AUTHENTICATE attribute of the private objects | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Portable OpenSSH | Reporter: | Jakub Jelen <jjelen> | ||||||||
| Component: | Smartcard | Assignee: | Assigned to nobody <unassigned-bugs> | ||||||||
| Status: | CLOSED FIXED | ||||||||||
| Severity: | enhancement | CC: | ahmedsayeed1982, djm, orion | ||||||||
| Priority: | P5 | Keywords: | patch, pkcs11 | ||||||||
| Version: | 7.3p1 | ||||||||||
| Hardware: | Other | ||||||||||
| OS: | Linux | ||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 2915 | ||||||||||
| Attachments: |
|
||||||||||
Comment on attachment 2890 [details] [PATCH] Honor PKCS#11 CKA_ALWAYS_AUTHENTICATE attribute of the private objects >@@ -316,6 +359,7 @@ pkcs11_rsa_private_encrypt(int flen, const u_char *from, u_char *to, RSA *rsa, > return (-1); > } > si->logged_in = 1; >+ login_performed = 1; ... >+ } else if (!login_performed && >+ pkcs11_always_authenticate(k11->provider, si, obj) < 0) { >+ error("Failed to re-authenticate to access ALWAYS_AUTHENTICATE object"); Can't we reuse si->logged_in here and skip the extra variable? Created attachment 3033 [details] patch sharing the login code We (In reply to Damien Miller from comment #1) > Can't we reuse si->logged_in here and skip the extra variable? We would need to reset the variable after the signing if you talk only about variable sharing. It would work, but the actual always-authenticate function would not get called for the second time. It would call the original login before SignInit with non-CONTEXT_SPECIFIC_LOGIN. It would work in some of the cases, but it would not be according to the PKCS#11 specification. For example, if the PINs are different, it would fail. I don't see a way how to retain the same functionality without this variable, but feel free to propose a solution. Though after the second thought (year after), sharing the code for C_Login, which is quite the same except the login type would make sense. I do not share the pkcs11_interactive check, because we need this prompt from non-interactive ssh-agent process using askpass. Created attachment 3225 [details]
revised patch after PKCS#11 ECDSA support landed
Pity there seems no way to test this using softhsm2 Unfortunately ... but you can try that with your yubikey and with OpenSC if you load the private key in the "SIGN KEY" slot 9c [0].
Note, that after [1] being merged in OpenSC last year, the trick with only single login does not work anymore so in the proposed patch, we should drop the did_login variable, otherwise it will not work (at least with OpenSC pkcs11 module). Therefore, in the single-shot connection, the pin is asked twice, which is unfortunate, but probably closest to the PIV specification.
One note for the code style:
+ struct pkcs11_slotinfo *si;
+ CK_FUNCTION_LIST *f;
+ CK_BBOOL flag = 0;
+ CK_ATTRIBUTE attr;
+ CK_RV rv;
^-- misaligned indentation (missing space in flag, attr)
[0] https://developers.yubico.com/PIV/Introduction/Certificate_slots.html
[1] https://github.com/OpenSC/OpenSC/pull/1256
This has been committed and will be in OpenSSH 8.0 closing resolved bugs as of 8.6p1 release [spam removed] |
Created attachment 2890 [details] [PATCH] Honor PKCS#11 CKA_ALWAYS_AUTHENTICATE attribute of the private objects We don't need to care about always_authenticate attribute in case of simple ssh connections, because the private key operation is performed only once (immediately after login). But this is a problem in ssh-agent which can authenticate more connections. This patch introduces the additional login (the pin is requested using SSH_ASKPASS if defined) if this attribute is not CK_FALSE.