On Sat, 2019-04-06 at 03:20 +1100, Damien Miller wrote:> On Fri, 5 Apr 2019, Jakub Jelen wrote: > > > There is also changed semantics of the ssh-keygen when listing keys > > from PKCS#11 modules. In the past, it was not needed to enter a PIN > > for > > this, but now. > > > > At least, it is not consistent with a comment in the function > > pkcs11_open_session(), which says > > > > 727 * if pin == NULL we delay login until key use > > > > Being logged in before listing keys prevents bug #2430, but as a > > side > > effect, even the ssh can not list keys before login and if the > > configuration contains a PKCS#11 module, the user is always > > prompted > > for a PIN, which is not very user friendly. > > > > I see this is a regression and the bug #2430 should get solved as > > proposed in the patches (will need some tweaks after the big > > refactoring). > > We'll take a look at this (and the other things you just reported) > after the release is done.Release is out with this regression. Is there any progress on this? The simplest thing how to reproduce is by extending the agent-pkcs11 regress testsuite with the following line, which previously worked fine, but now asks for a pin: ${SSHKEYGEN} -D ${TEST_SSH_PKCS11} Is this on a radar or should I create a new bug? I am using keys from PKCS#11 all the time and this prevents me from updating to the newer version. Regards, -- Jakub Jelen Senior Software Engineer Security Technologies Red Hat, Inc.
Jakub Jelen
2019-Apr-26 15:32 UTC
Regression regarding the PIN prompts for PKCS#11 (Was: Call for testing: OpenSSH 8.0)
On Wed, 2019-04-24 at 14:09 +0200, Jakub Jelen wrote:> On Sat, 2019-04-06 at 03:20 +1100, Damien Miller wrote: > > On Fri, 5 Apr 2019, Jakub Jelen wrote: > > > > > There is also changed semantics of the ssh-keygen when listing > > > keys > > > from PKCS#11 modules. In the past, it was not needed to enter a > > > PIN > > > for > > > this, but now. > > > > > > At least, it is not consistent with a comment in the function > > > pkcs11_open_session(), which says > > > > > > 727 * if pin == NULL we delay login until key use > > > > > > Being logged in before listing keys prevents bug #2430, but as a > > > side > > > effect, even the ssh can not list keys before login and if the > > > configuration contains a PKCS#11 module, the user is always > > > prompted > > > for a PIN, which is not very user friendly. > > > > > > I see this is a regression and the bug #2430 should get solved as > > > proposed in the patches (will need some tweaks after the big > > > refactoring). > > > > We'll take a look at this (and the other things you just reported) > > after the release is done. > > Release is out with this regression. Is there any progress on this? > The > simplest thing how to reproduce is by extending the agent-pkcs11 > regress testsuite with the following line, which previously worked > fine, but now asks for a pin: > > ${SSHKEYGEN} -D ${TEST_SSH_PKCS11} > > Is this on a radar or should I create a new bug? I am using keys from > PKCS#11 all the time and this prevents me from updating to the newer > version.Hello there, digging a bit in the git history, it looks like the regression was introduced by the commit 7a7fdca [1] authored by markus@, which is trying to fix a crash introduced by 41923ce [2]. That looks like also my fault that I preliminary approved this change probably without proper testing. Certainly the [2] is wrong -- there needs to be a way to process session_open function without calling to the C_Login and CKF_LOGIN_REQUIRED should not stay in the way (see the comments in the bug #2652). Actually I think both of the commits should get reverted since they are not addressing any problem, but just breaking the default use cases. The underlying problem of the bug #2652 is bug #2430 (still not addressed even though several patches were proposed). The attached patch is basically the revert that I am going to carry downstream to have the PKCS#11 working and I recommend to fix this also in openssh upstream before other people will start using this and complaining. I would be also happy to help with solving the underlying problem since there are indeed other users interested in that per the bug reports. [1] https://github.com/openssh/openssh-portable/commit/7a7fdca [2] https://github.com/openssh/openssh-portable/commit/41923ce [3] http://docs.oasis-open.org/pkcs11/pkcs11-base/v2.40/os/pkcs11-base-v2.40-os.html Regards, -- Jakub Jelen Senior Software Engineer Security Technologies Red Hat, Inc. -------------- next part -------------- A non-text attachment was scrubbed... Name: openssh_revert.patch Type: text/x-patch Size: 2136 bytes Desc: not available URL: <http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20190426/dfdb1336/attachment-0001.bin>
Damien Miller
2019-May-10 03:19 UTC
Regression regarding the PIN prompts for PKCS#11 (Was: Call for testing: OpenSSH 8.0)
Hi, Could you please create a new bug for this? The context is pretty hard to follow and I'd like something that captures it all so I can show it to some people who know more about PKCS#11 than I do. -d On Fri, 26 Apr 2019, Jakub Jelen wrote:> On Wed, 2019-04-24 at 14:09 +0200, Jakub Jelen wrote: > > On Sat, 2019-04-06 at 03:20 +1100, Damien Miller wrote: > > > On Fri, 5 Apr 2019, Jakub Jelen wrote: > > > > > > > There is also changed semantics of the ssh-keygen when listing > > > > keys > > > > from PKCS#11 modules. In the past, it was not needed to enter a > > > > PIN > > > > for > > > > this, but now. > > > > > > > > At least, it is not consistent with a comment in the function > > > > pkcs11_open_session(), which says > > > > > > > > 727 * if pin == NULL we delay login until key use > > > > > > > > Being logged in before listing keys prevents bug #2430, but as a > > > > side > > > > effect, even the ssh can not list keys before login and if the > > > > configuration contains a PKCS#11 module, the user is always > > > > prompted > > > > for a PIN, which is not very user friendly. > > > > > > > > I see this is a regression and the bug #2430 should get solved as > > > > proposed in the patches (will need some tweaks after the big > > > > refactoring). > > > > > > We'll take a look at this (and the other things you just reported) > > > after the release is done. > > > > Release is out with this regression. Is there any progress on this? > > The > > simplest thing how to reproduce is by extending the agent-pkcs11 > > regress testsuite with the following line, which previously worked > > fine, but now asks for a pin: > > > > ${SSHKEYGEN} -D ${TEST_SSH_PKCS11} > > > > Is this on a radar or should I create a new bug? I am using keys from > > PKCS#11 all the time and this prevents me from updating to the newer > > version. > > Hello there, > digging a bit in the git history, it looks like the regression was > introduced by the commit 7a7fdca [1] authored by markus@, which is > trying to fix a crash introduced by 41923ce [2]. That looks like also > my fault that I preliminary approved this change probably without > proper testing. Certainly the [2] is wrong -- there needs to be a way > to process session_open function without calling to the C_Login and > CKF_LOGIN_REQUIRED should not stay in the way (see the comments in the > bug #2652). > > Actually I think both of the commits should get reverted since they are > not addressing any problem, but just breaking the default use cases. > The underlying problem of the bug #2652 is bug #2430 (still not > addressed even though several patches were proposed). > > The attached patch is basically the revert that I am going to carry > downstream to have the PKCS#11 working and I recommend to fix this also > in openssh upstream before other people will start using this and > complaining. I would be also happy to help with solving the underlying > problem since there are indeed other users interested in that per the > bug reports. > > [1] https://github.com/openssh/openssh-portable/commit/7a7fdca > [2] https://github.com/openssh/openssh-portable/commit/41923ce > [3] > http://docs.oasis-open.org/pkcs11/pkcs11-base/v2.40/os/pkcs11-base-v2.40-os.html > > Regards, > -- > Jakub Jelen > Senior Software Engineer > Security Technologies > Red Hat, Inc. >