From 1138dbe4d8c24728b44afa0c601a240d9cd56f90 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 OpenPGP secret key; a protected key's passphrase comes from MFER_SIGN_KEY_PASSPHRASE or a prompt on the terminal. Verification keeps the rules of the --require-signature fix: one primary key in the embedded block, counted from its packets so that keys the library skips count too, exactly one signature, made by that key or one of its subkeys, and signer equal to its fingerprint. Tests make their keys in process. Model: opus-5-5 --- README.md | 13 +- docs/FORMAT.md | 14 +- go.mod | 2 + go.sum | 4 + internal/cli/check.go | 3 +- internal/cli/entry_test.go | 7 +- internal/cli/errmsg_test.go | 80 ++-- 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 | 79 ++++ internal/cli/signing_test.go | 91 +++++ 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 | 266 +++++++++++++ mfer/openpgp_test.go | 568 +++++++++++++++++++++++++++ mfer/scanner.go | 2 +- mfer/serialize.go | 34 +- 25 files changed, 1143 insertions(+), 1174 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..6609252 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,14 @@ 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 + - 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..378eb64 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,10 +142,10 @@ 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 diff --git a/go.mod b/go.mod index 66e394f..15e6614 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module sneak.berlin/go/mfer go 1.27.1 require ( + github.com/ProtonMail/go-crypto v1.5.2 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 +16,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..f8a78b4 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,7 @@ +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/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..408f6a6 100644 --- a/internal/cli/check.go +++ b/internal/cli/check.go @@ -21,7 +21,7 @@ import ( "sneak.berlin/go/mfer/mfer" ) -// fingerprintHexLen is the length of a full GPG key fingerprint in hex +// fingerprintHexLen is the length of a full OpenPGP key fingerprint in hex // characters. const fingerprintHexLen = 40 @@ -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 b27ea7e..31d0fde 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..66ac691 100644 --- a/internal/cli/errmsg_test.go +++ b/internal/cli/errmsg_test.go @@ -4,14 +4,18 @@ package cli import ( "bytes" "context" + "encoding/hex" "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,42 @@ 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. It is an Ed25519 +// key, which is quick to make. +func testSecretKey(t *testing.T, passphrase []byte) ([]byte, string) { + t.Helper() + + key, err := openpgp.NewEntity("MFER Test Key", "", "test@mfer.test", + &packet.Config{Algorithm: packet.PubKeyAlgoEdDSA}) + 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) 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 +192,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 +209,10 @@ func manifestSignedByAnotherKey( require.NoError(t, proto.Unmarshal( signedManifest(t, files)[len(mfer.MAGIC):], outer)) + // Armored blocks start on a line of their own; mfer, unlike gpg, ends + // one without a newline. outer.SigningPubKey = slices.Concat( - required.GetSigningPubKey(), outer.GetSigningPubKey()) + required.GetSigningPubKey(), []byte("\n"), outer.GetSigningPubKey()) 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 5e0e2e0..0c3fa38 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 @@ -433,7 +435,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 { @@ -450,6 +452,10 @@ func runFreshenHash( } } + if hasher.showProgress && hasher.filesToHash > 0 { + log.ProgressDone() + } + return nil } @@ -494,7 +500,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 @@ -528,7 +538,7 @@ func (mfa *CLIApp) freshenManifestOperation( totalHashBytes: totalHashBytes, filesToHash: filesToHash, startHash: time.Now(), - builder: newFreshenBuilder(cmd), + builder: builder, } err = runFreshenHash(ctx, hasher, scanner.entries) @@ -536,10 +546,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..298f370 --- /dev/null +++ b/internal/cli/signing.go @@ -0,0 +1,79 @@ +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. The passphrase of a protected key comes from +// MFER_SIGN_KEY_PASSPHRASE, or else from the terminal on stdin. +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 { + return opts, nil + } + + opts.Passphrase, err = mfa.readPassphrase(path) + if err != nil { + return nil, 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..39cb736 --- /dev/null +++ b/internal/cli/signing_test.go @@ -0,0 +1,91 @@ +//nolint:testpackage // white-box tests exercise unexported internals +package cli + +import ( + "path/filepath" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const testFlagSignKey = "--sign-key" + +// 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) + + 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")) + + fs := afero.NewMemMapFs() + require.NoError(t, afero.WriteFile(fs, "/key.asc", secretKey, 0o600)) + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "hello") + + opts := testOpts([]string{ + testApp, cmdGenerate, "-q", testFlagSignKey, "/key.asc", + "-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) +} 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..659363a --- /dev/null +++ b/mfer/openpgp.go @@ -0,0 +1,266 @@ +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, secret key and public key 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 + + // armorEnd starts the line that ends an armored block. + armorEnd = "-----END " +) + +var ( + errKeyCount = errors.New("must hold exactly one key") + errNoSecretKey = errors.New("signing key file holds no secret key") + errNoPassphrase = errors.New( + "signing key is protected and no passphrase was given") + errNotOneSignature = errors.New( + "signature must hold exactly one signature") +) + +// 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 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 +} + +// 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 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.PrivateKey == nil { + return nil, errNoSecretKey + } + + return key, nil +} + +// readOneKey returns the key in data, armored or binary, which must hold +// exactly one primary key. 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) + } + + 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 bodies of all its armored blocks, one after +// another. armor.Decode reads only the first block it finds, so dearmor +// decodes the text after each block's END line in turn, and a second key +// or signature after the first is read too. +func dearmor(data []byte) ([]byte, error) { + var binary []byte + + rest := data + + for { + block, err := armor.Decode(bytes.NewReader(rest)) + if errors.Is(err, io.EOF) { + break + } + + if err != nil { + return nil, err + } + + body, err := io.ReadAll(block.Body) + if err != nil { + return nil, err + } + + binary = append(binary, body...) + _, rest, _ = bytes.Cut(rest, []byte(armorEnd)) + } + + if binary == nil { + return data, nil + } + + return binary, 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++ + } + } +} diff --git a/mfer/openpgp_test.go b/mfer/openpgp_test.go new file mode 100644 index 0000000..06a1d83 --- /dev/null +++ b/mfer/openpgp_test.go @@ -0,0 +1,568 @@ +//nolint:testpackage // white-box tests exercise unexported internals +package mfer + +import ( + "bytes" + "context" + "io" + "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) +} + +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: in an +// armored block of its own, in the same armored block, 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{ + "armored block of its own": joinArmored( + armoredPublicKeys(t, other), armoredPublicKeys(t, signer)), + "same armored block": armoredPublicKeys(t, other, signer), + "secret key without user ID": joinArmored( + armoredPublicKeys(t, signer), armoredSecretKeys(t, 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{} + + manifest := rewriteOuter(t, signedTestManifest(t, testSigningOptions(t)), + func(outer *MFFileOuter) { + outer.SigningPubKey = joinArmored( + armoredPublicKeys(t, other), outer.GetSigningPubKey()) + }) + + _, err := NewManifestFromReader(bytes.NewReader(manifest)) + require.ErrorIs(t, err, errKeyCount) +} + +// TestManifestRefusesTwoSignatures loads a manifest whose signature field +// holds its good signature twice. Loading must refuse it. +func TestManifestRefusesTwoSignatures(t *testing.T) { + t.Parallel() + + manifest := rewriteOuter(t, signedTestManifest(t, testSigningOptions(t)), + func(outer *MFFileOuter) { + outer.Signature = joinArmored( + outer.GetSignature(), outer.GetSignature()) + }) + + _, err := NewManifestFromReader(bytes.NewReader(manifest)) + require.ErrorIs(t, err, errNotOneSignature) +} + +// 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 058c227..99e4086 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