On 7/6/26 4:54 PM, Clint Adams wrote:
On Mon, Jul 06, 2026 at 04:36:49PM +0200, Uwe Kleine-König wrote:
Packet ordering bases on comparing hashes
(Codec/Encryption/OpenPGP/Types.hs, `instance Ord Pkt`) and canonicalize

So technically the bug is in (or also in) hOpenPGP, but that's not
important for this discussion.

sorts using that Ord instance. Now hashable changed byte hashing in
https://github.com/haskell-unordered-containers/hashable/pull/301 and
thus canonicalization changed.

IMHO canonicalization shouldn't depend on an (unstable) hashing
algorithm. It should at least be documented that canonicalization is
unstable, or better, canonicalization should be fixed to be independent
of hashing.

I agree, and there's a FIXME comment about it in hOpenPGP's
Codec/Encryption/OpenPGP/Types/Internal/Pkt.hs , but I don't have a good
solution.  Do you have an idea for a stable mechanism that would do
the right thing?

While guided with claude (with the Fable 5 model) to find the issue, it also suggested me a way to fix it and implemented it with its . I don't speak haskell so I let you judge it as I'm unable to do it. I don't want to contribute to the AI slop things without the help of the tool to fix the issue so you'll find the patch and a script to test it in the attachements.

The patchs land in hopenpgp-tools on top of master 21d80c8e40cd.
From f38aef8eb2a0c925856a8a2f426742a9300af8ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Le=20Goffic?= <[email protected]>
Date: Mon, 6 Jul 2026 18:24:50 +0200
Subject: [PATCH 1/2] hokey canonicalize: make output independent of the
 hashable version

The canonical order produced by `hokey canonicalize` came from
hOpenPGP's Ord instances, whose tie-breaker is Data.Hashable.hash
(hOpenPGP commit 624a0466fd9f, "Ord instance for Pkt").  hashable
documents its hash as unstable, and hashable 1.4.5.0 switched
ByteString hashing from FNV to XXH3, silently changing the sort
order.  As a result two builds of the same hokey version produce
different "canonical" output depending on the hashable they were
compiled against (e.g. Debian trixie's 1.4.4.0 vs sid's 1.5.0.0),
defeating the purpose of canonicalization.

Sort on data that depends only on the input instead:

 * subkey packets: packet tag, then serialized wire bytes
   (previously: packet tag, then hash);
 * signature lists: keep the semantic order (creation time, type,
   issuer) and add the serialized wire bytes as a total tie-breaker,
   making the ordering deterministic where it previously fell back
   to input order;
 * user IDs and user attributes: unchanged semantic order, with the
   same tie-breakers for full determinism.

No new dependencies: Data.Binary and the Binary instances are
already in use for the conduit pipeline.

Tested by building the patched package on Debian trixie
(hashable 1.4.4.0, FNV) and sid (hashable 1.5.0.0, XXH3): the two
binaries now produce byte-identical, idempotent, packet-preserving
output for a 15-certificate keyring where unpatched builds diverge.

Closes: https://bugs.debian.org/1141576

Co-Authored-By: Claude Fable 5 <[email protected]>
---
 hokey.hs | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/hokey.hs b/hokey.hs
index 72043cf9ce9c..a3af2650a578 100644
--- a/hokey.hs
+++ b/hokey.hs
@@ -42,7 +42,8 @@ import Control.Monad.Trans.Writer.Lazy (execWriter, tell)
 import qualified Crypto.Hash as CH
 import qualified Crypto.Hash.Algorithms as CHA
 import qualified Data.Aeson as A
-import Data.Binary (get, put)
+import Data.Binary (Binary, get, put)
+import Data.Binary.Put (runPut)
 import qualified Data.ByteArray as BA
 import qualified Data.ByteString as B
 import qualified Data.ByteString.Base16 as Base16
@@ -54,10 +55,10 @@ import qualified Data.Conduit.List as CL
 import Data.Conduit.OpenPGP.Keyring (conduitToTKsDropping)
 import Data.Conduit.Serialization.Binary (conduitGet, conduitPut)
 import Data.Foldable (find)
-import Data.List (elemIndex, findIndex, intercalate, nub, sort, sortOn)
+import Data.List (elemIndex, findIndex, intercalate, nub, sort, sortBy, sortOn)
 import qualified Data.Map as Map
 import Data.Maybe (fromMaybe, listToMaybe, mapMaybe)
-import Data.Ord (Down(..))
+import Data.Ord (Down(..), comparing)
 import Data.Semigroup (Semigroup, (<>))
 import qualified Data.Set as Set
 import Data.Text (Text)
@@ -752,10 +753,32 @@ doCanonicalize =
   conduitPut .|
   CB.sinkHandle stdout
   where
+    -- The sort order must not depend on hash-based Ord instances:
+    -- Data.Hashable.hash is not stable across hashable releases (e.g.
+    -- FNV -> XXH3 for bytestrings in hashable-1.4.5.0), so Ord Pkt made
+    -- the output of `hokey canonicalize` differ between builds linking
+    -- different hashable versions.  Sort by packet tag and serialized
+    -- wire bytes instead, and use the wire bytes as a total tie-breaker
+    -- for the semantic signature ordering.
     canonicalize (TK k r ui ua s) =
