Compare --require-signature with the key that signed (closes #167)
check / check (push) Waiting to run

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
This commit is contained in:
2026-10-07 09:24:07 +00:00
parent 2a174e3ba2
commit 7f0bcfb228
11 changed files with 364 additions and 169 deletions
+11 -2
View File
@@ -37,7 +37,7 @@ The outer message contains:
| `uuid` | 105 | bytes | Random v4 UUID; must match the inner message UUID | | `uuid` | 105 | bytes | Random v4 UUID; must match the inner message UUID |
| `innerMessage` | 199 | bytes | Zstd-compressed serialized `MFFile` message | | `innerMessage` | 199 | bytes | Zstd-compressed serialized `MFFile` message |
| `signature` | 201 | bytes (optional) | GPG signature (ASCII-armored or binary) | | `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 | | `signingPubKey` | 203 | bytes (optional) | Full GPG signing public key |
### SHA-256 Hash ### SHA-256 Hash
@@ -137,7 +137,16 @@ Where:
compressed data) compressed data)
Components are separated by hyphens. The signature is produced by GPG over this 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 ## Deterministic Serialization
+9 -16
View File
@@ -126,10 +126,8 @@ func (mfa *CLIApp) fetchManifestToTemp(
} }
// verifyRequiredSigner enforces the --require-signature fingerprint // 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 // Validate fingerprint format: must be exactly 40 hex characters
if len(requiredSigner) != fingerprintHexLen { if len(requiredSigner) != fingerprintHexLen {
return fmt.Errorf("%w, got %d", errInvalidFingerprint, len(requiredSigner)) return fmt.Errorf("%w, got %d", errInvalidFingerprint, len(requiredSigner))
@@ -145,22 +143,17 @@ func verifyRequiredSigner(
errManifestNotSigned, requiredSigner) 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) // 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", 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 return nil
} }
@@ -330,7 +323,7 @@ func (mfa *CLIApp) checkManifestOperation(
// Check signature requirement // Check signature requirement
requiredSigner := cmd.String(flagRequireSignature) requiredSigner := cmd.String(flagRequireSignature)
if requiredSigner != "" { if requiredSigner != "" {
err = verifyRequiredSigner(ctx, chk, requiredSigner) err = verifyRequiredSigner(chk, requiredSigner)
if err != nil { if err != nil {
return err return err
} }
+27
View File
@@ -613,6 +613,33 @@ func runCheckAfterRewrite(t *testing.T, rewritten, msg string) {
assert.Equal(t, 1, exitCode, msg) 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) { func TestCheckCommandWithCorruptedFile(t *testing.T) {
t.Parallel() t.Parallel()
+39 -13
View File
@@ -9,12 +9,14 @@ import (
"os" "os"
"os/exec" "os/exec"
"path/filepath" "path/filepath"
"slices"
"testing" "testing"
"github.com/spf13/afero" "github.com/spf13/afero"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
urfcli "github.com/urfave/cli/v3" urfcli "github.com/urfave/cli/v3"
"google.golang.org/protobuf/proto"
"sneak.berlin/go/mfer/mfer" "sneak.berlin/go/mfer/mfer"
) )
@@ -86,8 +88,7 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
t.Run("invalid fingerprint length", func(t *testing.T) { t.Run("invalid fingerprint length", func(t *testing.T) {
t.Parallel() t.Parallel()
err := verifyRequiredSigner(context.Background(), err := verifyRequiredSigner(unsignedChecker(t), "12345678")
unsignedChecker(t), "12345678")
require.ErrorIs(t, err, errInvalidFingerprint) require.ErrorIs(t, err, errInvalidFingerprint)
assert.EqualError(t, err, assert.EqualError(t, err,
"invalid fingerprint: must be exactly 40 hex characters, got 8") "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.Run("manifest not signed", func(t *testing.T) {
t.Parallel() t.Parallel()
err := verifyRequiredSigner(context.Background(), err := verifyRequiredSigner(unsignedChecker(t), msgFpA)
unsignedChecker(t), msgFpA)
require.ErrorIs(t, err, errManifestNotSigned) require.ErrorIs(t, err, errManifestNotSigned)
assert.EqualError(t, err, assert.EqualError(t, err,
"manifest is not signed, but signature from "+msgFpA+" is required") "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 // 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 //nolint:paralleltest // signedManifest calls t.Setenv, which bars t.Parallel
func TestSignerMismatchMessage(t *testing.T) { func TestSignerMismatchMessage(t *testing.T) {
chk := signedChecker(t, chk := signedChecker(t,
signedManifest(t, map[string][]byte{"f.txt": []byte("signed file")})) 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) require.ErrorIs(t, err, errSignerMismatch)
assert.EqualError(t, err, assert.EqualError(t, err,
"embedded signing key fingerprint "+embeddedFP+ "embedded signing key fingerprint "+string(chk.Signer())+
" does not match required "+msgFpB) " does not match required "+msgFpB)
} }
@@ -191,6 +189,34 @@ func signedChecker(t *testing.T, manifest []byte) *mfer.Checker {
return chk 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) { func TestPathDoesNotExistMessage(t *testing.T) {
t.Parallel() t.Parallel()
+4 -6
View File
@@ -456,7 +456,8 @@ func fetchManifest(
requiredSigner := cmd.String(flagRequireSignature) requiredSigner := cmd.String(flagRequireSignature)
if requiredSigner != "" { if requiredSigner != "" {
err = verifyFetchedSigner(ctx, manifestData, requiredSigner) //nolint:contextcheck // mfer loads a manifest without a context
err = verifyFetchedSigner(manifestData, requiredSigner)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
@@ -538,9 +539,7 @@ func checkNoNameClash(files []*mfer.MFFilePath) error {
// exactly as check does. verifyRequiredSigner takes a Checker, which loads // 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 // its manifest from a file, so the manifest is handed to it as a file in
// memory. // memory.
func verifyFetchedSigner( func verifyFetchedSigner(manifestData []byte, requiredSigner string) error {
ctx context.Context, manifestData []byte, requiredSigner string,
) error {
memFs := afero.NewMemMapFs() memFs := afero.NewMemMapFs()
manifestPath := "/" + defaultManifestName manifestPath := "/" + defaultManifestName
@@ -549,7 +548,6 @@ func verifyFetchedSigner(
return err return err
} }
//nolint:contextcheck // mfer loads a manifest without a context
chk, err := mfer.NewChecker(&mfer.CheckerOptions{ chk, err := mfer.NewChecker(&mfer.CheckerOptions{
ManifestPath: manifestPath, ManifestPath: manifestPath,
BasePath: "/", BasePath: "/",
@@ -559,7 +557,7 @@ func verifyFetchedSigner(
return fmt.Errorf("failed to load manifest: %w", err) 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 // saveManifest writes the fetched manifest into dest under the default
+14 -5
View File
@@ -1117,8 +1117,10 @@ func TestFetchIntoDest(t *testing.T) {
// TestFetchRequireSignature runs fetch with --require-signature. A // TestFetchRequireSignature runs fetch with --require-signature. A
// manifest that is unsigned, or signed by another key, must stop fetch // manifest that is unsigned, or signed by another key, must stop fetch
// with check's message before it downloads or writes anything; the // 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 //nolint:paralleltest // signedManifest calls t.Setenv, which bars t.Parallel
func TestFetchRequireSignature(t *testing.T) { func TestFetchRequireSignature(t *testing.T) {
@@ -1133,9 +1135,7 @@ func TestFetchRequireSignature(t *testing.T) {
t.Run("signed", func(t *testing.T) { t.Run("signed", func(t *testing.T) {
manifest := signedManifest(t, files) 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, assertFetchRefused(t, manifest, files,
"embedded signing key fingerprint "+signer+" does not match required "+msgFpB, "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)) require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
assert.Equal(t, files[testFileTxt], filesUnder(t, dest)[testFileTxt]) 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 // TestFetchRefusesListedManifestName fetches manifests that list, at the
+7 -13
View File
@@ -15,7 +15,6 @@ import (
) )
var ( var (
errNoSigningPubKey = errors.New("manifest has no signing public key")
errManifestPathEmpty = errors.New("manifest path cannot be empty") errManifestPathEmpty = errors.New("manifest path cannot be empty")
errBasePathEmpty = errors.New("base 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 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 { func (c *Checker) Signer() []byte {
if !c.IsSigned() {
return nil
}
return c.signer return c.signer
} }
@@ -184,17 +189,6 @@ func (c *Checker) SigningPubKey() []byte {
return c.signingPubKey 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. // Check verifies all files against the manifest.
// Results are sent to the results channel as files are checked. // Results are sent to the results channel as files are checked.
// Progress updates are sent to the progress channel approximately once per second. // Progress updates are sent to the progress channel approximately once per second.
+13 -3
View File
@@ -7,6 +7,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"io" "io"
"strings"
"github.com/klauspost/compress/zstd" "github.com/klauspost/compress/zstd"
"github.com/spf13/afero" "github.com/spf13/afero"
@@ -28,6 +29,8 @@ var (
errInvalidManifestPath = errors.New("manifest contains invalid path") errInvalidManifestPath = errors.New("manifest contains invalid path")
errDecodedTooLarge = errors.New( errDecodedTooLarge = errors.New(
"manifest would take too much memory to decode") "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. // 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 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 { func (m *manifest) verifyOuterIntegrity() error {
h := sha256.New() h := sha256.New()
@@ -91,7 +96,7 @@ func (m *manifest) verifyOuterIntegrity() error {
} }
// Loading a manifest takes no context; gpgTimeout still bounds gpg. // Loading a manifest takes no context; gpgTimeout still bounds gpg.
err = gpgVerify( signingKey, err := gpgVerify(
context.Background(), context.Background(),
[]byte(sigString), []byte(sigString),
m.pbOuter.GetSignature(), m.pbOuter.GetSignature(),
@@ -101,6 +106,11 @@ func (m *manifest) verifyOuterIntegrity() error {
return fmt.Errorf("signature verification failed: %w", err) 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") log.Infof("signature verified successfully")
return nil return nil
+102 -75
View File
@@ -41,6 +41,15 @@ const (
// fields in a gpg fingerprint record (the fingerprint is field 10). // fields in a gpg fingerprint record (the fingerprint is field 10).
gpgFingerprintMinFields = 10 gpgFingerprintMinFields = 10
// gpgPrimaryKeyRecord starts the record of each primary public key in
// gpg --with-colons output.
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. // gpg option names used from more than one call site.
gpgOptArmor = "--armor" gpgOptArmor = "--armor"
gpgOptHomedir = "--homedir" gpgOptHomedir = "--homedir"
@@ -50,7 +59,10 @@ const (
var ( var (
errGPGKeyNotFound = errors.New("gpg key not found") errGPGKeyNotFound = errors.New("gpg key not found")
errFingerprintNotFound = errors.New("fingerprint not found for key") 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")
) )
// GPGKeyID represents a GPG key identifier (fingerprint or key ID). // GPGKeyID represents a GPG key identifier (fingerprint or key ID).
@@ -136,6 +148,40 @@ func parseFingerprint(colonOutput string) (string, bool) {
return "", false return "", false
} }
// countPrimaryKeys returns the number of primary keys in gpg --with-colons
// key listing output.
func countPrimaryKeys(colonOutput string) int {
count := 0
for line := range strings.SplitSeq(colonOutput, "\n") {
if strings.HasPrefix(line, gpgPrimaryKeyRecord) {
count++
}
}
return count
}
// parseSigningKey returns the fingerprint of the primary key that made a
// signature, the last field of the VALIDSIG line in gpg --verify status
// output, or ok=false unless there is exactly one such line.
func parseSigningKey(statusOutput string) (string, bool) {
var fingerprints []string
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. // gpgSign creates a detached signature of the data using the specified key.
// Returns the armored detached signature. // Returns the armored detached signature.
func gpgSign(ctx context.Context, data []byte, keyID GPGKeyID) ([]byte, error) { func gpgSign(ctx context.Context, data []byte, keyID GPGKeyID) ([]byte, error) {
@@ -187,12 +233,47 @@ func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
return []byte(fpr), nil return []byte(fpr), nil
} }
// gpgExtractPubKeyFingerprint imports a public key into a temporary keyring // gpgImportOneKey imports the public key in pubKeyFile into the keyring in
// and extracts its fingerprint. This verifies the key is valid and returns // gpgHome, which must then 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) { // Import the public key into the keyring
_, importStderr, err := runGPG(ctx, nil,
gpgArgs([]string{gpgOptHomedir, gpgHome, "--import"}, pubKeyFile)...,
)
if err != nil {
return fmt.Errorf(
"failed to import public key: %w: %s", err, importStderr.String(),
)
}
// List keys to count them
listStdout, listStderr, err := runGPG(ctx, nil,
gpgOptHomedir, gpgHome,
"--with-colons",
"--list-keys",
)
if err != nil {
return fmt.Errorf(
"failed to list keys: %w: %s", err, listStderr.String(),
)
}
keys := countPrimaryKeys(listStdout.String())
if keys != 1 {
return fmt.Errorf("%w, found %d", errSigningKeyCount, keys)
}
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 // Create temporary directory for GPG operations
tmpDir, err := os.MkdirTemp("", "mfer-gpg-fingerprint-*") tmpDir, err := os.MkdirTemp("", "mfer-gpg-verify-*")
if err != nil { if err != nil {
return "", fmt.Errorf("failed to create temp dir: %w", err) return "", fmt.Errorf("failed to create temp dir: %w", err)
} }
@@ -213,67 +294,12 @@ func gpgExtractPubKeyFingerprint(ctx context.Context, pubKey []byte) (string, er
return "", fmt.Errorf("failed to write public key: %w", err) 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 // Write signature to temp file
sigFile := filepath.Join(tmpDir, "signature.asc") sigFile := filepath.Join(tmpDir, "signature.asc")
err = os.WriteFile(sigFile, signature, privateFilePerms) err = os.WriteFile(sigFile, signature, privateFilePerms)
if err != nil { 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 // Write data to temp file
@@ -281,29 +307,30 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) error {
err = os.WriteFile(dataFile, data, privateFilePerms) err = os.WriteFile(dataFile, data, privateFilePerms)
if err != nil { 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 { 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, "--status-fd", "1", gpgOptVerify},
sigFile, dataFile)..., sigFile, dataFile)...,
) )
if err != nil { if err != nil {
return fmt.Errorf( return "", fmt.Errorf(
"signature verification failed: %w: %s", err, verifyStderr.String(), "signature verification failed: %w: %s", err, verifyStderr.String(),
) )
} }
return nil fingerprint, ok := parseSigningKey(verifyStdout.String())
if !ok {
return "", errNotOneGoodSignature
}
return fingerprint, nil
} }
+127 -27
View File
@@ -8,6 +8,7 @@ import (
"os" "os"
"os/exec" "os/exec"
"path/filepath" "path/filepath"
"slices"
"strconv" "strconv"
"strings" "strings"
"syscall" "syscall"
@@ -17,6 +18,7 @@ import (
"github.com/spf13/afero" "github.com/spf13/afero"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"google.golang.org/protobuf/proto"
) )
// testGPGEnv sets up a temporary GPG home directory with a test key. // testGPGEnv sets up a temporary GPG home directory with a test key.
@@ -35,7 +37,47 @@ func testGPGEnv(t *testing.T) (GPGKeyID, string) {
// Create temporary GPG home directory (0700 by default) // Create temporary GPG home directory (0700 by default)
gpgHome := t.TempDir() gpgHome := t.TempDir()
// Generate a test key with no passphrase genTestKey(t, gpgHome)
ctx, cancel := context.WithTimeout(context.Background(), gpgTimeout)
defer cancel()
// Get the key fingerprint
cmd := exec.CommandContext(ctx, "gpg",
"--list-keys", "--with-colons", "test@mfer.test")
cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome)
output, err := cmd.Output()
if err != nil {
t.Fatalf("failed to list test key: %v", err)
}
// Parse fingerprint from output
var keyID string
for line := range strings.SplitSeq(string(output), "\n") {
fields := strings.Split(line, ":")
if len(fields) >= gpgFingerprintMinFields &&
fields[0] == gpgFingerprintField {
keyID = fields[9]
break
}
}
if keyID == "" {
t.Fatal("failed to find test key fingerprint")
}
return GPGKeyID(keyID), gpgHome
}
// genTestKey generates a key with no passphrase for
// "MFER Test Key <test@mfer.test>" in gpgHome, which may already hold one.
func genTestKey(t *testing.T, gpgHome string) {
t.Helper()
keyParams := `%no-protection keyParams := `%no-protection
Key-Type: RSA Key-Type: RSA
Key-Length: 2048 Key-Length: 2048
@@ -60,36 +102,41 @@ Expire-Date: 0
if err != nil { if err != nil {
t.Skipf("failed to generate test GPG key: %v: %s", err, output) t.Skipf("failed to generate test GPG key: %v: %s", err, output)
} }
}
// Get the key fingerprint // signedTestManifest returns a manifest of one file signed with keyID.
cmd = exec.CommandContext(ctx, "gpg", func signedTestManifest(t *testing.T, keyID GPGKeyID) []byte {
"--list-keys", "--with-colons", "test@mfer.test") t.Helper()
cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome) b := NewBuilder()
b.SetSigningOptions(&SigningOptions{KeyID: keyID})
output, err = cmd.Output() content := []byte("signed file content")
if err != nil { _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, 0,
t.Fatalf("failed to list test key: %v", err) bytes.NewReader(content), nil)
} require.NoError(t, err)
// Parse fingerprint from output var buf bytes.Buffer
var keyID string
for line := range strings.SplitSeq(string(output), "\n") { require.NoError(t, b.Build(context.Background(), &buf))
fields := strings.Split(line, ":")
if len(fields) >= gpgFingerprintMinFields &&
fields[0] == gpgFingerprintField {
keyID = fields[9]
break return buf.Bytes()
} }
}
if keyID == "" { // rewriteOuter returns manifest with its outer message changed by edit.
t.Fatal("failed to find test key fingerprint") // 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()
return GPGKeyID(keyID), gpgHome 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) { func TestGPGSign(t *testing.T) {
@@ -265,9 +312,10 @@ func TestGPGVerify(t *testing.T) {
pubKey, err := gpgExportPublicKey(context.Background(), keyID) pubKey, err := gpgExportPublicKey(context.Background(), keyID)
require.NoError(t, err) 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) require.NoError(t, err)
assert.Equal(t, string(keyID), signingKey)
} }
func TestGPGVerifyInvalidSignature(t *testing.T) { func TestGPGVerifyInvalidSignature(t *testing.T) {
@@ -283,7 +331,7 @@ func TestGPGVerifyInvalidSignature(t *testing.T) {
// Try to verify with different data - should fail // Try to verify with different data - should fail
wrongData := []byte("different data") wrongData := []byte("different data")
err = gpgVerify(context.Background(), wrongData, sig, pubKey) _, err = gpgVerify(context.Background(), wrongData, sig, pubKey)
assert.Error(t, err) assert.Error(t, err)
} }
@@ -297,7 +345,7 @@ func TestGPGVerifyBadPublicKey(t *testing.T) {
// Try to verify with invalid public key - should fail // Try to verify with invalid public key - should fail
badPubKey := []byte("not a valid public key") 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) assert.Error(t, err)
} }
@@ -368,6 +416,58 @@ func TestManifestTamperedSignatureFails(t *testing.T) {
assert.Error(t, err) 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)
}
// 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)
t.Setenv("GNUPGHOME", gpgHome)
manifest := signedTestManifest(t, GPGKeyID("test@mfer.test"))
_, err := NewManifestFromReader(bytes.NewReader(manifest))
require.NoError(t, err)
}
func TestBuilderWithoutSigning(t *testing.T) { func TestBuilderWithoutSigning(t *testing.T) {
t.Parallel() t.Parallel()
+11 -9
View File
@@ -143,20 +143,15 @@ func (m *manifest) generateOuter(ctx context.Context) error {
} }
// signOuter signs the outer message with the configured GPG key and // 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. It signs with
// and exports the key by its fingerprint, so that a key ID matching more
// than one key cannot embed a key other than the one that signed.
func (m *manifest) signOuter(ctx context.Context) error { func (m *manifest) signOuter(ctx context.Context) error {
sigString, err := m.signatureString() sigString, err := m.signatureString()
if err != nil { if err != nil {
return fmt.Errorf("failed to generate signature string: %w", err) return fmt.Errorf("failed to generate signature string: %w", err)
} }
sig, 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) fingerprint, err := gpgGetKeyFingerprint(ctx, m.signingOptions.KeyID)
if err != nil { if err != nil {
return fmt.Errorf("failed to get key fingerprint: %w", err) return fmt.Errorf("failed to get key fingerprint: %w", err)
@@ -164,7 +159,14 @@ func (m *manifest) signOuter(ctx context.Context) error {
m.pbOuter.Signer = fingerprint m.pbOuter.Signer = fingerprint
pubKey, err := gpgExportPublicKey(ctx, m.signingOptions.KeyID) 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 { if err != nil {
return fmt.Errorf("failed to export public key: %w", err) return fmt.Errorf("failed to export public key: %w", err)
} }