Quick Check Oracle Allows for Partial Plaintext Recovery

[missytake]

This issue was discovered during the audit, see "M1" in https://delta.chat/assets/1907-otf-deltachat-rpgp-rustrsa-gb-reportv1.pdf:

"The RPGP implements OpenPGP's vulnerable quick check specification, which can allow for the first two bytes of every ciphertext block to be recovered.

OpenPGP implements a checksum known as quick check to ensure the proper decryption key is being used. However, this introduces an adaptive-chosen-ciphertext attack against its modified implementation of Cipher Feedback (CFB) mode of encryption. This allows an attacker to determine 16 bits of any block of plaintext with about 2^15 oracle queries for the initial setup work and 2^15 oracle queries for each block."

"The assessment team recommends not verifying the quick check bytes during the decryption
process by default. Then OpenPGP specification suggests various implementations only verify
quick-check operations when public key encryption is not used. See the reference “An Attack on
CFB Mode Encryption As Used By OpenPGP” for more details. The attack is acknowledged in the
RFC 4880 titled OpenPGP Message Format, under section 14. Security Considerations."

[link2xt]

Reading RFC 4880 link, it says "On the other hand, it is inconvenient to the user to be informed that they typed in the wrong passphrase only after a petabyte of data is decrypted. There are many cases in cryptographic engineering where the implementer must use care and wisdom, and this is one."

There are users who care about performance in this case and use rPGP to encrypt large filesystem archives: #258
@dadleyy Would it break any of your usecases if we simply remove quickcheck from symmetric encryption and the error is reported after decrypting the whole symmetrically-encrypted data rather than after the first block? If you don't expect users to type in the passphrase manually, this should not be a problem.

The fix is as simple as removing

rpgp/src/crypto/sym.rs

Lines 22 to 32 in 97edf8f

	// quick check, before decrypting the rest
	ensure_eq!(
	$prefix[$bs - 2],
	$prefix[$bs],
	"cfb decrypt, quick check part 1"
	);
	ensure_eq!(
	$prefix[$bs - 1],
	$prefix[$bs + 1],
	"cfb decrypt, quick check part 2"
	);

[dignifiedquire]

I wonder if this should be an option, given this is mostly only an issue in online systems

[link2xt]

Could be an option, but if it does not break anyone's use case I would prefer to just remove it completely and don't have to deal with it.

[link2xt]

I made a PR removing "quick check", we can decide whether to merge it or not: #285

[dadleyy]

@dadleyy Would it break any of your usecases if we simply remove quickcheck from symmetric encryption and the error is reported after decrypting the whole symmetrically-encrypted data rather than after the first block? If you don't expect users to type in the passphrase manually, this should not be a problem.

thanks for checking! no problems from my end.