-      TK k (sort r) (indepthsort ui) (indepthsort ua) (indepthsort s)
-    indepthsort :: (Ord a, Ord b) => [(a, [b])] -> [(a, [b])]
-    indepthsort = nub . sort . over (mapped . _2) sort
+      TK k
+        (sigsort r)
+        (indepthsort compare ui)
+        (indepthsort compare ua)
+        (indepthsort (\a b -> comparing pktTag a b <> comparing ser a b) s)
+    ser :: Binary a => a -> BL.ByteString
+    ser = runPut . put
+    sigsort :: [SignaturePayload] -> [SignaturePayload]
+    sigsort = sortBy (\a b -> compare a b <> comparing ser a b)
+    indepthsort ::
+         Eq a
+      => (a -> a -> Ordering)
+      -> [(a, [SignaturePayload])]
+      -> [(a, [SignaturePayload])]
+    indepthsort cmpa =
+      nub .
+      sortBy (\x y -> cmpa (fst x) (fst y) <> comparing (map ser . snd) x y) .
+      over (mapped . _2) sigsort
 
 doFetch :: FetchOptions -> IO ()
 doFetch o = do
-- 
2.55.0

From 8db7e5ab8cbd51e4cbe1d0bf93329eb4a5dcc812 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Le=20Goffic?= <[email protected]>
Date: Mon, 6 Jul 2026 20:52:33 +0200
Subject: [PATCH 2/2] Add container-based validation script for canonicalize
 determinism

validate-hokey-canonicalize.sh reproduces Debian bug #1141576 and
validates the fix in the previous commit, using throwaway Debian
containers (docker or podman, auto-detected or forced via ENGINE):

 1. reproduce the bug: the stock hopenpgp-tools packages from trixie
    (hashable 1.4.4.0, FNV) and sid (hashable 1.5.0.0, XXH3) produce
    different output for the same input certificate;
 2. build the patched package in both suites, taking the patch from
    the most recent commit touching hokey.hs;
 3. verify the patched binaries produce byte-identical output across
    suites, idempotent and packet-preserving.

A test certificate (1 primary key + 3 subkeys) is generated inside a
container when no certificate is supplied on the command line.

Co-Authored-By: Claude Fable 5 <[email protected]>
---
 validate-hokey-canonicalize.sh | 157 +++++++++++++++++++++++++++++++++
 1 file changed, 157 insertions(+)
 create mode 100755 validate-hokey-canonicalize.sh

