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, or whose signer field is not the fingerprint
gpg reports for the signing key on its VALIDSIG status line.
--require-signature compares with the signer field, which loading has
checked. Signing now signs with and exports the key by its fingerprint,
so a key ID matching two keys still writes a manifest that loads.
docs/FORMAT.md states what a verifier checks.

Model: opus-5-5

mfer/gpg.go

@@ -41,15 +41,19 @@ const (
 	// fields in a gpg fingerprint record (the fingerprint is field 10).
 	gpgFingerprintMinFields = 10
 
-	// gpgStatusPrefix starts each status line gpg writes to the file
+	// gpgPrimaryKeyRecord starts the record of each primary public key in
-	// descriptor named by --status-fd.
+	// gpg --with-colons output.
-	gpgStatusPrefix = "[GNUPG:]"
+	gpgPrimaryKeyRecord = "pub:"
+
+	// gpgValidSigStatus starts the status line gpg --verify writes for a
+	// good signature. Its last field is the fingerprint of the primary key
+	// that made the signature.
+	gpgValidSigStatus = "[GNUPG:] VALIDSIG "
 
 	// gpg option names used from more than one call site.
-	gpgOptArmor    = "--armor"
+	gpgOptArmor   = "--armor"
-	gpgOptHomedir  = "--homedir"
+	gpgOptHomedir = "--homedir"
-	gpgOptStatusFD = "--status-fd"
+	gpgOptVerify  = "--verify"
-	gpgOptVerify   = "--verify"
 )
 
 var (
@@ -59,8 +63,6 @@ var (
 		"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).
@@ -146,51 +148,53 @@ func parseFingerprint(colonOutput string) (string, bool) {
 	return "", false
 }
 
-// parseStatusLine returns the arguments of the status line for keyword in
+// countPrimaryKeys returns the number of primary keys in gpg --with-colons
-// gpg --status-fd output, or ok=false unless there is exactly one such line
+// key listing output.
-// and it has arguments.
+func countPrimaryKeys(colonOutput string) int {
-func parseStatusLine(statusOutput, keyword string) ([]string, bool) {
+	count := 0
-	var found [][]string
 
-	for line := range strings.SplitSeq(statusOutput, "\n") {
+	for line := range strings.SplitSeq(colonOutput, "\n") {
-		fields := strings.Fields(line)
+		if strings.HasPrefix(line, gpgPrimaryKeyRecord) {
-		if len(fields) > 2 && fields[0] == gpgStatusPrefix && fields[1] == keyword {
+			count++
-			found = append(found, fields[2:])
 		}
 	}
 
-	if len(found) != 1 {
+	return count
-		return nil, false
-	}
-
-	return found[0], true
 }
 
-// gpgSign creates an armored detached signature of data with the key gpg
+// parseSigningKey returns the fingerprint of the primary key that made a
-// picks for keyID, and returns it with the fingerprint of the key that made
+// signature, the last field of the VALIDSIG line in gpg --verify status
-// it, which is a subkey's when gpg signed with a subkey.
+// output, or ok=false unless there is exactly one such line.
-func gpgSign(
+func parseSigningKey(statusOutput string) (string, bool) {
-	ctx context.Context, data []byte, keyID GPGKeyID,
+	var fingerprints []string
-) ([]byte, string, error) {
+
-	// The signature goes to stdout, so the status lines go to stderr.
+	for line := range strings.SplitSeq(statusOutput, "\n") {
+		if strings.HasPrefix(line, gpgValidSigStatus) {
+			fields := strings.Fields(line)
+			fingerprints = append(fingerprints, fields[len(fields)-1])
+		}
+	}
+
+	if len(fingerprints) != 1 {
+		return "", false
+	}
+
+	return fingerprints[0], true
+}
+
+// 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) {
 	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())
 	}
 
-	// The last argument of SIG_CREATED is the fingerprint of the key that
+	return stdout.Bytes(), nil
-	// 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.
@@ -229,14 +233,12 @@ func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
 	return []byte(fpr), nil
 }
 
-// gpgImportOneKey imports the public key block in pubKeyFile into the
+// gpgImportOneKey imports the public key in pubKeyFile into the keyring in
-// keyring in gpgHome. The block must hold exactly one primary key.
+// gpgHome, which must then 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
+	// Import the public key into the keyring
-	// otherwise leaves empty; its messages go to stderr.
+	_, importStderr, err := runGPG(ctx, nil,
-	importStdout, importStderr, err := runGPG(ctx, nil,
+		gpgArgs([]string{gpgOptHomedir, gpgHome, "--import"}, pubKeyFile)...,
-		gpgArgs([]string{gpgOptHomedir, gpgHome, gpgOptStatusFD, "1", "--import"},
-			pubKeyFile)...,
 	)
 	if err != nil {
 		return fmt.Errorf(
@@ -244,16 +246,21 @@ func gpgImportOneKey(ctx context.Context, gpgHome, pubKeyFile string) error {
 		)
 	}
 
-	// The first argument of IMPORT_RES counts the primary keys gpg read
+	// List keys to count them
-	// from the block, those it then skipped (one with no user ID, for
+	listStdout, listStderr, err := runGPG(ctx, nil,
-	// example) included.
+		gpgOptHomedir, gpgHome,
-	result, ok := parseStatusLine(importStdout.String(), "IMPORT_RES")
+		"--with-colons",
-	if !ok {
+		"--list-keys",
-		return fmt.Errorf("%w, gpg reported no count", errSigningKeyCount)
+	)
+	if err != nil {
+		return fmt.Errorf(
+			"failed to list keys: %w: %s", err, listStderr.String(),
+		)
 	}
 
-	if result[0] != "1" {
+	keys := countPrimaryKeys(listStdout.String())
-		return fmt.Errorf("%w, found %s", errSigningKeyCount, result[0])
+	if keys != 1 {
+		return fmt.Errorf("%w, found %d", errSigningKeyCount, keys)
 	}
 
 	return nil
@@ -311,7 +318,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
 	// --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},
+		gpgArgs([]string{gpgOptHomedir, tmpDir, "--status-fd", "1", gpgOptVerify},
 			sigFile, dataFile)...,
 	)
 	if err != nil {
@@ -320,13 +327,10 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
 		)
 	}
 
-	// gpg writes a VALIDSIG line for each good signature. Its first
+	fingerprint, ok := parseSigningKey(verifyStdout.String())
-	// 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
+	return fingerprint, nil
 }

mfer/gpg_test.go

@@ -37,7 +37,7 @@ func testGPGEnv(t *testing.T) (GPGKeyID, string) {
 	// Create temporary GPG home directory (0700 by default)
 	gpgHome := t.TempDir()
 
-	genTestKey(t, gpgHome, testKeyParams)
+	genTestKey(t, gpgHome)
 
 	ctx, cancel := context.WithTimeout(context.Background(), gpgTimeout)
 	defer cancel()
@@ -73,20 +73,21 @@ func testGPGEnv(t *testing.T) (GPGKeyID, string) {
 	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
+// "MFER Test Key <test@mfer.test>" in gpgHome, which may already hold one.
-// keyParams in gpgHome, which may already hold one.
+func genTestKey(t *testing.T, gpgHome string) {
-func genTestKey(t *testing.T, gpgHome, keyParams string) {
 	t.Helper()
 
-	params := "%no-protection\n" + keyParams +
+	keyParams := `%no-protection
-		"Name-Real: MFER Test Key\nName-Email: test@mfer.test\n%commit\n"
+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(params), 0o600))
+	require.NoError(t, os.WriteFile(paramsFile, []byte(keyParams), 0o600))
 
 	ctx, cancel := context.WithTimeout(context.Background(), gpgTimeout)
 	defer cancel()
@@ -143,9 +144,8 @@ func TestGPGSign(t *testing.T) {
 	t.Setenv("GNUPGHOME", gpgHome)
 
 	data := []byte("test data to sign")
-	sig, signingKey, err := gpgSign(context.Background(), data, keyID)
+	sig, 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-----")
@@ -216,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)
 }
@@ -306,7 +306,7 @@ 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)
@@ -323,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)
@@ -340,7 +340,7 @@ 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
@@ -439,70 +439,6 @@ func TestManifestRefusesSecondEmbeddedKey(t *testing.T) {
 	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) {
@@ -523,7 +459,7 @@ func TestManifestRefusesSignerOtherThanSigningKey(t *testing.T) {
 // signed it, or loading refuses it.
 func TestBuilderSigningKeyIDMatchingTwoKeys(t *testing.T) {
 	_, gpgHome := testGPGEnv(t)
-	genTestKey(t, gpgHome, testKeyParams)
+	genTestKey(t, gpgHome)
 	t.Setenv("GNUPGHOME", gpgHome)
 
 	manifest := signedTestManifest(t, GPGKeyID("test@mfer.test"))
@@ -532,28 +468,6 @@ func TestBuilderSigningKeyIDMatchingTwoKeys(t *testing.T) {
 	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()
 
@@ -607,7 +521,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")
 }
@@ -632,7 +546,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,31 +143,29 @@ 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. The signer
+// embeds the signature, signer fingerprint, and public key. It signs with
-// and public key are those of the key gpg reports it signed with, so that
+// and exports the key by its fingerprint, so that a key ID matching more
-// a key ID matching more than one key cannot name or embed another key.
+// than one key cannot embed a key other than the one that signed.
 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, signingKey, err := gpgSign(ctx, []byte(sigString), m.signingOptions.KeyID)
+	fingerprint, err := gpgGetKeyFingerprint(ctx, m.signingOptions.KeyID)
-	if err != nil {
-		return fmt.Errorf("failed to sign manifest: %w", err)
-	}
-
-	m.pbOuter.Signature = sig
-
-	// 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
 
+	sig, err := gpgSign(ctx, []byte(sigString), GPGKeyID(fingerprint))
+	if err != nil {
+		return fmt.Errorf("failed to sign manifest: %w", err)
+	}
+
+	m.pbOuter.Signature = sig
+
 	pubKey, err := gpgExportPublicKey(ctx, GPGKeyID(fingerprint))
 	if err != nil {
 		return fmt.Errorf("failed to export public key: %w", err)