Limit how much fetch and check read for a manifest or a file (closes #168)

NewManifestFromReader reads at most one byte past MaxManifestSize, a new
constant of 258 MiB: the 256 MiB decompressed limit grown by zstd's worst
case of 1/256, plus 1 MiB for the signature, the signing key and the
other outer fields. It refuses a larger manifest. fetch, and check given
a URL, stop downloading a manifest one byte past the same size and report
it as too large; tests lower that size to keep their memory small. fetch
stops reading a file one byte past its listed size, so a longer body ends
in the size mismatch at once instead of filling the disk. docs/FORMAT.md
states the limit and gives the decompressed limit as 256 MiB, the size
the code uses.

Model: opus-5-5

docs/FORMAT.md

@@ -37,7 +37,7 @@ The outer message contains:
 | `uuid`            | 105    | bytes            | Random v4 UUID; must match the inner message UUID                        |
 | `innerMessage`    | 199    | bytes            | Zstd-compressed serialized `MFFile` message                              |
 | `signature`       | 201    | bytes (optional) | GPG signature (ASCII-armored or binary)                                  |
-| `signer`          | 202    | bytes (optional) | Full GPG key ID of the signer                                            |
+| `signer`          | 202    | bytes (optional) | Fingerprint of the signing key                                           |
 | `signingPubKey`   | 203    | bytes (optional) | Full GPG signing public key                                              |
 
 ### SHA-256 Hash
@@ -50,11 +50,14 @@ allows verifying data integrity before decompression.
 The `innerMessage` field is compressed with
 [Zstandard (zstd)](https://facebook.github.io/zstd/). Implementations must
 enforce a decompression size limit to prevent decompression bombs. The reference
-implementation limits decompressed size to 256 MB. It writes zstd frames with a
+implementation limits decompressed size to 256 MiB. It writes zstd frames with a
 window of at most 8 MiB, the largest window the zstd format recommends decoders
 support, and refuses frames that ask for a larger one. It also refuses an inner
 message whose file entries, hashes, timestamps and MIME types, counted at 176,
-112, 64 and 16 bytes each, add up to more than 8 times its size.
+112, 64 and 16 bytes each, add up to more than 8 times its size. It refuses a
+manifest file larger than 258 MiB without reading the rest of it: zstd's worst
+case grows a 256 MiB inner message by 1/256 to 257 MiB, and the last MiB is room
+for the signature, the signing key and the other outer fields.
 
 ## Inner Message (`MFFile`)
 
@@ -137,7 +140,16 @@ Where:
   compressed data)
 
 Components are separated by hyphens. The signature is produced by GPG over this
-canonical string and stored in the `signature` field of the outer message.
+canonical string and stored in the `signature` field of the outer message. The
+signing key's public key goes in `signingPubKey` and its fingerprint, in hex, in
+`signer`.
+
+A verifier accepts a signed manifest only if `signingPubKey` holds exactly one
+primary key, `signature` is one good signature over the canonical string made by
+that key (or one of its subkeys), and `signer` is that key's fingerprint. The
+reference implementation refuses to load a manifest that fails these checks;
+`check` and `fetch` given `--require-signature` then compare the required
+fingerprint with `signer`.
 
 ## Deterministic Serialization
 

internal/cli/check.go

@@ -112,10 +112,17 @@ func (mfa *CLIApp) fetchManifestToTemp(
 	}
 
 	tmpPath := tmpFile.Name()
-	_, cpErr := io.Copy(tmpFile, rc)
+
+	// Copying stops one byte past mfa.maxManifestSize, which is enough to
+	// tell that the manifest is too large.
+	written, cpErr := io.Copy(tmpFile, io.LimitReader(rc, mfa.maxManifestSize+1))
 	_ = rc.Close()
 	_ = tmpFile.Close()
 
+	if cpErr == nil && written > mfa.maxManifestSize {
+		cpErr = fmt.Errorf("%w of %d bytes", errManifestTooLarge, mfa.maxManifestSize)
+	}
+
 	if cpErr != nil {
 		_ = mfa.Fs.Remove(tmpPath)
 
@@ -126,10 +133,8 @@ func (mfa *CLIApp) fetchManifestToTemp(
 }
 
 // verifyRequiredSigner enforces the --require-signature fingerprint
-// against the manifest's embedded signing key.
+// against the key that made the manifest's signature.
-func verifyRequiredSigner(
+func verifyRequiredSigner(chk *mfer.Checker, requiredSigner string) error {
-	ctx context.Context, chk *mfer.Checker, requiredSigner string,
-) error {
 	// Validate fingerprint format: must be exactly 40 hex characters
 	if len(requiredSigner) != fingerprintHexLen {
 		return fmt.Errorf("%w, got %d", errInvalidFingerprint, len(requiredSigner))
@@ -145,22 +150,17 @@ func verifyRequiredSigner(
 			errManifestNotSigned, requiredSigner)
 	}
 
-	// Extract fingerprint from the embedded public key (not from the
+	// Loading the manifest checked that the signer is the fingerprint of
-	// signer field). This validates the key is importable and gets its
+	// the key that made the signature.
-	// actual fingerprint.
+	signer := string(chk.Signer())
-	embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP(ctx)
-	if err != nil {
-		return fmt.Errorf(
-			"failed to extract fingerprint from embedded signing key: %w", err)
-	}
 
 	// Compare fingerprints - must be exact match (case-insensitive)
-	if !strings.EqualFold(embeddedFP, requiredSigner) {
+	if !strings.EqualFold(signer, requiredSigner) {
 		return fmt.Errorf("embedded signing key fingerprint %s %w %s",
-			embeddedFP, errSignerMismatch, requiredSigner)
+			signer, errSignerMismatch, requiredSigner)
 	}
 
-	log.Infof("manifest signature verified (signer: %s)", embeddedFP)
+	log.Infof("manifest signature verified (signer: %s)", signer)
 
 	return nil
 }
@@ -330,7 +330,7 @@ func (mfa *CLIApp) checkManifestOperation(
 	// Check signature requirement
 	requiredSigner := cmd.String(flagRequireSignature)
 	if requiredSigner != "" {
-		err = verifyRequiredSigner(ctx, chk, requiredSigner)
+		err = verifyRequiredSigner(chk, requiredSigner)
 		if err != nil {
 			return err
 		}

internal/cli/entry.go

@@ -5,6 +5,7 @@ import (
 	"os"
 
 	"github.com/spf13/afero"
+	"sneak.berlin/go/mfer/mfer"
 )
 
 // NoColor disables colored output when set. Automatically true if the
@@ -56,14 +57,15 @@ func Run(appname, version, gitrev string) int {
 // RunWithOptions creates and runs the CLI application with the given options.
 func RunWithOptions(opts *RunOptions) int {
 	m := &CLIApp{
-		appname:  opts.Appname,
+		appname:         opts.Appname,
-		version:  opts.Version,
+		version:         opts.Version,
-		gitrev:   opts.Gitrev,
+		gitrev:          opts.Gitrev,
-		exitCode: 0,
+		exitCode:        0,
-		Stdin:    opts.Stdin,
+		maxManifestSize: mfer.MaxManifestSize,
-		Stdout:   opts.Stdout,
+		Stdin:           opts.Stdin,
-		Stderr:   opts.Stderr,
+		Stdout:          opts.Stdout,
-		Fs:       opts.Fs,
+		Stderr:          opts.Stderr,
+		Fs:              opts.Fs,
 	}
 
 	m.run(opts.Args)

internal/cli/entry_test.go

@@ -613,6 +613,33 @@ func runCheckAfterRewrite(t *testing.T, rewritten, msg string) {
 	assert.Equal(t, 1, exitCode, msg)
 }
 
+// TestCheckRequireSignatureRefusesOtherSigningKey runs check
+// --require-signature on a manifest signed by another key whose embedded
+// public key block also holds the required key. check must refuse it. It
+// needs gpg and is skipped without it, as the other signing tests are.
+//
+//nolint:paralleltest // signedManifest calls t.Setenv, which bars t.Parallel
+func TestCheckRequireSignatureRefusesOtherSigningKey(t *testing.T) {
+	content := []byte("signed file")
+	manifest, required := manifestSignedByAnotherKey(t,
+		map[string][]byte{testFileTxt: content})
+
+	fs := afero.NewMemMapFs()
+	require.NoError(t, fs.MkdirAll(testDir, 0o755))
+	require.NoError(t, afero.WriteFile(fs,
+		filepath.Join(testDir, testFileTxt), content, 0o644))
+	require.NoError(t, afero.WriteFile(fs, testManifest, manifest, 0o644))
+
+	opts := testOpts([]string{
+		testApp, cmdCheck, "-q", testFlagBase, testDir,
+		"--" + flagRequireSignature, required, testManifest,
+	}, fs)
+	assert.Equal(t, 1, runCLI(opts))
+	assert.Contains(t, testStderr(t, opts),
+		"failed to load manifest: signature verification failed: "+
+			"embedded public key block must hold exactly one key, found 2")
+}
+
 func TestCheckCommandWithCorruptedFile(t *testing.T) {
 	t.Parallel()
 

internal/cli/errmsg_test.go

@@ -9,12 +9,14 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"slices"
 	"testing"
 
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	urfcli "github.com/urfave/cli/v3"
+	"google.golang.org/protobuf/proto"
 	"sneak.berlin/go/mfer/mfer"
 )
 
@@ -86,8 +88,7 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
 	t.Run("invalid fingerprint length", func(t *testing.T) {
 		t.Parallel()
 
-		err := verifyRequiredSigner(context.Background(),
+		err := verifyRequiredSigner(unsignedChecker(t), "12345678")
-			unsignedChecker(t), "12345678")
 		require.ErrorIs(t, err, errInvalidFingerprint)
 		assert.EqualError(t, err,
 			"invalid fingerprint: must be exactly 40 hex characters, got 8")
@@ -96,8 +97,7 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
 	t.Run("manifest not signed", func(t *testing.T) {
 		t.Parallel()
 
-		err := verifyRequiredSigner(context.Background(),
+		err := verifyRequiredSigner(unsignedChecker(t), msgFpA)
-			unsignedChecker(t), msgFpA)
 		require.ErrorIs(t, err, errManifestNotSigned)
 		assert.EqualError(t, err,
 			"manifest is not signed, but signature from "+msgFpA+" is required")
@@ -105,23 +105,21 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
 }
 
 // TestSignerMismatchMessage drives verifyRequiredSigner against a real signed
-// manifest. The embedded fingerprint is whatever the generated key produced,
+// manifest. The signing key's fingerprint is whatever the generated key
-// so it is read back from the checker and substituted into the expected
+// produced, so it is read back from the checker and substituted into the
-// string; the required signer is a fixed value that cannot match it. Requires
+// expected string; the required signer is a fixed value that cannot match
-// gpg and is skipped where it is absent, as the other signing tests are.
+// it. Requires gpg and is skipped where it is absent, as the other signing
+// tests are.
 //
 //nolint:paralleltest // signedManifest calls t.Setenv, which bars t.Parallel
 func TestSignerMismatchMessage(t *testing.T) {
 	chk := signedChecker(t,
 		signedManifest(t, map[string][]byte{"f.txt": []byte("signed file")}))
 
-	embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP(context.Background())
+	err := verifyRequiredSigner(chk, msgFpB)
-	require.NoError(t, err)
-
-	err = verifyRequiredSigner(context.Background(), chk, msgFpB)
 	require.ErrorIs(t, err, errSignerMismatch)
 	assert.EqualError(t, err,
-		"embedded signing key fingerprint "+embeddedFP+
+		"embedded signing key fingerprint "+string(chk.Signer())+
 			" does not match required "+msgFpB)
 }
 
@@ -191,6 +189,34 @@ func signedChecker(t *testing.T, manifest []byte) *mfer.Checker {
 	return chk
 }
 
+// manifestSignedByAnotherKey returns a manifest of files and the
+// fingerprint of a throwaway key, the required key, that did not sign it.
+// The manifest is signed by a second throwaway key; its embedded public key
+// block holds the required key followed by the second key, and its signer
+// field names the required key.
+func manifestSignedByAnotherKey(
+	t *testing.T, files map[string][]byte,
+) ([]byte, string) {
+	t.Helper()
+
+	required := new(mfer.MFFileOuter)
+	require.NoError(t, proto.Unmarshal(
+		signedManifest(t, files)[len(mfer.MAGIC):], required))
+
+	outer := new(mfer.MFFileOuter)
+	require.NoError(t, proto.Unmarshal(
+		signedManifest(t, files)[len(mfer.MAGIC):], outer))
+
+	outer.SigningPubKey = slices.Concat(
+		required.GetSigningPubKey(), outer.GetSigningPubKey())
+	outer.Signer = required.GetSigner()
+
+	data, err := proto.Marshal(outer)
+	require.NoError(t, err)
+
+	return append([]byte(mfer.MAGIC), data...), string(required.GetSigner())
+}
+
 func TestPathDoesNotExistMessage(t *testing.T) {
 	t.Parallel()
 

internal/cli/fetch.go

@@ -361,7 +361,7 @@ func (mfa *CLIApp) fetchManifestOperation(
 		firstDelay: firstRetryDelay,
 	}
 
-	manifestData, files, err := fetchManifest(ctx, cmd, client, manifestURL)
+	manifestData, files, err := mfa.fetchManifest(ctx, cmd, client, manifestURL)
 	if err != nil {
 		return err
 	}
@@ -426,20 +426,22 @@ func (mfa *CLIApp) fetchManifestOperation(
 // that lists a file where fetch writes another or a mode outside 0777. It
 // returns the manifest as downloaded, to be saved once the files are in
 // place, and the files it lists.
-func fetchManifest(
+func (mfa *CLIApp) fetchManifest(
 	ctx context.Context, cmd *cli.Command, client retryingClient, manifestURL string,
 ) ([]byte, []*mfer.MFFilePath, error) {
 	log.Infof("fetching manifest from %s", manifestURL)
 
 	// Read the whole manifest before parsing it, so that a connection
 	// lost partway through is retried rather than reported as a bad
-	// manifest.
+	// manifest. Reading stops one byte past mfa.maxManifestSize, which is
+	// enough to tell that the manifest is too large.
 	var manifestData []byte
 
 	err := client.get(ctx, manifestURL, func(resp *http.Response) error {
 		var readErr error
 
-		manifestData, readErr = io.ReadAll(resp.Body)
+		manifestData, readErr = io.ReadAll(
+			io.LimitReader(resp.Body, mfa.maxManifestSize+1))
 
 		return readErr
 	})
@@ -447,6 +449,11 @@ func fetchManifest(
 		return nil, nil, fmt.Errorf("failed to fetch manifest: %w", err)
 	}
 
+	if int64(len(manifestData)) > mfa.maxManifestSize {
+		return nil, nil, fmt.Errorf("failed to fetch manifest: %w of %d bytes",
+			errManifestTooLarge, mfa.maxManifestSize)
+	}
+
 	// Parse manifest
 	//nolint:contextcheck // mfer loads a manifest without a context
 	manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData))
@@ -456,7 +463,8 @@ func fetchManifest(
 
 	requiredSigner := cmd.String(flagRequireSignature)
 	if requiredSigner != "" {
-		err = verifyFetchedSigner(ctx, manifestData, requiredSigner)
+		//nolint:contextcheck // mfer loads a manifest without a context
+		err = verifyFetchedSigner(manifestData, requiredSigner)
 		if err != nil {
 			return nil, nil, err
 		}
@@ -538,9 +546,7 @@ func checkNoNameClash(files []*mfer.MFFilePath) error {
 // exactly as check does. verifyRequiredSigner takes a Checker, which loads
 // its manifest from a file, so the manifest is handed to it as a file in
 // memory.
-func verifyFetchedSigner(
+func verifyFetchedSigner(manifestData []byte, requiredSigner string) error {
-	ctx context.Context, manifestData []byte, requiredSigner string,
-) error {
 	memFs := afero.NewMemMapFs()
 	manifestPath := "/" + defaultManifestName
 
@@ -549,7 +555,6 @@ func verifyFetchedSigner(
 		return err
 	}
 
-	//nolint:contextcheck // mfer loads a manifest without a context
 	chk, err := mfer.NewChecker(&mfer.CheckerOptions{
 		ManifestPath: manifestPath,
 		BasePath:     "/",
@@ -559,7 +564,7 @@ func verifyFetchedSigner(
 		return fmt.Errorf("failed to load manifest: %w", err)
 	}
 
-	return verifyRequiredSigner(ctx, chk, requiredSigner)
+	return verifyRequiredSigner(chk, requiredSigner)
 }
 
 // saveManifest writes the fetched manifest into dest under the default
@@ -910,8 +915,10 @@ func saveResponse(
 		progress:  progress,
 	}
 
-	// Copy content while hashing and reporting progress
+	// Copy content while hashing and reporting progress. One byte past
-	written, copyErr := io.Copy(pw, resp.Body)
+	// the listed size is enough for finishDownload to report a size
+	// mismatch.
+	written, copyErr := io.Copy(pw, io.LimitReader(resp.Body, expectedSize+1))
 
 	// Close file before checking errors (to flush writes)
 	closeErr := out.Close()

internal/cli/fetch_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	urfcli "github.com/urfave/cli/v3"
 	"google.golang.org/protobuf/proto"
 	"sneak.berlin/go/mfer/mfer"
 )
@@ -441,6 +442,75 @@ func TestFetchSizeMismatch(t *testing.T) {
 		"temp file should be cleaned up on size mismatch")
 }
 
+// zeros is an io.Reader of zero bytes without end.
+type zeros struct{}
+
+func (zeros) Read(p []byte) (int, error) {
+	clear(p)
+
+	return len(p), nil
+}
+
+// TestFetchStopsReadingFilePastListedSize serves a body that never ends
+// for a file listed at 16 bytes. fetch must stop reading one byte past
+// the listed size, report the size mismatch and remove its temp file.
+//
+//nolint:paralleltest // changes the process-global working directory
+func TestFetchStopsReadingFilePastListedSize(t *testing.T) {
+	server := httptest.NewServer(
+		http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+			_, _ = io.Copy(w, zeros{})
+		}))
+	defer server.Close()
+
+	chdirTemp(t)
+
+	err := downloadFile(context.Background(), testClient(),
+		server.URL+"/"+testFileTxt, ".", testFileTxt,
+		&mfer.MFFilePath{Path: testFileTxt, Size: 16}, nil)
+	require.ErrorIs(t, err, errSizeMismatch)
+	require.EqualError(t, err, "size mismatch: expected 16 bytes, got 17")
+	assert.NoFileExists(t, tempPathFor(testFileTxt))
+}
+
+// TestManifestDownloadStopsAtLimit serves a manifest that never ends to
+// fetch and to check, with the most they download of a manifest lowered
+// to 64 KiB. Each must stop reading at that limit, fail with an error
+// naming it and leave no temp file.
+func TestManifestDownloadStopsAtLimit(t *testing.T) {
+	server := httptest.NewServer(
+		http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+			_, _ = io.Copy(w, zeros{})
+		}))
+	defer server.Close()
+
+	tmpDir := t.TempDir()
+	t.Setenv("TMPDIR", tmpDir)
+
+	mfa := &CLIApp{Fs: afero.NewOsFs(), maxManifestSize: 64 << 10}
+
+	fetch := mfa.fetchCommand()
+	fetch.Action = mfa.fetchManifestOperation
+
+	check := mfa.checkCommand()
+	check.Action = mfa.checkManifestOperation
+
+	for _, cmd := range []*urfcli.Command{fetch, check} {
+		// Both operations log to the process-global logger.
+		err := runLocked(func() error {
+			return cmd.Run(context.Background(),
+				[]string{cmd.Name, server.URL + "/index.mf"})
+		})
+		require.ErrorIs(t, err, errManifestTooLarge, cmd.Name)
+		require.ErrorContains(t, err,
+			"maximum allowed size of 65536 bytes", cmd.Name)
+	}
+
+	leftover, err := os.ReadDir(tmpDir)
+	require.NoError(t, err)
+	assert.Empty(t, leftover)
+}
+
 //nolint:paralleltest // changes the process-global working directory
 func TestFetchProgress(t *testing.T) {
 	// Create source filesystem with a larger test file
@@ -1117,8 +1187,10 @@ func TestFetchIntoDest(t *testing.T) {
 // TestFetchRequireSignature runs fetch with --require-signature. A
 // manifest that is unsigned, or signed by another key, must stop fetch
 // with check's message before it downloads or writes anything; the
-// required key lets it through. The signed cases need gpg and are skipped
+// required key lets it through. A manifest signed by another key whose
-// without it, as the other signing tests are.
+// embedded public key block also holds the required key must stop fetch
+// too. The signed cases need gpg and are skipped without it, as the other
+// signing tests are.
 //
 //nolint:paralleltest // signedManifest calls t.Setenv, which bars t.Parallel
 func TestFetchRequireSignature(t *testing.T) {
@@ -1133,9 +1205,7 @@ func TestFetchRequireSignature(t *testing.T) {
 	t.Run("signed", func(t *testing.T) {
 		manifest := signedManifest(t, files)
 
-		signer, err := signedChecker(t, manifest).
+		signer := string(signedChecker(t, manifest).Signer())
-			ExtractEmbeddedSigningKeyFP(context.Background())
-		require.NoError(t, err)
 
 		assertFetchRefused(t, manifest, files,
 			"embedded signing key fingerprint "+signer+" does not match required "+msgFpB,
@@ -1153,6 +1223,15 @@ func TestFetchRequireSignature(t *testing.T) {
 		require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
 		assert.Equal(t, files[testFileTxt], filesUnder(t, dest)[testFileTxt])
 	})
+
+	t.Run("signed by another key embedded after the required one", func(t *testing.T) {
+		manifest, required := manifestSignedByAnotherKey(t, files)
+
+		assertFetchRefused(t, manifest, files,
+			"failed to parse manifest: signature verification failed: "+
+				"embedded public key block must hold exactly one key, found 2",
+			"--"+flagRequireSignature, required)
+	})
 }
 
 // TestFetchRefusesListedManifestName fetches manifests that list, at the

internal/cli/manifest_loader.go

@@ -23,6 +23,10 @@ const manifestFetchTimeout = 30 * time.Second
 // its message.
 var errHTTPStatus = errors.New("HTTP")
 
+// errManifestTooLarge indicates a manifest download that passed
+// CLIApp.maxManifestSize.
+var errManifestTooLarge = errors.New("manifest exceeds maximum allowed size")
+
 // isHTTPURL returns true if the string starts with http:// or https://.
 func isHTTPURL(s string) bool {
 	return strings.HasPrefix(s, "http://") || strings.HasPrefix(s, "https://")

internal/cli/mfer.go

@@ -59,6 +59,10 @@ type CLIApp struct {
 	exitCode    int
 	app         *cli.Command
 
+	// maxManifestSize is the most of a manifest that fetch, and check
+	// given a URL, download: mfer.MaxManifestSize, which tests lower.
+	maxManifestSize int64
+
 	Stdin  io.Reader // Standard input stream
 	Stdout io.Writer // Standard output stream for normal output
 	Stderr io.Writer // Standard error stream for diagnostics

mfer/checker.go

@@ -15,7 +15,6 @@ import (
 )
 
 var (
-	errNoSigningPubKey   = errors.New("manifest has no signing public key")
 	errManifestPathEmpty = errors.New("manifest path cannot be empty")
 	errBasePathEmpty     = errors.New("base path cannot be empty")
 )
@@ -173,8 +172,14 @@ func (c *Checker) IsSigned() bool {
 	return len(c.signature) > 0
 }
 
-// Signer returns the signer fingerprint if the manifest is signed, nil otherwise.
+// Signer returns the fingerprint of the key that made the manifest's
+// signature, which loading the manifest checked, or nil if the manifest is
+// not signed.
 func (c *Checker) Signer() []byte {
+	if !c.IsSigned() {
+		return nil
+	}
+
 	return c.signer
 }
 
@@ -184,17 +189,6 @@ func (c *Checker) SigningPubKey() []byte {
 	return c.signingPubKey
 }
 
-// ExtractEmbeddedSigningKeyFP imports the manifest's embedded public key into a
-// temporary keyring and extracts its fingerprint. This validates the key and
-// returns its actual fingerprint from the key material itself.
-func (c *Checker) ExtractEmbeddedSigningKeyFP(ctx context.Context) (string, error) {
-	if len(c.signingPubKey) == 0 {
-		return "", errNoSigningPubKey
-	}
-
-	return gpgExtractPubKeyFingerprint(ctx, c.signingPubKey)
-}
-
 // Check verifies all files against the manifest.
 // Results are sent to the results channel as files are checked.
 // Progress updates are sent to the progress channel approximately once per second.

mfer/constants.go

@@ -8,10 +8,17 @@ const (
 	ReleaseDate = "2025-12-17"
 
 	// MaxDecompressedSize is the maximum allowed size of decompressed manifest
-	// data (256 MB). This prevents decompression bombs from consuming excessive
+	// data (256 MiB). This prevents decompression bombs from consuming excessive
 	// memory.
 	MaxDecompressedSize int64 = 256 * 1024 * 1024
 
+	// MaxManifestSize is the largest manifest file mfer reads (258 MiB).
+	// zstd's worst case grows data it cannot compress by 1/256, so an inner
+	// message of MaxDecompressedSize compresses to at most 257 MiB; the
+	// last MiB is room for the signature, the signing key and the other
+	// outer fields.
+	MaxManifestSize = MaxDecompressedSize + MaxDecompressedSize/256 + 1<<20
+
 	// zstdWindowSize is the zstd window zstd.SpeedBestCompression gives mfer's writer.
 	zstdWindowSize = 8 << 20
 

mfer/deserialize.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/klauspost/compress/zstd"
 	"github.com/spf13/afero"
@@ -23,11 +24,14 @@ var (
 	errCompressedHashWrong  = errors.New("compressed data hash mismatch")
 	errSignatureNoPubKey    = errors.New("signature present but no public key")
 	errDecompressedTooLarge = errors.New("decompressed data exceeds maximum allowed size")
+	errManifestTooLarge     = errors.New("manifest exceeds maximum allowed size")
 	errUUIDMismatch         = errors.New("outer and inner UUID mismatch")
 	errInvalidFileFormat    = errors.New("invalid file format")
 	errInvalidManifestPath  = errors.New("manifest contains invalid path")
 	errDecodedTooLarge      = errors.New(
 		"manifest would take too much memory to decode")
+	errSignerNotSigningKey = errors.New(
+		"signer is not the fingerprint of the key that made the signature")
 )
 
 // validateUUID checks that the byte slice is the 16 bytes of a binary UUID.
@@ -60,8 +64,10 @@ func (m *manifest) validateOuterHeader() error {
 	return nil
 }
 
-// verifyOuterIntegrity checks the hash of the compressed payload and,
+// verifyOuterIntegrity checks the hash of the compressed payload and, if a
-// if a signature is present, verifies it against the embedded public key.
+// signature is present, verifies it against the embedded public key, which
+// must be one key, and checks that the signer field is that key's
+// fingerprint.
 func (m *manifest) verifyOuterIntegrity() error {
 	h := sha256.New()
 
@@ -91,7 +97,7 @@ func (m *manifest) verifyOuterIntegrity() error {
 	}
 
 	// Loading a manifest takes no context; gpgTimeout still bounds gpg.
-	err = gpgVerify(
+	signingKey, err := gpgVerify(
 		context.Background(),
 		[]byte(sigString),
 		m.pbOuter.GetSignature(),
@@ -101,6 +107,11 @@ func (m *manifest) verifyOuterIntegrity() error {
 		return fmt.Errorf("signature verification failed: %w", err)
 	}
 
+	if !strings.EqualFold(string(m.pbOuter.GetSigner()), signingKey) {
+		return fmt.Errorf("%w: signer %q, signing key %s",
+			errSignerNotSigningKey, m.pbOuter.GetSigner(), signingKey)
+	}
+
 	log.Infof("signature verified successfully")
 
 	return nil
@@ -308,13 +319,14 @@ func validateMagic(dat []byte) bool {
 	return bytes.Equal(got, expected)
 }
 
-// NewManifestFromReader reads a manifest from an io.Reader.
+// NewManifestFromReader reads a manifest from an io.Reader. It refuses a
+// manifest larger than MaxManifestSize, reading at most one byte past it.
 //
 //nolint:revive // unexported-return: exporting manifest is owner question 13
 func NewManifestFromReader(input io.Reader) (*manifest, error) {
 	m := &manifest{}
 
-	dat, err := io.ReadAll(input)
+	dat, err := readAtMost(input, MaxManifestSize)
 	if err != nil {
 		return nil, err
 	}
@@ -346,6 +358,21 @@ func NewManifestFromReader(input io.Reader) (*manifest, error) {
 	return m, nil
 }
 
+// readAtMost reads all of input, or refuses it with errManifestTooLarge
+// once it passes maxSize bytes, after reading one byte past maxSize.
+func readAtMost(input io.Reader, maxSize int64) ([]byte, error) {
+	dat, err := io.ReadAll(io.LimitReader(input, maxSize+1))
+	if err != nil {
+		return nil, err
+	}
+
+	if int64(len(dat)) > maxSize {
+		return nil, fmt.Errorf("%w of %d bytes", errManifestTooLarge, maxSize)
+	}
+
+	return dat, nil
+}
+
 // ManifestFromFileOptions configures NewManifestFromFile.
 type ManifestFromFileOptions struct {
 	// Path is the manifest file to read (required).

mfer/deserialize_test.go

@@ -0,0 +1,33 @@
+//nolint:testpackage // white-box tests exercise unexported internals
+package mfer
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestReadAtMost gives readAtMost exactly its maximum, which it must
+// return whole, and twice its maximum, which it must refuse after reading
+// one byte past the maximum, and no more. NewManifestFromReader reads
+// through it with MaxManifestSize; the test uses 64 KiB, since reading
+// MaxManifestSize under the race detector takes gigabytes of memory.
+func TestReadAtMost(t *testing.T) {
+	t.Parallel()
+
+	const maxSize = 64 << 10
+
+	dat, err := readAtMost(bytes.NewReader(make([]byte, maxSize)), maxSize)
+	require.NoError(t, err)
+	assert.Len(t, dat, maxSize)
+
+	input := bytes.NewReader(make([]byte, 2*maxSize))
+
+	_, err = readAtMost(input, maxSize)
+	require.ErrorIs(t, err, errManifestTooLarge)
+	require.EqualError(t, err,
+		"manifest exceeds maximum allowed size of 65536 bytes")
+	assert.Equal(t, maxSize-1, input.Len(), "bytes left unread")
+}

mfer/gpg.go

@@ -41,16 +41,26 @@ const (
 	// fields in a gpg fingerprint record (the fingerprint is field 10).
 	gpgFingerprintMinFields = 10
 
+	// gpgStatusPrefix starts each status line gpg writes to the file
+	// descriptor named by --status-fd.
+	gpgStatusPrefix = "[GNUPG:]"
+
 	// gpg option names used from more than one call site.
-	gpgOptArmor   = "--armor"
+	gpgOptArmor    = "--armor"
-	gpgOptHomedir = "--homedir"
+	gpgOptHomedir  = "--homedir"
-	gpgOptVerify  = "--verify"
+	gpgOptStatusFD = "--status-fd"
+	gpgOptVerify   = "--verify"
 )
 
 var (
 	errGPGKeyNotFound      = errors.New("gpg key not found")
 	errFingerprintNotFound = errors.New("fingerprint not found for key")
-	errImportedFPRNotFound = errors.New("fingerprint not found in imported key")
+	errSigningKeyCount     = errors.New(
+		"embedded public key block must hold exactly one key")
+	errNotOneGoodSignature = errors.New(
+		"gpg did not report exactly one good signature")
+	errSigningKeyNotReported = errors.New(
+		"gpg did not report the key that made the signature")
 )
 
 // GPGKeyID represents a GPG key identifier (fingerprint or key ID).
@@ -136,19 +146,67 @@ func parseFingerprint(colonOutput string) (string, bool) {
 	return "", false
 }
 
-// gpgSign creates a detached signature of the data using the specified key.
+// parseStatusLine returns the arguments of the status line for keyword in
-// Returns the armored detached signature.
+// gpg --status-fd output, or ok=false unless there is exactly one such line
-func gpgSign(ctx context.Context, data []byte, keyID GPGKeyID) ([]byte, error) {
+// and it has arguments.
+func parseStatusLine(statusOutput, keyword string) ([]string, bool) {
+	var found [][]string
+
+	for line := range strings.SplitSeq(statusOutput, "\n") {
+		fields := strings.Fields(line)
+		if len(fields) > 2 && fields[0] == gpgStatusPrefix && fields[1] == keyword {
+			found = append(found, fields[2:])
+		}
+	}
+
+	if len(found) != 1 {
+		return nil, false
+	}
+
+	return found[0], true
+}
+
+// gpgSign creates an armored detached signature of data with the key gpg
+// picks for keyID, and returns it with the fingerprint of the key that made
+// it, which is a subkey's when gpg signed with a subkey.
+func gpgSign(
+	ctx context.Context, data []byte, keyID GPGKeyID,
+) ([]byte, string, error) {
+	tmpDir, err := os.MkdirTemp("", "mfer-gpg-sign-*")
+	if err != nil {
+		return nil, "", fmt.Errorf("failed to create temp dir: %w", err)
+	}
+
+	defer func() { _ = os.RemoveAll(tmpDir) }()
+
+	sigFile := filepath.Join(tmpDir, "signature.asc")
+
+	// The signature goes to sigFile, so --status-fd 1 can send gpg's status
+	// lines to stdout; its messages go to stderr.
 	stdout, stderr, err := runGPG(ctx, bytes.NewReader(data),
 		"--detach-sign",
 		gpgOptArmor,
+		"--output", sigFile,
+		gpgOptStatusFD, "1",
 		"--local-user", string(keyID),
 	)
 	if err != nil {
-		return nil, fmt.Errorf("gpg sign failed: %w: %s", err, stderr.String())
+		return nil, "", fmt.Errorf("gpg sign failed: %w: %s", err, stderr.String())
 	}
 
-	return stdout.Bytes(), nil
+	// The last argument of SIG_CREATED is the fingerprint of the key that
+	// made the signature.
+	created, ok := parseStatusLine(stdout.String(), "SIG_CREATED")
+	if !ok {
+		return nil, "", fmt.Errorf("%w: %s", errSigningKeyNotReported, stderr.String())
+	}
+
+	sig, err := os.ReadFile(sigFile) //nolint:gosec // G304: inside tmpDir, made above
+	if err != nil {
+		return nil, "", fmt.Errorf("failed to read signature: %w", err)
+	}
+
+	return sig, created[len(created)-1], nil
 }
 
 // gpgExportPublicKey exports the public key for the specified key ID.
@@ -187,12 +245,44 @@ func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
 	return []byte(fpr), nil
 }
 
-// gpgExtractPubKeyFingerprint imports a public key into a temporary keyring
+// gpgImportOneKey imports the public key block in pubKeyFile into the
-// and extracts its fingerprint. This verifies the key is valid and returns
+// keyring in gpgHome. The block must hold exactly one primary key.
-// the actual fingerprint from the key material.
+func gpgImportOneKey(ctx context.Context, gpgHome, pubKeyFile string) error {
-func gpgExtractPubKeyFingerprint(ctx context.Context, pubKey []byte) (string, error) {
+	// --status-fd 1 sends gpg's status lines to stdout, which importing
+	// otherwise leaves empty; its messages go to stderr.
+	importStdout, importStderr, err := runGPG(ctx, nil,
+		gpgArgs([]string{gpgOptHomedir, gpgHome, gpgOptStatusFD, "1", "--import"},
+			pubKeyFile)...,
+	)
+	if err != nil {
+		return fmt.Errorf(
+			"failed to import public key: %w: %s", err, importStderr.String(),
+		)
+	}
+
+	// The first argument of IMPORT_RES counts the primary keys gpg read
+	// from the block, those it then skipped (one with no user ID, for
+	// example) included.
+	result, ok := parseStatusLine(importStdout.String(), "IMPORT_RES")
+	if !ok {
+		return fmt.Errorf("%w, gpg reported no count", errSigningKeyCount)
+	}
+
+	if result[0] != "1" {
+		return fmt.Errorf("%w, found %s", errSigningKeyCount, result[0])
+	}
+
+	return nil
+}
+
+// gpgVerify verifies a detached signature against data using the provided
+// public key, imported into a temporary keyring, and returns the
+// fingerprint of the primary key that made the signature. The public key
+// must hold exactly one primary key, so that a good signature can come
+// from no other key.
+func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, error) {
 	// Create temporary directory for GPG operations
-	tmpDir, err := os.MkdirTemp("", "mfer-gpg-fingerprint-*")
+	tmpDir, err := os.MkdirTemp("", "mfer-gpg-verify-*")
 	if err != nil {
 		return "", fmt.Errorf("failed to create temp dir: %w", err)
 	}
@@ -213,67 +303,12 @@ func gpgExtractPubKeyFingerprint(ctx context.Context, pubKey []byte) (string, er
 		return "", fmt.Errorf("failed to write public key: %w", err)
 	}
 
-	// Import the public key into the temporary keyring
-	_, importStderr, err := runGPG(ctx, nil,
-		gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)...,
-	)
-	if err != nil {
-		return "", fmt.Errorf(
-			"failed to import public key: %w: %s", err, importStderr.String(),
-		)
-	}
-
-	// List keys to get fingerprint
-	listStdout, listStderr, err := runGPG(ctx, nil,
-		"--homedir", tmpDir,
-		"--with-colons",
-		"--fingerprint",
-	)
-	if err != nil {
-		return "", fmt.Errorf(
-			"failed to list keys: %w: %s", err, listStderr.String(),
-		)
-	}
-
-	fpr, ok := parseFingerprint(listStdout.String())
-	if !ok {
-		return "", errImportedFPRNotFound
-	}
-
-	return fpr, nil
-}
-
-// gpgVerify verifies a detached signature against data using the provided public key.
-// It creates a temporary keyring to import the public key for verification.
-func gpgVerify(ctx context.Context, data, signature, pubKey []byte) error {
-	// Create temporary directory for GPG operations
-	tmpDir, err := os.MkdirTemp("", "mfer-gpg-verify-*")
-	if err != nil {
-		return fmt.Errorf("failed to create temp dir: %w", err)
-	}
-
-	defer func() { _ = os.RemoveAll(tmpDir) }()
-
-	// Set restrictive permissions
-	err = os.Chmod(tmpDir, privateDirPerms)
-	if err != nil {
-		return fmt.Errorf("failed to set temp dir permissions: %w", err)
-	}
-
-	// Write public key to temp file
-	pubKeyFile := filepath.Join(tmpDir, "pubkey.asc")
-
-	err = os.WriteFile(pubKeyFile, pubKey, privateFilePerms)
-	if err != nil {
-		return fmt.Errorf("failed to write public key: %w", err)
-	}
-
 	// Write signature to temp file
 	sigFile := filepath.Join(tmpDir, "signature.asc")
 
 	err = os.WriteFile(sigFile, signature, privateFilePerms)
 	if err != nil {
-		return fmt.Errorf("failed to write signature: %w", err)
+		return "", fmt.Errorf("failed to write signature: %w", err)
 	}
 
 	// Write data to temp file
@@ -281,29 +316,33 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) error {
 
 	err = os.WriteFile(dataFile, data, privateFilePerms)
 	if err != nil {
-		return fmt.Errorf("failed to write data: %w", err)
+		return "", fmt.Errorf("failed to write data: %w", err)
 	}
 
-	// Import the public key into the temporary keyring
+	err = gpgImportOneKey(ctx, tmpDir, pubKeyFile)
-	_, importStderr, err := runGPG(ctx, nil,
-		gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)...,
-	)
 	if err != nil {
-		return fmt.Errorf(
+		return "", err
-			"failed to import public key: %w: %s", err, importStderr.String(),
-		)
 	}
 
-	// Verify the signature
+	// --status-fd 1 sends gpg's status lines to stdout, which verifying a
-	_, verifyStderr, err := runGPG(ctx, nil,
+	// detached signature otherwise leaves empty; its messages go to stderr.
-		gpgArgs([]string{gpgOptHomedir, tmpDir, gpgOptVerify},
+	verifyStdout, verifyStderr, err := runGPG(ctx, nil,
+		gpgArgs([]string{gpgOptHomedir, tmpDir, gpgOptStatusFD, "1", gpgOptVerify},
 			sigFile, dataFile)...,
 	)
 	if err != nil {
-		return fmt.Errorf(
+		return "", fmt.Errorf(
 			"signature verification failed: %w: %s", err, verifyStderr.String(),
 		)
 	}
 
-	return nil
+	// gpg writes a VALIDSIG line for each good signature. Its first
+	// argument is the fingerprint of the key that made the signature,
+	// which may be a subkey; its last is that of the primary key.
+	valid, ok := parseStatusLine(verifyStdout.String(), "VALIDSIG")
+	if !ok {
+		return "", errNotOneGoodSignature
+	}
+
+	return valid[len(valid)-1], nil
 }

mfer/gpg_test.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"slices"
 	"strconv"
 	"strings"
 	"syscall"
@@ -17,6 +18,7 @@ import (
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/proto"
 )
 
 // testGPGEnv sets up a temporary GPG home directory with a test key.
@@ -35,39 +37,18 @@ func testGPGEnv(t *testing.T) (GPGKeyID, string) {
 	// Create temporary GPG home directory (0700 by default)
 	gpgHome := t.TempDir()
 
-	// Generate a test key with no passphrase
+	genTestKey(t, gpgHome, testKeyParams)
-	keyParams := `%no-protection
-Key-Type: RSA
-Key-Length: 2048
-Name-Real: MFER Test Key
-Name-Email: test@mfer.test
-Expire-Date: 0
-%commit
-`
-	paramsFile := filepath.Join(gpgHome, "key-params")
-	require.NoError(t, os.WriteFile(paramsFile, []byte(keyParams), 0o600))
 
 	ctx, cancel := context.WithTimeout(context.Background(), gpgTimeout)
 	defer cancel()
 
-	//nolint:gosec // paramsFile is a test-controlled path inside t.TempDir()
-	cmd := exec.CommandContext(ctx, "gpg",
-		"--batch", "--gen-key", paramsFile)
-
-	cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome)
-
-	output, err := cmd.CombinedOutput()
-	if err != nil {
-		t.Skipf("failed to generate test GPG key: %v: %s", err, output)
-	}
-
 	// Get the key fingerprint
-	cmd = exec.CommandContext(ctx, "gpg",
+	cmd := exec.CommandContext(ctx, "gpg",
 		"--list-keys", "--with-colons", "test@mfer.test")
 
 	cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome)
 
-	output, err = cmd.Output()
+	output, err := cmd.Output()
 	if err != nil {
 		t.Fatalf("failed to list test key: %v", err)
 	}
@@ -92,13 +73,79 @@ Expire-Date: 0
 	return GPGKeyID(keyID), gpgHome
 }
 
+// testKeyParams are the gpg key generation parameters of an RSA key that
+// signs and does not expire.
+const testKeyParams = "Key-Type: RSA\nKey-Length: 2048\nExpire-Date: 0\n"
+
+// genTestKey generates a key with no passphrase for
+// "MFER Test Key <test@mfer.test>" from the gpg key generation parameters
+// keyParams in gpgHome, which may already hold one.
+func genTestKey(t *testing.T, gpgHome, keyParams string) {
+	t.Helper()
+
+	params := "%no-protection\n" + keyParams +
+		"Name-Real: MFER Test Key\nName-Email: test@mfer.test\n%commit\n"
+	paramsFile := filepath.Join(gpgHome, "key-params")
+	require.NoError(t, os.WriteFile(paramsFile, []byte(params), 0o600))
+
+	ctx, cancel := context.WithTimeout(context.Background(), gpgTimeout)
+	defer cancel()
+
+	//nolint:gosec // paramsFile is a test-controlled path inside t.TempDir()
+	cmd := exec.CommandContext(ctx, "gpg",
+		"--batch", "--gen-key", paramsFile)
+
+	cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome)
+
+	output, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Skipf("failed to generate test GPG key: %v: %s", err, output)
+	}
+}
+
+// signedTestManifest returns a manifest of one file signed with keyID.
+func signedTestManifest(t *testing.T, keyID GPGKeyID) []byte {
+	t.Helper()
+
+	b := NewBuilder()
+	b.SetSigningOptions(&SigningOptions{KeyID: keyID})
+
+	content := []byte("signed file content")
+	_, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, 0,
+		bytes.NewReader(content), nil)
+	require.NoError(t, err)
+
+	var buf bytes.Buffer
+
+	require.NoError(t, b.Build(context.Background(), &buf))
+
+	return buf.Bytes()
+}
+
+// rewriteOuter returns manifest with its outer message changed by edit.
+// A signature stays good as long as edit leaves the UUID and hash alone.
+func rewriteOuter(t *testing.T, manifest []byte, edit func(*MFFileOuter)) []byte {
+	t.Helper()
+
+	outer := new(MFFileOuter)
+	require.NoError(t, proto.Unmarshal(manifest[len(MAGIC):], outer))
+
+	edit(outer)
+
+	data, err := proto.Marshal(outer)
+	require.NoError(t, err)
+
+	return append([]byte(MAGIC), data...)
+}
+
 func TestGPGSign(t *testing.T) {
 	keyID, gpgHome := testGPGEnv(t)
 	t.Setenv("GNUPGHOME", gpgHome)
 
 	data := []byte("test data to sign")
-	sig, err := gpgSign(context.Background(), data, keyID)
+	sig, signingKey, err := gpgSign(context.Background(), data, keyID)
 	require.NoError(t, err)
+	assert.Equal(t, string(keyID), signingKey)
 	assert.NotEmpty(t, sig)
 	assert.Contains(t, string(sig), "-----BEGIN PGP SIGNATURE-----")
 	assert.Contains(t, string(sig), "-----END PGP SIGNATURE-----")
@@ -163,15 +210,18 @@ func TestGPGOptionLikeKeyIDIsNotAnOption(t *testing.T) {
 	assert.NotContains(t, string(fpr), "gpg (GnuPG)")
 }
 
+// TestGPGSignInvalidKey signs with a key that has no secret key in the
+// keyring. The error must hold gpg's messages and none of its status lines.
 func TestGPGSignInvalidKey(t *testing.T) {
 	// Set up test environment (we need GNUPGHOME set)
 	_, gpgHome := testGPGEnv(t)
 	t.Setenv("GNUPGHOME", gpgHome)
 
 	data := []byte("test data")
-	_, err := gpgSign(context.Background(), data,
+	_, _, err := gpgSign(context.Background(), data,
 		GPGKeyID("NONEXISTENT_KEY_ID_12345"))
-	assert.Error(t, err)
+	require.Error(t, err)
+	assert.NotContains(t, err.Error(), gpgStatusPrefix)
 }
 
 func TestBuilderWithSigning(t *testing.T) {
@@ -259,15 +309,16 @@ func TestGPGVerify(t *testing.T) {
 	t.Setenv("GNUPGHOME", gpgHome)
 
 	data := []byte("test data to sign and verify")
-	sig, err := gpgSign(context.Background(), data, keyID)
+	sig, _, err := gpgSign(context.Background(), data, keyID)
 	require.NoError(t, err)
 
 	pubKey, err := gpgExportPublicKey(context.Background(), keyID)
 	require.NoError(t, err)
 
-	// Verify the signature
+	// Verify the signature; it names the key that made it
-	err = gpgVerify(context.Background(), data, sig, pubKey)
+	signingKey, err := gpgVerify(context.Background(), data, sig, pubKey)
 	require.NoError(t, err)
+	assert.Equal(t, string(keyID), signingKey)
 }
 
 func TestGPGVerifyInvalidSignature(t *testing.T) {
@@ -275,7 +326,7 @@ func TestGPGVerifyInvalidSignature(t *testing.T) {
 	t.Setenv("GNUPGHOME", gpgHome)
 
 	data := []byte("test data to sign")
-	sig, err := gpgSign(context.Background(), data, keyID)
+	sig, _, err := gpgSign(context.Background(), data, keyID)
 	require.NoError(t, err)
 
 	pubKey, err := gpgExportPublicKey(context.Background(), keyID)
@@ -283,7 +334,7 @@ func TestGPGVerifyInvalidSignature(t *testing.T) {
 
 	// Try to verify with different data - should fail
 	wrongData := []byte("different data")
-	err = gpgVerify(context.Background(), wrongData, sig, pubKey)
+	_, err = gpgVerify(context.Background(), wrongData, sig, pubKey)
 	assert.Error(t, err)
 }
 
@@ -292,12 +343,12 @@ func TestGPGVerifyBadPublicKey(t *testing.T) {
 	t.Setenv("GNUPGHOME", gpgHome)
 
 	data := []byte("test data")
-	sig, err := gpgSign(context.Background(), data, keyID)
+	sig, _, err := gpgSign(context.Background(), data, keyID)
 	require.NoError(t, err)
 
 	// Try to verify with invalid public key - should fail
 	badPubKey := []byte("not a valid public key")
-	err = gpgVerify(context.Background(), data, sig, badPubKey)
+	_, err = gpgVerify(context.Background(), data, sig, badPubKey)
 	assert.Error(t, err)
 }
 
@@ -368,6 +419,144 @@ func TestManifestTamperedSignatureFails(t *testing.T) {
 	assert.Error(t, err)
 }
 
+// TestManifestRefusesSecondEmbeddedKey loads a manifest whose embedded
+// public key block holds another key before the key that signed it.
+// Loading must refuse it, although the signature is good and the signer
+// field names the key that made it.
+func TestManifestRefusesSecondEmbeddedKey(t *testing.T) {
+	otherKey, otherHome := testGPGEnv(t)
+	t.Setenv("GNUPGHOME", otherHome)
+
+	otherPubKey, err := gpgExportPublicKey(context.Background(), otherKey)
+	require.NoError(t, err)
+
+	keyID, gpgHome := testGPGEnv(t)
+	t.Setenv("GNUPGHOME", gpgHome)
+
+	manifest := rewriteOuter(t, signedTestManifest(t, keyID),
+		func(outer *MFFileOuter) {
+			outer.SigningPubKey = slices.Concat(otherPubKey, outer.GetSigningPubKey())
+		})
+
+	_, err = NewManifestFromReader(bytes.NewReader(manifest))
+	require.ErrorIs(t, err, errSigningKeyCount)
+}
+
+// TestManifestRefusesSecondEmbeddedKeyWithoutUserID loads a manifest whose
+// embedded public key block holds, before the key that signed it, another
+// key with its user ID removed, which gpg skips on import. Loading must
+// refuse it: the block holds two keys.
+func TestManifestRefusesSecondEmbeddedKeyWithoutUserID(t *testing.T) {
+	otherKey, otherHome := testGPGEnv(t)
+	t.Setenv("GNUPGHOME", otherHome)
+
+	// Keeping only the user IDs that match "nobody" exports none.
+	otherPubKey, _, err := runGPG(context.Background(), nil,
+		gpgArgs([]string{
+			"--export", gpgOptArmor, "--export-filter", "keep-uid=uid = nobody",
+		}, string(otherKey))...)
+	require.NoError(t, err)
+
+	keyID, gpgHome := testGPGEnv(t)
+	t.Setenv("GNUPGHOME", gpgHome)
+
+	manifest := rewriteOuter(t, signedTestManifest(t, keyID),
+		func(outer *MFFileOuter) {
+			outer.SigningPubKey = slices.Concat(
+				otherPubKey.Bytes(), outer.GetSigningPubKey())
+		})
+
+	_, err = NewManifestFromReader(bytes.NewReader(manifest))
+	require.ErrorIs(t, err, errSigningKeyCount)
+}
+
+// TestManifestRefusesTwoSignatures loads a manifest whose signature field
+// holds its good signature twice. Loading must refuse it.
+func TestManifestRefusesTwoSignatures(t *testing.T) {
+	keyID, gpgHome := testGPGEnv(t)
+	t.Setenv("GNUPGHOME", gpgHome)
+
+	manifest := rewriteOuter(t, signedTestManifest(t, keyID),
+		func(outer *MFFileOuter) {
+			outer.Signature = slices.Concat(
+				outer.GetSignature(), outer.GetSignature())
+		})
+
+	_, err := NewManifestFromReader(bytes.NewReader(manifest))
+	require.ErrorIs(t, err, errNotOneGoodSignature)
+}
+
+// TestManifestSignedWithSubkey signs with a key whose primary key can only
+// certify, so gpg signs with its signing subkey. The manifest must load,
+// with the primary key's fingerprint as signer.
+func TestManifestSignedWithSubkey(t *testing.T) {
+	gpgHome := t.TempDir()
+	t.Setenv("GNUPGHOME", gpgHome)
+
+	genTestKey(t, gpgHome, "Key-Type: RSA\nKey-Length: 2048\nKey-Usage: cert\n"+
+		"Subkey-Type: RSA\nSubkey-Length: 2048\nSubkey-Usage: sign\n"+
+		"Expire-Date: 0\n")
+
+	primary, err := gpgGetKeyFingerprint(context.Background(), "test@mfer.test")
+	require.NoError(t, err)
+
+	m, err := NewManifestFromReader(bytes.NewReader(
+		signedTestManifest(t, GPGKeyID("test@mfer.test"))))
+	require.NoError(t, err)
+	assert.Equal(t, primary, m.pbOuter.GetSigner())
+}
+
+// TestManifestRefusesSignerOtherThanSigningKey loads a manifest whose
+// signer field names a key other than the one that made the signature.
+func TestManifestRefusesSignerOtherThanSigningKey(t *testing.T) {
+	keyID, gpgHome := testGPGEnv(t)
+	t.Setenv("GNUPGHOME", gpgHome)
+
+	manifest := rewriteOuter(t, signedTestManifest(t, keyID),
+		func(outer *MFFileOuter) {
+			outer.Signer = []byte(strings.Repeat("A", len(keyID)))
+		})
+
+	_, err := NewManifestFromReader(bytes.NewReader(manifest))
+	require.ErrorIs(t, err, errSignerNotSigningKey)
+}
+
+// TestBuilderSigningKeyIDMatchingTwoKeys signs with a key ID that two keys
+// in the keyring match. The manifest must embed and name only the key that
+// signed it, or loading refuses it.
+func TestBuilderSigningKeyIDMatchingTwoKeys(t *testing.T) {
+	_, gpgHome := testGPGEnv(t)
+	genTestKey(t, gpgHome, testKeyParams)
+	t.Setenv("GNUPGHOME", gpgHome)
+
+	manifest := signedTestManifest(t, GPGKeyID("test@mfer.test"))
+
+	_, err := NewManifestFromReader(bytes.NewReader(manifest))
+	require.NoError(t, err)
+}
+
+// TestBuilderSigningUserIDWithExpiredFirstKey signs with a user ID whose
+// first key in the keyring has expired. gpg signs with the other key for
+// that user ID, and the manifest must name and embed that key.
+func TestBuilderSigningUserIDWithExpiredFirstKey(t *testing.T) {
+	gpgHome := t.TempDir()
+	t.Setenv("GNUPGHOME", gpgHome)
+
+	// Made in 2020 and valid for one day.
+	genTestKey(t, gpgHome, "Key-Type: RSA\nKey-Length: 2048\n"+
+		"Creation-Date: 20200101T000000\nExpire-Date: 1d\n")
+
+	expired, err := gpgGetKeyFingerprint(context.Background(), "test@mfer.test")
+	require.NoError(t, err)
+
+	genTestKey(t, gpgHome, testKeyParams)
+
+	m, err := NewManifestFromReader(bytes.NewReader(
+		signedTestManifest(t, GPGKeyID("test@mfer.test"))))
+	require.NoError(t, err)
+	assert.NotEqual(t, expired, m.pbOuter.GetSigner())
+}
+
 func TestBuilderWithoutSigning(t *testing.T) {
 	t.Parallel()
 
@@ -421,7 +610,7 @@ func TestGPGTimeoutKillsGPG(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
 	defer cancel()
 
-	_, err := gpgSign(ctx, []byte("data"), GPGKeyID("any"))
+	_, _, err := gpgSign(ctx, []byte("data"), GPGKeyID("any"))
 	require.ErrorIs(t, err, context.DeadlineExceeded)
 	assert.Contains(t, err.Error(), "gpg sign failed: gpg timed out")
 }
@@ -446,7 +635,7 @@ func TestGPGCancelWhenChildHoldsOutput(t *testing.T) {
 	signErr := make(chan error, 1)
 
 	go func() {
-		_, err := gpgSign(ctx, []byte("data"), GPGKeyID("any"))
+		_, _, err := gpgSign(ctx, []byte("data"), GPGKeyID("any"))
 		signErr <- err
 	}()
 

mfer/serialize.go

@@ -143,28 +143,32 @@ func (m *manifest) generateOuter(ctx context.Context) error {
 }
 
 // signOuter signs the outer message with the configured GPG key and
-// embeds the signature, signer fingerprint, and public key.
+// embeds the signature, signer fingerprint, and public key. The signer
+// and public key are those of the key gpg reports it signed with, so that
+// a key ID matching more than one key cannot name or embed another key.
 func (m *manifest) signOuter(ctx context.Context) error {
 	sigString, err := m.signatureString()
 	if err != nil {
 		return fmt.Errorf("failed to generate signature string: %w", err)
 	}
 
-	sig, err := gpgSign(ctx, []byte(sigString), m.signingOptions.KeyID)
+	sig, signingKey, err := gpgSign(ctx, []byte(sigString), m.signingOptions.KeyID)
 	if err != nil {
 		return fmt.Errorf("failed to sign manifest: %w", err)
 	}
 
 	m.pbOuter.Signature = sig
 
-	fingerprint, err := gpgGetKeyFingerprint(ctx, m.signingOptions.KeyID)
+	// Listing the signing key, a subkey's included, puts its primary key's
+	// fingerprint first.
+	fingerprint, err := gpgGetKeyFingerprint(ctx, GPGKeyID(signingKey))
 	if err != nil {
 		return fmt.Errorf("failed to get key fingerprint: %w", err)
 	}
 
 	m.pbOuter.Signer = fingerprint
 
-	pubKey, err := gpgExportPublicKey(ctx, m.signingOptions.KeyID)
+	pubKey, err := gpgExportPublicKey(ctx, GPGKeyID(fingerprint))
 	if err != nil {
 		return fmt.Errorf("failed to export public key: %w", err)
 	}