Compare --require-signature with the key that signed (closes #167)

check and fetch --require-signature compared the required fingerprint
with the first key in the manifest's embedded public key block, while
gpg accepted a good signature by any key in that block.

Loading a signed manifest now refuses one whose embedded block holds
more than one primary key, counted as gpg reads the block, or whose
signer field is not the primary key fingerprint gpg reports for the
signature. --require-signature compares with the signer field, which
loading has checked. Signing names and embeds the key gpg reports it
signed with, so a key ID matching several keys still writes a manifest
that loads. docs/FORMAT.md states what a verifier checks.

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
@@ -137,7 +137,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

@@ -126,10 +126,8 @@ func (mfa *CLIApp) fetchManifestToTemp(
 }
 
 // verifyRequiredSigner enforces the --require-signature fingerprint
-// against the manifest's embedded signing key.
-func verifyRequiredSigner(
-	ctx context.Context, chk *mfer.Checker, requiredSigner string,
-) error {
+// against the key that made the manifest's signature.
+func verifyRequiredSigner(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 +143,17 @@ func verifyRequiredSigner(
 			errManifestNotSigned, requiredSigner)
 	}
 
-	// Extract fingerprint from the embedded public key (not from the
-	// signer field). This validates the key is importable and gets its
-	// actual fingerprint.
-	embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP(ctx)
-	if err != nil {
-		return fmt.Errorf(
-			"failed to extract fingerprint from embedded signing key: %w", err)
-	}
+	// Loading the manifest checked that the signer is the fingerprint of
+	// the key that made the signature.
+	signer := string(chk.Signer())
 
 	// 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 +323,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_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(),
-			unsignedChecker(t), "12345678")
+		err := verifyRequiredSigner(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(),
-			unsignedChecker(t), msgFpA)
+		err := verifyRequiredSigner(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,
-// so it is read back from the checker and substituted into the expected
-// string; the required signer is a fixed value that cannot match it. Requires
-// gpg and is skipped where it is absent, as the other signing tests are.
+// manifest. The signing key's fingerprint is whatever the generated key
+// produced, so it is read back from the checker and substituted into the
+// expected string; the required signer is a fixed value that cannot match
+// 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())
-	require.NoError(t, err)
-
-	err = verifyRequiredSigner(context.Background(), chk, msgFpB)
+	err := verifyRequiredSigner(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

@@ -456,7 +456,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 +539,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(
-	ctx context.Context, manifestData []byte, requiredSigner string,
-) error {
+func verifyFetchedSigner(manifestData []byte, requiredSigner string) error {
 	memFs := afero.NewMemMapFs()
 	manifestPath := "/" + defaultManifestName
 
@@ -549,7 +548,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 +557,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

internal/cli/fetch_test.go

@@ -1117,8 +1117,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
-// without it, as the other signing tests are.
+// required key lets it through. A manifest signed by another key whose
+// 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 +1135,7 @@ func TestFetchRequireSignature(t *testing.T) {
 	t.Run("signed", func(t *testing.T) {
 		manifest := signedManifest(t, files)
 
-		signer, err := signedChecker(t, manifest).
-			ExtractEmbeddedSigningKeyFP(context.Background())
-		require.NoError(t, err)
+		signer := string(signedChecker(t, manifest).Signer())
 
 		assertFetchRefused(t, manifest, files,
 			"embedded signing key fingerprint "+signer+" does not match required "+msgFpB,
@@ -1153,6 +1153,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

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/deserialize.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/klauspost/compress/zstd"
 	"github.com/spf13/afero"
@@ -28,6 +29,8 @@ var (
 	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 +63,10 @@ func (m *manifest) validateOuterHeader() error {
 	return nil
 }
 
-// verifyOuterIntegrity checks the hash of the compressed payload and,
-// if a signature is present, verifies it against the embedded public key.
+// verifyOuterIntegrity checks the hash of the compressed payload and, if a
+// 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 +96,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 +106,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

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"
-	gpgOptHomedir = "--homedir"
-	gpgOptVerify  = "--verify"
+	gpgOptArmor    = "--armor"
+	gpgOptHomedir  = "--homedir"
+	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,51 @@ func parseFingerprint(colonOutput string) (string, bool) {
 	return "", false
 }
 
-// gpgSign creates a detached signature of the data using the specified key.
-// Returns the armored detached signature.
-func gpgSign(ctx context.Context, data []byte, keyID GPGKeyID) ([]byte, error) {
+// parseStatusLine returns the arguments of the status line for keyword in
+// gpg --status-fd output, or ok=false unless there is exactly one such line
+// 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) {
+	// The signature goes to stdout, so the status lines go to stderr.
 	stdout, stderr, err := runGPG(ctx, bytes.NewReader(data),
 		"--detach-sign",
 		gpgOptArmor,
+		gpgOptStatusFD, "2",
 		"--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(stderr.String(), "SIG_CREATED")
+	if !ok {
+		return nil, "", fmt.Errorf("%w: %s", errSigningKeyNotReported, stderr.String())
+	}
+
+	return stdout.Bytes(), created[len(created)-1], nil
 }
 
 // gpgExportPublicKey exports the public key for the specified key ID.
@@ -187,12 +229,44 @@ func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
 	return []byte(fpr), nil
 }
 
-// gpgExtractPubKeyFingerprint imports a public key into a temporary keyring
-// and extracts its fingerprint. This verifies the key is valid and returns
-// the actual fingerprint from the key material.
-func gpgExtractPubKeyFingerprint(ctx context.Context, pubKey []byte) (string, error) {
+// gpgImportOneKey imports the public key block in pubKeyFile into the
+// keyring in gpgHome. The block must hold exactly one primary key.
+func gpgImportOneKey(ctx context.Context, gpgHome, pubKeyFile 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 +287,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 +300,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
-	_, importStderr, err := runGPG(ctx, nil,
-		gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)...,
-	)
+	err = gpgImportOneKey(ctx, tmpDir, pubKeyFile)
 	if err != nil {
-		return fmt.Errorf(
-			"failed to import public key: %w: %s", err, importStderr.String(),
-		)
+		return "", err
 	}
 
-	// Verify the signature
-	_, verifyStderr, err := runGPG(ctx, nil,
-		gpgArgs([]string{gpgOptHomedir, tmpDir, gpgOptVerify},
+	// --status-fd 1 sends gpg's status lines to stdout, which verifying a
+	// detached signature otherwise leaves empty; its messages go to stderr.
+	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
-	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))
+	genTestKey(t, gpgHome, testKeyParams)
 
 	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-----")
@@ -169,7 +216,7 @@ func TestGPGSignInvalidKey(t *testing.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)
 }
@@ -259,15 +306,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
-	err = gpgVerify(context.Background(), data, sig, pubKey)
+	// Verify the signature; it names the key that made it
+	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 +323,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 +331,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 +340,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 +416,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 +607,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 +632,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)
 	}