crypto: encryption is not authenticated

The current crypto implemented aes256-cfb with a magic is not secure against bit flipping attacks which is a very real vector for boot image protections like these.

Modes like CFB and anything in aes::cipher are what is called "confidentiality only" modes. There are two things you want to protect

Confidentiality: any part of the plaintext should not get out
Integrity: any outside tampering would not go undetected

In case of CFB you could just flip the i-th bit in ciphertext and the decryption will happily go though with the i-th bit in plaintext flipped. If you know what you are doing and are good at making educated guess on where to alter the boot image would lead to takeover.. The boot image will decrypt to whatever you want it to look like

To fix this there are two main solutions: HMAC which is essentially a hash-secured message seal and AEAD ciphers which adds secure check bits throughout the ciphertext to make sure it is has not been tampered with.

Usually with HMAC you get a constant size increase (256 bits for sha256) but with AEAD you have a small but linear increase in size. HMAC is more "leak proof" but there are not really effective ways to tamper AEAD protected ciphertext given you used them correctly ofc either.

It looks like you are just trying to protect a relatively small blob that you will decrypt fully in memory before use. I think for simplicity we should go with an HMAC based protection, now replacing the "magic" with the HMAC digest during encryption. During decryption the user puts in the password we derive the key and then use the key to redigest the encrypted boot image. Compare the two digests if they match then continue decrypting.

Or if you want to go full on with AEAD modes we could use gcm and xts, looks like RustCrypto has implementations for them as well in case in future you want to decrypt blocks larger than memory or O(1) random access

In the future if we want to go full on configurable you could make something like this and then parse parameters from string, for example aes256-cfb-magic (not authenticated) aes256-gcm-none (authenticated) aes256-cfb-sha256 (authenticated), like the whole deal in veracrypt, LUKS, etc. This would ofc need more planning and might be a future goal out of the scope of this issue.

pub struct Algorithm {
    cipher: Cipher,
    cipher_mode: CipherMode,
    hmac: Option<Hmac>,
}

Let me know your thoughts and I could get a PR started!

Alright I got this PR up, please take a look if you have the time!

I tried this and hmac + xts, both were pretty painless to implement but this required fewer dependencies and resulted in a bit less code, also, I didn't have to put anything in the plaintext, always nice to get to skip binary parsing.

Security-wise from my looking around it seems correct but it's complex so I'd love some feedback on it. The nonce is regenerated for each encryption, so it's never reused across encryptions so that seems fine, I'll have to figure out a way to make the updating more ergonomic though but that's outside the scope of this issue.

I think a choice of different methods could be a nice future improvement, but for now making sure that one alternative that's definitely(tm) correct is what I'd prefer!

MarcusGrass / boot-rs

crypto: encryption is not authenticated #1