Merge 310c7ce32f into 62c7e845d1

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")
 	}

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')
+}