openssh bugs - Sep 2015 - [Bug 2472] New: Add support to load additional certificates

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

            Bug ID: 2472
           Summary: Add support to load additional certificates
           Product: Portable OpenSSH
           Version: 7.1p1
          Hardware: All
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: ssh-agent
          Assignee: unassigned-bugs at mindrot.org
          Reporter: thomas.jarosch at intra2net.com

Created attachment 2715
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2715&action=edit
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

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

--- Comment #1 from Thomas Jarosch <thomas.jarosch at intra2net.com> ---
Created attachment 2716
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2716&action=edit
Patch part 2/3

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

--- Comment #2 from Thomas Jarosch <thomas.jarosch at intra2net.com> ---
Created attachment 2717
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2717&action=edit
Patch part 3/3

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org
             Status|NEW                         |ASSIGNED

--- Comment #3 from Damien Miller <djm at mindrot.org> ---
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.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

--- Comment #4 from Thomas Jarosch <thomas.jarosch at intra2net.com> ---
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.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

--- Comment #5 from Damien Miller <djm at mindrot.org> ---
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.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

--- Comment #6 from Thomas Jarosch <thomas.jarosch at intra2net.com> ---
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.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

--- Comment #7 from Thomas Jarosch <thomas.jarosch at intra2net.com> ---
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

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

--- Comment #8 from Thomas Jarosch <thomas.jarosch at intra2net.com> ---
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

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

Thomas Jarosch <thomas.jarosch at intra2net.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2715|0                           |1
        is obsolete|                            |
   Attachment #2716|0                           |1
        is obsolete|                            |
   Attachment #2717|0                           |1
        is obsolete|                            |

--- Comment #9 from Thomas Jarosch <thomas.jarosch at intra2net.com> ---
Created attachment 2933
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2933&action=edit
Updated patchset to current openssh code

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

Thomas Jarosch <thomas.jarosch at intra2net.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2933|0                           |1
        is obsolete|                            |

--- Comment #10 from Thomas Jarosch <thomas.jarosch at intra2net.com> ---
Created attachment 2934
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2934&action=edit
Tarball with updated patchset to current openssh code

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

Peter <peter at pean.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |peter at pean.org

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

--- Comment #11 from Peter <peter at pean.org> ---
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

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

--- Comment #12 from Peter <peter at pean.org> ---
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

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

--- Comment #13 from Thomas Jarosch <thomas.jarosch at intra2net.com> ---
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

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

--- Comment #14 from Thomas Jarosch <thomas.jarosch at intra2net.com> ---
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?

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

Jakub Jelen <jjelen at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jjelen at redhat.com

--- Comment #15 from Jakub Jelen <jjelen at redhat.com>
---
> > 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.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2934|0                           |1
        is obsolete|                            |

--- Comment #16 from Damien Miller <djm at mindrot.org> ---
Created attachment 3227
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3227&action=edit
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?

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

--- Comment #17 from Thomas Jarosch <thomas.jarosch at intra2net.com> ---
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.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

--- Comment #18 from Peter <peter at pean.org> ---
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.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

--- Comment #19 from Jakub Jelen <jjelen at redhat.com>
---
>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

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

--- Comment #20 from Peter <peter at pean.org> ---
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.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

Phil Frost <indigo at bitglue.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |indigo at bitglue.com

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

Celeste Liu <CoelacanthusHex at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |CoelacanthusHex at gmail.com

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2472

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |DUPLICATE

--- Comment #21 from Damien Miller <djm at mindrot.org> ---


*** This bug has been marked as a duplicate of bug 3613 ***

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.