[Bug 3553] New: PROTOCOL.key format specification is incorrect for encryption using AEAD transports

[bugzilla-daemon at mindrot.org]

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

            Bug ID: 3553
           Summary: PROTOCOL.key format specification is incorrect for
                    encryption using AEAD transports
           Product: Portable OpenSSH
           Version: 9.3p1
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P5
         Component: Documentation
          Assignee: unassigned-bugs at mindrot.org
          Reporter: thestr4d at gmail.com

`ssh-keygen` allows encrypting private keys with any symmetric
transport cipher that OpenSSH supports. This includes AEADs like
`aes256-gcm at openssh.com` and `chacha20-poly1305 at openssh.com`.
`ssh-keygen` does not default to using AEADs, but other ecosystem
players do so (e.g. 1Password uses `aes256-gcm at openssh.com` when a user
provides a passphrase during export of an SSH private key).

`PROTOCOL.key` specifies that the encrypted list of private keys is
encoded as a `string` (u32 length followed by that many bytes). A plain
reading of this specification is that this field contains the
ciphertext for whatever encryption scheme is being used (so for the
`none` scheme it is just the encoded plaintext, for `aes256-cbc` it is
the AES256-CBC-encrypted ciphertext, and so on).

However, when an AEAD is used, `ssh-keygen` does not encode the full
AEAD ciphertext in the `string` field. Instead, the fixed-length tag is
appended as raw bytes, and the `string` field only covers the
variable-length unauthenticated ciphertext part of the AEAD ciphertext
(or, hopefully equivalently for OpenSSH, the length field of the
`string` encodes the length of the plaintext, regardless of how it is
encrypted).

This leads to two problems:
- If key parsing is implemented following the specification in
`PROTOCOL.key`, the tag gets ignored, and the last `len(tag)` bytes of
the unauthenticated ciphertext are interpreted as the tag. This could
lead to a parser error if the plaintext is shorter than the tag, or if
the parser checks that all data is consumed (if the outer format does
not have follow-on data; the spec does not prohibit it), but is more
likely to result in a decryption error (leading the developer on a wild
goose chase trying to figure out why their cryptography library is
broken - hi!).
- Parsing of keys encoded by `ssh-keygen` (and by other ecosystem
players following its implementation-specific details) now needs to be
interleaved with interpretation. It is necessary to parse the
`ciphername` field, interpret it to determine what the expected tag
length is, and then provide that as input to the remainder of the
parser.

I don't know what the intended specification is here; it looks like
`ssh-keygen` was itself updated after the fact to parse this format
(https://github.com/openssh/openssh-portable/commit/63ebf019be863b2d90492a85e248cf55a6e87403).
That change was made in 2015, so it's too late to fix `ssh-keygen` to
follow the specification in `PROTOCOL.key`. `PROTOCOL.key` needs to be
treated as incorrect, and updated to specify the actual key format
generated and used in production.

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