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, and is
checked before any file is read. 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. An armored key or signature must be one block and
nothing else.

Model: opus-5-5

go.mod

@@ -4,6 +4,7 @@ go 1.27.1
 
 require (
 	github.com/ProtonMail/go-crypto v1.5.2
+	github.com/creack/pty v1.1.25-0.20260601142114-9246436fffe8
 	github.com/davecgh/go-spew v1.1.1
 	github.com/dustin/go-humanize v1.1.0
 	github.com/klauspost/compress v1.20.1

go.sum

@@ -2,6 +2,8 @@ github.com/ProtonMail/go-crypto v1.5.2 h1:cucYnvqcY7UOXVD//mSyjeaPY0SSN3v5cDkYPx
 github.com/ProtonMail/go-crypto v1.5.2/go.mod h1:/RaSu30DaKO4RY+XdV/ACcCcZkGr7AhUIduq5sjzzCo=
 github.com/cloudflare/circl v1.6.3 h1:9GPOhQGF9MCYUeXyMYlqTR6a5gTrgR/fBLXvUgtVcg8=
 github.com/cloudflare/circl v1.6.3/go.mod h1:2eXP6Qfat4O/Yhh8BznvKnJ+uzEoTQ6jVKJRn81BiS4=
+github.com/creack/pty v1.1.25-0.20260601142114-9246436fffe8 h1:CY3gjC7naqYGLMiywvj3suPfa1i0p/QEr7o8ujxL/2M=
+github.com/creack/pty v1.1.25-0.20260601142114-9246436fffe8/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/dustin/go-humanize v1.1.0 h1:dbKTrvD0klcbBV/h4AWJdMuZogJACoMlvWIWZ5b2xWg=

internal/cli/errmsg_test.go

@@ -5,11 +5,11 @@ import (
 	"bytes"
 	"context"
 	"encoding/hex"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"path/filepath"
-	"slices"
 	"strings"
 	"testing"
 
@@ -209,10 +209,26 @@ 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(), []byte("\n"), outer.GetSigningPubKey())
+	// One armored block holding both keys, as gpg --export --armor writes
+	// two keys.
+	var block bytes.Buffer
+
+	w, err := armor.Encode(&block, openpgp.PublicKeyType, nil)
+	require.NoError(t, err)
+
+	for _, key := range [][]byte{
+		required.GetSigningPubKey(), outer.GetSigningPubKey(),
+	} {
+		decoded, err := armor.Decode(bytes.NewReader(key))
+		require.NoError(t, err)
+
+		_, err = io.Copy(w, decoded.Body)
+		require.NoError(t, err)
+	}
+
+	require.NoError(t, w.Close())
+
+	outer.SigningPubKey = block.Bytes()
 	outer.Signer = required.GetSigner()
 
 	data, err := proto.Marshal(outer)

internal/cli/signing.go

@@ -24,7 +24,8 @@ var errNoPassphrase = errors.New(
 
 // 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.
+// MFER_SIGN_KEY_PASSPHRASE, or else from the terminal on stdin, and must
+// unlock the key.
 func (mfa *CLIApp) signingOptions(path string) (*mfer.SigningOptions, error) {
 	secretKey, err := afero.ReadFile(mfa.Fs, path)
 	if err != nil {
@@ -48,6 +49,13 @@ func (mfa *CLIApp) signingOptions(path string) (*mfer.SigningOptions, error) {
 		return nil, err
 	}
 
+	// gen and freshen read the signing options before any file, so a
+	// wrong passphrase stops them before they hash anything.
+	err = mfer.CheckSigningKey(opts)
+	if err != nil {
+		return nil, fmt.Errorf("%s: %w", path, err)
+	}
+
 	return opts, nil
 }
 

internal/cli/signing_test.go

@@ -2,15 +2,22 @@
 package cli
 
 import (
+	"bufio"
+	"io"
 	"path/filepath"
+	"strings"
 	"testing"
 
+	"github.com/creack/pty"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-const testFlagSignKey = "--sign-key"
+const (
+	testFlagSignKey = "--sign-key"
+	testKeyFile     = "/key.asc"
+)
 
 // TestGenAndFreshenSignWithKeyFile runs gen, then freshen after a file is
 // added, with --sign-key naming a key file: one key with no passphrase and
@@ -73,12 +80,12 @@ func TestSignWithProtectedKeyNeedsPassphrase(t *testing.T) {
 	secretKey, _ := testSecretKey(t, []byte("secret"))
 
 	fs := afero.NewMemMapFs()
-	require.NoError(t, afero.WriteFile(fs, "/key.asc", secretKey, 0o600))
+	require.NoError(t, afero.WriteFile(fs, testKeyFile, secretKey, 0o600))
 	require.NoError(t, fs.MkdirAll(testDir, 0o755))
 	writeTestFile(t, fs, testFile1, "hello")
 
 	opts := testOpts([]string{
-		testApp, cmdGenerate, "-q", testFlagSignKey, "/key.asc",
+		testApp, cmdGenerate, "-q", testFlagSignKey, testKeyFile,
 		"-o", testMF, testDir,
 	}, fs)
 	assert.Equal(t, 1, runCLI(opts))
@@ -89,3 +96,93 @@ func TestSignWithProtectedKeyNeedsPassphrase(t *testing.T) {
 	require.NoError(t, err)
 	assert.False(t, exists)
 }
+
+// TestSignWithWrongPassphraseFailsFirst runs gen on a directory and
+// freshen on a manifest, neither of which exists, with a protected key and
+// a wrong MFER_SIGN_KEY_PASSPHRASE. Each must fail to unlock the key: it
+// does that before it reads any file, so a missing file goes unnoticed.
+func TestSignWithWrongPassphraseFailsFirst(t *testing.T) {
+	t.Setenv(envSignKeyPassphrase, "wrong")
+
+	secretKey, _ := testSecretKey(t, []byte("right"))
+
+	fs := afero.NewMemMapFs()
+	require.NoError(t, afero.WriteFile(fs, testKeyFile, secretKey, 0o600))
+
+	for _, args := range [][]string{
+		{
+			testApp, cmdGenerate, "-q", testFlagSignKey, testKeyFile,
+			"-o", testMF, "/missing",
+		},
+		{testApp, cmdFreshen, "-q", testFlagSignKey, testKeyFile, "/missing.mf"},
+	} {
+		opts := testOpts(args, fs)
+		assert.Equal(t, 1, runCLI(opts), args[1])
+		assert.Contains(t, testStderr(t, opts),
+			testKeyFile+": unlock signing key", args[1])
+	}
+}
+
+// TestGenAsksForPassphraseOnTerminal runs gen with a protected key, no
+// MFER_SIGN_KEY_PASSPHRASE, and a terminal as stdin and stderr. gen must
+// ask for the passphrase on stderr, and sign with what is typed after the
+// prompt.
+func TestGenAsksForPassphraseOnTerminal(t *testing.T) {
+	t.Setenv(envSignKeyPassphrase, "")
+
+	secretKey, fingerprint := testSecretKey(t, []byte("passphrase"))
+
+	fs := afero.NewOsFs()
+	keyFile := filepath.Join(t.TempDir(), "key.asc")
+	root := t.TempDir()
+	manifestPath := filepath.Join(root, defaultManifestName)
+
+	require.NoError(t, afero.WriteFile(fs, keyFile, secretKey, 0o600))
+	writeTestFile(t, fs, filepath.Join(root, testFileTxt), "hello")
+
+	terminal, tty, err := pty.Open()
+	require.NoError(t, err)
+
+	t.Cleanup(func() { _ = terminal.Close() })
+
+	opts := testOpts([]string{
+		testApp, cmdGenerate, "-q", testFlagSignKey, keyFile,
+		"-o", manifestPath, root,
+	}, fs)
+	opts.Stdin = tty
+	opts.Stderr = tty
+
+	exitCode := make(chan int, 1)
+
+	go func() {
+		exitCode <- runCLI(opts)
+
+		// Once gen has ended, reading the terminal fails instead of
+		// waiting for a prompt that will not come.
+		_ = tty.Close()
+	}()
+
+	prompt := "Passphrase for " + keyFile + ": "
+	output := bufio.NewReader(terminal)
+	written := ""
+
+	for !strings.HasSuffix(written, prompt) {
+		b, err := output.ReadByte()
+		require.NoError(t, err, "gen wrote %q and no prompt", written)
+
+		written += string(b)
+	}
+
+	_, err = terminal.WriteString("passphrase\n")
+	require.NoError(t, err)
+
+	code := <-exitCode
+	rest, _ := io.ReadAll(output)
+	require.Equal(t, 0, code, "gen wrote %q", rest)
+
+	check := testOpts([]string{
+		testApp, cmdCheck, "-q",
+		"--" + flagRequireSignature, fingerprint, manifestPath,
+	}, fs)
+	require.Equal(t, 0, runCLI(check), testStderr(t, check))
+}

mfer/openpgp.go

@@ -23,8 +23,10 @@ const (
 	secretKeyPacketTag = 5
 	publicKeyPacketTag = 6
 
-	// armorEnd starts the line that ends an armored block.
-	armorEnd = "-----END "
+	// armorBegin and armorEnd start the lines that begin and end an
+	// armored block.
+	armorBegin = "-----BEGIN "
+	armorEnd   = "-----END "
 )
 
 var (
@@ -34,6 +36,8 @@ var (
 		"signing key is protected and no passphrase was given")
 	errNotOneSignature = errors.New(
 		"signature must hold exactly one signature")
+	errNotOneArmoredBlock = errors.New(
+		"must be exactly one armored block and nothing else")
 )
 
 // SigningOptions holds the key a manifest is signed with.
@@ -57,6 +61,16 @@ func SecretKeyIsProtected(secretKey []byte) (bool, error) {
 	return isProtected(key), nil
 }
 
+// CheckSigningKey fails unless opts can sign: opts.SecretKey must hold one
+// primary key with its secret key, and opts.Passphrase must unlock it when
+// it is protected. It lets a caller find a wrong passphrase before it
+// builds a manifest.
+func CheckSigningKey(opts *SigningOptions) error {
+	_, err := readSigningKey(opts)
+
+	return err
+}
+
 // readSigningKey returns the key in opts.SecretKey, unlocked with
 // opts.Passphrase if it is protected.
 func readSigningKey(opts *SigningOptions) (*openpgp.Entity, error) {
@@ -206,39 +220,32 @@ func verifySignature(data, signature, pubKey []byte) (string, error) {
 }
 
 // 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.
+// not armored, or else the body of its armored block. Armored data must be
+// one block and nothing else: its first line is the only BEGIN line and
+// its last line the only END line, white space around them aside.
+// armor.Decode skips any text before a BEGIN line and reads only the first
+// block, so without this a second key or signature would go unseen.
 func dearmor(data []byte) ([]byte, error) {
-	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 {
+	if !bytes.Contains(data, []byte(armorBegin)) {
 		return data, nil
 	}
 
-	return binary, nil
+	text := bytes.TrimSpace(data)
+	lastLine := text[bytes.LastIndexByte(text, '\n')+1:]
+
+	if !bytes.HasPrefix(text, []byte(armorBegin)) ||
+		bytes.Count(text, []byte(armorBegin)) != 1 ||
+		!bytes.HasPrefix(lastLine, []byte(armorEnd)) ||
+		bytes.Count(text, []byte(armorEnd)) != 1 {
+		return nil, errNotOneArmoredBlock
+	}
+
+	block, err := armor.Decode(bytes.NewReader(text))
+	if err != nil {
+		return nil, err
+	}
+
+	return io.ReadAll(block.Body)
 }
 
 // countPackets returns how many packets in the binary OpenPGP data have

mfer/openpgp_test.go

@@ -403,12 +403,12 @@ func TestManifestSignedByGPGLoads(t *testing.T) {
 }
 
 // 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.
+// public key block holds another key besides the key that signed it: as a
+// public key, as a secret key with no user ID, which openpgp.ReadKeyRing
+// skips, and written as a subkey packet at the start of the block, which
+// openpgp.ReadKeyRing reads as a primary key. Loading must refuse each,
+// although the signature is good and the signer field names the key that
+// made it.
 func TestManifestRefusesSecondEmbeddedKey(t *testing.T) {
 	t.Parallel()
 
@@ -424,12 +424,9 @@ func TestManifestRefusesSecondEmbeddedKey(t *testing.T) {
 	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),
+		"public key":                 armoredPublicKeys(t, other, signer),
+		"secret key without user ID": armoredSecretKeys(t, signer, otherWithoutUserID),
+		"subkey packet first":        armoredPublicKeys(t, otherAsSubkey, signer),
 	} {
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
@@ -454,10 +451,12 @@ func TestManifestRefusesSecondEmbeddedKeyWithoutUserID(t *testing.T) {
 	other := newTestKey(t, nil)
 	other.Identities = map[string]*openpgp.Identity{}
 
-	manifest := rewriteOuter(t, signedTestManifest(t, testSigningOptions(t)),
+	signer := newTestKey(t, nil)
+
+	manifest := rewriteOuter(t, signedTestManifest(t,
+		&SigningOptions{SecretKey: armoredSecretKeys(t, signer)}),
 		func(outer *MFFileOuter) {
-			outer.SigningPubKey = joinArmored(
-				armoredPublicKeys(t, other), outer.GetSigningPubKey())
+			outer.SigningPubKey = armoredPublicKeys(t, other, signer)
 		})
 
 	_, err := NewManifestFromReader(bytes.NewReader(manifest))
@@ -465,20 +464,63 @@ func TestManifestRefusesSecondEmbeddedKeyWithoutUserID(t *testing.T) {
 }
 
 // TestManifestRefusesTwoSignatures loads a manifest whose signature field
-// holds its good signature twice. Loading must refuse it.
+// holds its good signature twice, not armored. Loading must refuse it.
 func TestManifestRefusesTwoSignatures(t *testing.T) {
 	t.Parallel()
 
 	manifest := rewriteOuter(t, signedTestManifest(t, testSigningOptions(t)),
 		func(outer *MFFileOuter) {
-			outer.Signature = joinArmored(
-				outer.GetSignature(), outer.GetSignature())
+			sig, err := dearmor(outer.GetSignature())
+			require.NoError(t, err)
+
+			outer.Signature = slices.Concat(sig, sig)
 		})
 
 	_, err := NewManifestFromReader(bytes.NewReader(manifest))
 	require.ErrorIs(t, err, errNotOneSignature)
 }
 
+// TestManifestRefusesFieldNotOneArmoredBlock loads manifests whose
+// signature or embedded public key block holds its good armored block with
+// something else: a second armored block, text after the END line, or many
+// END lines before the block. Decoding the block once for each END line
+// before it would take time and memory that grow with the square of the
+// field's size. Loading must refuse each.
+func TestManifestRefusesFieldNotOneArmoredBlock(t *testing.T) {
+	t.Parallel()
+
+	manifest := signedTestManifest(t, testSigningOptions(t))
+
+	for name, change := range map[string]func([]byte) []byte{
+		"second armored block": func(block []byte) []byte {
+			return joinArmored(block, block)
+		},
+		"text after the END line": func(block []byte) []byte {
+			return slices.Concat(block, []byte("\nmore text\n"))
+		},
+		"END lines before the block": func(block []byte) []byte {
+			return slices.Concat(
+				[]byte(strings.Repeat(armorEnd+"\n", 1000)), block)
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			for _, changed := range [][]byte{
+				rewriteOuter(t, manifest, func(outer *MFFileOuter) {
+					outer.Signature = change(outer.GetSignature())
+				}),
+				rewriteOuter(t, manifest, func(outer *MFFileOuter) {
+					outer.SigningPubKey = change(outer.GetSigningPubKey())
+				}),
+			} {
+				_, err := NewManifestFromReader(bytes.NewReader(changed))
+				require.ErrorIs(t, err, errNotOneArmoredBlock)
+			}
+		})
+	}
+}
+
 // TestManifestSignedWithSubkey signs with a key that has a signing subkey,
 // which signs in place of the primary key. The manifest must load, with
 // the primary key's fingerprint as signer.