openssh bugs - Apr 2015 - [Bug 2390] New: PROTOCOL.key mis-describes private section

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

            Bug ID: 2390
           Summary: PROTOCOL.key mis-describes private section
           Product: Portable OpenSSH
           Version: 6.8p1
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P5
         Component: Documentation
          Assignee: unassigned-bugs at mindrot.org
          Reporter: anakin at pobox.com

Created attachment 2607
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2607&action=edit
test key file

The file openssh/PROTOCOL.key documents the (optionally) encrypted
section of a new-format private key file as containing

        uint32  checkint
        uint32  checkint
        string  privatekey1
        string  comment1
        ...

Therefore, I expect each private key to be wrapped in a single large
SSH-2 "string", inside which I expect a second layer of data
marshalling such as the key type string and various mpints.

However, in fact, this doesn't seem to be true: as far as I can see,
the individual pieces of the private key just appear at the top level
of the encrypted string, and whoever is reading the file must recognise
the key type (either here or in the prior sequence of public keys) and
use it to know how many bits and pieces to expect.

As evidence I attach a test key file that I just generated using the
command "./ssh-keygen -t rsa -o -f testkey" and no passphrase.
Hex-dumping the base64 content, the relevant section is here:

00000140  DE DF 00 00 03 C8 92 77 30 13 92 77 30 13 00 00
.......w0..w0...
00000150  00 07 73 73 68 2D 72 73 61 00 00 01 01 00 AC 84
..ssh-rsa.......

Offset 0x142 is just beyond the end of the public key component, so the
next thing we see is the uint32 0x3C8 which is the length field of the
entire encrypted section (though not encrypted in this case). After
that, we see two identical copies of the uint32 0x92773013 (the
duplicated 'checkint'), and then immediately after that the 00 00 00 07
length field of the string "ssh-rsa" - with no intervening length
field
large enough to span the whole private key. In other words, the
individual pieces of the private key data appear inline without a large
wrapping "string".

In fact, my analysis of the entire 'encrypted' section in this key
looks like this:

        uint32  0x92773013 (checkint)
        uint32  0x92773013 (checkint again)
        string  "ssh-rsa"
        mpint   modulus
        mpint   0x10001 (public exponent)
        mpint   private exponent
        mpint   iqmp
        mpint   p
        mpint   q
        string  "simon at resolution" (key comment)
        data    0x01 0x02 0x03 (padding)

so you can see that a decoder of this format has to see "ssh-rsa" and
know that that means six mpints follow, otherwise they cannot know
where to find the key comment or the start of the next private key (if
there were one).

I realise it's too late to actually change the key format now, but
PROTOCOL.key should be corrected to stop claiming a wrapping string.
Ideally it should also contain enough information to understand all the
supported key types, or at the very least, how to skip over each one to
the next.

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

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

--- Comment #1 from Simon Tatham <anakin at pobox.com> ---
Created attachment 2608
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2608&action=edit
hex dump of testkey

Also attached the full hex dump of the decoded base64 in my test key
file.

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

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

Simon Tatham <anakin at pobox.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |anakin at pobox.com

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

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

Colin Watson <cjwatson at debian.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cjwatson at debian.org

--- Comment #2 from Colin Watson <cjwatson at debian.org> ---
Created attachment 3174
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3174&action=edit
Make PROTOCOL.key agree more closely with reality

I ran into this independently and put together the attached patch to
improve the protocol documentation.  It probably shouldn't close this
bug as I haven't done anything about describing the internal encoding
of each private key, but it's a step forward.

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

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED
                 CC|                            |djm at mindrot.org

--- Comment #3 from Damien Miller <djm at mindrot.org> ---
This was mostly fixed in openssh-8.7, except for the s/char/byte/. I
just committed a fix for that too. Thanks

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |3418


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=3418
[Bug 3418] tracking bug for openssh-9.1
-- 
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=2390

Stephanie <tylercashea33 at icloud.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tylercashea33 at icloud.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=2390

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #4 from Damien Miller <djm at mindrot.org> ---
Closing bugs from OpenSSH 9.1 release cycle

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