Subject: [Openpgp-dt] SEDfail and open questions

From: Justus Winter <[email-masked]>
Date: Wed, 13 Oct 2021 17:41:54 +0200

Hello,

tl;dr: I think SED support is broken in many implementations, including
GnuPG, and this creates a accidental compatibility with SEIPD,
exacerbating the Efail problem.

I was in the process of writing a stern warning against the
Symmetrically Encrypted Data (SED) packet.  I always thought that you
could convert a Symmetrically Encrypted Integrity Protected Data (SEIPD)
packet into a SED packet by changing the header and stripping off the
MDC packet.  However, I don't think that is true, with a caveat.

The reason why you cannot (at least trivially) convert a SEIPD packet
into a SED packet is that the SED packet uses CFB with a special
re-synchronization step (see [0]) after the initialization, whereas
SEIPD does not do this step.  Therefore, the ciphers modes should not be
compatible.

0: https://openpgp-wg.gitlab.io/rfc4880bis/#name-openpgp-cfb-mode

Then, I wanted to confirm that my reading of the RFC wrt the
re-synchronization step matches what is actually implemented.  I set out
to read some implementations.

For GnuPG, this is handled in libgcrypt by the function cipher_sync.  It
is activated using the flag GCRY_CIPHER_ENABLE_SYNC.  However, I don't
think passing this flag to gcry_cipher_open actually causes the sync
operation to be executed.  I have written a program that seems to
support this (see attachment):

cargo run
   Compiling gcrypt-cfb-sync-test v0.1.0 (/home/teythoon/repos/pep/tmp/gcrypt-cfb-sync-test)
    Finished dev [unoptimized + debuginfo] target(s) in 0.26s
     Running `/home/teythoon/repos/pep/tmp/gcrypt-cfb-sync-test/target/debug/gcrypt-cfb-sync-test`
Encrypting without sync, decrypting without sync... expecting success, decryption successful.
Encrypting without sync, decrypting with sync... expecting failure, decryption successful.
Encrypting with sync, decrypting without sync... expecting failure, decryption successful.
Encrypting with sync, decrypting with sync... expecting success, decryption successful.

It follows that GnuPG (and dkgpg which also uses libgcrypt) does not
correctly implement SED, but a SED variant that does not do the CFB
re-synchronization step.

Reading OpenPGP.js, I don't see the re-synchronization either.

Reading RNP, I see that they are doing the re-synchronization.

What's the takeaway?  If SED is broken in GnuPG and noone complained,
that likely means that SED is no longer in use, and that is a good
thing.  However, the accidental compatibility allows the conversion of
SEIPD packets into SED packets, paving the way for Efail-style attacks
on implementations willing to decrypt SED packets via the packet
downgrade.

Please check my findings.

Justus

---

From: Justus Winter <[email-masked]>
Date: Thu, 14 Oct 2021 10:22:45 +0200

NIIBE Yutaka <[email-masked]> writes:

> Justus Winter <[email-masked]> wrote:
>> operation to be executed.
>
> A user need to call gcry_cipher_sync (actually, it's a macro):
>
>  -- Function: gcry_error_t gcry_cipher_sync (gcry_cipher_hd_t H)
>
>      Perform the OpenPGP sync operation on context H.  Note that this is
>      a no-op unless the context was created with the flag
>      'GCRY_CIPHER_ENABLE_SYNC'
> -- 

Indeed.  I see that being invoked in GnuPG and dkgpg, so they are fine.
Sorry for the noise.  What about OpenPGP.js?

Justus

---

From: Daniel Huigens <[email-masked]>
Date: Thu, 14 Oct 2021 09:09:59 +0000

Hello :)

Regarding OpenPGP.js, I believe we do implement it correctly, but
(when encrypting) instead of resynchronizing the CFB instance, we just
create a new one; that's why there's two calls to `cfb.encrypt` [0].
Due to the way CFB mode works, we can pass the last `blocksize` bytes of
ciphertext as the IV to the second CFB instance, and it produces the
same result.

For decryption, I realized at some point that resynchronization
(or having two instances) is technically not required; instead we can
start directly with decrypting `ciphertext.subarray(blocksize + 2)`
using `ciphertext.subarray(2, blocksize + 2)` as the IV, for a similar
reason. This means we never decrypt the first `blocksize + 2` bytes,
but they decrypt to random bytes anyway, so we don't care about them.

(This allows us to use an existing library for CFB mode without
modification for both SED and SEIPD packets, despite the difference.)

That being said, converting an SEIPD packet to an SED packet should
still be possible, by inserting two bytes at the start of the first
block, as described by the EFAIL paper which in turn references [1].

In OpenPGP.js, decrypting SED packets has been behind a config flag
(disabled by default) for a long time, and a cursory GitHub search
seems to indicate that not many people use it, so that would support
removing it entirely. In OpenPGP.js v5, we've also removed the config
option to generate it entirely, so there's no straightforward way to do
so (without creating packets manually) even though the code is still
there. I would support MUST NOT generate and also MUST NOT decrypt
(or SHOULD NOT decrypt if there would not be consensus for that).

Best,
Daniel

[0]: https://github.com/openpgpjs/openpgpjs/blob/master/src/packet/symmetrically_encrypted_data.js#L111-L112
[1]: https://www.metzdowd.com/pipermail/cryptography/2015-October/026685.html

---

From: Justus Winter <[email-masked]>
Date: Thu, 14 Oct 2021 12:04:27 +0200

> modification for both SED and SEIPD packets, despite the difference.)

Ah yes, I can see it now.

> [1]: https://www.metzdowd.com/pipermail/cryptography/2015-October/026685.html

That is a great pointer, thanks.

> (or SHOULD NOT decrypt if there would not be consensus for that).

FTR, in Sequoia we didn't implement support for SED packets.  We may do
so in the future, because we want to be able to produce tools to decrypt
archived messages.  When we do, our policy mechanism will prevent the
use of SED packets unless the user explicitly opted into that.

Thanks unconfusing me!

Justus