Enforce real timeouts on gpg subprocess calls (closes #62)
check / check (push) Successful in 1m43s
check / check (push) Successful in 1m43s
Every gpg run now has a one-minute deadline (gpgTimeout) on top of its caller's context and is killed when either ends. A timeout is reported as "gpg timed out" under the failing operation instead of "signal: killed". Only gpg itself is killed; WaitDelay (one second) stops the run from waiting on a process gpg left behind that still holds its output, such as a wrapper script that does not exec the real gpg. Builder.Build and Checker.ExtractEmbeddedSigningKeyFP take a context, so ToManifest's context now reaches signing and the contextcheck suppression calling signing non-cancellable is gone. Manifest loading takes no context, so its signature check is bounded by the timeout alone. Model: opus-5-5
This commit is contained in:
@@ -2,6 +2,7 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/hex"
|
||||
"errors"
|
||||
"fmt"
|
||||
@@ -126,7 +127,9 @@ func (mfa *CLIApp) fetchManifestToTemp(url string) (string, error) {
|
||||
|
||||
// verifyRequiredSigner enforces the --require-signature fingerprint
|
||||
// against the manifest's embedded signing key.
|
||||
func verifyRequiredSigner(chk *mfer.Checker, requiredSigner string) error {
|
||||
func verifyRequiredSigner(
|
||||
ctx context.Context, chk *mfer.Checker, requiredSigner string,
|
||||
) error {
|
||||
// Validate fingerprint format: must be exactly 40 hex characters
|
||||
if len(requiredSigner) != fingerprintHexLen {
|
||||
return fmt.Errorf("%w, got %d", errInvalidFingerprint, len(requiredSigner))
|
||||
@@ -145,7 +148,7 @@ func verifyRequiredSigner(chk *mfer.Checker, requiredSigner string) error {
|
||||
// Extract fingerprint from the embedded public key (not from the
|
||||
// signer field). This validates the key is importable and gets its
|
||||
// actual fingerprint.
|
||||
embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP()
|
||||
embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP(ctx)
|
||||
if err != nil {
|
||||
return fmt.Errorf(
|
||||
"failed to extract fingerprint from embedded signing key: %w", err)
|
||||
@@ -303,7 +306,7 @@ func (mfa *CLIApp) checkManifestOperation(ctx *cli.Context) error {
|
||||
// Check signature requirement
|
||||
requiredSigner := ctx.String("require-signature")
|
||||
if requiredSigner != "" {
|
||||
err = verifyRequiredSigner(chk, requiredSigner)
|
||||
err = verifyRequiredSigner(ctx.Context, chk, requiredSigner)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -83,7 +83,8 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
|
||||
t.Run("invalid fingerprint length", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
err := verifyRequiredSigner(unsignedChecker(t), "12345678")
|
||||
err := verifyRequiredSigner(context.Background(),
|
||||
unsignedChecker(t), "12345678")
|
||||
require.ErrorIs(t, err, errInvalidFingerprint)
|
||||
assert.EqualError(t, err,
|
||||
"invalid fingerprint: must be exactly 40 hex characters, got 8")
|
||||
@@ -92,7 +93,8 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
|
||||
t.Run("manifest not signed", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
err := verifyRequiredSigner(unsignedChecker(t), msgFpA)
|
||||
err := verifyRequiredSigner(context.Background(),
|
||||
unsignedChecker(t), msgFpA)
|
||||
require.ErrorIs(t, err, errManifestNotSigned)
|
||||
assert.EqualError(t, err,
|
||||
"manifest is not signed, but signature from "+msgFpA+" is required")
|
||||
@@ -109,10 +111,10 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
|
||||
func TestSignerMismatchMessage(t *testing.T) {
|
||||
chk := signedChecker(t)
|
||||
|
||||
embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP()
|
||||
embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP(context.Background())
|
||||
require.NoError(t, err)
|
||||
|
||||
err = verifyRequiredSigner(chk, msgFpB)
|
||||
err = verifyRequiredSigner(context.Background(), chk, msgFpB)
|
||||
require.ErrorIs(t, err, errSignerMismatch)
|
||||
assert.EqualError(t, err,
|
||||
"embedded signing key fingerprint "+embeddedFP+
|
||||
@@ -160,7 +162,7 @@ func signedChecker(t *testing.T) *mfer.Checker {
|
||||
|
||||
var buf bytes.Buffer
|
||||
|
||||
require.NoError(t, b.Build(&buf))
|
||||
require.NoError(t, b.Build(context.Background(), &buf))
|
||||
|
||||
fs := afero.NewMemMapFs()
|
||||
require.NoError(t, afero.WriteFile(fs, "/index.mf", buf.Bytes(), 0o644))
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"errors"
|
||||
"fmt"
|
||||
@@ -304,7 +305,7 @@ func (h *freshenHasher) processEntry(e *freshenEntry) error {
|
||||
// writeFreshenedManifest writes the manifest atomically (write to a
|
||||
// temp file, then rename over the target).
|
||||
func writeFreshenedManifest(
|
||||
afs afero.Fs, builder *mfer.Builder, manifestPath string,
|
||||
ctx context.Context, afs afero.Fs, builder *mfer.Builder, manifestPath string,
|
||||
) error {
|
||||
tmpPath := manifestPath + ".tmp"
|
||||
|
||||
@@ -313,7 +314,7 @@ func writeFreshenedManifest(
|
||||
return fmt.Errorf("failed to create temp file: %w", err)
|
||||
}
|
||||
|
||||
err = builder.Build(outFile)
|
||||
err = builder.Build(ctx, outFile)
|
||||
_ = outFile.Close()
|
||||
|
||||
if err != nil {
|
||||
@@ -530,7 +531,7 @@ func (mfa *CLIApp) freshenManifestOperation(ctx *cli.Context) error {
|
||||
}
|
||||
|
||||
// Write updated manifest atomically (write to temp, then rename)
|
||||
err = writeFreshenedManifest(mfa.Fs, hasher.builder, manifestPath)
|
||||
err = writeFreshenedManifest(ctx.Context, mfa.Fs, hasher.builder, manifestPath)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
+6
-4
@@ -3,6 +3,7 @@
|
||||
package mfer
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"errors"
|
||||
"fmt"
|
||||
@@ -281,8 +282,9 @@ func (b *Builder) SetSigningOptions(opts *SigningOptions) {
|
||||
b.signingOptions = opts
|
||||
}
|
||||
|
||||
// Build finalizes the manifest and writes it to the writer.
|
||||
func (b *Builder) Build(w io.Writer) error {
|
||||
// Build finalizes the manifest and writes it to the writer. ctx bounds the
|
||||
// gpg runs that sign the manifest when signing options are set.
|
||||
func (b *Builder) Build(ctx context.Context, w io.Writer) error {
|
||||
b.mu.Lock()
|
||||
defer b.mu.Unlock()
|
||||
|
||||
@@ -308,13 +310,13 @@ func (b *Builder) Build(w io.Writer) error {
|
||||
}
|
||||
|
||||
// Generate outer wrapper
|
||||
err := m.generateOuter()
|
||||
err := m.generateOuter(ctx)
|
||||
if err != nil {
|
||||
return fmt.Errorf("build: generate outer: %w", err)
|
||||
}
|
||||
|
||||
// Generate final output
|
||||
err = m.generate()
|
||||
err = m.generate(ctx)
|
||||
if err != nil {
|
||||
return fmt.Errorf("build: generate: %w", err)
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ package mfer
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -113,7 +114,7 @@ func TestBuilderBuild(t *testing.T) {
|
||||
|
||||
var buf bytes.Buffer
|
||||
|
||||
err = b.Build(&buf)
|
||||
err = b.Build(context.Background(), &buf)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Should have magic bytes
|
||||
@@ -177,7 +178,7 @@ func TestBuilderDeterministicOutput(t *testing.T) {
|
||||
|
||||
var buf bytes.Buffer
|
||||
|
||||
err := b.Build(&buf)
|
||||
err := b.Build(context.Background(), &buf)
|
||||
require.NoError(t, err)
|
||||
|
||||
return buf.Bytes()
|
||||
@@ -325,7 +326,7 @@ func TestBuilderBuildRoundTrip(t *testing.T) {
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
require.NoError(t, b.Build(&buf))
|
||||
require.NoError(t, b.Build(context.Background(), &buf))
|
||||
|
||||
m, err := NewManifestFromReader(&buf)
|
||||
require.NoError(t, err)
|
||||
@@ -383,7 +384,7 @@ func TestManifestString(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
|
||||
var buf bytes.Buffer
|
||||
require.NoError(t, b.Build(&buf))
|
||||
require.NoError(t, b.Build(context.Background(), &buf))
|
||||
|
||||
m, err := NewManifestFromReader(&buf)
|
||||
require.NoError(t, err)
|
||||
@@ -397,7 +398,7 @@ func TestBuilderBuildEmpty(t *testing.T) {
|
||||
|
||||
var buf bytes.Buffer
|
||||
|
||||
err := b.Build(&buf)
|
||||
err := b.Build(context.Background(), &buf)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Should still produce valid manifest with 0 files
|
||||
@@ -416,7 +417,7 @@ func TestBuilderOmitsCreatedAtByDefault(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
|
||||
var buf bytes.Buffer
|
||||
require.NoError(t, b.Build(&buf))
|
||||
require.NoError(t, b.Build(context.Background(), &buf))
|
||||
|
||||
m, err := NewManifestFromReader(&buf)
|
||||
require.NoError(t, err)
|
||||
@@ -438,7 +439,7 @@ func TestBuilderIncludesCreatedAtWhenRequested(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
|
||||
var buf bytes.Buffer
|
||||
require.NoError(t, b.Build(&buf))
|
||||
require.NoError(t, b.Build(context.Background(), &buf))
|
||||
|
||||
m, err := NewManifestFromReader(&buf)
|
||||
require.NoError(t, err)
|
||||
@@ -464,7 +465,7 @@ func TestBuilderDeterministicFileOrder(t *testing.T) {
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
require.NoError(t, b.Build(&buf))
|
||||
require.NoError(t, b.Build(context.Background(), &buf))
|
||||
m, err := NewManifestFromReader(&buf)
|
||||
require.NoError(t, err)
|
||||
|
||||
|
||||
+2
-2
@@ -164,12 +164,12 @@ func (c *Checker) SigningPubKey() []byte {
|
||||
// ExtractEmbeddedSigningKeyFP imports the manifest's embedded public key into a
|
||||
// temporary keyring and extracts its fingerprint. This validates the key and
|
||||
// returns its actual fingerprint from the key material itself.
|
||||
func (c *Checker) ExtractEmbeddedSigningKeyFP() (string, error) {
|
||||
func (c *Checker) ExtractEmbeddedSigningKeyFP(ctx context.Context) (string, error) {
|
||||
if len(c.signingPubKey) == 0 {
|
||||
return "", errNoSigningPubKey
|
||||
}
|
||||
|
||||
return gpgExtractPubKeyFingerprint(c.signingPubKey)
|
||||
return gpgExtractPubKeyFingerprint(ctx, c.signingPubKey)
|
||||
}
|
||||
|
||||
// Check verifies all files against the manifest.
|
||||
|
||||
@@ -61,7 +61,7 @@ func createTestManifest(
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
require.NoError(t, builder.Build(&buf))
|
||||
require.NoError(t, builder.Build(context.Background(), &buf))
|
||||
require.NoError(t, afero.WriteFile(fs, manifestPath, buf.Bytes(), 0o644))
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ package mfer
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"errors"
|
||||
"fmt"
|
||||
@@ -92,7 +93,9 @@ func (m *manifest) verifyOuterIntegrity() error {
|
||||
)
|
||||
}
|
||||
|
||||
// Loading a manifest takes no context; gpgTimeout still bounds gpg.
|
||||
err = gpgVerify(
|
||||
context.Background(),
|
||||
[]byte(sigString),
|
||||
m.pbOuter.GetSignature(),
|
||||
m.pbOuter.GetSigningPubKey(),
|
||||
|
||||
@@ -3,6 +3,7 @@ package mfer
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"fmt"
|
||||
"testing"
|
||||
@@ -122,7 +123,7 @@ func TestDeserializeValidManifestRoundTrips(t *testing.T) {
|
||||
require.NoError(t, b.AddFileWithHash("dir/file.txt", 123, ModTime{}, hash))
|
||||
|
||||
var buf bytes.Buffer
|
||||
require.NoError(t, b.Build(&buf))
|
||||
require.NoError(t, b.Build(context.Background(), &buf))
|
||||
|
||||
m, err := NewManifestFromReader(bytes.NewReader(buf.Bytes()))
|
||||
require.NoError(t, err)
|
||||
|
||||
+4
-2
@@ -2,6 +2,7 @@
|
||||
package mfer
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -80,6 +81,7 @@ func TestSerializeInternalErrorMessagesVerbatim(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m := &manifest{}
|
||||
require.EqualError(t, m.generate(), "internal error: pbInner not set")
|
||||
require.EqualError(t, m.generateOuter(), "internal error")
|
||||
require.EqualError(t, m.generate(context.Background()),
|
||||
"internal error: pbInner not set")
|
||||
require.EqualError(t, m.generateOuter(context.Background()), "internal error")
|
||||
}
|
||||
|
||||
+48
-15
@@ -10,9 +10,21 @@ import (
|
||||
"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
|
||||
@@ -66,8 +78,20 @@ func gpgArgs(opts []string, positional ...string) []string {
|
||||
}
|
||||
|
||||
// runGPG runs the gpg binary in batch mode with the given arguments and
|
||||
// optional stdin, returning captured stdout and stderr.
|
||||
func runGPG(stdin io.Reader, args ...string) (*bytes.Buffer, *bytes.Buffer, error) {
|
||||
// optional stdin, returning captured stdout and 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
|
||||
@@ -76,7 +100,8 @@ func runGPG(stdin io.Reader, args ...string) (*bytes.Buffer, *bytes.Buffer, erro
|
||||
// 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
|
||||
context.Background(), "gpg", fullArgs...)
|
||||
ctx, "gpg", fullArgs...)
|
||||
cmd.WaitDelay = gpgWaitDelay
|
||||
cmd.Stdin = stdin
|
||||
|
||||
var stdout, stderr bytes.Buffer
|
||||
@@ -85,6 +110,14 @@ func runGPG(stdin io.Reader, args ...string) (*bytes.Buffer, *bytes.Buffer, erro
|
||||
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("gpg timed out: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
return &stdout, &stderr, err
|
||||
}
|
||||
@@ -105,8 +138,8 @@ func parseFingerprint(colonOutput string) (string, bool) {
|
||||
|
||||
// gpgSign creates a detached signature of the data using the specified key.
|
||||
// Returns the armored detached signature.
|
||||
func gpgSign(data []byte, keyID GPGKeyID) ([]byte, error) {
|
||||
stdout, stderr, err := runGPG(bytes.NewReader(data),
|
||||
func gpgSign(ctx context.Context, data []byte, keyID GPGKeyID) ([]byte, error) {
|
||||
stdout, stderr, err := runGPG(ctx, bytes.NewReader(data),
|
||||
"--detach-sign",
|
||||
gpgOptArmor,
|
||||
"--local-user", string(keyID),
|
||||
@@ -120,8 +153,8 @@ func gpgSign(data []byte, keyID GPGKeyID) ([]byte, error) {
|
||||
|
||||
// gpgExportPublicKey exports the public key for the specified key ID.
|
||||
// Returns the armored public key.
|
||||
func gpgExportPublicKey(keyID GPGKeyID) ([]byte, error) {
|
||||
stdout, stderr, err := runGPG(nil,
|
||||
func gpgExportPublicKey(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
|
||||
stdout, stderr, err := runGPG(ctx, nil,
|
||||
gpgArgs([]string{"--export", gpgOptArmor}, string(keyID))...,
|
||||
)
|
||||
if err != nil {
|
||||
@@ -136,8 +169,8 @@ func gpgExportPublicKey(keyID GPGKeyID) ([]byte, error) {
|
||||
}
|
||||
|
||||
// gpgGetKeyFingerprint gets the full fingerprint for a key ID.
|
||||
func gpgGetKeyFingerprint(keyID GPGKeyID) ([]byte, error) {
|
||||
stdout, stderr, err := runGPG(nil,
|
||||
func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
|
||||
stdout, stderr, err := runGPG(ctx, nil,
|
||||
gpgArgs([]string{"--with-colons", "--fingerprint"}, string(keyID))...,
|
||||
)
|
||||
if err != nil {
|
||||
@@ -157,7 +190,7 @@ func gpgGetKeyFingerprint(keyID GPGKeyID) ([]byte, error) {
|
||||
// gpgExtractPubKeyFingerprint imports a public key into a temporary keyring
|
||||
// and extracts its fingerprint. This verifies the key is valid and returns
|
||||
// the actual fingerprint from the key material.
|
||||
func gpgExtractPubKeyFingerprint(pubKey []byte) (string, error) {
|
||||
func gpgExtractPubKeyFingerprint(ctx context.Context, pubKey []byte) (string, error) {
|
||||
// Create temporary directory for GPG operations
|
||||
tmpDir, err := os.MkdirTemp("", "mfer-gpg-fingerprint-*")
|
||||
if err != nil {
|
||||
@@ -181,7 +214,7 @@ func gpgExtractPubKeyFingerprint(pubKey []byte) (string, error) {
|
||||
}
|
||||
|
||||
// Import the public key into the temporary keyring
|
||||
_, importStderr, err := runGPG(nil,
|
||||
_, importStderr, err := runGPG(ctx, nil,
|
||||
gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)...,
|
||||
)
|
||||
if err != nil {
|
||||
@@ -191,7 +224,7 @@ func gpgExtractPubKeyFingerprint(pubKey []byte) (string, error) {
|
||||
}
|
||||
|
||||
// List keys to get fingerprint
|
||||
listStdout, listStderr, err := runGPG(nil,
|
||||
listStdout, listStderr, err := runGPG(ctx, nil,
|
||||
"--homedir", tmpDir,
|
||||
"--with-colons",
|
||||
"--fingerprint",
|
||||
@@ -212,7 +245,7 @@ func gpgExtractPubKeyFingerprint(pubKey []byte) (string, error) {
|
||||
|
||||
// gpgVerify verifies a detached signature against data using the provided public key.
|
||||
// It creates a temporary keyring to import the public key for verification.
|
||||
func gpgVerify(data, signature, pubKey []byte) error {
|
||||
func gpgVerify(ctx context.Context, data, signature, pubKey []byte) error {
|
||||
// Create temporary directory for GPG operations
|
||||
tmpDir, err := os.MkdirTemp("", "mfer-gpg-verify-*")
|
||||
if err != nil {
|
||||
@@ -252,7 +285,7 @@ func gpgVerify(data, signature, pubKey []byte) error {
|
||||
}
|
||||
|
||||
// Import the public key into the temporary keyring
|
||||
_, importStderr, err := runGPG(nil,
|
||||
_, importStderr, err := runGPG(ctx, nil,
|
||||
gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)...,
|
||||
)
|
||||
if err != nil {
|
||||
@@ -262,7 +295,7 @@ func gpgVerify(data, signature, pubKey []byte) error {
|
||||
}
|
||||
|
||||
// Verify the signature
|
||||
_, verifyStderr, err := runGPG(nil,
|
||||
_, verifyStderr, err := runGPG(ctx, nil,
|
||||
gpgArgs([]string{gpgOptHomedir, tmpDir, gpgOptVerify},
|
||||
sigFile, dataFile)...,
|
||||
)
|
||||
|
||||
+100
-20
@@ -4,11 +4,14 @@ package mfer
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"io"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/spf13/afero"
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -43,8 +46,11 @@ Expire-Date: 0
|
||||
paramsFile := filepath.Join(gpgHome, "key-params")
|
||||
require.NoError(t, os.WriteFile(paramsFile, []byte(keyParams), 0o600))
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), gpgTimeout)
|
||||
defer cancel()
|
||||
|
||||
//nolint:gosec // paramsFile is a test-controlled path inside t.TempDir()
|
||||
cmd := exec.CommandContext(context.Background(), "gpg",
|
||||
cmd := exec.CommandContext(ctx, "gpg",
|
||||
"--batch", "--gen-key", paramsFile)
|
||||
|
||||
cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome)
|
||||
@@ -55,7 +61,7 @@ Expire-Date: 0
|
||||
}
|
||||
|
||||
// Get the key fingerprint
|
||||
cmd = exec.CommandContext(context.Background(), "gpg",
|
||||
cmd = exec.CommandContext(ctx, "gpg",
|
||||
"--list-keys", "--with-colons", "test@mfer.test")
|
||||
|
||||
cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome)
|
||||
@@ -90,7 +96,7 @@ func TestGPGSign(t *testing.T) {
|
||||
t.Setenv("GNUPGHOME", gpgHome)
|
||||
|
||||
data := []byte("test data to sign")
|
||||
sig, err := gpgSign(data, keyID)
|
||||
sig, err := gpgSign(context.Background(), data, keyID)
|
||||
require.NoError(t, err)
|
||||
assert.NotEmpty(t, sig)
|
||||
assert.Contains(t, string(sig), "-----BEGIN PGP SIGNATURE-----")
|
||||
@@ -101,7 +107,7 @@ func TestGPGExportPublicKey(t *testing.T) {
|
||||
keyID, gpgHome := testGPGEnv(t)
|
||||
t.Setenv("GNUPGHOME", gpgHome)
|
||||
|
||||
pubKey, err := gpgExportPublicKey(keyID)
|
||||
pubKey, err := gpgExportPublicKey(context.Background(), keyID)
|
||||
require.NoError(t, err)
|
||||
assert.NotEmpty(t, pubKey)
|
||||
assert.Contains(t, string(pubKey), "-----BEGIN PGP PUBLIC KEY BLOCK-----")
|
||||
@@ -112,7 +118,7 @@ func TestGPGGetKeyFingerprint(t *testing.T) {
|
||||
keyID, gpgHome := testGPGEnv(t)
|
||||
t.Setenv("GNUPGHOME", gpgHome)
|
||||
|
||||
fingerprint, err := gpgGetKeyFingerprint(keyID)
|
||||
fingerprint, err := gpgGetKeyFingerprint(context.Background(), keyID)
|
||||
require.NoError(t, err)
|
||||
assert.NotEmpty(t, fingerprint)
|
||||
// The fingerprint should be 40 hex chars
|
||||
@@ -146,12 +152,12 @@ func TestGPGOptionLikeKeyIDIsNotAnOption(t *testing.T) {
|
||||
_, gpgHome := testGPGEnv(t)
|
||||
t.Setenv("GNUPGHOME", gpgHome)
|
||||
|
||||
pubKey, err := gpgExportPublicKey(GPGKeyID("--version"))
|
||||
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(GPGKeyID("--version"))
|
||||
fpr, err := gpgGetKeyFingerprint(context.Background(), GPGKeyID("--version"))
|
||||
require.Error(t, err)
|
||||
assert.NotContains(t, string(fpr), "gpg (GnuPG)")
|
||||
}
|
||||
@@ -162,7 +168,8 @@ func TestGPGSignInvalidKey(t *testing.T) {
|
||||
t.Setenv("GNUPGHOME", gpgHome)
|
||||
|
||||
data := []byte("test data")
|
||||
_, err := gpgSign(data, GPGKeyID("NONEXISTENT_KEY_ID_12345"))
|
||||
_, err := gpgSign(context.Background(), data,
|
||||
GPGKeyID("NONEXISTENT_KEY_ID_12345"))
|
||||
assert.Error(t, err)
|
||||
}
|
||||
|
||||
@@ -185,7 +192,7 @@ func TestBuilderWithSigning(t *testing.T) {
|
||||
// Build the manifest
|
||||
var buf bytes.Buffer
|
||||
|
||||
err = b.Build(&buf)
|
||||
err = b.Build(context.Background(), &buf)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Parse the manifest and verify signature fields are populated
|
||||
@@ -251,14 +258,14 @@ func TestGPGVerify(t *testing.T) {
|
||||
t.Setenv("GNUPGHOME", gpgHome)
|
||||
|
||||
data := []byte("test data to sign and verify")
|
||||
sig, err := gpgSign(data, keyID)
|
||||
sig, err := gpgSign(context.Background(), data, keyID)
|
||||
require.NoError(t, err)
|
||||
|
||||
pubKey, err := gpgExportPublicKey(keyID)
|
||||
pubKey, err := gpgExportPublicKey(context.Background(), keyID)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify the signature
|
||||
err = gpgVerify(data, sig, pubKey)
|
||||
err = gpgVerify(context.Background(), data, sig, pubKey)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
@@ -267,15 +274,15 @@ func TestGPGVerifyInvalidSignature(t *testing.T) {
|
||||
t.Setenv("GNUPGHOME", gpgHome)
|
||||
|
||||
data := []byte("test data to sign")
|
||||
sig, err := gpgSign(data, keyID)
|
||||
sig, err := gpgSign(context.Background(), data, keyID)
|
||||
require.NoError(t, err)
|
||||
|
||||
pubKey, err := gpgExportPublicKey(keyID)
|
||||
pubKey, err := gpgExportPublicKey(context.Background(), keyID)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Try to verify with different data - should fail
|
||||
wrongData := []byte("different data")
|
||||
err = gpgVerify(wrongData, sig, pubKey)
|
||||
err = gpgVerify(context.Background(), wrongData, sig, pubKey)
|
||||
assert.Error(t, err)
|
||||
}
|
||||
|
||||
@@ -284,12 +291,12 @@ func TestGPGVerifyBadPublicKey(t *testing.T) {
|
||||
t.Setenv("GNUPGHOME", gpgHome)
|
||||
|
||||
data := []byte("test data")
|
||||
sig, err := gpgSign(data, keyID)
|
||||
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(data, sig, badPubKey)
|
||||
err = gpgVerify(context.Background(), data, sig, badPubKey)
|
||||
assert.Error(t, err)
|
||||
}
|
||||
|
||||
@@ -312,7 +319,7 @@ func TestManifestSignatureVerification(t *testing.T) {
|
||||
// Build the manifest
|
||||
var buf bytes.Buffer
|
||||
|
||||
err = b.Build(&buf)
|
||||
err = b.Build(context.Background(), &buf)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Parse the manifest - signature should be verified during load
|
||||
@@ -341,7 +348,7 @@ func TestManifestTamperedSignatureFails(t *testing.T) {
|
||||
|
||||
var buf bytes.Buffer
|
||||
|
||||
err = b.Build(&buf)
|
||||
err = b.Build(context.Background(), &buf)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Tamper with the signature by replacing some bytes
|
||||
@@ -375,7 +382,7 @@ func TestBuilderWithoutSigning(t *testing.T) {
|
||||
// Build the manifest
|
||||
var buf bytes.Buffer
|
||||
|
||||
err = b.Build(&buf)
|
||||
err = b.Build(context.Background(), &buf)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Parse the manifest and verify signature fields are empty
|
||||
@@ -390,3 +397,76 @@ func TestBuilderWithoutSigning(t *testing.T) {
|
||||
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.
|
||||
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.Contains(t, err.Error(), "gpg sign failed: gpg timed out")
|
||||
}
|
||||
|
||||
// TestGPGTimeoutWhenChildHoldsOutput 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 shortly after the deadline
|
||||
// instead of waiting for sleep to exit. The fake gpg writes the process ID
|
||||
// of sleep to a file so that the test can kill it before returning.
|
||||
func TestGPGTimeoutWhenChildHoldsOutput(t *testing.T) {
|
||||
pidFile := filepath.Join(t.TempDir(), "sleep.pid")
|
||||
t.Setenv("PATH", fakeGPGPath(t,
|
||||
"#!/bin/sh\nsleep 3 &\necho $! >'"+pidFile+"'\nwait\n"))
|
||||
t.Cleanup(func() {
|
||||
pid, err := os.ReadFile(pidFile) //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)
|
||||
require.NoError(t, sleep.Kill())
|
||||
})
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
|
||||
defer cancel()
|
||||
|
||||
start := time.Now()
|
||||
_, err := gpgSign(ctx, []byte("data"), GPGKeyID("any"))
|
||||
require.ErrorIs(t, err, context.DeadlineExceeded)
|
||||
assert.Less(t, time.Since(start), 3*time.Second,
|
||||
"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)
|
||||
}
|
||||
|
||||
+1
-2
@@ -283,8 +283,7 @@ func (s *Scanner) ToManifest(
|
||||
}
|
||||
|
||||
// Build and write manifest
|
||||
//nolint:contextcheck // Build's GPG signing exec is not cancellable by design
|
||||
return builder.Build(w)
|
||||
return builder.Build(ctx, w)
|
||||
}
|
||||
|
||||
// configureBuilder constructs a manifest builder configured from the
|
||||
|
||||
+9
-8
@@ -2,6 +2,7 @@ package mfer
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"errors"
|
||||
"fmt"
|
||||
@@ -50,13 +51,13 @@ func newTimestampFromTime(t time.Time) *Timestamp {
|
||||
}
|
||||
}
|
||||
|
||||
func (m *manifest) generate() error {
|
||||
func (m *manifest) generate(ctx context.Context) error {
|
||||
if m.pbInner == nil {
|
||||
return errInnerNotSet
|
||||
}
|
||||
|
||||
if m.pbOuter == nil {
|
||||
e := m.generateOuter()
|
||||
e := m.generateOuter(ctx)
|
||||
if e != nil {
|
||||
return e
|
||||
}
|
||||
@@ -77,7 +78,7 @@ func (m *manifest) generate() error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *manifest) generateOuter() error {
|
||||
func (m *manifest) generateOuter(ctx context.Context) error {
|
||||
if m.pbInner == nil {
|
||||
return errInternal
|
||||
}
|
||||
@@ -135,7 +136,7 @@ func (m *manifest) generateOuter() error {
|
||||
|
||||
// Sign the manifest if signing options are provided
|
||||
if m.signingOptions != nil && m.signingOptions.KeyID != "" {
|
||||
return m.signOuter()
|
||||
return m.signOuter(ctx)
|
||||
}
|
||||
|
||||
return nil
|
||||
@@ -143,27 +144,27 @@ func (m *manifest) generateOuter() error {
|
||||
|
||||
// signOuter signs the outer message with the configured GPG key and
|
||||
// embeds the signature, signer fingerprint, and public key.
|
||||
func (m *manifest) signOuter() error {
|
||||
func (m *manifest) signOuter(ctx context.Context) error {
|
||||
sigString, err := m.signatureString()
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to generate signature string: %w", err)
|
||||
}
|
||||
|
||||
sig, err := gpgSign([]byte(sigString), m.signingOptions.KeyID)
|
||||
sig, err := gpgSign(ctx, []byte(sigString), m.signingOptions.KeyID)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to sign manifest: %w", err)
|
||||
}
|
||||
|
||||
m.pbOuter.Signature = sig
|
||||
|
||||
fingerprint, err := gpgGetKeyFingerprint(m.signingOptions.KeyID)
|
||||
fingerprint, err := gpgGetKeyFingerprint(ctx, m.signingOptions.KeyID)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to get key fingerprint: %w", err)
|
||||
}
|
||||
|
||||
m.pbOuter.Signer = fingerprint
|
||||
|
||||
pubKey, err := gpgExportPublicKey(m.signingOptions.KeyID)
|
||||
pubKey, err := gpgExportPublicKey(ctx, m.signingOptions.KeyID)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to export public key: %w", err)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user