rnpgp / rnp

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

Support decryption with hidden recipients. #1275

Closed duckdalbe closed 1 year ago

duckdalbe commented 4 years ago

RNP reports that it cannot decrypt if no Key ID of the Public-Key Encrypted Session Key Packets matches a known private key.

It would be great if it instead could check for "hidden" recipients. In case the Key ID 0x00000000 is present, RNP should try to decrypt with all known private keys. See https://tools.ietf.org/html/rfc4880#section-5.1 (last paragraph of that section).

Background: Currently Thunderbird 78 an later fails to decrypt messages where the user is a hidden recipient. Such ciphertexts could e.g. be created by Enigmail, or in general with GnuPG's hidden-encrypt-to config option. Downstream issue: https://thunderbird.topicbox.com/groups/e2ee/T7e31aaa38399d4b3-Mefc2734288f0dfa17d01b823/fails-to-decrypt-with-recipient-key-id-0x00000000-hidden-recipient.

ni4 commented 4 years ago

Thanks for reporting, we'll add corresponding functionality once get to this issue.

andrey-utkin commented 2 years ago

I'd be interested to look into this. Seems clear how to write a test for it, and then the test will guide my investigation.

ni4 commented 2 years ago

@andrey-utkin Feel free to take it! The only thing is that I'd prefer to make this feature configurablte via the SecurityContext, so user may enable or disable this. Feel free to ask additional questions if you have.

andrey-utkin commented 2 years ago

Sorry for the slow start.

Test so far:

# - create a keypair in rnp
rnpkeys --generate-key --password '' --userid 1275@rnp

# - export pubkey from rnp
rnpkeys --export-key --userid 1275@rnp > rnp.pubkey.asc

# - import pubkey to gnupg
gpg --import < rnp.pubkey.asc

# - encrypt using the pubkey with gnupg with hidden-encrypt-to option
echo hello | gpg --trust-model always --armor  --recipient 1275@rnp --encrypt > present-to.pgp-msg.asc
echo hello | gpg --trust-model always --armor  --hidden-recipient 1275@rnp --encrypt > hidden-to.pgp-msg.asc

# - decrypt this with rnp
rnp --decrypt present-to.pgp-msg.asc # works, prints "hello"

# https://github.com/rnpgp/rnp/issues/1275
# [init_encrypted_src() /var/tmp/portage/app-crypt/rnp-9999/work/rnp-9999/src/librepgp/stream-parse.cpp:2163] failed to obtain decrypting key or password
# exit code 1
rnp --decrypt hidden-to.pgp-msg.asc

I will proceed by investigating it and in parallel fitting the test into some existing test suite. Pythonic one seems to be well-suited for such things, is that right?

ni4 commented 2 years ago

I will proceed by investigating it and in parallel fitting the test into some existing test suite. Pythonic one seems to be well-suited for such things, is that right?

I'd prefer to have FFI API test as well, since people who use library (like Thunderbird) could use approach which differs from a CLI one. Also it would be good to have test for different cases, like alread-preloaded secret keys, keys loaded by demand via key provider, trying multiple secret keys and so on.

andrey-utkin commented 2 years ago

I got it working with the clumsiest hack ever. Cleaning up and refactoring now, will submit into a branch later. The meaning of these hooks becomes less certain with this extension of functionality in mind:

typedef void pgp_on_recipients_func_t(const std::vector<pgp_pk_sesskey_t> &recipients,
                                      const std::vector<pgp_sk_sesskey_t> &passwords,
                                      void *                               param);
typedef void pgp_decryption_start_func_t(pgp_pk_sesskey_t *pubenc,
                                         pgp_sk_sesskey_t *symenc,
                                         void *            param);

on_recipients() gets zeros-filled entries. Making it see the effective secret keys would take some extra care, but it's unclear whether anybody really needs that extra complexity. pgp_decryption_start() is supposed to receive the "pubenc" packet corresponding to the decryption key used. But it will either hold zeros instead of effective key id, or have a patched pubenc which is not verbatim and refers to a key which potentially wasn't in on_recipients() data.

My judgement is that optimal behaviour is

ni4 commented 2 years ago

@andrey-utkin Good news! Agree with your approach: on_recipients() should have original recipient list (so implementation which uses corresponding FFI API may see that original recipient was hidden), and used_recipient should be set to the key data which was used for the decryption.

andrey-utkin commented 2 years ago

This is my branch so far: https://github.com/andrey-utkin/rnp/tree/autkin-1275-decrypt-hidden-to History will be cleaned up for PR submission. FFI test is underway. Will look into "make this feature configurablte via the SecurityContext" next and add tests for that.

ni4 commented 2 years ago

@andrey-utkin Great! Could you please create a draft PR from your branch? It would be easier to discuss changes there (and would run CI including linters and so on)?

andrey-utkin commented 2 years ago

Created a draft PR as requested: https://github.com/rnpgp/rnp/pull/1792