search

rnpgp / rnp

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

rnp processes unauthenticated partial chunks of AEAD encrypted data packets #807

Open teythoon opened 5 years ago

teythoon commented 5 years ago

Description

First, rnp still lacks a way to responsibly disclose problems with their code.

Second, rnp processes unauthenticated partial chunks of AEAD encrypted data packets.

Steps to Reproduce

  1. Import this key: https://gitlab.com/sequoia-pgp/sequoia/raw/master/openpgp/tests/data/keys/testy-private.pgp?inline=false

  2. Dearmor this file:

-----BEGIN PGP ARMORED FILE-----

QlpoOTFBWSZTWYdZK6YCAm3//+v25z/zON3+7fbbpu9au9Lvu7vDbHGbv/uvVuJb
3crusAEbamyA00Bo0A0aAAGnqNAaA00wg0NDQBoYQaNBoDEHqaNManqBtJkHpBmU
9NI9BCqmjQaaBo0YjQZANBoNDRpo0YmmgAxAGhoMmgDAT0QGmBGjBGTAEZMjCVKg
AANAAAAaAGgNAAAAAAAAAAAAAAAADQyDEaf4AAEAAEEvIEAAIAAGRUuoAAEAAAAA
Fj+0P/kYY/hXJimJSnwxH01b5wjoxb8WdkQM/OI+OKXvenBLrp7NTFDSmYd0jdPJ
5qoyGQOegWiG9/AERTPt5GhPYUg7tsKgW4n3L2MPmNv9gVsk9opSn4Myu/PNIYgH
eWVIJ81P8Cd/a47dh3YEoJVFb8lsVjzckImjKvB8nQMOeoLtSIwwBUXexStMzzc9
Qi6UljgciNAjLa454cJxEei+CT9UVoQWB5VAHwMICJfNqZ2tox3E9BlUr9zB4ypJ
G8J9B0/KCwmngrInWI0yDML6wxlzNYsbtTj1QMCo9XxwUKbVMlvWX5S/SpWp+8br
5lM8m/FaxItXAAAQAALFqTXdyxdyRThQkIdZK6Y=
=pbek
-----END PGP ARMORED FILE-----
  1. Decrypt the message.

Expected Behavior

rnp MUST NOT process unauthenticated data.

Actual Behavior

rnp processes unauthenticated data:

% ../pgpzoo/rnp/src/rnp/rnp --decrypt --homedir=$GNUPGHOME exachunks.pgp
[init_packet_sequence() /home/teythoon/repos/pep/pgpzoo/rnp/src/librepgp/stream-parse.cpp:2147] unexpected pkt 60
[rnp_process_file() /home/teythoon/repos/pep/pgpzoo/rnp/src/rnp/rnpcli.cpp:987] error 0x10000001

Note the "unexpected pkt 60". This changes with the "session key" I choose to "decrypt" the stream of zeros.

Impact

Processing unauthenticated data negates the security benefits of doing AEAD.

What you can do

dewyatt commented 5 years ago

I had some trouble with the provided data, but I was able to confirm this behavior.

$ gpg --output - --dearmor exachunks.asc | bzip2 -d | src/rnp/rnp --decrypt
[init_packet_sequence() /tmp/rnp/src/librepgp/stream-parse.cpp:2147] unexpected pkt 60
[rnp_process_file() /tmp/rnp/src/rnp/rnpcli.cpp:987] error 0x10000001
ni4 commented 5 years ago

Yeah, currently AEAD works this way since it handles all sizes of chunks specified in RFC4880-bis, which may be too large to cache somewhere in memory. I know there is an ongoing discussion in RFC working group regarding AEAD chunk size, so probably good idea is to wait till it ends with some decision. While we can cache chunk of small size and output larger ones this will not be a 100% correct solution.

teythoon commented 5 years ago

If you think that there is now way to safely implement AEAD as proposed in RFC4880-bis06, please speak up on the IETF mailing list, because your current position there is

You can say, MUST support 1K to 16K, SHOULD support up to 256K and MAY support larger sizes.

rrrooommmaaa commented 4 years ago

@ni4 Can I take this one or are there more prioritized?

ni4 commented 4 years ago

@rrrooommmaaa As for me this one is a bit of lower priority and still requires some discussion on implementation logic. As for me the simplest and the most urgent one is #1103, then #1099, and if you like Windows stuff - very important but a way more time-consuming would be #997.