rnpgp / rnp

RNP: high performance C++ OpenPGP library used by Mozilla Thunderbird
https://www.rnpgp.org
Other
198 stars 55 forks source link

`rnp_decrypt` is happy to "decrypt" cleartext; should offer comparable "protection info" as `rnp_op_verify_get_protection_info` #1826

Open dkg opened 2 years ago

dkg commented 2 years ago

When decrypting, if i don't care about signature verification, the rnp_decrypt interface is nice and simple.

However, it will happily transform this "encrypted" message to the cleartext string test test:

-----BEGIN PGP MESSAGE-----

owE7LZDEkFSS/qkktbhEAURwAQA=
=OIAi
-----END PGP MESSAGE-----

And the user of librnp will be unaware that there was no actual cryptographic protection on the message (this is just an ASCII-Armored Literal Data packet).

It would be nice to offer the equivalent data as that provided by rnp_op_verify_get_protection_info -- at the very least the boolean output, without having to indicate that i might be interested in signature verification.

ni4 commented 2 years ago

Hm, this would require adding new function as this doesn't have any state parameters. It's signature could be like rnp_result_t rnp_decrypt_ex(rnp_ffi_t ffi, rnp_input_t input, rnp_output_t output, rnp_op_verify_t *op), allowing to retrieve encryption/signature status once needed (or pass NULL to op to just decrypt).

Another solution could be to return some specific error code from rnp_decrypt(), like RNP_ERROR_NOT_ENCRYPTED if there were no encryption layer.

dkg commented 2 years ago

I'd also be interested to know if the encryption was malleable (e.g. no MDC or AEAD), or using a weak cipher (IDEA, 3DES, CAST5), not just if it was cleartext. See for example the guidance in the crypto-refresh of RFC 4880, which says:

Implementations MAY decrypt data that uses IDEA, TripleDES, or CAST5 for the sake of reading older messages or new messages from legacy clients. An Implementation that decrypts data using IDEA, TripleDES, or CAST5 SHOULD generate a deprecation warning about the symmetric algorithm, indicating that message confidentiality is suspect.

and the section on ciphertext malleability which says:

When an OpenPGP implementation discovers that it is decrypting data that appears to be malleable, it MUST indicate a clear error message that the integrity of the message is suspect, SHOULD NOT release decrypted data to the user, and SHOULD halt with an error.

I could also imagine wanting a deprecation warning if the decryption resulted from decrypting with a weak secret key (e.g. RSA-512, or ElGamal).

These are subtly different responses (and different warnings to raise), and potentially more than one of them could be true, so i'm not sure that a simple single return code is sufficient.

I'm reluctant to have rnp_decrypt return an rnp_op_verify_t because there are so many different unrelated/irrelevant things that you might do with an rnp_op_verify_t, and i think it makes the API pretty confusing to try to use.

could it just return some sort of rnp_protection_t object that only reports this information?

ni4 commented 2 years ago

Sorry, somehow missed to reply. It would be a bit overhead for the FFI - as we cannot have structs, we'll need to return some opaque object and have functions to retrieve all the required properties, like a protection info, recipient count, recipient used to decrypt and so on. That would duplicate already existing functionality of the rnp_op_verify_get_*, confusing the user. Need to think more on this.