Bug 2390 - PROTOCOL.key mis-describes private section
Summary: PROTOCOL.key mis-describes private section
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: Documentation (show other bugs)
Version: 6.8p1
Hardware: All All
: P5 normal
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks: V_9_1
  Show dependency treegraph
 
Reported: 2015-04-27 04:04 AEST by Simon Tatham
Modified: 2022-10-04 21:58 AEDT (History)
4 users (show)

See Also:


Attachments
test key file (1.78 KB, application/octet-stream)
2015-04-27 04:04 AEST, Simon Tatham
no flags Details
hex dump of testkey (6.01 KB, text/plain)
2015-04-27 04:05 AEST, Simon Tatham
no flags Details
Make PROTOCOL.key agree more closely with reality (1.36 KB, patch)
2018-08-28 20:23 AEST, Colin Watson
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Tatham 2015-04-27 04:04:16 AEST
Created attachment 2607 [details]
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@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.
Comment 1 Simon Tatham 2015-04-27 04:05:21 AEST
Created attachment 2608 [details]
hex dump of testkey

Also attached the full hex dump of the decoded base64 in my test key file.
Comment 2 Colin Watson 2018-08-28 20:23:02 AEST
Created attachment 3174 [details]
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.
Comment 3 Damien Miller 2022-07-01 14:46:47 AEST
This was mostly fixed in openssh-8.7, except for the s/char/byte/. I just committed a fix for that too. Thanks
Comment 4 Damien Miller 2022-10-04 21:58:03 AEDT
Closing bugs from OpenSSH 9.1 release cycle