On Thu 2024-05-30 18:34:13 +0200, Andreas Metzler wrote: > the issue report refers to two patches, one of these is already part of > 2.2.43. The other one[1] seemed pretty straightforward to backport > (functions moved to other files and cipher_filter_ocb instead of > cipher_filter_aead). I would appreciate a second pair of eyes.
I wish the patch wasn't so large, even if it appears to backport
cleanly!
I confess i don't fully understand the logic around iobufs and
I'm also more than a little distressed that the bulk of this logic and
buffer fiddling seems to be about deciding whether to compress the input
stream or not, on the basis of whether it's using SEPIDv1 (or LibrePGP's
non-standard OCB mode) vs. the deprecated, obsolete SED.
Nothing should be generating SED packets today, in 2024. The idea that
the compression layer is going to defend against chosen ciphertext
attacks is… lacking in cryptographic rigor.
Furthermore, the idea that GnuPG will (or will not) add a compression
layer depending on the first 5 bytes of the input stream is pretty
strange. Consider this deeply weird comparison, where depending on the
input data, the same GnuPG pipeline will compress or not compress:
```
0 dkg@alice:~$ packetlist() { printf '%s\n' "$1" | gpg --compress-algo=ZIP
--encrypt -r "$PGPID" | gpg --unwrap --decrypt | gpg --list-packets; }
0 dkg@alice:~$ diff -u <(packetlist %PDFx) <(packetlist %PDF-)
gpg: encrypted with 255-bit ECDH key, ID 38024D718ABA3F3B, created 2023-12-06
"Daniel Kahn Gillmor"
gpg: encrypted with 255-bit ECDH key, ID 38024D718ABA3F3B, created 2023-12-06
"Daniel Kahn Gillmor"
--- /dev/fd/63 2024-05-31 17:08:37.339457042 -0400
+++ /dev/fd/62 2024-05-31 17:08:37.339457042 -0400
@@ -1,6 +1,4 @@
-# off=0 ctb=a3 tag=8 hlen=1 plen=0 indeterminate
-:compressed packet: algo=1
-# off=2 ctb=cb tag=11 hlen=2 plen=12 new-ctb
+# off=0 ctb=cb tag=11 hlen=2 plen=12 new-ctb
:literal data packet:
mode b (62), created 1717189717, name="",
raw data: 6 bytes
1 dkg@alice:~$
```
Seems to me like the approach that would reduce the overall amount of
complexity is to have an explicit default that does not vary choice of
compression algorithm, based on the choice of incoming cleartext, and
then to actually respect any user-specified --compress-algo, which
doesn't seem to be happening when the first five octets of the input
stream match this magic list.
Of course, i'd be happy to just rip compression out of the OpenPGP
specification entirely anyway -- people who want to compress before
encrypting have many different ways of doing it irrespective of the
OpenPGP spec.
I think we should include an autopkgtest for the gnupg2 source package
as well that explicitly ensures that future updates don't break epg.el.
I'm including it here, but i'll also put it in the autopkgtests to
ensure we don't hit this regression again.
I propose to push this test plus your patch into salsa later today and
if it passes cleanly, i'll go ahead with the upload.
--dkg
#!/bin/sh set -e # Author: Daniel Kahn Gillmor <d...@fifthhorseman.net> # Emacs has epg mode, which expects a certain behavior from GnuPG. # # GnuPG upstream doesn't think that it is a bug to break those # expectations (see https://dev.gnupg.org/T6481) but we want to avoid # having those problems in debian. (see # https://bugs.debian.org/1071552) # This test ensures that a simple attempt to send signed, encrypted # PGP/MIME mail with emacs doesn't break with the current version of # GnuPG. WORKDIR="$(mktemp -d)" mkdir -m 0700 "$WORKDIR/a" "$WORKDIR/b" "$WORKDIR/out" OUTDIR="${AUTOPKGTEST_ARTIFACTS:-$WORKDIR/out}" cleanup() { find "$WORKDIR/out" -type f -print0 | xargs --null --no-run-if-empty -- head -v -n100 printf "Cleaning up working directory '%s'\n" "$WORKDIR" rm -rf "$WORKDIR" } trap cleanup EXIT GPG() { home=$1 shift gpg --quiet --homedir "$WORKDIR/$home" --batch --pinentry-mode=loopback --passphrase='' --no-tty --status-file="$WORKDIR/status" "$@" } GPG a --quick-gen-key "Alice <al...@example.org>" ALICE_FPR=$(grep KEY_CREATED "$WORKDIR/status" | cut -f4 -d\ ) GPG b --quick-gen-key "Bob <b...@example.net>" BOB_FPR=$(grep KEY_CREATED "$WORKDIR/status" | cut -f4 -d\ ) GPG b --export "Bob <b...@example.net>" | GPG a --import # Alice certifies Bob's certificate locally so that her GnuPG installation knows it is valid: GPG a --quick-lsign-key "$BOB_FPR" "Bob <b...@example.net>" cat > "$WORKDIR/mailscript.el" <<EOF (let ((user-mail-address "al...@example.org") (user-full-name "Alice") (mail-host-address "example.com")) (message-mail "Bob <b...@example.net>" "this is a test") (message-goto-body) (insert "we need to see whether easypg can handle encryption (see https://dev.gnupg.org/T6481)") (mml-secure-message-sign-encrypt) (let ((mml-secure-openpgp-sign-with-sender t) (message-send-mail-function (lambda () (write-region nil nil "sample.eml")))) (message-send-and-exit)) ) EOF # emacs message mode is chatty to stderr even on success, so we store it to keep autopkgtest happy. (cd "$WORKDIR" && TERM=dumb GNUPGHOME="$WORKDIR/a" timeout 10s emacs --quick --no-window-system --script mailscript.el 2> "$OUTDIR/stderr") head -v -n 100 "$OUTDIR/stderr" cp "$WORKDIR/sample.eml" "$OUTDIR/" GPG a --output "$OUTDIR/alice.key" --armor --export-secret-key "$ALICE_FPR" GPG b --output "$OUTDIR/bob.key" --armor --export-secret-key "$BOB_FPR"
signature.asc
Description: PGP signature
