search

FiloSottile / age

A simple, modern and secure encryption tool (and Go library) with small explicit keys, no config options, and UNIX-style composability.
https://age-encryption.org
BSD 3-Clause "New" or "Revised" License
17.05k stars 500 forks source link

Specific SSH RSA key lengths break backward compatibility #397

Closed 0x2b3bfa0 closed 2 years ago

0x2b3bfa0 commented 2 years ago

Environment

Intent

Using a 3072 bit SSH RSA key:

Result

$ ssh-keygen -m pem -t rsa -b 3072 -f 3072.pem
Generating public/private rsa key pair.
Enter passphrase (empty for no passphrase): 
Enter same passphrase again: 
Your identification has been saved in 30712.pem
Your public key has been saved in 30712.pem.pub
The key fingerprint is: ···
The key's randomart image is: ···
$ age-v1.0.0-beta6 --recipients-file 3072.pem.pub |
> age-v1.0.0-beta7 --decrypt --identity 3072.pem
Error: failed to read header: parsing age header: malformed body line "--- gGh7s3ehHo9x4fzygcKGXxRnmUnOB/wF1KfoBZc0aWw\n": reached footer without previous stanza being closed
Note: this might be a file encrypted with an old beta version of rage. Use rage to decrypt it.
[ Did age not do what you expected? Could an error be more useful? Tell us: https://filippo.io/age/report ]
$ age-v1.0.0-beta7 --recipients-file 3072.pem.pub |
> age-v1.0.0-beta6 --decrypt --identity 3072.pem
Error: failed to read header: parsing age header: malformed body line "\n": line is empty
[ Did age not do what you expected? Could an error be more useful? Tell us: https://filippo.io/age/report ]

📖 Note: key length provided for illustrative purposes; ssh-keygen(1) generates 3072 bit SSH RSA keys by default.

Changes

Commit message for 15df6e2cf71bd7457d6a7e1c15754030dc6a9304

We are going to reuse the stanza format for IPC in the plugin protocol, but in that context we need stanzas to be self-closing. Currently they almost are, but if the body is 0 modulo 48,[^48] there is no way to know if the stanza is over after the last line.

Now, all stanzas have to end with a short line, even if empty.

No ciphertexts generated by age in the past are affected, but 3% of the ciphertexts generated by rage will now stop working. They are still supported by rage going forward. If it turns out to be a common issue, we can add an exception.

[^48]: Actually modulo 64

Release notes for 1.0.0-beta7

[...] The header format was also changed to expect a short line at the end of every stanza, which allows the format to be used in a streaming protocol. No encrypted files produced by cmd/age are affected. A small number of encrypted files produced by rage are affected, and won't decrypt with newer versions of age. They still decrypt with any version of rage. [...]

Details

This bug affects any SSH RSA key whose length in bits l satisfies the following condition, being 3072 the most common example:

$$\left\lceil\left\lceil\frac{l}{8}\right\rceil\times\frac{4}{3}\right\rceil\equiv0\pmod{64}$$

0x2b3bfa0 commented 2 years ago

Fix

As suggested in the commit message for 15df6e2cf71bd7457d6a7e1c15754030dc6a9304, it would be possible to add an exception to the following checks:

https://github.com/FiloSottile/age/blob/15df6e2cf71bd7457d6a7e1c15754030dc6a9304/internal/format/format.go#L169-L171

https://github.com/FiloSottile/age/blob/15df6e2cf71bd7457d6a7e1c15754030dc6a9304/internal/format/format.go#L183-L185

Unfortunately, changes to the newlineWriter.Write function also affected the header MAC computation, invalidating the message.

https://github.com/FiloSottile/age/blob/15df6e2cf71bd7457d6a7e1c15754030dc6a9304/age.go#L200-L202

Would computing and checking the MAC for both formats be worth the effort?

FiloSottile commented 2 years ago

Thank you for the detailed report!

No encrypted files produced by cmd/age are affected.

Ah, yep, this part turned out to be inaccurate, I didn't think about RSA key sizes.

Since no one noticed until now, it doesn't sound it's particularly disruptive, and we didn't commit to backwards compatibility until v1.0.0-rc.1. However, I need to make the error message mention this possibility.

Actually modulo 64

I think 48 is correct. The length of the body in bytes before encoding has to be a multiple of 48. The 4/3 term in your formula is part of the encoding itself.

0x2b3bfa0 commented 2 years ago

Since no one noticed until now, it doesn't sound it's particularly disruptive, and we didn't commit to backwards compatibility until v1.0.0-rc.1. However, I need to make the error message mention this possibility.

Definitely! A good error message sounds much better than an overcomplicated workaround.

I think 48 is correct. The length of the body in bytes before encoding has to be a multiple of 48. The 4/3 term in your formula is part of the encoding itself.

I was thinking of the length of the body after the Base64 encoding step, not before. 🤦🏼‍♂️ Both numbers are “correct” then, although 48 probably makes more sense in this context.

$$\left\lceil\frac{l}{8}\right\rceil\equiv0\pmod{48}$$