openssh bugs - Sep 2016 - [Bug 2617] New: sign_and_send_pubkey: no separate private key for certificate

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

            Bug ID: 2617
           Summary: sign_and_send_pubkey: no separate private key for
                    certificate
           Product: Portable OpenSSH
           Version: 7.3p1
          Hardware: 68k
                OS: Mac OS X
            Status: NEW
          Severity: normal
          Priority: P5
         Component: ssh
          Assignee: unassigned-bugs at mindrot.org
          Reporter: pllewis72 at gmail.com

This worked back in openssh 6.  I'd just recently updated to OSX 10.12
and it stopped right after.  Openssh 7.2+ seems to be a point in which
I know it has changed.  I have since tested this on Ubuntu 16.04 with
openssh 7.2 with same results, so it's not a platform issue.  I also
updated ssh through homebrew on the mac to 7.3p1.  

First look on bugzilla, I thought it was related to the 2550 bug
(https://bugzilla.mindrot.org/show_bug.cgi?id=2550), but that was fixed
in 7.3p1.

The process using ssh certificate authentication through an SSH proxy
host.  The private key is in the downloaded certificate.  Openssh is
now looking for a separate ssh private key file.

Via 7.3 failure:
ssh -vvv -o 'ProxyCommand ssh -i ~/.ssh/bastion_key
my.name@<BASTIONHOST> -W %h:%p' ec2-user@<EC2HOST> -i
~/.ssh/bastion_key
OpenSSH_7.3p1, LibreSSL 2.4.2
debug1: Reading configuration data /Users/user/.ssh/config
debug1: /Users/user/.ssh/config line 27: Applying options for 10.*
debug1: Reading configuration data /usr/local/etc/ssh/ssh_config
debug1: Executing proxy command: exec ssh -i ~/.ssh/bastion_key
my.name@<BASTIONHOST> -W <EC2HOST>:22
debug1: permanently_drop_suid: ######
debug1: key_load_public: No such file or directory
debug1: identity file /Users/user/.ssh/bastion_key type -1
debug1: identity file /Users/user/.ssh/bastion_key-cert type 5
debug1: Enabling compatibility mode for protocol 2.0
debug1: Local version string SSH-2.0-OpenSSH_7.3
no such identity: /Users/user/.ssh/bastion_key-cert: No such file or
directory
Permission denied (publickey).
ssh_exchange_identification: Connection closed by remote host

When I check out the bastion file, I get the following:
$ ls -l ~/.ssh/bastion_key*
-rw------- 1 user group 1675 Sep 26 14:09 /Users/user/.ssh/bastion_key
-rw-r--r-- 1 user group 1539 Sep 26 14:09
/Users/user/.ssh/bastion_key-cert.pub


Docker container with OpenSSH 6.6 works(docker is why its all as root):
[root at 18be76b35451 ~]# ssh -vvv -o 'ProxyCommand ssh -i
~/.ssh/bastion_key my.name@<BASTIONHOST> -W %h:%p'
ec2-user@<EC2HOST>
-i ~/.ssh/bastion_key
OpenSSH_6.6.1, OpenSSL 1.0.1e-fips 11 Feb 2013
debug1: Reading configuration data /etc/ssh/ssh_config
debug1: /etc/ssh/ssh_config line 56: Applying options for *
debug1: Executing proxy command: exec ssh -i ~/.ssh/bastion_key
my.name@<BASTIONHOST> -W <EC2HOST>:22
debug1: permanently_set_uid: 0/0
debug1: permanently_drop_suid: 0
debug3: Incorrect RSA1 identifier
debug3: Could not load "/root/.ssh/bastion_key" as a RSA1 public key
debug1: identity file /root/.ssh/bastion_key type -1
debug1: ssh_rsa_verify: signature correct
debug1: identity file /root/.ssh/bastion_key-cert type 5
debug1: Enabling compatibility mode for protocol 2.0
debug1: Local version string SSH-2.0-OpenSSH_6.6.1
debug1: Remote protocol version 2.0, remote software version
OpenSSH_6.6.1
debug1: match: OpenSSH_6.6.1 pat OpenSSH_6.6.1* compat 0x04000000
debug2: fd 5 setting O_NONBLOCK
debug2: fd 4 setting O_NONBLOCK
debug3: load_hostkeys: loading entries for host "<EC2HOST>" from
file
"/root/.ssh/known_hosts"
debug3: load_hostkeys: found key type ECDSA in file
/root/.ssh/known_hosts:2
debug3: load_hostkeys: loaded 1 keys
....

[root at 18be76b35451 ~]# ls -l ~/.ssh/bastion_key*
-rw------- 1 root root 1679 Sep 26 18:25 /root/.ssh/bastion_key
-rw-r--r-- 1 root root 1539 Sep 26 18:25
/root/.ssh/bastion_key-cert.pub

Let me know if more logs are needed.  I can do more debugging also if
this isn't the right data.

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

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

Peter <pllewis72 at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 OS|Mac OS X                    |All

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

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

Peter <pllewis72 at gmail.com> changed:

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

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

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

Peter <pllewis72 at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Hardware|68k                         |All

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

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org

--- Comment #1 from Damien Miller <djm at mindrot.org> ---
Created attachment 2884
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2884&action=edit
probable fix

I think this patch should fix the problem. Could you please test it?

-- 
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=2617

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2594


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2594
[Bug 2594] Tracking bug for OpenSSH 7.4 release
-- 
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=2617

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned-bugs at mindrot.org |djm at mindrot.org

--- Comment #2 from Damien Miller <djm at mindrot.org> ---
Created attachment 2886
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2886&action=edit
revised fix

Previous fix had a problem, please try this one

-- 
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=2617

Damien Miller <djm at mindrot.org> changed:

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

-- 
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=2617

--- Comment #3 from Peter <pllewis72 at gmail.com> ---
I was able to test and confirm this resolved the issue.  

Thanks for the fix.  Do you have an ideas when either p2 or 7.4 will be
released?

Thanks again.

-- 
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=2617

Adam Eijdenberg <adam at continusec.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |adam at continusec.com

--- Comment #4 from Adam Eijdenberg <adam at continusec.com> ---
I found this bug after preparing a similar patch (including tests).

Although the patch provided here is simpler, it fails when using the
new CertificateFile configuration line (which was introduced in the
commit that broke the old behaviour).

e.g. the following config:

CertificateFile
/Users/aeijdenberg/.ssh/id_androgogic_shortlived_rsa-cert.pub
IdentityFile /Users/aeijdenberg/.ssh/id_androgogic_shortlived_rsa

debug1: Offering RSA-CERT public key:
/Users/aeijdenberg/.ssh/id_androgogic_shortlived_rsa-cert.pub
debug1: Server accepts key: pkalg ssh-rsa-cert-v01 at openssh.com blen
1540
debug1: sign_and_send_pubkey: no separate private key for certificate
"/Users/aeijdenberg/.ssh/id_androgogic_shortlived_rsa-cert.pub"
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@         WARNING: UNPROTECTED PRIVATE KEY FILE!          @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
Permissions 0644 for
'/Users/aeijdenberg/.ssh/id_androgogic_shortlived_rsa-cert.pub' are too
open.
It is required that your private key files are NOT accessible by
others.
This private key will be ignored.
Load key
"/Users/aeijdenberg/.ssh/id_androgogic_shortlived_rsa-cert.pub": bad
permissions
debug1: Trying private key:
/Users/aeijdenberg/.ssh/id_androgogic_shortlived_rsa
debug1: Authentications that can continue: publickey,password
debug1: No more authentication methods to try.
Permission denied (publickey,password).

(and just changing the permissions didn't seem to help, it instead
prompted me for a password for the cert file, which doesn't need one)

Commenting out the explicit reference in config to CertificateFile
makes it work again.

Here is the alternate patch I had put together - it includes tests, and
also addresses a few other somewhat related issues:
https://github.com/openssh/openssh-portable/pull/53

-- 
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=2617

--- Comment #5 from Damien Miller <djm at mindrot.org> ---
(In reply to Adam Eijdenberg from comment #4)
> I found this bug after preparing a similar patch (including tests).
> 
> Although the patch provided here is simpler, it fails when using the
> new CertificateFile configuration line (which was introduced in the
> commit that broke the old behaviour).
I think your pull request goes a bit beyond what's going on here, by
removing the restrictions that CertificateFile-loaded keys must have a
corresponding plain public key. IMO that's a fine goal, but it's not
strictly a regression like this is.

-- 
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=2617

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at zip.com.au
   Attachment #2886|                            |ok?(dtucker at zip.com.au)
              Flags|                            |

-- 
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=2617

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2886|ok?(dtucker at zip.com.au)     |ok+
              Flags|                            |

-- 
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=2617

--- Comment #6 from Damien Miller <djm at mindrot.org> ---
Created attachment 2899
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2899&action=edit
try to support IdentityFile w/ no key.pub with CertificateFile

This attempts to make CertificateFile work when a key provided by
IdentityFile has no public copy on disk by considering IdentityFile
keys that did not load a public half if the filename matches, possibly
without .pub/-cert.pub

-- 
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=2617

--- Comment #7 from Damien Miller <djm at mindrot.org> ---
Patch for the IdentityFile case has been committed and will be in
OpenSSH 7.4.

If anyone could test the 2nd patch for CertificateFile it would be
greatly appreciated; the window for the 7.4 release is rapidly
closing...

-- 
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=2617

--- Comment #8 from Adam Eijdenberg <adam at continusec.com> ---
Hi Damien,

I've tested your patch against the same tests I included in my original
PR (https://github.com/openssh/openssh-portable/pull/53) however I'm
seeing the same segfaults that I encountered when I tried to make mine.
:)

The problem (I think) is that identity_sign() calls
identity_sign_encode() before doing anything, and
identity_sign_encode() attempts to dereference id->key->type which is
problematic since id->key is NULL.

I'll attach a patch that addresses the segfaults, and separately a
patch with the tests that I'd put in the original PR (even though this
patch doesn't address all of them).

Cheers, Adam

-- 
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=2617

--- Comment #9 from Adam Eijdenberg <adam at continusec.com> ---
Created attachment 2901
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2901&action=edit
Allow for id->key being NULL before being passed to identity_sign()

Allow for id->key being NULL before being passed to identity_sign()

-- 
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=2617

--- Comment #10 from Adam Eijdenberg <adam at continusec.com> ---
Created attachment 2902
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2902&action=edit
Tests

This attachment is a patch to add the tests that I had in the original
PR.

To run:

cd regress/
PATH=`pwd`/..:$PATH:. TEST_SHELL=/bin/sh sh test-exec.sh `pwd`
cert-file.sh

-- 
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=2617

--- Comment #11 from Adam Eijdenberg <adam at continusec.com> ---
Created attachment 2903
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2903&action=edit
Load key files for matching cert names

This patch adds to the previous ones to make all the tests actually
pass.

It mirrors the logic for loading keys (which check for matching certs
filenames, if none are explicitly specified), and looks for keys which
match cert filenames if no keys are explicitly specified.

It also disables use of id_rsa (and other defaults) when an explicit
CertificateFile is specified (similar to when an IdentityFile is
specified) and also when IdentitiesOnly is specified (and that is
likely worth discussion as to whether that's the right thing to do or
not).

-- 
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=2617

Adam Eijdenberg <adam at continusec.com> changed:

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

--- Comment #12 from Adam Eijdenberg <adam at continusec.com> ---
Created attachment 2904
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2904&action=edit
Tests (fixed patch format)

-- 
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=2617

Adam Eijdenberg <adam at continusec.com> changed:

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

--- Comment #13 from Adam Eijdenberg <adam at continusec.com> ---
Created attachment 2905
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2905&action=edit
Tests

(third time lucky formatting the attachment correctly, sorry about the
spam)

-- 
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=2617

Adam Eijdenberg <adam at continusec.com> changed:

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

--- Comment #14 from Adam Eijdenberg <adam at continusec.com> ---
Created attachment 2906
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2906&action=edit
Allow for id->key being NULL before being passed to identity_sign()

(fixed patch attachment format)

-- 
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=2617

--- Comment #15 from Damien Miller <djm at mindrot.org> ---
Created attachment 2909
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2909&action=edit
consolidated and tweaked patches

Thanks indeed for taking the time to write regression tests.

I've merged most of the patches to this one. It does not include your
changes to load keys specified via CertificateFile but not IdentityFile
- I want to think about those a bit more and I'd like to get the rest
of it in before release if possible.

-- 
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=2617

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2909|                            |ok?(dtucker at zip.com.au)
              Flags|                            |

--- Comment #16 from Damien Miller <djm at mindrot.org> ---
Comment on attachment 2909
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2909
consolidated and tweaked patches

Note to Darren: the changes in identity_sign(), etc are necessary
because we'll now let identities with id->key == NULL in for the case
where a certificate doesn't have a .pub file that corresponds to the
private file.

-- 
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=2617

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2909|ok?(dtucker at zip.com.au)     |ok+
              Flags|                            |

--- Comment #17 from Darren Tucker <dtucker at zip.com.au> ---
Comment on attachment 2909
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2909
consolidated and tweaked patches

however I'm not all that familiar with this code, so you might want to
also get Markus to take a look

-- 
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=2617

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2647

--- Comment #18 from Damien Miller <djm at mindrot.org> ---
OpenSSH 7.4 release is closing; punt the bugs to 7.5


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2647
[Bug 2647] Tracking bug for OpenSSH 7.5 release
-- 
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=2617

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2594                        |


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2594
[Bug 2594] Tracking bug for OpenSSH 7.4 release
-- 
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=2617

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #19 from Damien Miller <djm at mindrot.org> ---
Patch is applied, this will be in OpenSSH 7.5

-- 
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=2617

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #20 from Damien Miller <djm at mindrot.org> ---
Close all resolved bugs after release of OpenSSH 7.7.

-- 
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.