From 310c7ce32f859b31fba54f1654314e7c31bf2d2f Mon Sep 17 00:00:00 2001 From: sandaruwijewardhana Date: Sun, 23 Aug 2026 21:49:15 +0530 Subject: [PATCH] fix(provenance): decode each armored keyring block from its own reader armor.Decode wraps its input in a 100-byte bufio.Reader and may read past the end of a block, so decoding concatenated armored blocks from a single shared reader silently dropped the block that follows, depending on alignment. Slice out each -----BEGIN ... -----END block and decode each from its own reader. Closes #32567 Signed-off-by: sandaruwijewardhana --- pkg/provenance/sign.go | 28 ++++++++++++++++++++++--- pkg/provenance/sign_test.go | 42 +++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/pkg/provenance/sign.go b/pkg/provenance/sign.go index b4d5f7541..141b6177d 100644 --- a/pkg/provenance/sign.go +++ b/pkg/provenance/sign.go @@ -394,12 +394,31 @@ func loadKeyRing(ringpath string) (openpgp.EntityList, error) { // way GnuPG imports them. func loadArmoredKeyRing(data []byte) (openpgp.EntityList, error) { var ring openpgp.EntityList - r := bytes.NewReader(data) + + // armor.Decode wraps its input in a 100-byte bufio.Reader and may read past + // the end of a block ("the given Reader is not usable after calling this + // function: an arbitrary amount of data may have been read past the end of + // the block"). Decoding several concatenated blocks from a single shared + // reader can therefore consume - and silently drop - the block that follows, + // so hand each block its own reader. + rest := data for { - block, err := armor.Decode(r) - if errors.Is(err, io.EOF) { + begin := bytes.Index(rest, []byte("-----BEGIN ")) + if begin < 0 { break } + end := bytes.Index(rest[begin:], []byte("-----END ")) + if end < 0 { + return nil, errors.New("armored keyring is missing an end line") + } + end += begin + if nl := bytes.IndexByte(rest[end:], '\n'); nl >= 0 { + end += nl + 1 + } else { + end = len(rest) + } + + block, err := armor.Decode(bytes.NewReader(rest[begin:end])) if err != nil { return nil, err } @@ -411,7 +430,10 @@ func loadArmoredKeyRing(data []byte) (openpgp.EntityList, error) { return nil, err } ring = append(ring, entities...) + + rest = rest[end:] } + if len(ring) == 0 { return nil, errors.New("no keys found") } diff --git a/pkg/provenance/sign_test.go b/pkg/provenance/sign_test.go index f3e049999..16feead2a 100644 --- a/pkg/provenance/sign_test.go +++ b/pkg/provenance/sign_test.go @@ -25,6 +25,7 @@ import ( "strings" "testing" + "github.com/ProtonMail/go-crypto/openpgp" "github.com/ProtonMail/go-crypto/openpgp/armor" pgperrors "github.com/ProtonMail/go-crypto/openpgp/errors" "github.com/ProtonMail/go-crypto/openpgp/packet" @@ -422,3 +423,44 @@ func readSumFile(sumfile string) (string, error) { parts := strings.SplitN(sig, " ", 2) return parts[0], nil } + +// TestLoadKeyRingArmoredMultiBlockAlignment guards against the armor.Decode +// over-read described in #32567: because armor.Decode buffers ahead and may +// read past the end of a block, decoding concatenated armored blocks from a +// single shared reader silently dropped the block that follows, depending on +// the preceding block's length. Unlike the fixed fixture in +// TestLoadKeyRingArmoredMultiBlock, this sweeps a range of first-block lengths +// so it exercises the alignment rather than passing on it. +func TestLoadKeyRingArmoredMultiBlockAlignment(t *testing.T) { + base, err := os.ReadFile(testArmoredPubfile) + require.NoError(t, err) + if !bytes.HasSuffix(base, []byte("\n")) { + base = append(base, '\n') + } + + for namePadding := 0; namePadding <= 24; namePadding++ { + keyring := append(append([]byte{}, armoredTestPublicKey(t, namePadding)...), base...) + + ring, err := loadArmoredKeyRing(keyring) + require.NoErrorf(t, err, "namePadding=%d", namePadding) + require.Lenf(t, ring, 2, "namePadding=%d: a concatenated key block was dropped", namePadding) + } +} + +// armoredTestPublicKey generates a throwaway Ed25519 key whose UID name is padded +// with namePadding extra characters, and returns its ASCII-armored public key +// block, newline terminated like `gpg --export --armor`. Varying namePadding +// varies the length of the encoded block, which is what the alignment sweep needs. +func armoredTestPublicKey(t *testing.T, namePadding int) []byte { + t.Helper() + entity, err := openpgp.NewEntity(strings.Repeat("a", namePadding)+" Example", "", "test@example.com", + &packet.Config{Algorithm: packet.PubKeyAlgoEdDSA}) + require.NoError(t, err) + + var buf bytes.Buffer + w, err := armor.Encode(&buf, openpgp.PublicKeyType, nil) + require.NoError(t, err) + require.NoError(t, entity.Serialize(w)) + require.NoError(t, w.Close()) + return append(buf.Bytes(), '\n') +}