diff --git a/validate-hokey-canonicalize.sh b/validate-hokey-canonicalize.sh
new file mode 100755
index 000000000000..8bba71e6c481
--- /dev/null
+++ b/validate-hokey-canonicalize.sh
@@ -0,0 +1,157 @@
+#!/bin/sh
+# validate-hokey-canonicalize.sh — reproduce Debian bug #1141576 and validate
+# the "make output independent of the hashable version" patch.
+#
+# The bug: `hokey canonicalize` sorts packets via hOpenPGP's Ord instances,
+# whose tie-breaker is Data.Hashable.hash.  hashable 1.4.5.0 switched byte
+# hashing from FNV to XXH3, so builds linking hashable < 1.4.5 (Debian trixie)
+# and >= 1.4.5 (Debian sid) produce different "canonical" output for the same
+# input certificate.
+#
+# This script, using throwaway Debian containers:
+#   1. reproduces the divergence with the stock hopenpgp-tools packages
+#      from trixie (hashable 1.4.4.0, FNV) and sid (hashable 1.5.0.0, XXH3);
+#   2. builds the patched package in both suites (the patch is taken from
+#      the HEAD commit of this repository via `git format-patch`);
+#   3. verifies that both patched binaries produce byte-identical and
+#      idempotent output, with no packet lost.
+#
+# Usage:
+#   [ENGINE=docker|podman] ./validate-hokey-canonicalize.sh [certificate...]
+#
+# With no argument a fresh test certificate (1 primary key + 3 subkeys) is
+# generated inside a container.  Note: step 1 is probabilistic for an
+# arbitrary certificate — the FNV and XXH3 orders can coincide by chance;
+# steps 2-3 (the actual validation) are deterministic.
+#
+# Requires: docker or podman, network access.  Runtime: ~10-20 min
+# (two package builds).  Everything happens in a temp dir printed at start.
+
+set -eu
+
+if [ -n "${ENGINE:-}" ]; then
+	:
+elif command -v podman >/dev/null 2>&1; then
+	ENGINE=podman
+elif command -v docker >/dev/null 2>&1; then
+	ENGINE=docker
+else
+	echo "error: neither podman nor docker found" >&2
+	exit 1
+fi
+
+SUITES="trixie sid"
+REPO_DIR="$(cd "$(dirname "$0")" && pwd)"
+WORK="$(mktemp -d)"
+echo "== engine: $ENGINE, workdir: $WORK"
+
+run() { # run SUITE SCRIPT
+	"$ENGINE" run --rm -e DEBIAN_FRONTEND=noninteractive -e TERM=dumb \
+	    -v "$WORK":/work -w /work "debian:$1" sh -ec "$2"
+}
+
+# --- step 0: patch and input certificate --------------------------------
+
+fixcommit="$(git -C "$REPO_DIR" log -1 --format=%H -- hokey.hs)"
+git -C "$REPO_DIR" format-patch -1 --stdout "$fixcommit" > "$WORK/fix.patch"
+echo "== patch: $(git -C "$REPO_DIR" log --format=%s -1 "$fixcommit") ($(wc -l < "$WORK/fix.patch") lines)"
+
+if [ $# -gt 0 ]; then
+	i=0
+	for f in "$@"; do
+		i=$((i + 1))
+		cp "$f" "$WORK/cert$i.pgp"
+	done
+	echo "== using $i user-supplied certificate(s)"
+else
+	echo "== generating a test certificate (1 primary + 3 subkeys)"
+	run trixie '
+		apt-get update -qq && apt-get install -y -qq gnupg >/dev/null
+		export GNUPGHOME=$(mktemp -d)
+		gpg -q --batch --pinentry-mode loopback --passphrase "" \
+		    --quick-gen-key "Test User <[email protected]>" ed25519 cert 0
+		fpr=$(gpg --with-colons --list-keys | awk -F: "/^fpr/ {print \$10; exit}")
+		for algo in ed25519 cv25519 nistp256; do
+			gpg -q --batch --pinentry-mode loopback --passphrase "" \
+			    --quick-add-key "$fpr" "$algo" "" 0
+		done
+		gpg --export "$fpr" > /work/cert1.pgp
+	'
+fi
+
+# --- step 1: reproduce the bug with stock hokey --------------------------
+
+for suite in $SUITES; do
+	echo "== [$suite] canonicalizing with stock hopenpgp-tools"
+	run "$suite" '
+		apt-get update -qq && apt-get install -y -qq hopenpgp-tools >/dev/null
+		for c in cert*.pgp; do
+			hokey canonicalize < "$c" 2>/dev/null > "$c.stock-'"$suite"'"
+		done
+	'
+done
+
+bug=0
+for c in "$WORK"/cert*.pgp; do
+	if ! cmp -s "$c.stock-trixie" "$c.stock-sid"; then
+		bug=1
+	fi
+done
+if [ $bug -eq 1 ]; then
+	echo "== BUG REPRODUCED: stock trixie and sid outputs differ"
+else
+	echo "== note: stock outputs happen to agree for this input (possible" \
+	     "by chance); the validation below is still meaningful"
+fi
+
+# --- step 2: build and run the patched package in both suites ------------
+
+for suite in $SUITES; do
+	echo "== [$suite] building patched hopenpgp-tools (~5-10 min)"
+	run "$suite" '
+		sed -i "s/^Types: deb$/Types: deb deb-src/" \
+		    /etc/apt/sources.list.d/debian.sources
+		apt-get update -qq
+		apt-get install -y -qq dpkg-dev build-essential fakeroot gnupg \
+		    >/dev/null 2>&1
+		apt-get build-dep -y -qq hopenpgp-tools >/dev/null 2>&1
+		mkdir /build && cd /build
+		apt-get source hopenpgp-tools >/dev/null 2>&1
+		cd haskell-hopenpgp-tools-*
+		patch -p1 < /work/fix.patch
+		dpkg-buildpackage -us -uc -b > /work/build-'"$suite"'.log 2>&1
+		dpkg -i /build/hopenpgp-tools_*.deb >/dev/null 2>&1
+		cd /work
+		for c in cert*.pgp; do
+			hokey canonicalize < "$c" 2>/dev/null > "$c.patched-'"$suite"'"
+			# idempotency: canonicalizing twice must be a no-op
+			hokey canonicalize < "$c.patched-'"$suite"'" 2>/dev/null \
+				| cmp -s - "$c.patched-'"$suite"'"
+			# packet preservation: same number of packets in and out
+			in=$(gpg --list-packets "$c" 2>/dev/null | grep -c "^:")
+			out=$(gpg --list-packets "$c.patched-'"$suite"'" 2>/dev/null \
+				| grep -c "^:")
+			[ "$in" = "$out" ]
+		done
+	'
+done
+
+# --- step 3: verdict ------------------------------------------------------
+
+fail=0
+for c in "$WORK"/cert*.pgp; do
+	if cmp -s "$c.patched-trixie" "$c.patched-sid"; then
+		echo "== PASS: $(basename "$c"): patched outputs are byte-identical"
+	else
+		echo "== FAIL: $(basename "$c"): patched outputs differ"
+		fail=1
+	fi
+done
+
+if [ $fail -eq 0 ]; then
+	echo "== RESULT: PASS — patched hokey canonicalize is independent of" \
+	     "the hashable version (and idempotent, packet-preserving)"
+else
+	echo "== RESULT: FAIL — see $WORK"
+fi
+exit $fail
-- 
2.55.0

Reply via email to