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

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