From e00ec787e84a80ba5a8c1cc1fae3f9a58e133d45 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Thu, 8 Oct 2026 01:02:03 +0000 Subject: [PATCH] Sign and verify manifests in Go with OpenPGP instead of running gpg (closes #181) mfer ran the gpg binary to sign, export keys and verify, so signing and loading signed manifests failed wherever gpg is missing. It now uses github.com/ProtonMail/go-crypto/openpgp. --sign-key and MFER_SIGN_KEY name a file holding one version 4 OpenPGP secret key; a protected key's passphrase comes from MFER_SIGN_KEY_PASSPHRASE or a terminal prompt. gen and freshen check that the key can sign before they read any file. Verification keeps the rules of the --require-signature fix: one primary key in the embedded block, counted from its packets, exactly one signature, made by that key or a subkey, and signer equal to its fingerprint. A DSA key is refused, and so is an armored field that is not one well-formed block. Model: opus-5-5 --- README.md | 16 +- docs/FORMAT.md | 18 +- go.mod | 3 + go.sum | 6 + internal/cli/check.go | 5 +- internal/cli/entry_test.go | 7 +- internal/cli/errmsg_test.go | 106 +++-- internal/cli/export.go | 1 - internal/cli/fetch.go | 2 - internal/cli/fetch_test.go | 13 +- internal/cli/freshen.go | 32 +- internal/cli/gen.go | 19 +- internal/cli/list.go | 1 - internal/cli/mfer.go | 6 +- internal/cli/signing.go | 86 ++++ internal/cli/signing_test.go | 209 ++++++++++ mfer/builder.go | 6 +- mfer/deserialize.go | 5 +- mfer/deserialize_fuzz_test.go | 8 - mfer/gpg.go | 358 ---------------- mfer/gpg_test.go | 703 ------------------------------- mfer/openpgp.go | 363 ++++++++++++++++ mfer/openpgp_test.go | 754 ++++++++++++++++++++++++++++++++++ mfer/scanner.go | 2 +- mfer/serialize.go | 34 +- 25 files changed, 1585 insertions(+), 1178 deletions(-) create mode 100644 internal/cli/signing.go create mode 100644 internal/cli/signing_test.go delete mode 100644 mfer/gpg.go delete mode 100644 mfer/gpg_test.go create mode 100644 mfer/openpgp.go create mode 100644 mfer/openpgp_test.go diff --git a/README.md b/README.md index 4daf28f..9ba95de 100644 --- a/README.md +++ b/README.md @@ -257,8 +257,9 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues). - Should the manifest signature format be GnuPG signatures, or those from OpenBSD's signify (of which there is a good - [golang implementation](https://github.com/frankbraun/gosignify))? Still open, - as question 10 on [issue 82](https://git.eeqj.de/sneak/mfer/issues/82). + [golang implementation](https://github.com/frankbraun/gosignify))? Settled + under question 10 on [issue 82](https://git.eeqj.de/sneak/mfer/issues/82): + OpenPGP signatures, which mfer makes and checks itself without running `gpg`. - Should the on-disk serialization format be proto3 or json? Settled: it is proto3, see `docs/FORMAT.md` and `mfer/mf.proto`. @@ -296,6 +297,17 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues). - leaves out hidden files unless given `--include-dotfiles`, and symlinks unless given `--follow-symlinks`, which lists each symlink to a file under its own name with the contents of the file it points to +- `mfer gen --sign-key key.asc` / `mfer freshen --sign-key key.asc` + - signs the manifest with the OpenPGP secret key in `key.asc`, armored or + binary, as `gpg --export-secret-keys` writes it; `MFER_SIGN_KEY` names the + file too. mfer signs it itself and does not need `gpg`. A file holding + more than one key is refused, and a key held only on a smartcard cannot + sign. The key must be a version 4 key, whose 40-character fingerprint is + what `--require-signature` takes, and not a DSA key. A key that cannot + sign, because it has expired or been revoked or its passphrase is wrong, + stops `gen` and `freshen` before they read any file + - takes a protected key's passphrase from `MFER_SIGN_KEY_PASSPHRASE`, or + else asks for it at the terminal - `mfer fetch https://example.com/stuff/` - fetches `/stuff/index.mf` and downloads all files listed in manifest into the current directory, or the one given with `--dest`, and assures diff --git a/docs/FORMAT.md b/docs/FORMAT.md index e21ce06..663b7e4 100644 --- a/docs/FORMAT.md +++ b/docs/FORMAT.md @@ -6,7 +6,7 @@ Version 1.0 An `.mf` file is a binary manifest that describes a directory tree of files, including their paths, sizes, and cryptographic checksums. It supports optional -GPG signatures for integrity verification and optional timestamps and file +OpenPGP signatures for integrity verification and optional timestamps and file permissions for metadata preservation. Nothing goes in the 1.0 manifest that 1.0 does not read or write: no field is @@ -36,9 +36,9 @@ The outer message contains: | `sha256` | 104 | bytes | SHA-256 hash of the **compressed** `innerMessage` (corruption detection) | | `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) | +| `signature` | 201 | bytes (optional) | OpenPGP detached signature (ASCII-armored or binary) | | `signer` | 202 | bytes (optional) | Fingerprint of the signing key | -| `signingPubKey` | 203 | bytes (optional) | Full GPG signing public key | +| `signingPubKey` | 203 | bytes (optional) | Full OpenPGP public key of the signing key (ASCII-armored or binary) | ### SHA-256 Hash @@ -142,17 +142,19 @@ Where: - `` is the hex-encoded SHA-256 hash from the outer message (covering 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. The -signing key's public key goes in `signingPubKey` and its fingerprint, in hex, in -`signer`. +Components are separated by hyphens. The signature is an OpenPGP detached +signature over this canonical string, 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`. +fingerprint with `signer`. It also refuses a manifest whose `signingPubKey` +holds a DSA key or subkey, since checking the self-signatures of a DSA key with +very large numbers can take hours. ## Deterministic Serialization diff --git a/go.mod b/go.mod index 66e394f..5e3752d 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,8 @@ module sneak.berlin/go/mfer go 1.27.1 require ( + github.com/ProtonMail/go-crypto v1.5.2 + github.com/creack/pty v1.1.25-0.20260601142114-9246436fffe8 github.com/davecgh/go-spew v1.1.1 github.com/dustin/go-humanize v1.1.0 github.com/klauspost/compress v1.20.1 @@ -15,6 +17,7 @@ require ( ) require ( + github.com/cloudflare/circl v1.6.3 // indirect github.com/klauspost/cpuid/v2 v2.4.0 // indirect github.com/minio/sha256-simd v1.0.1 // indirect github.com/mr-tron/base58 v1.3.0 // indirect diff --git a/go.sum b/go.sum index 5007fbb..7c041b8 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,9 @@ +github.com/ProtonMail/go-crypto v1.5.2 h1:cucYnvqcY7UOXVD//mSyjeaPY0SSN3v5cDkYPxumINk= +github.com/ProtonMail/go-crypto v1.5.2/go.mod h1:/RaSu30DaKO4RY+XdV/ACcCcZkGr7AhUIduq5sjzzCo= +github.com/cloudflare/circl v1.6.3 h1:9GPOhQGF9MCYUeXyMYlqTR6a5gTrgR/fBLXvUgtVcg8= +github.com/cloudflare/circl v1.6.3/go.mod h1:2eXP6Qfat4O/Yhh8BznvKnJ+uzEoTQ6jVKJRn81BiS4= +github.com/creack/pty v1.1.25-0.20260601142114-9246436fffe8 h1:CY3gjC7naqYGLMiywvj3suPfa1i0p/QEr7o8ujxL/2M= +github.com/creack/pty v1.1.25-0.20260601142114-9246436fffe8/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/dustin/go-humanize v1.1.0 h1:dbKTrvD0klcbBV/h4AWJdMuZogJACoMlvWIWZ5b2xWg= diff --git a/internal/cli/check.go b/internal/cli/check.go index fceb118..9b63b36 100644 --- a/internal/cli/check.go +++ b/internal/cli/check.go @@ -21,8 +21,8 @@ import ( "sneak.berlin/go/mfer/mfer" ) -// fingerprintHexLen is the length of a full GPG key fingerprint in hex -// characters. +// fingerprintHexLen is the length in hex characters of the fingerprint of +// an OpenPGP version 4 key, the only version mfer signs with. const fingerprintHexLen = 40 var ( @@ -320,7 +320,6 @@ func (mfa *CLIApp) checkManifestOperation( log.Infof("checking manifest %s with base %s", manifestPath, basePath) // Create checker - //nolint:contextcheck // mfer loads a manifest without a context chk, err := mfer.NewChecker(&mfer.CheckerOptions{ ManifestPath: manifestPath, BasePath: basePath, diff --git a/internal/cli/entry_test.go b/internal/cli/entry_test.go index bf2c718..73c266a 100644 --- a/internal/cli/entry_test.go +++ b/internal/cli/entry_test.go @@ -654,11 +654,10 @@ func runCheckAfterRewrite(t *testing.T, rewritten, msg string) { // 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 +// public key block also holds the required key. check must refuse it. func TestCheckRequireSignatureRefusesOtherSigningKey(t *testing.T) { + t.Parallel() + content := []byte("signed file") manifest, required := manifestSignedByAnotherKey(t, map[string][]byte{testFileTxt: content}) diff --git a/internal/cli/errmsg_test.go b/internal/cli/errmsg_test.go index ac5fb5f..e3dd9c6 100644 --- a/internal/cli/errmsg_test.go +++ b/internal/cli/errmsg_test.go @@ -4,14 +4,18 @@ package cli import ( "bytes" "context" + "encoding/hex" + "io" "net/http" "net/http/httptest" "os" - "os/exec" "path/filepath" - "slices" + "strings" "testing" + "github.com/ProtonMail/go-crypto/openpgp" + "github.com/ProtonMail/go-crypto/openpgp/armor" + "github.com/ProtonMail/go-crypto/openpgp/packet" "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -108,11 +112,10 @@ func TestVerifyRequiredSignerMessages(t *testing.T) { // 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 +// it. func TestSignerMismatchMessage(t *testing.T) { + t.Parallel() + chk := signedChecker(t, signedManifest(t, map[string][]byte{"f.txt": []byte("signed file")})) @@ -123,43 +126,48 @@ func TestSignerMismatchMessage(t *testing.T) { " does not match required "+msgFpB) } -// signedManifest returns a manifest of files signed by a throwaway GPG key -// generated in a temporary GNUPGHOME, which it leaves set for the rest of -// the test. +// testSecretKey returns a new OpenPGP key with its secret key, armored, as +// gpg --export-secret-keys --armor writes it, and the key's fingerprint. +// The key is protected by passphrase unless that is nil. config sets how +// the key is made; without one it is an Ed25519 key, which is quick to +// make. +func testSecretKey( + t *testing.T, passphrase []byte, config *packet.Config, +) ([]byte, string) { + t.Helper() + + if config == nil { + config = &packet.Config{Algorithm: packet.PubKeyAlgoEdDSA} + } + + key, err := openpgp.NewEntity("MFER Test Key", "", "test@mfer.test", config) + require.NoError(t, err) + + if passphrase != nil { + require.NoError(t, key.EncryptPrivateKeys(passphrase, nil)) + } + + var buf bytes.Buffer + + w, err := armor.Encode(&buf, openpgp.PrivateKeyType, nil) + require.NoError(t, err) + require.NoError(t, key.SerializePrivateWithoutSigning(w, nil)) + require.NoError(t, w.Close()) + + return buf.Bytes(), strings.ToUpper(hex.EncodeToString(key.PrimaryKey.Fingerprint)) +} + +// signedManifest returns a manifest of files signed by a new OpenPGP key. func signedManifest(t *testing.T, files map[string][]byte) []byte { t.Helper() - _, err := exec.LookPath("gpg") - if err != nil { - t.Skip("gpg not installed, skipping signing test") - } - - gpgHome := t.TempDir() - params := "%no-protection\n" + - "Key-Type: RSA\nKey-Length: 2048\n" + - "Name-Real: MFER Test Key\nName-Email: test@mfer.test\n" + - "Expire-Date: 0\n%commit\n" - paramsFile := filepath.Join(gpgHome, "key-params") - require.NoError(t, os.WriteFile(paramsFile, []byte(params), 0o600)) - - //nolint:gosec // paramsFile is a test-controlled path inside t.TempDir() - cmd := exec.CommandContext(context.Background(), "gpg", - "--batch", "--gen-key", paramsFile) - - cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome) - - out, err := cmd.CombinedOutput() - if err != nil { - t.Skipf("failed to generate test GPG key: %v: %s", err, out) - } - - t.Setenv("GNUPGHOME", gpgHome) + secretKey, _ := testSecretKey(t, nil, nil) b := mfer.NewBuilder() - b.SetSigningOptions(&mfer.SigningOptions{KeyID: mfer.GPGKeyID("test@mfer.test")}) + b.SetSigningOptions(&mfer.SigningOptions{SecretKey: secretKey}) for path, content := range files { - _, err = b.AddFile(mfer.RelFilePath(path), mfer.FileSize(len(content)), + _, err := b.AddFile(mfer.RelFilePath(path), mfer.FileSize(len(content)), mfer.ModTime{}, 0, bytes.NewReader(content), nil) require.NoError(t, err) } @@ -190,8 +198,8 @@ func signedChecker(t *testing.T, manifest []byte) *mfer.Checker { } // 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 +// fingerprint of a new key, the required key, that did not sign it. +// The manifest is signed by a second new 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( @@ -207,8 +215,26 @@ func manifestSignedByAnotherKey( require.NoError(t, proto.Unmarshal( signedManifest(t, files)[len(mfer.MAGIC):], outer)) - outer.SigningPubKey = slices.Concat( - required.GetSigningPubKey(), outer.GetSigningPubKey()) + // One armored block holding both keys, as gpg --export --armor writes + // two keys. + var block bytes.Buffer + + w, err := armor.Encode(&block, openpgp.PublicKeyType, nil) + require.NoError(t, err) + + for _, key := range [][]byte{ + required.GetSigningPubKey(), outer.GetSigningPubKey(), + } { + decoded, err := armor.Decode(bytes.NewReader(key)) + require.NoError(t, err) + + _, err = io.Copy(w, decoded.Body) + require.NoError(t, err) + } + + require.NoError(t, w.Close()) + + outer.SigningPubKey = block.Bytes() outer.Signer = required.GetSigner() data, err := proto.Marshal(outer) diff --git a/internal/cli/export.go b/internal/cli/export.go index d537039..b343a9e 100644 --- a/internal/cli/export.go +++ b/internal/cli/export.go @@ -36,7 +36,6 @@ func (mfa *CLIApp) exportManifestOperation( defer func() { _ = rc.Close() }() - //nolint:contextcheck // mfer loads a manifest without a context manifest, err := mfer.NewManifestFromReader(rc) if err != nil { return fmt.Errorf("parse manifest: %w", err) diff --git a/internal/cli/fetch.go b/internal/cli/fetch.go index 8b0f569..6f2573f 100644 --- a/internal/cli/fetch.go +++ b/internal/cli/fetch.go @@ -455,7 +455,6 @@ func (mfa *CLIApp) fetchManifest( } // Parse manifest - //nolint:contextcheck // mfer loads a manifest without a context manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData)) if err != nil { return nil, nil, fmt.Errorf("parse manifest: %w", err) @@ -463,7 +462,6 @@ func (mfa *CLIApp) fetchManifest( requiredSigner := cmd.String(flagRequireSignature) if requiredSigner != "" { - //nolint:contextcheck // mfer loads a manifest without a context err = verifyFetchedSigner(manifestData, requiredSigner) if err != nil { return nil, nil, err diff --git a/internal/cli/fetch_test.go b/internal/cli/fetch_test.go index 8d61b20..98b5bdc 100644 --- a/internal/cli/fetch_test.go +++ b/internal/cli/fetch_test.go @@ -1191,20 +1191,23 @@ func TestFetchIntoDest(t *testing.T) { // with check's message before it downloads or writes anything; the // 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 +// too. func TestFetchRequireSignature(t *testing.T) { + t.Parallel() + files := map[string][]byte{testFileTxt: []byte("signed file")} t.Run("unsigned", func(t *testing.T) { + t.Parallel() + assertFetchRefused(t, manifestOf(t, files), files, "manifest is not signed, but signature from "+msgFpA+" is required", "--"+flagRequireSignature, msgFpA) }) t.Run("signed", func(t *testing.T) { + t.Parallel() + manifest := signedManifest(t, files) signer := string(signedChecker(t, manifest).Signer()) @@ -1227,6 +1230,8 @@ func TestFetchRequireSignature(t *testing.T) { }) t.Run("signed by another key embedded after the required one", func(t *testing.T) { + t.Parallel() + manifest, required := manifestSignedByAnotherKey(t, files) assertFetchRefused(t, manifest, files, diff --git a/internal/cli/freshen.go b/internal/cli/freshen.go index eace12a..d3f25ae 100644 --- a/internal/cli/freshen.go +++ b/internal/cli/freshen.go @@ -339,7 +339,7 @@ func writeFreshenedManifest( // newFreshenBuilder constructs the manifest builder configured from CLI // flags. -func newFreshenBuilder(cmd *cli.Command) *mfer.Builder { +func (mfa *CLIApp) newFreshenBuilder(cmd *cli.Command) (*mfer.Builder, error) { builder := mfer.NewBuilder() if cmd.Bool("include-timestamps") { builder.SetIncludeTimestamps(true) @@ -347,13 +347,15 @@ func newFreshenBuilder(cmd *cli.Command) *mfer.Builder { // Set up signing options if sign-key is provided if signKey := cmd.String("sign-key"); signKey != "" { - builder.SetSigningOptions(&mfer.SigningOptions{ - KeyID: mfer.GPGKeyID(signKey), - }) - log.Infof("signing manifest with GPG key: %s", signKey) + signing, err := mfa.signingOptions(signKey) + if err != nil { + return nil, err + } + + builder.SetSigningOptions(signing) } - return builder + return builder, nil } // freshenScan runs the scan phase against the loaded manifest entries @@ -441,7 +443,7 @@ func hashTotals(entries []*freshenEntry) (int64, int64) { } // runFreshenHash processes every entry through the hasher, aborting if -// the context is canceled. +// the context is canceled, and ends the hasher's progress line. func runFreshenHash( ctx context.Context, hasher *freshenHasher, entries []*freshenEntry, ) error { @@ -458,6 +460,10 @@ func runFreshenHash( } } + if hasher.showProgress && hasher.filesToHash > 0 { + log.ProgressDone() + } + return nil } @@ -502,7 +508,11 @@ func (mfa *CLIApp) freshenManifestOperation( return err } - //nolint:contextcheck // mfer loads a manifest without a context + builder, err := mfa.newFreshenBuilder(cmd) + if err != nil { + return err + } + existingByPath, err := mfa.loadExistingEntries(manifestPath) if err != nil { return err @@ -536,7 +546,7 @@ func (mfa *CLIApp) freshenManifestOperation( totalHashBytes: totalHashBytes, filesToHash: filesToHash, startHash: time.Now(), - builder: newFreshenBuilder(cmd), + builder: builder, } err = runFreshenHash(ctx, hasher, scanner.entries) @@ -544,10 +554,6 @@ func (mfa *CLIApp) freshenManifestOperation( return err } - if showProgress && filesToHash > 0 { - log.ProgressDone() - } - // Print summary log.Infof("freshen complete: %d unchanged, %d changed, %d added, %d removed", scanner.unchanged, scanner.changed, scanner.added, removed) diff --git a/internal/cli/gen.go b/internal/cli/gen.go index 4ea3e36..4a24882 100644 --- a/internal/cli/gen.go +++ b/internal/cli/gen.go @@ -124,7 +124,7 @@ func (mfa *CLIApp) outputPath(cmd *cli.Command) (string, error) { // the path the manifest is written to. func (mfa *CLIApp) buildScannerOptions( cmd *cli.Command, output string, -) *mfer.ScannerOptions { +) (*mfer.ScannerOptions, error) { opts := &mfer.ScannerOptions{ IncludeDotfiles: cmd.Bool("include-dotfiles"), FollowSymLinks: cmd.Bool("follow-symlinks"), @@ -145,13 +145,15 @@ func (mfa *CLIApp) buildScannerOptions( // Set up signing options if sign-key is provided if signKey := cmd.String("sign-key"); signKey != "" { - opts.SigningOptions = &mfer.SigningOptions{ - KeyID: mfer.GPGKeyID(signKey), + signing, err := mfa.signingOptions(signKey) + if err != nil { + return nil, err } - log.Infof("signing manifest with GPG key: %s", signKey) + + opts.SigningOptions = signing } - return opts + return opts, nil } // enumerateInputs runs the enumeration phase over the argument paths, @@ -275,7 +277,12 @@ func (mfa *CLIApp) generateManifestOperation( return err } - s := mfer.NewScannerWithOptions(mfa.buildScannerOptions(cmd, outputPath)) + opts, err := mfa.buildScannerOptions(cmd, outputPath) + if err != nil { + return err + } + + s := mfer.NewScannerWithOptions(opts) // Phase 1: Enumeration - collect paths and stat files err = mfa.runEnumeratePhase(cmd, s) diff --git a/internal/cli/list.go b/internal/cli/list.go index ed0fff9..0c50016 100644 --- a/internal/cli/list.go +++ b/internal/cli/list.go @@ -29,7 +29,6 @@ func (mfa *CLIApp) listManifestOperation(ctx context.Context, cmd *cli.Command) defer func() { _ = rc.Close() }() - //nolint:contextcheck // mfer loads a manifest without a context manifest, err := mfer.NewManifestFromReader(rc) if err != nil { return fmt.Errorf("parse manifest: %w", err) diff --git a/internal/cli/mfer.go b/internal/cli/mfer.go index a19db67..bd51420 100644 --- a/internal/cli/mfer.go +++ b/internal/cli/mfer.go @@ -169,7 +169,7 @@ func requireSignatureFlag() *cli.StringFlag { return &cli.StringFlag{ Name: flagRequireSignature, Aliases: []string{"S"}, - Usage: "Require manifest to be signed by the specified GPG key ID", + Usage: "Require manifest to be signed by the OpenPGP key with this fingerprint", Sources: cli.EnvVars("MFER_REQUIRE_SIGNATURE"), } } @@ -229,7 +229,7 @@ func (mfa *CLIApp) generateCommand() *cli.Command { &cli.StringFlag{ Name: "sign-key", Aliases: []string{"s"}, - Usage: "GPG key ID to sign the manifest with", + Usage: "OpenPGP secret key file to sign the manifest with", Sources: cli.EnvVars("MFER_SIGN_KEY"), }, &cli.StringFlag{ @@ -319,7 +319,7 @@ func (mfa *CLIApp) freshenCommand() *cli.Command { &cli.StringFlag{ Name: "sign-key", Aliases: []string{"s"}, - Usage: "GPG key ID to sign the manifest with", + Usage: "OpenPGP secret key file to sign the manifest with", Sources: cli.EnvVars("MFER_SIGN_KEY"), }, &cli.BoolFlag{ diff --git a/internal/cli/signing.go b/internal/cli/signing.go new file mode 100644 index 0000000..2c446d4 --- /dev/null +++ b/internal/cli/signing.go @@ -0,0 +1,86 @@ +package cli + +import ( + "errors" + "fmt" + "os" + + "github.com/spf13/afero" + "golang.org/x/term" + "sneak.berlin/go/mfer/internal/log" + "sneak.berlin/go/mfer/mfer" +) + +// envSignKeyPassphrase names the environment variable holding the +// passphrase of a protected signing key. +// +//nolint:gosec // G101: the name of a variable, not a credential +const envSignKeyPassphrase = "MFER_SIGN_KEY_PASSPHRASE" + +// errNoPassphrase indicates a protected signing key whose passphrase is +// neither in the environment nor can be asked for on a terminal. +var errNoPassphrase = errors.New( + "signing key is protected: set " + envSignKeyPassphrase + " to its passphrase") + +// signingOptions returns the signing options for the OpenPGP secret key in +// the file path, which must be able to sign. The passphrase of a protected +// key comes from MFER_SIGN_KEY_PASSPHRASE, or else from the terminal on +// stdin, and must unlock the key. +func (mfa *CLIApp) signingOptions(path string) (*mfer.SigningOptions, error) { + secretKey, err := afero.ReadFile(mfa.Fs, path) + if err != nil { + return nil, fmt.Errorf("read signing key: %w", err) + } + + protected, err := mfer.SecretKeyIsProtected(secretKey) + if err != nil { + return nil, fmt.Errorf("%s: %w", path, err) + } + + log.Infof("signing manifest with the OpenPGP key in %s", path) + + opts := &mfer.SigningOptions{SecretKey: secretKey} + if protected { + opts.Passphrase, err = mfa.readPassphrase(path) + if err != nil { + return nil, err + } + } + + // gen and freshen read the signing options before any file, so a key + // that cannot sign, or a wrong passphrase, stops them before they hash + // anything. + err = mfer.CheckSigningKey(opts) + if err != nil { + return nil, fmt.Errorf("%s: %w", path, err) + } + + return opts, nil +} + +// readPassphrase returns MFER_SIGN_KEY_PASSPHRASE when it is set, or else +// asks for the passphrase of the key in the file path on the terminal on +// stdin. +func (mfa *CLIApp) readPassphrase(path string) ([]byte, error) { + passphrase := os.Getenv(envSignKeyPassphrase) + if passphrase != "" { + return []byte(passphrase), nil + } + + stdin, ok := mfa.Stdin.(*os.File) + if !ok || !term.IsTerminal(int(stdin.Fd())) { + return nil, errNoPassphrase + } + + _, _ = fmt.Fprintf(mfa.Stderr, "Passphrase for %s: ", path) + + typed, err := term.ReadPassword(int(stdin.Fd())) + + _, _ = fmt.Fprintln(mfa.Stderr) + + if err != nil { + return nil, fmt.Errorf("read passphrase: %w", err) + } + + return typed, nil +} diff --git a/internal/cli/signing_test.go b/internal/cli/signing_test.go new file mode 100644 index 0000000..77062ba --- /dev/null +++ b/internal/cli/signing_test.go @@ -0,0 +1,209 @@ +//nolint:testpackage // white-box tests exercise unexported internals +package cli + +import ( + "bufio" + "io" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/ProtonMail/go-crypto/openpgp/packet" + "github.com/creack/pty" + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + testFlagSignKey = "--sign-key" + testKeyFile = "/key.asc" +) + +// TestGenAndFreshenSignWithKeyFile runs gen, then freshen after a file is +// added, with --sign-key naming a key file: one key with no passphrase and +// one protected by the passphrase in MFER_SIGN_KEY_PASSPHRASE. check +// --require-signature must accept each manifest as signed by that key. +// freshen leaves its manifest out of the listing only on the real +// filesystem, so the test uses that. +func TestGenAndFreshenSignWithKeyFile(t *testing.T) { + for name, passphrase := range map[string][]byte{ + "unprotected": nil, + "protected": []byte("passphrase"), + } { + t.Run(name, func(t *testing.T) { + t.Setenv(envSignKeyPassphrase, string(passphrase)) + + secretKey, fingerprint := testSecretKey(t, passphrase, nil) + + fs := afero.NewOsFs() + keyFile := filepath.Join(t.TempDir(), "key.asc") + root := t.TempDir() + manifestPath := filepath.Join(root, defaultManifestName) + + require.NoError(t, afero.WriteFile(fs, keyFile, secretKey, 0o600)) + writeTestFile(t, fs, filepath.Join(root, testFileTxt), "hello") + + opts := testOpts([]string{ + testApp, cmdGenerate, "-q", testFlagSignKey, keyFile, + "-o", manifestPath, root, + }, fs) + require.Equal(t, 0, runCLI(opts), testStderr(t, opts)) + + check := []string{ + testApp, cmdCheck, "-q", + "--" + flagRequireSignature, fingerprint, manifestPath, + } + + opts = testOpts(check, fs) + require.Equal(t, 0, runCLI(opts), testStderr(t, opts)) + + writeTestFile(t, fs, filepath.Join(root, "added.txt"), "added") + + opts = testOpts([]string{ + testApp, cmdFreshen, "-q", testFlagSignKey, keyFile, manifestPath, + }, fs) + require.Equal(t, 0, runCLI(opts), testStderr(t, opts)) + + opts = testOpts(check, fs) + require.Equal(t, 0, runCLI(opts), testStderr(t, opts)) + assert.Len(t, manifestFiles(t, fs, manifestPath), 2) + }) + } +} + +// TestSignWithProtectedKeyNeedsPassphrase runs gen with a protected key, +// with MFER_SIGN_KEY_PASSPHRASE empty and no terminal to ask on. gen must +// fail, naming the variable, and write no manifest. +func TestSignWithProtectedKeyNeedsPassphrase(t *testing.T) { + t.Setenv(envSignKeyPassphrase, "") + + secretKey, _ := testSecretKey(t, []byte("secret"), nil) + + fs := afero.NewMemMapFs() + require.NoError(t, afero.WriteFile(fs, testKeyFile, secretKey, 0o600)) + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "hello") + + opts := testOpts([]string{ + testApp, cmdGenerate, "-q", testFlagSignKey, testKeyFile, + "-o", testMF, testDir, + }, fs) + assert.Equal(t, 1, runCLI(opts)) + assert.Contains(t, testStderr(t, opts), + "signing key is protected: set MFER_SIGN_KEY_PASSPHRASE to its passphrase") + + exists, err := afero.Exists(fs, testMF) + require.NoError(t, err) + assert.False(t, exists) +} + +// TestSignWithKeyThatCannotSignFailsFirst runs gen on a directory and +// freshen on a manifest, neither of which exists, with keys that cannot +// sign: a protected key with a wrong MFER_SIGN_KEY_PASSPHRASE, a key that +// expired in 2020, and a version 6 key. Each run must fail on the key: it +// checks the key before it reads any file, so a missing file goes +// unnoticed. +func TestSignWithKeyThatCannotSignFailsFirst(t *testing.T) { + t.Setenv(envSignKeyPassphrase, "wrong") + + wrongPassphrase, _ := testSecretKey(t, []byte("right"), nil) + + made := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC) + expired, _ := testSecretKey(t, nil, &packet.Config{ + Algorithm: packet.PubKeyAlgoEdDSA, + Time: func() time.Time { return made }, + KeyLifetimeSecs: uint32((24 * time.Hour).Seconds()), + }) + + version6, _ := testSecretKey(t, nil, &packet.Config{ + Algorithm: packet.PubKeyAlgoEd25519, + V6Keys: true, + }) + + for want, secretKey := range map[string][]byte{ + "unlock signing key": wrongPassphrase, + "signing key cannot sign": expired, + "signing key must be an OpenPGP version 4 key": version6, + } { + fs := afero.NewMemMapFs() + require.NoError(t, afero.WriteFile(fs, testKeyFile, secretKey, 0o600)) + + for _, args := range [][]string{ + { + testApp, cmdGenerate, "-q", testFlagSignKey, testKeyFile, + "-o", testMF, "/missing", + }, + {testApp, cmdFreshen, "-q", testFlagSignKey, testKeyFile, "/missing.mf"}, + } { + opts := testOpts(args, fs) + assert.Equal(t, 1, runCLI(opts), args[1], want) + assert.Contains(t, testStderr(t, opts), testKeyFile+": "+want, args[1]) + } + } +} + +// TestGenAsksForPassphraseOnTerminal runs gen with a protected key, no +// MFER_SIGN_KEY_PASSPHRASE, and a terminal as stdin and stderr. gen must +// ask for the passphrase on stderr, and sign with what is typed after the +// prompt. +func TestGenAsksForPassphraseOnTerminal(t *testing.T) { + t.Setenv(envSignKeyPassphrase, "") + + secretKey, fingerprint := testSecretKey(t, []byte("passphrase"), nil) + + fs := afero.NewOsFs() + keyFile := filepath.Join(t.TempDir(), "key.asc") + root := t.TempDir() + manifestPath := filepath.Join(root, defaultManifestName) + + require.NoError(t, afero.WriteFile(fs, keyFile, secretKey, 0o600)) + writeTestFile(t, fs, filepath.Join(root, testFileTxt), "hello") + + terminal, tty, err := pty.Open() + require.NoError(t, err) + + t.Cleanup(func() { _ = terminal.Close() }) + + opts := testOpts([]string{ + testApp, cmdGenerate, "-q", testFlagSignKey, keyFile, + "-o", manifestPath, root, + }, fs) + opts.Stdin = tty + opts.Stderr = tty + + exitCode := make(chan int, 1) + + go func() { + exitCode <- runCLI(opts) + + // Once gen has ended, reading the terminal fails instead of + // waiting for a prompt that will not come. + _ = tty.Close() + }() + + prompt := "Passphrase for " + keyFile + ": " + output := bufio.NewReader(terminal) + written := "" + + for !strings.HasSuffix(written, prompt) { + b, err := output.ReadByte() + require.NoError(t, err, "gen wrote %q and no prompt", written) + + written += string(b) + } + + _, err = terminal.WriteString("passphrase\n") + require.NoError(t, err) + + code := <-exitCode + rest, _ := io.ReadAll(output) + require.Equal(t, 0, code, "gen wrote %q", rest) + + check := testOpts([]string{ + testApp, cmdCheck, "-q", + "--" + flagRequireSignature, fingerprint, manifestPath, + }, fs) + require.Equal(t, 0, runCLI(check), testStderr(t, check)) +} diff --git a/mfer/builder.go b/mfer/builder.go index ddc98c6..a6a0d84 100644 --- a/mfer/builder.go +++ b/mfer/builder.go @@ -290,7 +290,7 @@ func (b *Builder) SetIncludeTimestamps(include bool) { b.includeTimestamps = include } -// SetSigningOptions sets the GPG signing options for the manifest. +// SetSigningOptions sets the key the manifest is signed with. // If opts is non-nil, the manifest will be signed when Build() is called. func (b *Builder) SetSigningOptions(opts *SigningOptions) { b.mu.Lock() @@ -299,8 +299,8 @@ func (b *Builder) SetSigningOptions(opts *SigningOptions) { b.signingOptions = opts } -// Build finalizes the manifest and writes it to the writer. ctx bounds the -// gpg runs that sign the manifest when signing options are set. +// Build finalizes the manifest and writes it to the writer. When signing +// options are set, it does not sign once ctx has ended. func (b *Builder) Build(ctx context.Context, w io.Writer) error { b.mu.Lock() defer b.mu.Unlock() diff --git a/mfer/deserialize.go b/mfer/deserialize.go index ddbaffd..0d3b95c 100644 --- a/mfer/deserialize.go +++ b/mfer/deserialize.go @@ -2,7 +2,6 @@ package mfer import ( "bytes" - "context" "crypto/sha256" "errors" "fmt" @@ -93,9 +92,7 @@ func (m *manifest) verifyOuterIntegrity() error { return fmt.Errorf("build signature string: %w", err) } - // Loading a manifest takes no context; gpgTimeout still bounds gpg. - signingKey, err := gpgVerify( - context.Background(), + signingKey, err := verifySignature( []byte(sigString), m.pbOuter.GetSignature(), m.pbOuter.GetSigningPubKey(), diff --git a/mfer/deserialize_fuzz_test.go b/mfer/deserialize_fuzz_test.go index 5767082..a6a97c7 100644 --- a/mfer/deserialize_fuzz_test.go +++ b/mfer/deserialize_fuzz_test.go @@ -19,14 +19,6 @@ import ( // input and of the decompressed data it may read, plus room for the // decoder's window buffers. A panic or a hang fails the test on its own. func FuzzNewManifestFromReader(f *testing.F) { - // A signed manifest makes the parser write the key and signature to a - // temporary directory and run gpg on them. With gpg off the PATH and - // temporary files kept in the test's own directory, no process is - // started and nothing is written elsewhere; such input ends in an - // error instead. - f.Setenv("PATH", "") - f.Setenv("TMPDIR", f.TempDir()) - f.Fuzz(func(t *testing.T, data []byte) { var before, after runtime.MemStats diff --git a/mfer/gpg.go b/mfer/gpg.go deleted file mode 100644 index 21b89c3..0000000 --- a/mfer/gpg.go +++ /dev/null @@ -1,358 +0,0 @@ -package mfer - -import ( - "bytes" - "context" - "errors" - "fmt" - "io" - "os" - "os/exec" - "path/filepath" - "strings" - "time" -) - -const ( - // gpgTimeout bounds every gpg run, which can otherwise wait forever on - // a passphrase prompt or a stalled gpg-agent. A minute leaves a person - // time to type a passphrase or touch a smartcard. - gpgTimeout = time.Minute - - // gpgWaitDelay is how long a gpg run keeps waiting for gpg's stdout - // and stderr to close once gpg has been killed or has exited. Reading - // what gpg itself wrote takes far less; only a process gpg left behind - // holds them open longer. - gpgWaitDelay = time.Second - - // privateDirPerms is the permission mode for temporary GPG home - // directories. - privateDirPerms os.FileMode = 0o700 - - // privateFilePerms is the permission mode for temporary key, - // signature, and data files. - privateFilePerms os.FileMode = 0o600 - - // gpgFingerprintField is the record type tag for fingerprint lines - // in gpg --with-colons output. - gpgFingerprintField = "fpr" - - // gpgFingerprintMinFields is the minimum number of colon-separated - // 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" - gpgOptStatusFD = "--status-fd" - gpgOptVerify = "--verify" -) - -var ( - errGPGKeyNotFound = errors.New("GPG key not found") - errFingerprintNotFound = errors.New("fingerprint not found for 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). -type GPGKeyID string - -// SigningOptions contains options for GPG signing. -type SigningOptions struct { - KeyID GPGKeyID -} - -// gpgArgs builds a gpg argument list from opts followed by positional -// arguments, separated by an explicit "--" end-of-options marker. -// -// This matters because key IDs reach gpg as bare positional arguments -// (from --sign-key / MFER_SIGN_KEY) and gpg would otherwise parse a value -// beginning with "-" as one of its own options. Callers must route every -// non-option argument through here. -func gpgArgs(opts []string, positional ...string) []string { - args := make([]string, 0, len(opts)+1+len(positional)) - args = append(args, opts...) - args = append(args, "--") - args = append(args, positional...) - - return args -} - -// runGPG runs the gpg binary in batch mode with the given arguments and -// optional stdin, returning captured stdout and stderr. If gpg fails, the -// error ends with what gpg wrote to stderr. gpg is killed when ctx ends or -// gpgTimeout passes, whichever comes first. -func runGPG( - ctx context.Context, stdin io.Reader, args ...string, -) (*bytes.Buffer, *bytes.Buffer, error) { - // exec.CommandContext kills only gpg itself. A gpg-agent that gpg - // starts runs detached and holds none of gpg's output, but another - // process gpg leaves behind (a wrapper script that runs the real gpg - // without exec, for example) can keep gpg's stdout or stderr open, and - // Run would wait for it to exit. WaitDelay stops that wait - // gpgWaitDelay after the kill; that process is left running. - ctx, cancel := context.WithTimeout(ctx, gpgTimeout) - defer cancel() - - fullArgs := append([]string{"--batch", "--no-tty"}, args...) - - // G204: the executable name is a compile-time constant. The arguments - // are not, so the guarantee that matters is placement: every - // caller-supplied value is passed either as the value of a named - // option or after the "--" end-of-options marker inserted by gpgArgs, - // and therefore cannot be reinterpreted by gpg as an option. - cmd := exec.CommandContext( //nolint:gosec // G204: see comment above - ctx, "gpg", fullArgs...) - cmd.WaitDelay = gpgWaitDelay - cmd.Stdin = stdin - - var stdout, stderr bytes.Buffer - - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - err := cmd.Run() - if err != nil && ctx.Err() != nil { - // gpg was killed because ctx ended, which Run reports only as - // "signal: killed"; return the reason instead. - err = ctx.Err() - if errors.Is(err, context.DeadlineExceeded) { - err = fmt.Errorf("timed out: %w", err) - } - } - - if err != nil { - err = withStderr(err, &stderr) - } - - return &stdout, &stderr, err -} - -// withStderr returns err followed by what gpg wrote to stderr, or err alone -// when gpg wrote nothing. -func withStderr(err error, stderr *bytes.Buffer) error { - messages := strings.TrimSpace(stderr.String()) - if messages == "" { - return err - } - - return fmt.Errorf("%w: %s", err, messages) -} - -// parseFingerprint extracts the first fingerprint from gpg --with-colons -// output, or returns ok=false if none is present. -func parseFingerprint(colonOutput string) (string, bool) { - for line := range strings.SplitSeq(colonOutput, "\n") { - fields := strings.Split(line, ":") - if len(fields) >= gpgFingerprintMinFields && - fields[0] == gpgFingerprintField { - return fields[9], true - } - } - - return "", false -} - -// 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) { - tmpDir, err := os.MkdirTemp("", "mfer-gpg-sign-*") - if err != nil { - return nil, "", err - } - - defer func() { _ = os.RemoveAll(tmpDir) }() - - sigFile := filepath.Join(tmpDir, "signature.asc") - - // The signature goes to sigFile, so --status-fd 1 can send gpg's status - // lines to stdout; its messages go to stderr. - stdout, stderr, err := runGPG(ctx, bytes.NewReader(data), - "--detach-sign", - gpgOptArmor, - "--output", sigFile, - gpgOptStatusFD, "1", - "--local-user", string(keyID), - ) - if err != nil { - return nil, "", fmt.Errorf("gpg sign: %w", err) - } - - // The last argument of SIG_CREATED is the fingerprint of the key that - // made the signature. - created, ok := parseStatusLine(stdout.String(), "SIG_CREATED") - if !ok { - return nil, "", withStderr(errSigningKeyNotReported, stderr) - } - - sig, err := os.ReadFile(sigFile) //nolint:gosec // G304: inside tmpDir, made above - if err != nil { - return nil, "", err - } - - return sig, created[len(created)-1], nil -} - -// gpgExportPublicKey exports the public key for the specified key ID. -// Returns the armored public key. -func gpgExportPublicKey(ctx context.Context, keyID GPGKeyID) ([]byte, error) { - stdout, _, err := runGPG(ctx, nil, - gpgArgs([]string{"--export", gpgOptArmor}, string(keyID))..., - ) - if err != nil { - return nil, fmt.Errorf("gpg export: %w", err) - } - - if stdout.Len() == 0 { - return nil, fmt.Errorf("%w: %s", errGPGKeyNotFound, keyID) - } - - return stdout.Bytes(), nil -} - -// gpgGetKeyFingerprint gets the full fingerprint for a key ID. -func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) { - stdout, _, err := runGPG(ctx, nil, - gpgArgs([]string{"--with-colons", "--fingerprint"}, string(keyID))..., - ) - if err != nil { - return nil, fmt.Errorf("gpg fingerprint lookup: %w", err) - } - - fpr, ok := parseFingerprint(stdout.String()) - if !ok { - return nil, fmt.Errorf("%w: %s", errFingerprintNotFound, keyID) - } - - return []byte(fpr), nil -} - -// 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, _, err := runGPG(ctx, nil, - gpgArgs([]string{gpgOptHomedir, gpgHome, gpgOptStatusFD, "1", "--import"}, - pubKeyFile)..., - ) - if err != nil { - return fmt.Errorf("gpg import: %w", err) - } - - // 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-verify-*") - if err != nil { - return "", err - } - - defer func() { _ = os.RemoveAll(tmpDir) }() - - // Set restrictive permissions - err = os.Chmod(tmpDir, privateDirPerms) - if err != nil { - return "", err - } - - // Write public key to temp file - pubKeyFile := filepath.Join(tmpDir, "pubkey.asc") - - err = os.WriteFile(pubKeyFile, pubKey, privateFilePerms) - if err != nil { - return "", err - } - - // Write signature to temp file - sigFile := filepath.Join(tmpDir, "signature.asc") - - err = os.WriteFile(sigFile, signature, privateFilePerms) - if err != nil { - return "", err - } - - // Write data to temp file - dataFile := filepath.Join(tmpDir, "data") - - err = os.WriteFile(dataFile, data, privateFilePerms) - if err != nil { - return "", err - } - - err = gpgImportOneKey(ctx, tmpDir, pubKeyFile) - if err != nil { - return "", 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, _, err := runGPG(ctx, nil, - gpgArgs([]string{gpgOptHomedir, tmpDir, gpgOptStatusFD, "1", gpgOptVerify}, - sigFile, dataFile)..., - ) - if err != nil { - return "", fmt.Errorf("gpg verify: %w", err) - } - - // 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 -} diff --git a/mfer/gpg_test.go b/mfer/gpg_test.go deleted file mode 100644 index af59eb2..0000000 --- a/mfer/gpg_test.go +++ /dev/null @@ -1,703 +0,0 @@ -//nolint:testpackage // white-box tests exercise unexported internals -package mfer - -import ( - "bytes" - "context" - "io" - "os" - "os/exec" - "path/filepath" - "slices" - "strconv" - "strings" - "syscall" - "testing" - "time" - - "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. -// Returns the key ID and the GPG home directory; callers must point -// GNUPGHOME at the returned directory (via t.Setenv) before using the -// gpg helpers under test. -func testGPGEnv(t *testing.T) (GPGKeyID, string) { - t.Helper() - - // Check if gpg is installed - _, err := exec.LookPath("gpg") - if err != nil { - t.Skip("gpg not installed, skipping signing test") - } - - // Create temporary GPG home directory (0700 by default) - gpgHome := t.TempDir() - - genTestKey(t, gpgHome, testKeyParams) - - 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 -} - -// 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 " 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, 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-----") -} - -func TestGPGExportPublicKey(t *testing.T) { - keyID, gpgHome := testGPGEnv(t) - t.Setenv("GNUPGHOME", gpgHome) - - pubKey, err := gpgExportPublicKey(context.Background(), keyID) - require.NoError(t, err) - assert.NotEmpty(t, pubKey) - assert.Contains(t, string(pubKey), "-----BEGIN PGP PUBLIC KEY BLOCK-----") - assert.Contains(t, string(pubKey), "-----END PGP PUBLIC KEY BLOCK-----") -} - -func TestGPGGetKeyFingerprint(t *testing.T) { - keyID, gpgHome := testGPGEnv(t) - t.Setenv("GNUPGHOME", gpgHome) - - fingerprint, err := gpgGetKeyFingerprint(context.Background(), keyID) - require.NoError(t, err) - assert.NotEmpty(t, fingerprint) - // The fingerprint should be 40 hex chars - assert.Len(t, fingerprint, 40, "fingerprint should be 40 hex chars") -} - -// TestGPGArgsSeparatesPositionals pins that caller-supplied values are -// placed after an end-of-options marker. Key IDs arrive from --sign-key -// and MFER_SIGN_KEY as bare positional arguments, so without the marker -// a value beginning with "-" would be parsed by gpg as one of its own -// options. -func TestGPGArgsSeparatesPositionals(t *testing.T) { - t.Parallel() - - assert.Equal(t, - []string{"--opt-a", "--opt-b", "--", "--version"}, - gpgArgs([]string{"--opt-a", "--opt-b"}, "--version")) - - assert.Equal(t, - []string{"--opt-c", "--", "sig", "data"}, - gpgArgs([]string{"--opt-c"}, "sig", "data")) - - assert.Equal(t, []string{"--opt-d", "--"}, - gpgArgs([]string{"--opt-d"})) -} - -// TestGPGOptionLikeKeyIDIsNotAnOption drives real gpg with a key ID that -// looks like an option and asserts it is treated as a (nonexistent) key -// rather than executed as gpg's own --version. -func TestGPGOptionLikeKeyIDIsNotAnOption(t *testing.T) { - _, gpgHome := testGPGEnv(t) - t.Setenv("GNUPGHOME", gpgHome) - - pubKey, err := gpgExportPublicKey(context.Background(), GPGKeyID("--version")) - require.Error(t, err) - require.ErrorIs(t, err, errGPGKeyNotFound) - assert.NotContains(t, string(pubKey), "gpg (GnuPG)") - - fpr, err := gpgGetKeyFingerprint(context.Background(), GPGKeyID("--version")) - require.Error(t, err) - assert.NotContains(t, string(fpr), "gpg (GnuPG)") -} - -// TestGPGSignInvalidKey signs with a key that has no secret key in the -// keyring. The error must hold gpg's messages and none of its status lines. -func TestGPGSignInvalidKey(t *testing.T) { - // Set up test environment (we need GNUPGHOME set) - _, gpgHome := testGPGEnv(t) - t.Setenv("GNUPGHOME", gpgHome) - - data := []byte("test data") - _, _, err := gpgSign(context.Background(), data, - GPGKeyID("NONEXISTENT_KEY_ID_12345")) - require.Error(t, err) - assert.NotContains(t, err.Error(), gpgStatusPrefix) -} - -func TestBuilderWithSigning(t *testing.T) { - keyID, gpgHome := testGPGEnv(t) - t.Setenv("GNUPGHOME", gpgHome) - - // Create a builder with signing options - b := NewBuilder() - b.SetSigningOptions(&SigningOptions{ - KeyID: keyID, - }) - - // Add a test file - content := []byte("test file content") - reader := bytes.NewReader(content) - _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, 0, reader, nil) - require.NoError(t, err) - - // Build the manifest - var buf bytes.Buffer - - err = b.Build(context.Background(), &buf) - require.NoError(t, err) - - // Parse the manifest and verify signature fields are populated - manifest, err := NewManifestFromReader(&buf) - require.NoError(t, err) - require.NotNil(t, manifest.pbOuter) - - assert.NotEmpty(t, manifest.pbOuter.GetSignature(), - "signature should be populated") - assert.NotEmpty(t, manifest.pbOuter.GetSigner(), "signer should be populated") - assert.NotEmpty(t, manifest.pbOuter.GetSigningPubKey(), - "signing public key should be populated") - - // Verify signature is a valid PGP signature - assert.Contains(t, string(manifest.pbOuter.GetSignature()), - "-----BEGIN PGP SIGNATURE-----") - - // Verify public key is a valid PGP public key block - assert.Contains(t, string(manifest.pbOuter.GetSigningPubKey()), - "-----BEGIN PGP PUBLIC KEY BLOCK-----") -} - -func TestScannerWithSigning(t *testing.T) { - keyID, gpgHome := testGPGEnv(t) - t.Setenv("GNUPGHOME", gpgHome) - - // Create in-memory filesystem with test files - fs := afero.NewMemMapFs() - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, - afero.WriteFile(fs, "/testdir/file1.txt", []byte("content1"), 0o644)) - require.NoError(t, - afero.WriteFile(fs, "/testdir/file2.txt", []byte("content2"), 0o644)) - - // Create scanner with signing options - opts := &ScannerOptions{ - Fs: fs, - SigningOptions: &SigningOptions{ - KeyID: keyID, - }, - } - s := NewScannerWithOptions(opts) - - // Enumerate files - require.NoError(t, s.EnumeratePath("/testdir", nil)) - assert.Equal(t, FileCount(2), s.FileCount()) - - // Generate signed manifest - var buf bytes.Buffer - require.NoError(t, s.ToManifest(context.Background(), &buf, nil)) - - // Parse and verify - manifest, err := NewManifestFromReader(&buf) - require.NoError(t, err) - - assert.NotEmpty(t, manifest.pbOuter.GetSignature()) - assert.NotEmpty(t, manifest.pbOuter.GetSigner()) - assert.NotEmpty(t, manifest.pbOuter.GetSigningPubKey()) -} - -func TestGPGVerify(t *testing.T) { - keyID, gpgHome := testGPGEnv(t) - t.Setenv("GNUPGHOME", gpgHome) - - data := []byte("test data to sign and verify") - sig, _, err := gpgSign(context.Background(), data, keyID) - require.NoError(t, err) - - pubKey, err := gpgExportPublicKey(context.Background(), keyID) - require.NoError(t, err) - - // 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) { - keyID, gpgHome := testGPGEnv(t) - t.Setenv("GNUPGHOME", gpgHome) - - data := []byte("test data to sign") - sig, _, err := gpgSign(context.Background(), data, keyID) - require.NoError(t, err) - - pubKey, err := gpgExportPublicKey(context.Background(), keyID) - require.NoError(t, err) - - // Try to verify with different data - should fail - wrongData := []byte("different data") - _, err = gpgVerify(context.Background(), wrongData, sig, pubKey) - assert.Error(t, err) -} - -func TestGPGVerifyBadPublicKey(t *testing.T) { - keyID, gpgHome := testGPGEnv(t) - t.Setenv("GNUPGHOME", gpgHome) - - data := []byte("test data") - 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) - assert.Error(t, err) -} - -func TestManifestSignatureVerification(t *testing.T) { - keyID, gpgHome := testGPGEnv(t) - t.Setenv("GNUPGHOME", gpgHome) - - // Create a builder with signing options - b := NewBuilder() - b.SetSigningOptions(&SigningOptions{ - KeyID: keyID, - }) - - // Add a test file - content := []byte("test file content for verification") - reader := bytes.NewReader(content) - _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, 0, reader, nil) - require.NoError(t, err) - - // Build the manifest - var buf bytes.Buffer - - err = b.Build(context.Background(), &buf) - require.NoError(t, err) - - // Parse the manifest - signature should be verified during load - manifest, err := NewManifestFromReader(&buf) - require.NoError(t, err) - require.NotNil(t, manifest) - - // Signature should be present and valid - assert.NotEmpty(t, manifest.pbOuter.GetSignature()) -} - -func TestManifestTamperedSignatureFails(t *testing.T) { - keyID, gpgHome := testGPGEnv(t) - t.Setenv("GNUPGHOME", gpgHome) - - // Create a signed manifest - b := NewBuilder() - b.SetSigningOptions(&SigningOptions{ - KeyID: keyID, - }) - - content := []byte("test file content") - reader := bytes.NewReader(content) - _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, 0, reader, nil) - require.NoError(t, err) - - var buf bytes.Buffer - - err = b.Build(context.Background(), &buf) - require.NoError(t, err) - - // Tamper with the signature by replacing some bytes - data := buf.Bytes() - // Find and modify a byte in the signature portion - for i := range data { - if i > 100 && data[i] == 'A' { - data[i] = 'B' - - break - } - } - - // Try to load the tampered manifest - should fail - _, err = NewManifestFromReader(bytes.NewReader(data)) - 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() - - // Create a builder without signing options - b := NewBuilder() - - // Add a test file - content := []byte("test file content") - reader := bytes.NewReader(content) - _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, 0, reader, nil) - require.NoError(t, err) - - // Build the manifest - var buf bytes.Buffer - - err = b.Build(context.Background(), &buf) - require.NoError(t, err) - - // Parse the manifest and verify signature fields are empty - manifest, err := NewManifestFromReader(&buf) - require.NoError(t, err) - require.NotNil(t, manifest.pbOuter) - - assert.Empty(t, manifest.pbOuter.GetSignature(), - "signature should be empty when not signing") - assert.Empty(t, manifest.pbOuter.GetSigner(), - "signer should be empty when not signing") - assert.Empty(t, manifest.pbOuter.GetSigningPubKey(), - "signing public key should be empty when not signing") -} - -// fakeGPGPath writes script as an executable named gpg into a temporary -// directory and returns a PATH value with that directory first. -func fakeGPGPath(t *testing.T, script string) string { - t.Helper() - - binDir := t.TempDir() - //nolint:gosec // G306: the fake gpg has to be executable - require.NoError(t, os.WriteFile(filepath.Join(binDir, "gpg"), - []byte(script), 0o700)) - - return binDir + string(os.PathListSeparator) + os.Getenv("PATH") -} - -// TestGPGTimeoutKillsGPG puts a fake gpg that never finishes first on -// PATH and checks that a run past its deadline is killed and reported as -// a timeout of the named operation, instead of hanging. The fake gpg writes -// nothing to stderr, so the message ends with the timeout. -func TestGPGTimeoutKillsGPG(t *testing.T) { - t.Setenv("PATH", fakeGPGPath(t, "#!/bin/sh\nexec sleep 10\n")) - - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) - defer cancel() - - _, _, err := gpgSign(ctx, []byte("data"), GPGKeyID("any")) - require.ErrorIs(t, err, context.DeadlineExceeded) - assert.EqualError(t, err, "gpg sign: timed out: context deadline exceeded") -} - -// TestGPGSignKeyNotReportedKeepsStderr puts a fake gpg first on PATH that -// exits cleanly without reporting the key that signed, and checks that what -// it wrote to stderr is in the message. -func TestGPGSignKeyNotReportedKeepsStderr(t *testing.T) { - t.Setenv("PATH", fakeGPGPath(t, - "#!/bin/sh\necho 'gpg: note from the fake gpg' >&2\n")) - - _, _, err := gpgSign(context.Background(), []byte("data"), GPGKeyID("any")) - require.ErrorIs(t, err, errSigningKeyNotReported) - assert.EqualError(t, err, - "gpg did not report the key that made the signature: "+ - "gpg: note from the fake gpg") -} - -// TestGPGFailureKeepsStderr puts a fake gpg first on PATH that writes to -// stderr and exits non-zero, and checks that what it wrote ends the message. -func TestGPGFailureKeepsStderr(t *testing.T) { - t.Setenv("PATH", fakeGPGPath(t, - "#!/bin/sh\necho 'gpg: signing failed: No secret key' >&2\nexit 2\n")) - - _, _, err := gpgSign(context.Background(), []byte("data"), GPGKeyID("any")) - assert.EqualError(t, err, - "gpg sign: exit status 2: gpg: signing failed: No secret key") -} - -// TestGPGCancelWhenChildHoldsOutput uses a fake gpg that runs sleep as a -// child instead of exec-ing it, the way a wrapper script around the real -// gpg might. Killing the fake gpg leaves sleep holding its stdout and -// stderr open; the call must still return once ctx ends instead of waiting -// for sleep to exit. The fake gpg writes the process ID of sleep to a named -// pipe; the test ends ctx only after reading it, so sleep is running by -// then, and kills sleep before returning. -func TestGPGCancelWhenChildHoldsOutput(t *testing.T) { - pidPipe := filepath.Join(t.TempDir(), "sleep.pid") - require.NoError(t, syscall.Mkfifo(pidPipe, 0o600)) - // sleep outlasts the 10 s wait below, so a call that waits for it fails. - t.Setenv("PATH", fakeGPGPath(t, - "#!/bin/sh\nsleep 60 &\necho $! >'"+pidPipe+"'\nwait\n")) - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - signErr := make(chan error, 1) - - go func() { - _, _, err := gpgSign(ctx, []byte("data"), GPGKeyID("any")) - signErr <- err - }() - - pid, err := os.ReadFile(pidPipe) //nolint:gosec // G304: path inside t.TempDir() - require.NoError(t, err) - - n, err := strconv.Atoi(strings.TrimSpace(string(pid))) - require.NoError(t, err) - - sleep, err := os.FindProcess(n) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, sleep.Kill()) }) - - cancel() - - // The call should return about gpgWaitDelay (one second) after the - // cancel. 10 s is far above that and well under the 30 s test timeout, - // which would abort the whole package before the cleanup kills sleep. - select { - case err := <-signErr: - require.ErrorIs(t, err, context.Canceled) - case <-time.After(10 * time.Second): - t.Fatal("the call waited for the child holding gpg's output to exit") - } -} - -// TestBuildPassesContextToSigning checks that a caller can cancel the gpg -// runs that sign a manifest through the context given to Build. -func TestBuildPassesContextToSigning(t *testing.T) { - t.Parallel() - - b := NewBuilder() - b.SetSigningOptions(&SigningOptions{KeyID: "any"}) - - ctx, cancel := context.WithCancel(context.Background()) - cancel() - - require.ErrorIs(t, b.Build(ctx, io.Discard), context.Canceled) -} diff --git a/mfer/openpgp.go b/mfer/openpgp.go new file mode 100644 index 0000000..c5c9c40 --- /dev/null +++ b/mfer/openpgp.go @@ -0,0 +1,363 @@ +package mfer + +import ( + "bytes" + "encoding/hex" + "errors" + "fmt" + "io" + "slices" + "strings" + + "github.com/ProtonMail/go-crypto/openpgp" + "github.com/ProtonMail/go-crypto/openpgp/armor" + pgperrors "github.com/ProtonMail/go-crypto/openpgp/errors" + "github.com/ProtonMail/go-crypto/openpgp/packet" +) + +const ( + // The tags of OpenPGP signature, key and subkey packets (RFC 9580, + // section 5). Subkeys have tags of their own, so each secret or public + // key packet is one primary key. + signaturePacketTag = 2 + secretKeyPacketTag = 5 + publicKeyPacketTag = 6 + secretSubkeyPacketTag = 7 + publicSubkeyPacketTag = 14 + + // In the body of a key or subkey packet the algorithm octet follows + // the version octet and the four-octet creation time, and from version + // 5 on a four-octet length as well (RFC 9580, section 5.5.2). + keyAlgorithmOffset = 5 + firstKeyVersionWithLength = 5 + keyAlgorithmOffsetAfterLength = 9 + + // signingKeyVersion is the only OpenPGP key version mfer signs with. + // Its fingerprints are 40 hex characters, the length + // --require-signature takes. + signingKeyVersion = 4 + + // armorBegin and armorEnd start the lines that begin and end an + // armored block. + armorBegin = "-----BEGIN " + armorEnd = "-----END " +) + +var ( + errKeyCount = errors.New("must hold exactly one key") + errDSAKey = errors.New("must not hold a DSA key") + errNoSecretKey = errors.New("signing key file holds no secret key") + errNotV4Key = errors.New("signing key must be an OpenPGP version 4 key, " + + "the only kind whose fingerprint --require-signature takes") + errNoPassphrase = errors.New( + "signing key is protected and no passphrase was given") + errNotOneSignature = errors.New( + "signature must hold exactly one signature") + errNotOneArmoredBlock = errors.New( + "must be exactly one armored block and nothing else") + errMalformedArmor = errors.New("armor is malformed") +) + +// SigningOptions holds the key a manifest is signed with. +type SigningOptions struct { + // SecretKey is an OpenPGP secret key, armored or binary, as + // gpg --export-secret-keys writes it. It must hold one primary key. + SecretKey []byte + // Passphrase unlocks SecretKey when it is protected. + Passphrase []byte +} + +// SecretKeyIsProtected reports whether the OpenPGP secret key secretKey, +// armored or binary, needs a passphrase to sign. It fails unless +// secretKey holds one version 4 primary key with its secret key. +func SecretKeyIsProtected(secretKey []byte) (bool, error) { + key, err := readSecretKey(secretKey) + if err != nil { + return false, err + } + + return isProtected(key), nil +} + +// CheckSigningKey fails unless opts can sign now: opts.SecretKey must hold +// one version 4 primary key with a secret key that may sign and has not +// expired or been revoked, and opts.Passphrase must unlock it when it is +// protected. It lets a caller find a key that cannot sign before it builds +// a manifest. +func CheckSigningKey(opts *SigningOptions) error { + key, err := readSigningKey(opts) + if err != nil { + return err + } + + // Signing nothing fails wherever signing the manifest would. + err = openpgp.DetachSign(io.Discard, key, bytes.NewReader(nil), nil) + if err != nil { + return fmt.Errorf("signing key cannot sign: %w", err) + } + + return nil +} + +// readSigningKey returns the key in opts.SecretKey, unlocked with +// opts.Passphrase if it is protected. +func readSigningKey(opts *SigningOptions) (*openpgp.Entity, error) { + key, err := readSecretKey(opts.SecretKey) + if err != nil { + return nil, err + } + + if !isProtected(key) { + return key, nil + } + + if len(opts.Passphrase) == 0 { + return nil, errNoPassphrase + } + + err = key.DecryptPrivateKeys(opts.Passphrase) + if err != nil { + return nil, fmt.Errorf("unlock signing key: %w", err) + } + + return key, nil +} + +// readSecretKey returns the one key in secretKey, armored or binary, +// which must be a version 4 key and include its secret key. +func readSecretKey(secretKey []byte) (*openpgp.Entity, error) { + key, err := readOneKey(secretKey, "signing key file") + if err != nil { + return nil, err + } + + if key.PrimaryKey.Version != signingKeyVersion { + return nil, fmt.Errorf("%w; this key is version %d", + errNotV4Key, key.PrimaryKey.Version) + } + + if key.PrivateKey == nil { + return nil, errNoSecretKey + } + + return key, nil +} + +// readOneKey returns the key in data, armored or binary, which must hold +// exactly one primary key and no DSA key or subkey. what names data in +// errors. +func readOneKey(data []byte, what string) (*openpgp.Entity, error) { + packets, err := dearmor(data) + if err != nil { + return nil, fmt.Errorf("read %s: %w", what, err) + } + + keys, err := countPackets(packets, secretKeyPacketTag, publicKeyPacketTag) + if err != nil { + return nil, fmt.Errorf("read %s: %w", what, err) + } + + if keys != 1 { + return nil, fmt.Errorf("%s %w, found %d", what, errKeyCount, keys) + } + + // openpgp.ReadKeyRing checks every self-signature, and a DSA key with + // very large numbers makes each check take seconds to minutes. + dsa, err := holdsDSAKey(packets) + if err != nil { + return nil, fmt.Errorf("read %s: %w", what, err) + } + + if dsa { + return nil, fmt.Errorf("%s %w", what, errDSAKey) + } + + keyring, err := openpgp.ReadKeyRing(bytes.NewReader(packets)) + if err != nil { + return nil, fmt.Errorf("read %s: %w", what, err) + } + + // openpgp.ReadKeyRing also reads a subkey packet at the start as a + // primary key. + if len(keyring) != 1 { + return nil, fmt.Errorf("%s %w, found %d", what, errKeyCount, len(keyring)) + } + + return keyring[0], nil +} + +// isProtected reports whether any secret key in key needs a passphrase. +func isProtected(key *openpgp.Entity) bool { + if key.PrivateKey.Encrypted { + return true + } + + for _, subkey := range key.Subkeys { + if subkey.PrivateKey != nil && subkey.PrivateKey.Encrypted { + return true + } + } + + return false +} + +// armoredPublicKey returns the public part of key, armored. +func armoredPublicKey(key *openpgp.Entity) ([]byte, error) { + var buf bytes.Buffer + + w, err := armor.Encode(&buf, openpgp.PublicKeyType, nil) + if err != nil { + return nil, err + } + + err = key.Serialize(w) + if err != nil { + return nil, fmt.Errorf("write public key: %w", err) + } + + err = w.Close() + if err != nil { + return nil, err + } + + return buf.Bytes(), nil +} + +// fingerprint returns the fingerprint of key's primary key in upper-case +// hex, as gpg prints it. +func fingerprint(key *openpgp.Entity) string { + return strings.ToUpper(hex.EncodeToString(key.PrimaryKey.Fingerprint)) +} + +// verifySignature checks that signature is one good OpenPGP signature +// over data, made by the one primary key in pubKey or one of its subkeys, +// and returns that primary key's fingerprint. signature and pubKey may each +// be armored or binary. +func verifySignature(data, signature, pubKey []byte) (string, error) { + key, err := readOneKey(pubKey, "embedded public key block") + if err != nil { + return "", err + } + + sigData, err := dearmor(signature) + if err != nil { + return "", fmt.Errorf("read signature: %w", err) + } + + sigs, err := countPackets(sigData, signaturePacketTag) + if err != nil { + return "", fmt.Errorf("read signature: %w", err) + } + + if sigs != 1 { + return "", fmt.Errorf("%w, found %d", errNotOneSignature, sigs) + } + + _, err = openpgp.CheckDetachedSignature(openpgp.EntityList{key}, + bytes.NewReader(data), bytes.NewReader(sigData), nil) + // A manifest outlives its signing key, so a signature by a key that + // has expired since is still good. + if err != nil && !errors.Is(err, pgperrors.ErrKeyExpired) { + return "", fmt.Errorf("verify signature: %w", err) + } + + return fingerprint(key), nil +} + +// dearmor returns the binary OpenPGP data in data: data itself when it is +// not armored, or else the body of its armored block. Armored data must be +// one block and nothing else: its first line is the only BEGIN line and +// its last line the only END line, white space around them aside. +// armor.Decode skips any text before a BEGIN line and reads only the first +// block, so without this a second key or signature would go unseen. +func dearmor(data []byte) ([]byte, error) { + if !bytes.Contains(data, []byte(armorBegin)) { + return data, nil + } + + text := bytes.TrimSpace(data) + lastLine := text[bytes.LastIndexByte(text, '\n')+1:] + + if !bytes.HasPrefix(text, []byte(armorBegin)) || + bytes.Count(text, []byte(armorBegin)) != 1 || + !bytes.HasPrefix(lastLine, []byte(armorEnd)) || + bytes.Count(text, []byte(armorEnd)) != 1 { + return nil, errNotOneArmoredBlock + } + + // armor.Decode passes over a block it cannot read, such as one with a + // header line that has no colon, and returns io.EOF on finding no + // other. + block, err := armor.Decode(bytes.NewReader(text)) + if err != nil { + return nil, errMalformedArmor + } + + body, err := io.ReadAll(block.Body) + if err != nil { + return nil, fmt.Errorf("%w: %w", errMalformedArmor, err) + } + + return body, nil +} + +// countPackets returns how many packets in the binary OpenPGP data have +// one of tags. It reads only each packet's header, so it also counts +// packets that openpgp.ReadKeyRing skips, such as a key with no user ID +// or of an algorithm it does not know. +func countPackets(data []byte, tags ...uint8) (int, error) { + packets := packet.NewOpaqueReader(bytes.NewReader(data)) + count := 0 + + for { + p, err := packets.Next() + if errors.Is(err, io.EOF) { + return count, nil + } + + if err != nil { + return 0, err + } + + if slices.Contains(tags, p.Tag) { + count++ + } + } +} + +// holdsDSAKey reports whether any key or subkey packet in the binary +// OpenPGP data holds a DSA key. It reads each packet's algorithm octet +// rather than parsing the packet, since parsing a secret key packet checks +// its numbers, which for a DSA key with very large numbers is as slow as +// checking a self-signature. +func holdsDSAKey(data []byte) (bool, error) { + packets := packet.NewOpaqueReader(bytes.NewReader(data)) + + for { + p, err := packets.Next() + if errors.Is(err, io.EOF) { + return false, nil + } + + if err != nil { + return false, err + } + + if !slices.Contains([]uint8{ + secretKeyPacketTag, publicKeyPacketTag, + secretSubkeyPacketTag, publicSubkeyPacketTag, + }, p.Tag) { + continue + } + + offset := keyAlgorithmOffset + if len(p.Contents) > 0 && p.Contents[0] >= firstKeyVersionWithLength { + offset = keyAlgorithmOffsetAfterLength + } + + if len(p.Contents) > offset && + packet.PublicKeyAlgorithm(p.Contents[offset]) == packet.PubKeyAlgoDSA { + return true, nil + } + } +} diff --git a/mfer/openpgp_test.go b/mfer/openpgp_test.go new file mode 100644 index 0000000..efcaa86 --- /dev/null +++ b/mfer/openpgp_test.go @@ -0,0 +1,754 @@ +//nolint:testpackage // white-box tests exercise unexported internals +package mfer + +import ( + "bytes" + "context" + "crypto/dsa" //nolint:staticcheck // SA1019: tests need a DSA key to refuse + "io" + "math/big" + "os" + "path/filepath" + "slices" + "strconv" + "strings" + "testing" + "time" + + "github.com/ProtonMail/go-crypto/openpgp" + "github.com/ProtonMail/go-crypto/openpgp/armor" + "github.com/ProtonMail/go-crypto/openpgp/packet" + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" +) + +// newTestKey returns a new Ed25519 key, which is quick to make, for +// "MFER Test Key ". config may set when it is made and how +// long it lasts. +func newTestKey(t *testing.T, config *packet.Config) *openpgp.Entity { + t.Helper() + + if config == nil { + config = &packet.Config{} + } + + config.Algorithm = packet.PubKeyAlgoEdDSA + + key, err := openpgp.NewEntity("MFER Test Key", "", "test@mfer.test", config) + require.NoError(t, err) + + return key +} + +// armoredSecretKeys returns keys with their secret keys in one armored +// block, as gpg --export-secret-keys --armor writes them. +func armoredSecretKeys(t *testing.T, keys ...*openpgp.Entity) []byte { + t.Helper() + + var buf bytes.Buffer + + w, err := armor.Encode(&buf, openpgp.PrivateKeyType, nil) + require.NoError(t, err) + + for _, key := range keys { + require.NoError(t, key.SerializePrivateWithoutSigning(w, nil)) + } + + require.NoError(t, w.Close()) + + return buf.Bytes() +} + +// armoredPublicKeys returns the public parts of keys in one armored block, +// as gpg --export --armor writes them. +func armoredPublicKeys(t *testing.T, keys ...*openpgp.Entity) []byte { + t.Helper() + + var buf bytes.Buffer + + w, err := armor.Encode(&buf, openpgp.PublicKeyType, nil) + require.NoError(t, err) + + for _, key := range keys { + require.NoError(t, key.Serialize(w)) + } + + require.NoError(t, w.Close()) + + return buf.Bytes() +} + +// testSigningOptions returns signing options for a new key with no +// passphrase. +func testSigningOptions(t *testing.T) *SigningOptions { + t.Helper() + + return &SigningOptions{SecretKey: armoredSecretKeys(t, newTestKey(t, nil))} +} + +// joinArmored returns the armored block first followed by the armored +// block second on the next line. armor.Encode ends a block without a +// newline, unlike gpg, and a block only starts at the start of a line. +func joinArmored(first, second []byte) []byte { + return slices.Concat(first, []byte("\n"), second) +} + +// signedTestManifest returns a manifest of one file signed with opts. +func signedTestManifest(t *testing.T, opts *SigningOptions) []byte { + t.Helper() + + b := NewBuilder() + b.SetSigningOptions(opts) + + 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 TestBuilderWithSigning(t *testing.T) { + t.Parallel() + + key := newTestKey(t, nil) + + // Create a builder with signing options + b := NewBuilder() + b.SetSigningOptions(&SigningOptions{SecretKey: armoredSecretKeys(t, key)}) + + // Add a test file + content := []byte("test file content") + reader := bytes.NewReader(content) + _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, 0, reader, nil) + require.NoError(t, err) + + // Build the manifest + var buf bytes.Buffer + + err = b.Build(context.Background(), &buf) + require.NoError(t, err) + + // Parse the manifest and verify signature fields are populated + manifest, err := NewManifestFromReader(&buf) + require.NoError(t, err) + require.NotNil(t, manifest.pbOuter) + + assert.NotEmpty(t, manifest.pbOuter.GetSignature(), + "signature should be populated") + assert.NotEmpty(t, manifest.pbOuter.GetSigningPubKey(), + "signing public key should be populated") + + // The signer is the key's fingerprint in 40 upper-case hex characters. + assert.Equal(t, fingerprint(key), string(manifest.pbOuter.GetSigner())) + assert.Regexp(t, "^[0-9A-F]{40}$", string(manifest.pbOuter.GetSigner())) + + // Verify signature is a valid PGP signature + assert.Contains(t, string(manifest.pbOuter.GetSignature()), + "-----BEGIN PGP SIGNATURE-----") + + // Verify public key is a valid PGP public key block + assert.Contains(t, string(manifest.pbOuter.GetSigningPubKey()), + "-----BEGIN PGP PUBLIC KEY BLOCK-----") +} + +func TestScannerWithSigning(t *testing.T) { + t.Parallel() + + // Create in-memory filesystem with test files + fs := afero.NewMemMapFs() + require.NoError(t, fs.MkdirAll("/testdir", 0o755)) + require.NoError(t, + afero.WriteFile(fs, "/testdir/file1.txt", []byte("content1"), 0o644)) + require.NoError(t, + afero.WriteFile(fs, "/testdir/file2.txt", []byte("content2"), 0o644)) + + // Create scanner with signing options + opts := &ScannerOptions{ + Fs: fs, + SigningOptions: testSigningOptions(t), + } + s := NewScannerWithOptions(opts) + + // Enumerate files + require.NoError(t, s.EnumeratePath("/testdir", nil)) + assert.Equal(t, FileCount(2), s.FileCount()) + + // Generate signed manifest + var buf bytes.Buffer + require.NoError(t, s.ToManifest(context.Background(), &buf, nil)) + + // Parse and verify + manifest, err := NewManifestFromReader(&buf) + require.NoError(t, err) + + assert.NotEmpty(t, manifest.pbOuter.GetSignature()) + assert.NotEmpty(t, manifest.pbOuter.GetSigner()) + assert.NotEmpty(t, manifest.pbOuter.GetSigningPubKey()) +} + +// TestSigningWithBinarySecretKey signs with a secret key that is not +// armored, as gpg --export-secret-keys writes it without --armor. +func TestSigningWithBinarySecretKey(t *testing.T) { + t.Parallel() + + var secretKey bytes.Buffer + + require.NoError(t, newTestKey(t, nil).SerializePrivateWithoutSigning(&secretKey, nil)) + + _, err := NewManifestFromReader(bytes.NewReader( + signedTestManifest(t, &SigningOptions{SecretKey: secretKey.Bytes()}))) + require.NoError(t, err) +} + +// TestSigningWithProtectedKey signs with a key protected by a passphrase: +// with the passphrase, without one, and with a wrong one. +func TestSigningWithProtectedKey(t *testing.T) { + t.Parallel() + + key := newTestKey(t, nil) + require.NoError(t, key.EncryptPrivateKeys([]byte("right"), nil)) + secretKey := armoredSecretKeys(t, key) + + protected, err := SecretKeyIsProtected(secretKey) + require.NoError(t, err) + assert.True(t, protected) + + _, err = NewManifestFromReader(bytes.NewReader(signedTestManifest(t, + &SigningOptions{SecretKey: secretKey, Passphrase: []byte("right")}))) + require.NoError(t, err) + + b := NewBuilder() + b.SetSigningOptions(&SigningOptions{SecretKey: secretKey}) + require.ErrorIs(t, b.Build(context.Background(), io.Discard), errNoPassphrase) + + b.SetSigningOptions(&SigningOptions{ + SecretKey: secretKey, Passphrase: []byte("wrong"), + }) + assert.ErrorContains(t, b.Build(context.Background(), io.Discard), + "unlock signing key") +} + +func TestSecretKeyIsProtectedWithoutPassphrase(t *testing.T) { + t.Parallel() + + protected, err := SecretKeyIsProtected(testSigningOptions(t).SecretKey) + require.NoError(t, err) + assert.False(t, protected) +} + +// TestSigningKeyFileWithTwoKeys signs with a key file that holds two keys, +// as gpg writes it for a user ID that two keys have. It names no one key +// to sign with, so signing must fail. +func TestSigningKeyFileWithTwoKeys(t *testing.T) { + t.Parallel() + + b := NewBuilder() + b.SetSigningOptions(&SigningOptions{SecretKey: armoredSecretKeys(t, + newTestKey(t, nil), newTestKey(t, nil))}) + + err := b.Build(context.Background(), io.Discard) + require.ErrorIs(t, err, errKeyCount) + assert.EqualError(t, err, "signing key file must hold exactly one key, found 2") +} + +// TestSigningKeyFileWithoutSecretKey signs with a file that holds only a +// public key. +func TestSigningKeyFileWithoutSecretKey(t *testing.T) { + t.Parallel() + + b := NewBuilder() + b.SetSigningOptions(&SigningOptions{ + SecretKey: armoredPublicKeys(t, newTestKey(t, nil)), + }) + require.ErrorIs(t, b.Build(context.Background(), io.Discard), errNoSecretKey) +} + +// TestCheckSigningKeyRefusesKeyThatCannotSign checks a key that can sign, +// and keys that read and need no passphrase but cannot sign now: one that +// expired in 2020 and one that has been revoked. CheckSigningKey must +// refuse each of the two, as signing a manifest with it would fail. +func TestCheckSigningKeyRefusesKeyThatCannotSign(t *testing.T) { + t.Parallel() + + require.NoError(t, CheckSigningKey(testSigningOptions(t))) + + made := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC) + expired := newTestKey(t, &packet.Config{ + Time: func() time.Time { return made }, + KeyLifetimeSecs: uint32((24 * time.Hour).Seconds()), + }) + + revoked := newTestKey(t, nil) + require.NoError(t, revoked.RevokeKey(packet.KeyRetired, "", nil)) + + for name, key := range map[string]*openpgp.Entity{ + "expired": expired, + "revoked": revoked, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + assert.ErrorContains(t, CheckSigningKey( + &SigningOptions{SecretKey: armoredSecretKeys(t, key)}), + "signing key cannot sign") + }) + } +} + +// TestSigningRefusesVersion6Key signs with an OpenPGP version 6 key, whose +// fingerprint is 64 hex characters. mfer signs only with version 4 keys. +func TestSigningRefusesVersion6Key(t *testing.T) { + t.Parallel() + + key, err := openpgp.NewEntity("MFER Test Key", "", "test@mfer.test", + &packet.Config{V6Keys: true, Algorithm: packet.PubKeyAlgoEd25519}) + require.NoError(t, err) + + secretKey := armoredSecretKeys(t, key) + + _, err = SecretKeyIsProtected(secretKey) + require.ErrorIs(t, err, errNotV4Key) + + b := NewBuilder() + b.SetSigningOptions(&SigningOptions{SecretKey: secretKey}) + require.ErrorIs(t, b.Build(context.Background(), io.Discard), errNotV4Key) +} + +// TestMalformedArmorIsNamed reads a signing key file, a signature and an +// embedded public key block whose armor is malformed: a header line with +// no colon, or no blank line after the BEGIN line. Each must fail saying +// the armor is malformed. +func TestMalformedArmorIsNamed(t *testing.T) { + t.Parallel() + + opts := testSigningOptions(t) + manifest := signedTestManifest(t, opts) + + for name, change := range map[string]func([]byte) []byte{ + "header line with no colon": func(block []byte) []byte { + return bytes.Replace(block, + []byte("-----\n"), []byte("-----\nno colon\n"), 1) + }, + "no blank line after the BEGIN line": func(block []byte) []byte { + return bytes.Replace(block, []byte("-----\n\n"), []byte("-----\n"), 1) + }, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + _, err := SecretKeyIsProtected(change(opts.SecretKey)) + require.ErrorIs(t, err, errMalformedArmor) + + for _, changed := range [][]byte{ + rewriteOuter(t, manifest, func(outer *MFFileOuter) { + outer.Signature = change(outer.GetSignature()) + }), + rewriteOuter(t, manifest, func(outer *MFFileOuter) { + outer.SigningPubKey = change(outer.GetSigningPubKey()) + }), + } { + _, err = NewManifestFromReader(bytes.NewReader(changed)) + require.ErrorIs(t, err, errMalformedArmor) + } + }) + } +} + +func TestVerifySignature(t *testing.T) { + t.Parallel() + + key := newTestKey(t, nil) + data := []byte("test data to sign and verify") + + var armored, binary bytes.Buffer + + require.NoError(t, openpgp.ArmoredDetachSign(&armored, key, + bytes.NewReader(data), nil)) + require.NoError(t, openpgp.DetachSign(&binary, key, bytes.NewReader(data), nil)) + + pubKey, err := armoredPublicKey(key) + require.NoError(t, err) + + var binaryPubKey bytes.Buffer + + require.NoError(t, key.Serialize(&binaryPubKey)) + + // Verifying names the key that made the signature, whether the + // signature and key are armored or not. + signer, err := verifySignature(data, armored.Bytes(), pubKey) + require.NoError(t, err) + assert.Equal(t, fingerprint(key), signer) + + signer, err = verifySignature(data, binary.Bytes(), binaryPubKey.Bytes()) + require.NoError(t, err) + assert.Equal(t, fingerprint(key), signer) + + // A signature over other data is bad. + _, err = verifySignature([]byte("different data"), armored.Bytes(), pubKey) + require.Error(t, err) + + // A public key that is not one cannot verify anything. + _, err = verifySignature(data, armored.Bytes(), []byte("not a public key")) + assert.Error(t, err) +} + +// TestVerifySignatureKeyExpiredSince verifies a signature made in 2020 by +// a key that expired a day after it was made. The signature is still good. +func TestVerifySignatureKeyExpiredSince(t *testing.T) { + t.Parallel() + + made := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC) + config := &packet.Config{ + Time: func() time.Time { return made }, + KeyLifetimeSecs: uint32((24 * time.Hour).Seconds()), + } + key := newTestKey(t, config) + data := []byte("signed in 2020") + + var sig bytes.Buffer + + require.NoError(t, openpgp.ArmoredDetachSign(&sig, key, bytes.NewReader(data), config)) + + pubKey, err := armoredPublicKey(key) + require.NoError(t, err) + + signer, err := verifySignature(data, sig.Bytes(), pubKey) + require.NoError(t, err) + assert.Equal(t, fingerprint(key), signer) +} + +func TestManifestSignatureVerification(t *testing.T) { + t.Parallel() + + // Parse the manifest - signature should be verified during load + manifest, err := NewManifestFromReader(bytes.NewReader( + signedTestManifest(t, testSigningOptions(t)))) + require.NoError(t, err) + require.NotNil(t, manifest) + + // Signature should be present and valid + assert.NotEmpty(t, manifest.pbOuter.GetSignature()) +} + +func TestManifestTamperedSignatureFails(t *testing.T) { + t.Parallel() + + // Change one character of the signature's base64 body, which starts + // after the blank line that ends the armor headers. + data := rewriteOuter(t, signedTestManifest(t, testSigningOptions(t)), + func(outer *MFFileOuter) { + sig := outer.GetSignature() + i := bytes.Index(sig, []byte("\n\n")) + len("\n\n") + 20 + + sig[i]++ + }) + + // Try to load the tampered manifest - should fail + _, err := NewManifestFromReader(bytes.NewReader(data)) + assert.Error(t, err) +} + +// TestManifestSignedByGPGLoads loads the signed seed of +// FuzzNewManifestFromReader, a manifest signed with gpg before mfer signed +// and verified manifests itself. +func TestManifestSignedByGPGLoads(t *testing.T) { + t.Parallel() + + seed, err := os.ReadFile(filepath.Join( + "testdata", "fuzz", "FuzzNewManifestFromReader", "signed")) + require.NoError(t, err) + + // After its header line the seed holds the manifest as []byte("..."). + _, quoted, found := strings.Cut(string(seed), "[]byte(") + require.True(t, found) + + manifest, err := strconv.Unquote( + strings.TrimSuffix(strings.TrimSpace(quoted), ")")) + require.NoError(t, err) + + m, err := NewManifestFromReader(strings.NewReader(manifest)) + require.NoError(t, err) + assert.Equal(t, "4F562BFB863FDC6B51B4EE88872A51176CEF23AE", + string(m.pbOuter.GetSigner())) +} + +// TestManifestRefusesSecondEmbeddedKey loads manifests whose embedded +// public key block holds another key besides the key that signed it: as a +// public key, as a secret key with no user ID, which openpgp.ReadKeyRing +// skips, and written as a subkey packet at the start of the block, which +// openpgp.ReadKeyRing reads as a primary key. Loading must refuse each, +// although the signature is good and the signer field names the key that +// made it. +func TestManifestRefusesSecondEmbeddedKey(t *testing.T) { + t.Parallel() + + other := newTestKey(t, nil) + signer := newTestKey(t, nil) + manifest := signedTestManifest(t, + &SigningOptions{SecretKey: armoredSecretKeys(t, signer)}) + + otherWithoutUserID := newTestKey(t, nil) + otherWithoutUserID.Identities = map[string]*openpgp.Identity{} + + otherAsSubkey := newTestKey(t, nil) + otherAsSubkey.PrimaryKey.IsSubkey = true + + for name, block := range map[string][]byte{ + "public key": armoredPublicKeys(t, other, signer), + "secret key without user ID": armoredSecretKeys(t, signer, otherWithoutUserID), + "subkey packet first": armoredPublicKeys(t, otherAsSubkey, signer), + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + embedded := rewriteOuter(t, manifest, func(outer *MFFileOuter) { + outer.SigningPubKey = block + }) + + _, err := NewManifestFromReader(bytes.NewReader(embedded)) + require.ErrorIs(t, err, errKeyCount) + }) + } +} + +// TestManifestRefusesSecondEmbeddedKeyWithoutUserID loads a manifest whose +// embedded public key block holds, before the key that signed it, another +// key with no user ID, which openpgp.ReadKeyRing skips. Loading must +// refuse it: the block holds two keys. +func TestManifestRefusesSecondEmbeddedKeyWithoutUserID(t *testing.T) { + t.Parallel() + + other := newTestKey(t, nil) + other.Identities = map[string]*openpgp.Identity{} + + signer := newTestKey(t, nil) + + manifest := rewriteOuter(t, signedTestManifest(t, + &SigningOptions{SecretKey: armoredSecretKeys(t, signer)}), + func(outer *MFFileOuter) { + outer.SigningPubKey = armoredPublicKeys(t, other, signer) + }) + + _, err := NewManifestFromReader(bytes.NewReader(manifest)) + require.ErrorIs(t, err, errKeyCount) +} + +// dsaKeyPacket returns a public key packet holding a DSA key, or a public +// subkey packet when isSubkey. Its numbers are not a working key: loading +// must refuse the packet before it uses them. +func dsaKeyPacket(t *testing.T, isSubkey bool) []byte { + t.Helper() + + key := packet.NewDSAPublicKey(time.Now(), &dsa.PublicKey{ + P: big.NewInt(23), Q: big.NewInt(11), G: big.NewInt(4), Y: big.NewInt(8), + }) + key.IsSubkey = isSubkey + + var buf bytes.Buffer + + require.NoError(t, key.Serialize(&buf)) + + return buf.Bytes() +} + +// TestManifestRefusesDSAKey loads manifests whose embedded public key +// block holds a DSA key: alone, or as a subkey after the key that signed +// the manifest. openpgp.ReadKeyRing checks a key's self-signatures, which +// for a DSA key with very large numbers takes minutes each. Loading must +// refuse each block before that. +func TestManifestRefusesDSAKey(t *testing.T) { + t.Parallel() + + signer := newTestKey(t, nil) + manifest := signedTestManifest(t, + &SigningOptions{SecretKey: armoredSecretKeys(t, signer)}) + + var signerKey bytes.Buffer + + require.NoError(t, signer.Serialize(&signerKey)) + + for name, block := range map[string][]byte{ + "DSA key": dsaKeyPacket(t, false), + "DSA subkey": slices.Concat(signerKey.Bytes(), dsaKeyPacket(t, true)), + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + embedded := rewriteOuter(t, manifest, func(outer *MFFileOuter) { + outer.SigningPubKey = block + }) + + _, err := NewManifestFromReader(bytes.NewReader(embedded)) + require.ErrorIs(t, err, errDSAKey) + }) + } +} + +// TestManifestRefusesTwoSignatures loads a manifest whose signature field +// holds its good signature twice, not armored. Loading must refuse it. +func TestManifestRefusesTwoSignatures(t *testing.T) { + t.Parallel() + + manifest := rewriteOuter(t, signedTestManifest(t, testSigningOptions(t)), + func(outer *MFFileOuter) { + sig, err := dearmor(outer.GetSignature()) + require.NoError(t, err) + + outer.Signature = slices.Concat(sig, sig) + }) + + _, err := NewManifestFromReader(bytes.NewReader(manifest)) + require.ErrorIs(t, err, errNotOneSignature) +} + +// TestManifestRefusesFieldNotOneArmoredBlock loads manifests whose +// signature or embedded public key block holds its good armored block with +// something else: a second armored block, text after the END line, or many +// END lines before the block. Decoding the block once for each END line +// before it would take time and memory that grow with the square of the +// field's size. Loading must refuse each. +func TestManifestRefusesFieldNotOneArmoredBlock(t *testing.T) { + t.Parallel() + + manifest := signedTestManifest(t, testSigningOptions(t)) + + for name, change := range map[string]func([]byte) []byte{ + "second armored block": func(block []byte) []byte { + return joinArmored(block, block) + }, + "text after the END line": func(block []byte) []byte { + return slices.Concat(block, []byte("\nmore text\n")) + }, + "END lines before the block": func(block []byte) []byte { + return slices.Concat( + []byte(strings.Repeat(armorEnd+"\n", 1000)), block) + }, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + for _, changed := range [][]byte{ + rewriteOuter(t, manifest, func(outer *MFFileOuter) { + outer.Signature = change(outer.GetSignature()) + }), + rewriteOuter(t, manifest, func(outer *MFFileOuter) { + outer.SigningPubKey = change(outer.GetSigningPubKey()) + }), + } { + _, err := NewManifestFromReader(bytes.NewReader(changed)) + require.ErrorIs(t, err, errNotOneArmoredBlock) + } + }) + } +} + +// TestManifestSignedWithSubkey signs with a key that has a signing subkey, +// which signs in place of the primary key. The manifest must load, with +// the primary key's fingerprint as signer. +func TestManifestSignedWithSubkey(t *testing.T) { + t.Parallel() + + key := newTestKey(t, nil) + require.NoError(t, key.AddSigningSubkey( + &packet.Config{Algorithm: packet.PubKeyAlgoEdDSA})) + + m, err := NewManifestFromReader(bytes.NewReader(signedTestManifest(t, + &SigningOptions{SecretKey: armoredSecretKeys(t, key)}))) + require.NoError(t, err) + assert.Equal(t, fingerprint(key), string(m.pbOuter.GetSigner())) + + block, err := armor.Decode(bytes.NewReader(m.pbOuter.GetSignature())) + require.NoError(t, err) + + p, err := packet.Read(block.Body) + require.NoError(t, err) + + sig, ok := p.(*packet.Signature) + require.True(t, ok) + + subkey := key.Subkeys[len(key.Subkeys)-1].PublicKey + assert.Equal(t, subkey.KeyId, *sig.IssuerKeyId, + "the signing subkey made the signature") +} + +// TestManifestRefusesSignerOtherThanSigningKey loads a manifest whose +// signer field names a key other than the one that made the signature. +func TestManifestRefusesSignerOtherThanSigningKey(t *testing.T) { + t.Parallel() + + manifest := rewriteOuter(t, signedTestManifest(t, testSigningOptions(t)), + func(outer *MFFileOuter) { + outer.Signer = []byte(strings.Repeat("A", len(outer.GetSigner()))) + }) + + _, err := NewManifestFromReader(bytes.NewReader(manifest)) + require.ErrorIs(t, err, errSignerNotSigningKey) +} + +func TestBuilderWithoutSigning(t *testing.T) { + t.Parallel() + + // Create a builder without signing options + b := NewBuilder() + + // Add a test file + content := []byte("test file content") + reader := bytes.NewReader(content) + _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, 0, reader, nil) + require.NoError(t, err) + + // Build the manifest + var buf bytes.Buffer + + err = b.Build(context.Background(), &buf) + require.NoError(t, err) + + // Parse the manifest and verify signature fields are empty + manifest, err := NewManifestFromReader(&buf) + require.NoError(t, err) + require.NotNil(t, manifest.pbOuter) + + assert.Empty(t, manifest.pbOuter.GetSignature(), + "signature should be empty when not signing") + assert.Empty(t, manifest.pbOuter.GetSigner(), + "signer should be empty when not signing") + assert.Empty(t, manifest.pbOuter.GetSigningPubKey(), + "signing public key should be empty when not signing") +} + +// TestBuildPassesContextToSigning checks that Build does not sign once the +// context given to it has ended. +func TestBuildPassesContextToSigning(t *testing.T) { + t.Parallel() + + b := NewBuilder() + b.SetSigningOptions(&SigningOptions{SecretKey: []byte("any")}) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + require.ErrorIs(t, b.Build(ctx, io.Discard), context.Canceled) +} diff --git a/mfer/scanner.go b/mfer/scanner.go index 5326bde..2dcfbc2 100644 --- a/mfer/scanner.go +++ b/mfer/scanner.go @@ -58,7 +58,7 @@ type ScannerOptions struct { IncludePermissions bool // Fs is the filesystem to use, defaults to OsFs if nil. Fs afero.Fs - // SigningOptions holds GPG signing options (nil = no signing). + // SigningOptions holds the key to sign with (nil = no signing). SigningOptions *SigningOptions // Seed, if set, derives a deterministic UUID from this seed. Seed string diff --git a/mfer/serialize.go b/mfer/serialize.go index 6688ed9..f40eac8 100644 --- a/mfer/serialize.go +++ b/mfer/serialize.go @@ -7,9 +7,11 @@ import ( "errors" "fmt" "math" + "strings" "time" "uuid" + "github.com/ProtonMail/go-crypto/openpgp" "github.com/klauspost/compress/zstd" "google.golang.org/protobuf/proto" ) @@ -133,44 +135,48 @@ func (m *manifest) generateOuter(ctx context.Context) error { } // Sign the manifest if signing options are provided - if m.signingOptions != nil && m.signingOptions.KeyID != "" { + if m.signingOptions != nil { return m.signOuter(ctx) } return nil } -// signOuter signs the outer message with the configured GPG key and -// 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. +// signOuter signs the outer message with the secret key in the signing +// options and embeds the signature, the key's fingerprint and its public +// key. func (m *manifest) signOuter(ctx context.Context) error { + // Unlocking a protected key can take a while; do not start once ctx + // has ended. + err := ctx.Err() + if err != nil { + return err + } + sigString, err := m.signatureString() if err != nil { return fmt.Errorf("build signature string: %w", err) } - sig, signingKey, err := gpgSign(ctx, []byte(sigString), m.signingOptions.KeyID) + key, err := readSigningKey(m.signingOptions) if err != nil { return err } - m.pbOuter.Signature = sig + var sig bytes.Buffer - // Listing the signing key, a subkey's included, puts its primary key's - // fingerprint first. - fingerprint, err := gpgGetKeyFingerprint(ctx, GPGKeyID(signingKey)) + err = openpgp.ArmoredDetachSign(&sig, key, strings.NewReader(sigString), nil) if err != nil { - return err + return fmt.Errorf("sign manifest: %w", err) } - m.pbOuter.Signer = fingerprint - - pubKey, err := gpgExportPublicKey(ctx, GPGKeyID(fingerprint)) + pubKey, err := armoredPublicKey(key) if err != nil { return err } + m.pbOuter.Signature = sig.Bytes() + m.pbOuter.Signer = []byte(fingerprint(key)) m.pbOuter.SigningPubKey = pubKey return nil