Created attachment 2715 [details] Patch part 1/3 Add support to load additional certificates for already loaded private keys. Useful if the private key is on a PKCS#11 hardware token. The private keys inside ssh-agent are now using a refcount to share the private parts between "Identities". The reason for this change was that the PKCS#11 code might have redirected ("wrap") the RSA functions to a hardware token. We don't want to mess with those internals. Tested with an OpenGPG card. Patch developed against 6.9p and applies to original 6.9, too. Original patch from openssh-unixdev has been split into three smaller patches for easier review. It has also been updated for version 7.1p1. (KEY_RSA_CERT_V00 / KEY_DSA_CERT_V00 was removed). Original submission: https://marc.info/?l=openssh-unix-dev&m=143792343407993&w=2
Created attachment 2716 [details] Patch part 2/3
Created attachment 2717 [details] Patch part 3/3
Is this still necessary with the support for additional certificates that landed in HEAD recently? https://anongit.mindrot.org/openssh.git/commit/?id=4e44a79a07d4b88b6a4e5e8c1bed5f58c841b1b8 The new code explicitly tries to match additional certs to known private keys.
The new code goes in the right direction. I don't think it covers the use case when you ssh into one machine and then want to use agent forwarding to ssh into the next machine? That use case is covered by this patch. Also certificate support is currently being added to the ssh-agent emulation of gnupg's 2.x gpg-agent. That would benefit from the new ssh-add command, too.
Looking at the patch, I like the idea but I don't think we need to modify ssh-agent to accommodate it. Couldn't ssh-add just graft the extra certificates to the private key and send them? This is similar to how it send implicit *-cert.pub certificates now. It might be a little more hassle for the user, since they will need to have their private keys available at the same time as their certificates, but IMO users shouldn't be able to add keys to an agent *without* presenting their private section.
I'm not sure if the "implicit send certificates" approach might be very cumbersome when using PKCS#11 tokens. How would one specify the filename for the public certs when using PKCS#11? Also: How would it pick up multiple certs for the same private key? We plan on using at least two certs for separates access privileges. btw: Thanks for your time for reviewing this.
Hi Damien, cooking this patchset a little further: (In reply to Damien Miller from comment #5) > Looking at the patch, I like the idea but I don't think we need to > modify ssh-agent to accommodate it. > > Couldn't ssh-add just graft the extra certificates to the private > key and send them? This is similar to how it send implicit > *-cert.pub certificates now. it's been a while, but I remember vaguely that if you remove a certificate again with the current upstream code, it will call sshkey_free(id->key) and this will kill the PKCS#11 provider, too. -> refcounting is needed, especially if multiple certs reference the same PKCS#11 token / private key. I could split the refcounting and the "key shadowing" into two distinct code changes if there's a chance of upstreaming the concept in general. Not sure if it's worth the effort since it almost touches the same code places. > It might be a little more hassle for the user, since they will need > to have their private keys available at the same time as their > certificates, but IMO users shouldn't be able to add keys to an > agent *without* presenting their private section. if you want to go this route, there are still two unsolved riddles here: - How would one specify the filename for the public certs when using PKCS#11? - Also: How would it pick up multiple certs for the same private key? Also agent-forwarding probably won't work, you would need to copy the certificates files to the machine you want to hop to the next machine. Cheers, Thomas
I'll post an updated patchset again current git f25ee13b3e81fd80efeb871dc150fe49d7fc8afd. (this is openssh 7.4p1+) The code is also available here (for easier review access): https://github.com/thomasjfox/openssh-portable/tree/cert-smartcard-support
Created attachment 2933 [details] Updated patchset to current openssh code
Created attachment 2934 [details] Tarball with updated patchset to current openssh code
Hi Thomas, Thank you for your work, this seems to be exactly what Im looking for. I have my keys on a PCKS#11 provider and need to use the agent to forward my certificates. I tried to add these patches to 7.6p1 but it fails: [tl2:~/openssh-7.6p1] petera$ patch < 20 2017-01-30-0001-sshkey-API-Add-new-sshkey_is_private-function.patch 2017-01-30-0002-ssh-agent-Add-support-to-load-additional-certificate.patch 2017-01-30-0003-ssh-add-Support-adding-an-additional-certificate.patch [tl2:~/openssh-7.6p1] petera$ patch < 2017-01-30-0002-ssh-agent-Add-support-to-load-additional-certificate.patch patching file ssh-agent.c Hunk #1 succeeded at 114 (offset 5 lines). Hunk #2 succeeded at 187 with fuzz 2 (offset -7 lines). Hunk #3 FAILED at 238. Hunk #4 succeeded at 243 (offset -8 lines). Hunk #5 FAILED at 289. Hunk #6 FAILED at 304. Hunk #7 FAILED at 360. Hunk #8 FAILED at 425. Hunk #9 succeeded at 332 (offset -116 lines). Hunk #10 FAILED at 693. Hunk #11 succeeded at 616 with fuzz 2 (offset -220 lines). 6 out of 11 hunks FAILED -- saving rejects to file ssh-agent.c.rej [tl2:~/openssh-7.6p1] petera$ patch < 20 2017-01-30-0001-sshkey-API-Add-new-sshkey_is_private-function.patch 2017-01-30-0002-ssh-agent-Add-support-to-load-additional-certificate.patch 2017-01-30-0003-ssh-add-Support-adding-an-additional-certificate.patch [tl2:~/openssh-7.6p1] petera$ patch < 2017-01-30-0003-ssh-add-Support-adding-an-additional-certificate.patch patching file ssh-add.1 Hunk #1 succeeded at 121 (offset -1 lines). patching file ssh-add.c Hunk #1 FAILED at 178. Hunk #2 FAILED at 440. Hunk #3 succeeded at 453 (offset -8 lines). Hunk #4 FAILED at 479. Hunk #5 FAILED at 508. Hunk #6 succeeded at 509 (offset -7 lines). Hunk #7 FAILED at 608. Hunk #8 FAILED at 617. 6 out of 8 hunks FAILED -- saving rejects to file ssh-add.c.rej
Worked fine to add the patches to 7.4 but then I get this: [tl2:~/openssh-7.4p1] petera$ ./ssh-agent -P /usr/lib64/opensc-pkcs11.so -d setenv SSH_AUTH_SOCK /tmp/ssh-hW8Tsd3WfC0h/agent.22437; echo Agent pid 22437; debug2: fd 3 setting O_NONBLOCK debug2: fd 4 setting O_NONBLOCK debug1: type 20 debug1: process_add_smartcard_key: add /usr/lib64/opensc-pkcs11.so Segmentation fault
Hi Peter, I can look into porting the patches to the newest openssh version. Right now I'm in an update release crunch period at work, so not much time for other things atm. Hopefully there is time for this either at the end of December 2017 or at the end of January 2018. Can you try to run the pkcs11 enabled ssh-agent via valgrind? That way we could get a backtrace of the crash. Actually the patches should improve the pkcs11 handling. Without the added refcounting it could happen that openssh accesses an pkcs11 provider that's already unloaded. At least with the "old" openssh 6.9 / 7.4. Cheers, Thomas
Hi Damien, I've began working on this patch set again. It's ported to openssh 7.6p1 already. What I don't like about the implementation is that it creates an "empty" private key via sshkey_add_private() in ssh-add to reuse the existing sshkey_private_serialize() infrastructure. Later on ssh-agent uses the new sshkey_is_private() "hack" to determine if it's a just cert or full private key. A cleaner approach would be this: - Add SSH2_AGENTC_ADD_CERTIFICATE_CONSTRAINED on-the-wire id - Add sshkey_cert_serialize() and _deserialize() - Load certificate via "ssh-add some-cert-file.pub" if a matching private key is already available (either loaded or on a PKCS11 token). The clean extension to the ssh-agent protocol could be added to https://tools.ietf.org/html/draft-miller-ssh-agent-02 and also be re-used by gpg2's ssh-agent emulation in the near future. I've also checked the discussion on the resolved bug 2436 and it also had the goal to use multiple certificates. Having ssh-agent support for this would be the next step. [side note: The current PKCS11 code in ssh-add skips loading *any* certificate. This might be due to the refcounting issue as outlined in comment 7] What do you think? You mentioned earlier: > but IMO users shouldn't be able to add keys to an agent *without* > presenting their private section. Can you elaborate a little more on this? Do you see a security risk?
> > but IMO users shouldn't be able to add keys to an agent *without* > > presenting their private section. > > Can you elaborate a little more on this? Do you see a security risk? If the server would accept such key, it would be a big security issue of that server. I believe it is just a good practice making sane also the client applications that is not going to allow potentially broken/breaking configuration. You can send the public key/certificate tests but you can really not authenticate without the private counterpart. Thank you for the work on the patch. It sounds like a useful feature to do and support. But I am not sure if this is the best way how to do that. Your proposal about adding SSH2_AGENTC_ADD_CERTIFICATE_CONSTRAINED sounds significantly better even for the price of extending the protocol for one more message. For the patch to be more acceptable, I believe few test cases to verify the general functionality would be good. There is already one almost-working test with ssh-agent and soft-pkcs11 module, but I elaborated on it more in the bug #2817, which is solving different problem of PKCS#11 support, but can be used as a reference for the test cases.
Created attachment 3227 [details] add SSH2_AGENTC_ADD_CERTIFICATES to add certificates for matching with private keys This is an implementation of a SSH2_AGENTC_ADD_CERTIFICATES message in ssh-agent to load one or more certificates that will be matched to private keys if/when they are loaded. I'm not convinced that being able to add certificates to one's agent yields any security problem. The authenticator is possession of the private key, and access to an agent socket is already approximately equivalent to that - an attacker could get equivalent results without ever touching the agent by grafting a certificate to an agent key themselves. BTW, it is already possible to use specify certificates in ssh that will be used with keys from the agent of PKCS#11 tokens. Maybe this isn't needed at all?
Yes, the original patch is not needed anymore thanks to other improvements in openssh. We rolled out the changes in December 2018 and so far no complaints :) Basically openssh gained support to sign certificates with private keys reachable via the ssh-agent, this works transparently with gpg-agent's ssh agent protocol implementation + an openpgp smartcard. Also the ability to specify additional certificates on the command line solved the issue for us. I was about the write that we will abandon the patch since it's no longer needed with a recent openssh.
Hi! Im sorry but Im not really following. If I have a private key loaded from a PKCS#11 token, how do I load the corresponding certificate into the agent? Cant find anything about it in the ssh-add manual. Thanks for your help. /Peter.
From what I understand, currently the ssh-agent can work with certificate keys that are available locally to the client. The issue is that they can not be added to the agent with the keys on smartcard so both of them could be atomically forwarded to be used on remote hosts, which was one of the requests in this bug (comment #11). Clearly updating this will require updating also the ssh-agent protocol [1] and other tools talking this protocol if we do not want to break time (draft is already expired). The question is if is reasonable to extend the protocol this way or the extension negotiation mechanism (since it is available) should be used. The protocol is already used for a long time, but no official RFC is out so [1] https://tools.ietf.org/html/draft-miller-ssh-agent-02
Yes, thats what I do today. I distribute my certificate files so that are available locally on all machines that I need it. But thats not a very scalable solution. If you are using a combination of PKCS#11 tokens, agent forwarding and certificates this is to my knowledge to only way to go for the moment.