Hi, first, there is a bug in pin detection: if no pin is supplied to function, the exit condition is skipped. Also if no pin is supplied, login is skipped even when card requires login. Proposed patch is here: https://github.com/danielkucera/openssh-portable/commit/d6be677d1befd84fdbef0259316ebf4383feef6c
Created attachment 3032 [details] patch
Comment on attachment 3032 [details] patch >diff --git a/ssh-pkcs11.c b/ssh-pkcs11.c >index d1f750db0..938535638 100644 >--- a/ssh-pkcs11.c >+++ b/ssh-pkcs11.c >@@ -366,19 +366,16 @@ pkcs11_open_session(struct pkcs11_provider *p, CK_ULONG slotidx, char *pin) > > f = p->function_list; > login_required = p->slotinfo[slotidx].token.flags & CKF_LOGIN_REQUIRED; >- if (pin && login_required && !strlen(pin)) { >- error("pin required"); >- return (-1); >- } >+ I'm not sure I understand why this section is removed - could you explain it?
(In reply to Damien Miller from comment #2) > Comment on attachment 3032 [details] > patch > > >diff --git a/ssh-pkcs11.c b/ssh-pkcs11.c > >index d1f750db0..938535638 100644 > >--- a/ssh-pkcs11.c > >+++ b/ssh-pkcs11.c > >@@ -366,19 +366,16 @@ pkcs11_open_session(struct pkcs11_provider *p, CK_ULONG slotidx, char *pin) > > > > f = p->function_list; > > login_required = p->slotinfo[slotidx].token.flags & CKF_LOGIN_REQUIRED; > >- if (pin && login_required && !strlen(pin)) { > >- error("pin required"); > >- return (-1); > >- } > >+ > > I'm not sure I understand why this section is removed - could you > explain it? Because in my case, the pkcs library says it requires login but if you don't pass it as argument to C_Login, it will ask for it. Thus we should not exit with error here.
(In reply to Daniel Kucera from comment #3) > Because in my case, the pkcs library says it requires login but if > you don't pass it as argument to C_Login, it will ask for it. Thus > we should not exit with error here. * if you don't pass PIN as argument.
(In reply to Damien Miller from comment #2) > Comment on attachment 3032 [details] > patch > > >diff --git a/ssh-pkcs11.c b/ssh-pkcs11.c > >index d1f750db0..938535638 100644 > >--- a/ssh-pkcs11.c > >+++ b/ssh-pkcs11.c > >@@ -366,19 +366,16 @@ pkcs11_open_session(struct pkcs11_provider *p, CK_ULONG slotidx, char *pin) > > > > f = p->function_list; > > login_required = p->slotinfo[slotidx].token.flags & CKF_LOGIN_REQUIRED; > >- if (pin && login_required && !strlen(pin)) { > >- error("pin required"); > >- return (-1); > >- } > >+ > > I'm not sure I understand why this section is removed - could you > explain it? Oh, I remember now: It's because if pin is not set (is null), login_required is not evaluated so no error is returned so this check is useless. And we don't even need to return error here, login can be performed by external library after calling C_Login with pin set to zero. CKF_LOGIN_REQUIRED only means C_Login has to be called, not that the pin has to be set.
Created attachment 3124 [details] allow deferring the PIN prompt to reader keyboard Well ... the pkcs11_open_session() is called from pkcs11_add_provider() and that is called either from ssh, ssh-pkcs11-helper or from ssh-keygen. (1) The ssh and ssh-keygen call this function with NULL pin. The ssh asks for the PIN later. This is fine. (2) The ssh-pkcs11-provider and ssh-keygen (CA signing) call this function directly with pin as provided by user (can be zero-length string), and in the second case can be also NULL (preferred way). Given that, the first condition is certainly not useless. It makes sense to fail before opening session if we know that we can not provide a pin. There is possibility that the PIN provided by user (through ssh-agent protocol) is empty string and in that case, we do not have any way how to prompt for the PIN later. Theoretically, there is still a way to ask using askpass, but it is not implemented at this moment. But the other part is true. The interactive-login already detects the CKF_PROTECTED_AUTHENTICATION_PATH flag, that is used for logging into the token from reader keypad. I believe the same thing should be also supported in the ssh-agent process, but since the pin prompt is in different process than the actual connection to PKCS#11 library, the user just needs to submit empty PIN and it needs to be detected later in ssh-agent, but certainly not based only on the PIN value, but on the proper flags of the token. In the case of using reader keypad, the pin should be a NULL_PTR as recommended by specification [1]. Daniel, can you try the attached patch (should apply on master), if it solves your problem? [1] http://docs.oasis-open.org/pkcs11/pkcs11-base/v2.40/os/pkcs11-base-v2.40-os.html
Ahoj Jakub, I tried it but it doesn't work: $ ./ssh-keygen -D /usr/lib/eidklient/libpkcs11_sig_x64.so -e cannot read public key from pkcs11 $
Created attachment 3125 [details] patch_v2
This one I uploaded (patch_v2) works.
Thank you for testing the patch. But your changes again change the semantics and issue the pinpad login even if the PIN is NULL, which is not what you generally want. Or is your card requiring the login also for the listing of public keys? What do you get if you try to list the public objects from pkcs11-tool? pkcs11-tool -O /usr/lib/eidklient/libpkcs11_sig_x64.so
(In reply to Jakub Jelen from comment #10) > Thank you for testing the patch. But your changes again change the > semantics and issue the pinpad login even if the PIN is NULL, which > is not what you generally want. But if CKF_LOGIN_REQUIRED is set why would one want to skip login? > > Or is your card requiring the login also for the listing of public > keys? What do you get if you try to list the public objects from > pkcs11-tool? > > pkcs11-tool -O /usr/lib/eidklient/libpkcs11_sig_x64.so My card requires login for absolutely everything $ pkcs11-tool -vvv --module /usr/lib/eidklient/libpkcs11_sig_x64.so -O Using slot 0 with a present token (0x1) $ pkcs11-tool -vvv --module /usr/lib/eidklient/libpkcs11_sig_x64.so -l -O Using slot 0 with a present token (0x1) Private Key Object; RSA label: 571cd7f3-0935-4218-b7cf-4b43af29d1bc ID: ... Usage: decrypt, sign Access: always authenticate Certificate Object; type = X.509 cert label: 571cd7f3-0935-4218-b7cf-4b43af29d1bc ID: ...
(In reply to Daniel Kucera from comment #11) > (In reply to Jakub Jelen from comment #10) > > Thank you for testing the patch. But your changes again change the > > semantics and issue the pinpad login even if the PIN is NULL, which > > is not what you generally want. > > But if CKF_LOGIN_REQUIRED is set why would one want to skip login? The PKCS#11 specification does not say what can and what can not be accessed if this flag is provided: > CKF_LOGIN_REQUIRED: True if there are *some* cryptographic functions that a user MUST be logged in to perform From: http://docs.oasis-open.org/pkcs11/pkcs11-base/v2.40/os/pkcs11-base-v2.40-os.html We do not skip login for the private-key operations, but only for the listing of the keys, which is a valid use case. > > Or is your card requiring the login also for the listing of public > > keys? What do you get if you try to list the public objects from > > pkcs11-tool? > > > > pkcs11-tool -O /usr/lib/eidklient/libpkcs11_sig_x64.so > > My card requires login for absolutely everything > > $ pkcs11-tool -vvv --module /usr/lib/eidklient/libpkcs11_sig_x64.so > -O > Using slot 0 with a present token (0x1) > $ pkcs11-tool -vvv --module /usr/lib/eidklient/libpkcs11_sig_x64.so > -l -O > Using slot 0 with a present token (0x1) > Private Key Object; RSA > label: 571cd7f3-0935-4218-b7cf-4b43af29d1bc > ID: ... > Usage: decrypt, sign > Access: always authenticate > Certificate Object; type = X.509 cert > label: 571cd7f3-0935-4218-b7cf-4b43af29d1bc > ID: ... Yes, this is the same problem as described in the bug #2430 some while back, which I hit with some soft tokens and that are also visible in eID cards as I tried to point out. Prompting for the PIN for public key operations is nothing we would like to do automatically, so there really should be some switch to do the login before listing the keys or the login should be proposed explicitly by for example a PIN in PKCS#11 URI.
(In reply to Jakub Jelen from comment #12) > > Prompting for the PIN for public key operations is nothing we would > like to do automatically, so there really should be some switch to > do the login before listing the keys or the login should be proposed > explicitly by for example a PIN in PKCS#11 URI. I see two reasonable options here: either to check return of all functions for CKR_USER_NOT_LOGGED_IN return code and retry them after login or login always when CKF_LOGIN_REQUIRED is set. Moreover, not every time when you call login with NULL pin you are required to put it in. In my case the library ask for it only time to time (you can see my usecase here: https://blog.danman.eu/ssh-autentifikacia-s-eid-obcianskym-preukazom-pod-linuxom/ ) probably because it keeps the session with card open.
(In reply to Daniel Kucera from comment #13) > (In reply to Jakub Jelen from comment #12) > > Prompting for the PIN for public key operations is nothing we would > > like to do automatically, so there really should be some switch to > > do the login before listing the keys or the login should be proposed > > explicitly by for example a PIN in PKCS#11 URI. > > I see two reasonable options here: either to check return of all > functions for CKR_USER_NOT_LOGGED_IN return code and retry them > after login If you do not see any objects on the card before login, you will not get any such error so this will not resolve your problem in any way. > or login always when CKF_LOGIN_REQUIRED is set. That is not sane default behavior. With most of the cards, certificates and public keys are visible without login. For the few others, there should be configuration option to handle this case as I initially proposed in the referenced bug. > Moreover, not every time when you call login with NULL pin you are > required to put it in. In my case the library ask for it only time > to time (you can see my usecase here: > https://blog.danman.eu/ssh-autentifikacia-s-eid-obcianskym-preukazom- > pod-linuxom/ ) probably because it keeps the session with card open. From the log, it looks like CardOS V5.0 card, which should work also with the latest OpenSC. The PKCS#11 module you are using is probably somehow holding the login state of your card and presents you its own PIN pad in GUI. That is certainly not a standard behavior of PKCS#11 modules nor cards.
One more thing. Will a *ssh-agent* work for you with stock OpenSSH? To my understanding, it already does a login before listing the keys, so a workaround could be using the keys from ssh-agent: eval `ssh-agent` ssh-add -s /usr/lib/eidklient/libpkcs11_sig_x64.so ssh user@moj.server.sk
(In reply to Jakub Jelen from comment #15) > One more thing. Will a *ssh-agent* work for you with stock OpenSSH? > To my understanding, it already does a login before listing the > keys, so a workaround could be using the keys from ssh-agent: > > eval `ssh-agent` > ssh-add -s /usr/lib/eidklient/libpkcs11_sig_x64.so > ssh user@moj.server.sk $ ssh-add -s /usr/lib/eidklient/libpkcs11_sig_x64.so Enter passphrase for PKCS#11: Could not add card "/usr/lib/eidklient/libpkcs11_sig_x64.so": agent refused operation What kind of passphrase does it ask for? I tried card pin but without success.
Sorry, I forgot about the pinpad. For the reader virtual keypad, you need to use the patch that I attached to the comment #6 (applied to ssh-agent and ssh-pkcs11-provider, which complicates installation). It should be still prompting for the pin, but if you just press enter, you should get past that and should allow to read the keys, if I see right. Unfortunately, the ssh-add does not know if there is pinpad at that moment so it can not skip this prompt, but needs to send empty string in this case.
(In reply to Jakub Jelen from comment #17) > Sorry, I forgot about the pinpad. For the reader virtual keypad, you > need to use the patch that I attached to the comment #6 (applied to > ssh-agent and ssh-pkcs11-provider, which complicates installation). > > It should be still prompting for the pin, but if you just press > enter, you should get past that and should allow to read the keys, > if I see right. > > Unfortunately, the ssh-add does not know if there is pinpad at that > moment so it can not skip this prompt, but needs to send empty > string in this case. After applying patch: it doesn't work with empty string pin: $ ./ssh-add -s /usr/lib/eidklient/libpkcs11_sig_x64.so Enter passphrase for PKCS#11: Could not add card "/usr/lib/eidklient/libpkcs11_sig_x64.so": agent refused operation but it does with correct card pin: $ ./ssh-add -s /usr/lib/eidklient/libpkcs11_sig_x64.so Enter passphrase for PKCS#11: Card added: /usr/lib/eidklient/libpkcs11_sig_x64.so $ ./ssh-add -L ssh-rsa AAAAB3... /usr/lib/eidklient/libpkcs11_sig_x64.so ssh-rsa AAAAB3... /usr/lib/eidklient/libpkcs11_sig_x64.so ssh-rsa AAAAB3... /usr/lib/eidklient/libpkcs11_sig_x64.so
Maybe it still needs some care. I don't have a slovak EiD so I can not verify this use case. Anyway, can you try the patch attached in the bug #2430? It should allow you to use the keys from ssh client and ssh-keygen by trying to login if there were no public keys visible before.
(In reply to Jakub Jelen from comment #19) > Maybe it still needs some care. I don't have a slovak EiD so I can > not verify this use case. > > Anyway, can you try the patch attached in the bug #2430? It should > allow you to use the keys from ssh client and ssh-keygen by trying > to login if there were no public keys visible before. Yes, that patch works fine. First time it asks for pin using software keypad reader, next times it works without asking. Used command: ./ssh -I /usr/lib/eidklient/libpkcs11_sig_x64.so server
Created attachment 3226 [details] update patch to post-ECDSA PKCS#11 key merge This updates the patch after the PKCS#11 ECDSA code has landed. Note that this patch is now atop the one on bug 2638
The new patch looks good to me.
Looks OK to me too.
This has been committed and will be in OpenSSH 8.0
Move resolved bugs -> CLOSED after 8.0 release
[spam removed]