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

@@ -302,10 +302,7 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues).
       binary, as `gpg --export-secret-keys` writes it; `MFER_SIGN_KEY` names the
       file too. mfer signs it itself and does not need `gpg`. A file holding
       more than one key is refused, and a key held only on a smartcard cannot
-      sign. The key must be a version 4 key, whose 40-character fingerprint is
-      what `--require-signature` takes, and not a DSA key. A key that cannot
-      sign, because it has expired or been revoked or its passphrase is wrong,
-      stops `gen` and `freshen` before they read any file
+      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/`

docs/FORMAT.md

@@ -152,10 +152,7 @@ primary key, `signature` is one good signature over the canonical string made by
 that key (or one of its subkeys), and `signer` is that key's fingerprint. The
 reference implementation refuses to load a manifest that fails these checks;
 `check` and `fetch` given `--require-signature` then compare the required
-fingerprint with `signer`. It also refuses a manifest whose `signingPubKey`
-holds a DSA key or subkey, or any secret key or subkey, since checking the
-self-signatures of a DSA key or the numbers of a secret key can take hours when
-those numbers are very large.
+fingerprint with `signer`.
 
 ## Deterministic Serialization
 

go.mod

@@ -4,7 +4,6 @@ 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,8 +2,6 @@ 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/check.go

@@ -21,8 +21,8 @@ import (
 	"sneak.berlin/go/mfer/mfer"
 )
 
-// fingerprintHexLen is the length in hex characters of the fingerprint of
-// an OpenPGP version 4 key, the only version mfer signs with.
+// fingerprintHexLen is the length of a full OpenPGP key fingerprint in hex
+// characters.
 const fingerprintHexLen = 40
 
 var (

internal/cli/entry_test.go

@@ -1368,51 +1368,6 @@ func TestGenerateSeveralPathsDefaultOutput(t *testing.T) {
 	}
 }
 
-// testLink is the name of the symlink to a tree that the
-// DirectoryNamedThroughSymlink tests make in a temp dir.
-const testLink = "link"
-
-// TestGenerateDirectoryNamedThroughSymlink runs gen on a directory named
-// through a symlink, given as the argument or as the working directory: the
-// manifest lists the files in the directory the symlink points to, and
-// leaves out a symlink inside it, as gen does without --follow-symlinks.
-//
-//nolint:paralleltest // changes the process-global working directory
-func TestGenerateDirectoryNamedThroughSymlink(t *testing.T) {
-	// Paths are relative to a temp dir holding data and link, a symlink to
-	// data.
-	for name, tc := range map[string]struct {
-		workDir string
-		args    []string
-	}{
-		"argument":          {".", []string{testLink}},
-		"working directory": {testLink, nil},
-	} {
-		t.Run(name, func(t *testing.T) {
-			root := t.TempDir()
-			data := filepath.Join(root, "data")
-
-			fs := afero.NewOsFs()
-			writeTestTree(t, fs, data)
-			require.NoError(t,
-				os.Symlink(testFileTxt, filepath.Join(data, "alias.txt")))
-			require.NoError(t, os.Symlink(data, filepath.Join(root, testLink)))
-
-			// t.Chdir sets PWD to the path it is given, as a shell does, and
-			// os.Getwd returns PWD when it names the working directory.
-			t.Chdir(filepath.Join(root, tc.workDir))
-
-			opts := testOpts(slices.Concat(
-				[]string{testApp, cmdGenerate, "-q"}, tc.args,
-			), fs)
-			require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
-
-			assert.ElementsMatch(t, []string{testFileTxt, "sub/nested.txt"},
-				manifestPaths(t, fs, filepath.Join(data, defaultManifestName)))
-		})
-	}
-}
-
 // TestGenerateBytesDoNotDependOnOutput runs gen --seed on one tree, each
 // time writing to another file, over a file already there and beside an
 // earlier run's temp file: neither is listed, and what is listed depends

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"
 
@@ -128,19 +128,13 @@ func TestSignerMismatchMessage(t *testing.T) {
 
 // testSecretKey returns a new OpenPGP key with its secret key, armored, as
 // gpg --export-secret-keys --armor writes it, and the key's fingerprint.
-// The key is protected by passphrase unless that is nil. config sets how
-// the key is made; without one it is an Ed25519 key, which is quick to
-// make.
-func testSecretKey(
-	t *testing.T, passphrase []byte, config *packet.Config,
-) ([]byte, string) {
+// 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()
 
-	if config == nil {
-		config = &packet.Config{Algorithm: packet.PubKeyAlgoEdDSA}
-	}
-
-	key, err := openpgp.NewEntity("MFER Test Key", "", "test@mfer.test", config)
+	key, err := openpgp.NewEntity("MFER Test Key", "", "test@mfer.test",
+		&packet.Config{Algorithm: packet.PubKeyAlgoEdDSA})
 	require.NoError(t, err)
 
 	if passphrase != nil {
@@ -161,7 +155,7 @@ func testSecretKey(
 func signedManifest(t *testing.T, files map[string][]byte) []byte {
 	t.Helper()
 
-	secretKey, _ := testSecretKey(t, nil, nil)
+	secretKey, _ := testSecretKey(t, nil)
 
 	b := mfer.NewBuilder()
 	b.SetSigningOptions(&mfer.SigningOptions{SecretKey: secretKey})
@@ -215,26 +209,10 @@ func manifestSignedByAnotherKey(
 	require.NoError(t, proto.Unmarshal(
 		signedManifest(t, files)[len(mfer.MAGIC):], outer))
 
-	// 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()
+	// 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())
 	outer.Signer = required.GetSigner()
 
 	data, err := proto.Marshal(outer)

internal/cli/freshen.go

@@ -381,14 +381,6 @@ func (mfa *CLIApp) freshenScan(
 		}
 	}
 
-	// The walk does not follow a symlink at its top, so a base directory
-	// named through one is resolved first. If that fails, the base is
-	// walked as named and the walk reports the problem.
-	resolved, err := filepath.EvalSymlinks(absBase)
-	if err == nil {
-		absBase = resolved
-	}
-
 	scanner := &freshenScanner{
 		fs:                 mfa.Fs,
 		absBase:            absBase,
@@ -400,7 +392,7 @@ func (mfa *CLIApp) freshenScan(
 		existingByPath:     existingByPath,
 	}
 
-	err = afero.Walk(mfa.Fs, absBase, scanner.walk)
+	err := afero.Walk(mfa.Fs, absBase, scanner.walk)
 
 	if showProgress {
 		log.ProgressDone()

internal/cli/freshen_test.go

@@ -336,49 +336,6 @@ func TestFreshenResolvesEntriesAgainstManifestDirectory(t *testing.T) {
 	}
 }
 
-// TestFreshenDirectoryNamedThroughSymlink adds a file to a tree after gen
-// made its manifest, then freshens it with the tree named through a symlink,
-// given as the argument or as the working directory: the manifest lists the
-// files in the tree, and leaves out a symlink inside it, as freshen does
-// without --follow-symlinks.
-//
-//nolint:paralleltest // changes the process-global working directory
-func TestFreshenDirectoryNamedThroughSymlink(t *testing.T) {
-	// Paths are relative to a temp dir holding link, a symlink to the tree.
-	for name, tc := range map[string]struct {
-		workDir string
-		args    []string
-	}{
-		"argument":          {".", []string{testLink}},
-		"working directory": {testLink, nil},
-	} {
-		t.Run(name, func(t *testing.T) {
-			fs := afero.NewOsFs()
-			tree, manifestPath := setupFreshenDir(t, fs,
-				map[string]string{testFileTxt: "in the tree"})
-			writeTestFile(t, fs, filepath.Join(tree, "later.txt"), "added later")
-			require.NoError(t,
-				os.Symlink(testFileTxt, filepath.Join(tree, "alias.txt")))
-
-			root := t.TempDir()
-			require.NoError(t, os.Symlink(tree, filepath.Join(root, testLink)))
-
-			// t.Chdir sets PWD to the path it is given, as a shell does, and
-			// os.Getwd returns PWD when it names the working directory.
-			t.Chdir(filepath.Join(root, tc.workDir))
-
-			opts := testOpts(slices.Concat(
-				[]string{testApp, cmdFreshen, "-q"}, tc.args,
-			), fs)
-			require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
-
-			assertManifestLists(t, fs, tree, manifestPath, map[string]string{
-				testFileTxt: "in the tree", "later.txt": "added later",
-			})
-		})
-	}
-}
-
 // TestFreshenRecordEntryMtimePresence pins the behavior of recordEntry
 // with respect to MFFilePath.Mtime, which is a message pointer with
 // proto3 field presence and may legitimately be absent.

internal/cli/signing.go

@@ -23,9 +23,8 @@ var errNoPassphrase = errors.New(
 	"signing key is protected: set " + envSignKeyPassphrase + " to its passphrase")
 
 // signingOptions returns the signing options for the OpenPGP secret key in
-// the file path, which must be able to sign. The passphrase of a protected
-// key comes from MFER_SIGN_KEY_PASSPHRASE, or else from the terminal on
-// stdin, and must unlock the key.
+// 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 {
@@ -40,19 +39,13 @@ func (mfa *CLIApp) signingOptions(path string) (*mfer.SigningOptions, error) {
 	log.Infof("signing manifest with the OpenPGP key in %s", path)
 
 	opts := &mfer.SigningOptions{SecretKey: secretKey}
-	if protected {
-		opts.Passphrase, err = mfa.readPassphrase(path)
-		if err != nil {
-			return nil, err
-		}
+	if !protected {
+		return opts, nil
 	}
 
-	// gen and freshen read the signing options before any file, so a key
-	// that cannot sign, or a wrong passphrase, stops them before they hash
-	// anything.
-	err = mfer.CheckSigningKey(opts)
+	opts.Passphrase, err = mfa.readPassphrase(path)
 	if err != nil {
-		return nil, fmt.Errorf("%s: %w", path, err)
+		return nil, err
 	}
 
 	return opts, nil

internal/cli/signing_test.go

@@ -2,24 +2,15 @@
 package cli
 
 import (
-	"bufio"
-	"io"
 	"path/filepath"
-	"strings"
 	"testing"
-	"time"
 
-	"github.com/ProtonMail/go-crypto/openpgp/packet"
-	"github.com/creack/pty"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-const (
-	testFlagSignKey = "--sign-key"
-	testKeyFile     = "/key.asc"
-)
+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
@@ -35,7 +26,7 @@ func TestGenAndFreshenSignWithKeyFile(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			t.Setenv(envSignKeyPassphrase, string(passphrase))
 
-			secretKey, fingerprint := testSecretKey(t, passphrase, nil)
+			secretKey, fingerprint := testSecretKey(t, passphrase)
 
 			fs := afero.NewOsFs()
 			keyFile := filepath.Join(t.TempDir(), "key.asc")
@@ -79,15 +70,15 @@ func TestGenAndFreshenSignWithKeyFile(t *testing.T) {
 func TestSignWithProtectedKeyNeedsPassphrase(t *testing.T) {
 	t.Setenv(envSignKeyPassphrase, "")
 
-	secretKey, _ := testSecretKey(t, []byte("secret"), nil)
+	secretKey, _ := testSecretKey(t, []byte("secret"))
 
 	fs := afero.NewMemMapFs()
-	require.NoError(t, afero.WriteFile(fs, testKeyFile, secretKey, 0o600))
+	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, testKeyFile,
+		testApp, cmdGenerate, "-q", testFlagSignKey, "/key.asc",
 		"-o", testMF, testDir,
 	}, fs)
 	assert.Equal(t, 1, runCLI(opts))
@@ -98,112 +89,3 @@ func TestSignWithProtectedKeyNeedsPassphrase(t *testing.T) {
 	require.NoError(t, err)
 	assert.False(t, exists)
 }
-
-// TestSignWithKeyThatCannotSignFailsFirst runs gen on a directory and
-// freshen on a manifest, neither of which exists, with keys that cannot
-// sign: a protected key with a wrong MFER_SIGN_KEY_PASSPHRASE, a key that
-// expired in 2020, and a version 6 key. Each run must fail on the key: it
-// checks the key before it reads any file, so a missing file goes
-// unnoticed.
-func TestSignWithKeyThatCannotSignFailsFirst(t *testing.T) {
-	t.Setenv(envSignKeyPassphrase, "wrong")
-
-	wrongPassphrase, _ := testSecretKey(t, []byte("right"), nil)
-
-	made := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
-	expired, _ := testSecretKey(t, nil, &packet.Config{
-		Algorithm:       packet.PubKeyAlgoEdDSA,
-		Time:            func() time.Time { return made },
-		KeyLifetimeSecs: uint32((24 * time.Hour).Seconds()),
-	})
-
-	version6, _ := testSecretKey(t, nil, &packet.Config{
-		Algorithm: packet.PubKeyAlgoEd25519,
-		V6Keys:    true,
-	})
-
-	for want, secretKey := range map[string][]byte{
-		"unlock signing key":                           wrongPassphrase,
-		"signing key cannot sign":                      expired,
-		"signing key must be an OpenPGP version 4 key": version6,
-	} {
-		fs := afero.NewMemMapFs()
-		require.NoError(t, afero.WriteFile(fs, testKeyFile, secretKey, 0o600))
-
-		for _, args := range [][]string{
-			{
-				testApp, cmdGenerate, "-q", testFlagSignKey, testKeyFile,
-				"-o", testMF, "/missing",
-			},
-			{testApp, cmdFreshen, "-q", testFlagSignKey, testKeyFile, "/missing.mf"},
-		} {
-			opts := testOpts(args, fs)
-			assert.Equal(t, 1, runCLI(opts), args[1], want)
-			assert.Contains(t, testStderr(t, opts), testKeyFile+": "+want, args[1])
-		}
-	}
-}
-
-// TestGenAsksForPassphraseOnTerminal runs gen with a protected key, no
-// MFER_SIGN_KEY_PASSPHRASE, and a terminal as stdin and stderr. gen must
-// ask for the passphrase on stderr, and sign with what is typed after the
-// prompt.
-func TestGenAsksForPassphraseOnTerminal(t *testing.T) {
-	t.Setenv(envSignKeyPassphrase, "")
-
-	secretKey, fingerprint := testSecretKey(t, []byte("passphrase"), nil)
-
-	fs := afero.NewOsFs()
-	keyFile := filepath.Join(t.TempDir(), "key.asc")
-	root := t.TempDir()
-	manifestPath := filepath.Join(root, defaultManifestName)
-
-	require.NoError(t, afero.WriteFile(fs, keyFile, secretKey, 0o600))
-	writeTestFile(t, fs, filepath.Join(root, testFileTxt), "hello")
-
-	terminal, tty, err := pty.Open()
-	require.NoError(t, err)
-
-	t.Cleanup(func() { _ = terminal.Close() })
-
-	opts := testOpts([]string{
-		testApp, cmdGenerate, "-q", testFlagSignKey, keyFile,
-		"-o", manifestPath, root,
-	}, fs)
-	opts.Stdin = tty
-	opts.Stderr = tty
-
-	exitCode := make(chan int, 1)
-
-	go func() {
-		exitCode <- runCLI(opts)
-
-		// Once gen has ended, reading the terminal fails instead of
-		// waiting for a prompt that will not come.
-		_ = tty.Close()
-	}()
-
-	prompt := "Passphrase for " + keyFile + ": "
-	output := bufio.NewReader(terminal)
-	written := ""
-
-	for !strings.HasSuffix(written, prompt) {
-		b, err := output.ReadByte()
-		require.NoError(t, err, "gen wrote %q and no prompt", written)
-
-		written += string(b)
-	}
-
-	_, err = terminal.WriteString("passphrase\n")
-	require.NoError(t, err)
-
-	code := <-exitCode
-	rest, _ := io.ReadAll(output)
-	require.Equal(t, 0, code, "gen wrote %q", rest)
-
-	check := testOpts([]string{
-		testApp, cmdCheck, "-q",
-		"--" + flagRequireSignature, fingerprint, manifestPath,
-	}, fs)
-	require.Equal(t, 0, runCLI(check), testStderr(t, check))
-}

mfer/openpgp.go

@@ -16,47 +16,24 @@ import (
 )
 
 const (
-	// The tags of OpenPGP signature, key and subkey packets (RFC 9580,
-	// section 5). Subkeys have tags of their own, so each secret or public
-	// key packet is one primary key.
-	signaturePacketTag    = 2
-	secretKeyPacketTag    = 5
-	publicKeyPacketTag    = 6
-	secretSubkeyPacketTag = 7
-	publicSubkeyPacketTag = 14
+	// 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
 
-	// In the body of a key or subkey packet the algorithm octet follows
-	// the version octet and the four-octet creation time, and from version
-	// 5 on a four-octet length as well (RFC 9580, section 5.5.2).
-	keyAlgorithmOffset            = 5
-	firstKeyVersionWithLength     = 5
-	keyAlgorithmOffsetAfterLength = 9
-
-	// signingKeyVersion is the only OpenPGP key version mfer signs with.
-	// Its fingerprints are 40 hex characters, the length
-	// --require-signature takes.
-	signingKeyVersion = 4
-
-	// armorBegin and armorEnd start the lines that begin and end an
-	// armored block.
-	armorBegin = "-----BEGIN "
-	armorEnd   = "-----END "
+	// armorEnd starts the line that ends an armored block.
+	armorEnd = "-----END "
 )
 
 var (
-	errKeyCount    = errors.New("must hold exactly one key")
-	errDSAKey      = errors.New("must not hold a DSA key")
-	errSecretKey   = errors.New("must not hold a secret key")
-	errNoSecretKey = errors.New("signing key file holds no secret key")
-	errNotV4Key    = errors.New("signing key must be an OpenPGP version 4 key, " +
-		"the only kind whose fingerprint --require-signature takes")
+	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")
-	errNotOneArmoredBlock = errors.New(
-		"must be exactly one armored block and nothing else")
-	errMalformedArmor = errors.New("armor is malformed")
 )
 
 // SigningOptions holds the key a manifest is signed with.
@@ -70,7 +47,7 @@ type SigningOptions struct {
 
 // SecretKeyIsProtected reports whether the OpenPGP secret key secretKey,
 // armored or binary, needs a passphrase to sign. It fails unless
-// secretKey holds one version 4 primary key with its secret key.
+// secretKey holds one primary key with its secret key.
 func SecretKeyIsProtected(secretKey []byte) (bool, error) {
 	key, err := readSecretKey(secretKey)
 	if err != nil {
@@ -80,26 +57,6 @@ func SecretKeyIsProtected(secretKey []byte) (bool, error) {
 	return isProtected(key), nil
 }
 
-// CheckSigningKey fails unless opts can sign now: opts.SecretKey must hold
-// one version 4 primary key with a secret key that may sign and has not
-// expired or been revoked, and opts.Passphrase must unlock it when it is
-// protected. It lets a caller find a key that cannot sign before it builds
-// a manifest.
-func CheckSigningKey(opts *SigningOptions) error {
-	key, err := readSigningKey(opts)
-	if err != nil {
-		return err
-	}
-
-	// Signing nothing fails wherever signing the manifest would.
-	err = openpgp.DetachSign(io.Discard, key, bytes.NewReader(nil), nil)
-	if err != nil {
-		return fmt.Errorf("signing key cannot sign: %w", err)
-	}
-
-	return nil
-}
-
 // readSigningKey returns the key in opts.SecretKey, unlocked with
 // opts.Passphrase if it is protected.
 func readSigningKey(opts *SigningOptions) (*openpgp.Entity, error) {
@@ -125,18 +82,13 @@ func readSigningKey(opts *SigningOptions) (*openpgp.Entity, error) {
 }
 
 // readSecretKey returns the one key in secretKey, armored or binary,
-// which must be a version 4 key and include its secret key.
+// which must include its secret key.
 func readSecretKey(secretKey []byte) (*openpgp.Entity, error) {
-	key, err := readOneKey(secretKey, "signing key file", false)
+	key, err := readOneKey(secretKey, "signing key file")
 	if err != nil {
 		return nil, err
 	}
 
-	if key.PrimaryKey.Version != signingKeyVersion {
-		return nil, fmt.Errorf("%w; this key is version %d",
-			errNotV4Key, key.PrimaryKey.Version)
-	}
-
 	if key.PrivateKey == nil {
 		return nil, errNoSecretKey
 	}
@@ -145,9 +97,8 @@ func readSecretKey(secretKey []byte) (*openpgp.Entity, error) {
 }
 
 // readOneKey returns the key in data, armored or binary, which must hold
-// exactly one primary key and no DSA key or subkey, and when publicOnly no
-// secret key or subkey either. what names data in errors.
-func readOneKey(data []byte, what string, publicOnly bool) (*openpgp.Entity, error) {
+// 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)
@@ -162,29 +113,6 @@ func readOneKey(data []byte, what string, publicOnly bool) (*openpgp.Entity, err
 		return nil, fmt.Errorf("%s %w, found %d", what, errKeyCount, keys)
 	}
 
-	// openpgp.ReadKeyRing checks the numbers of every secret key it reads,
-	// and an ElGamal secret subkey with a very large prime makes that take
-	// minutes.
-	secretKeys, err := countPackets(packets, secretKeyPacketTag, secretSubkeyPacketTag)
-	if err != nil {
-		return nil, fmt.Errorf("read %s: %w", what, err)
-	}
-
-	if publicOnly && secretKeys != 0 {
-		return nil, fmt.Errorf("%s %w", what, errSecretKey)
-	}
-
-	// openpgp.ReadKeyRing also checks every self-signature, and a DSA key
-	// with very large numbers makes each check take seconds to minutes.
-	dsa, err := holdsDSAKey(packets)
-	if err != nil {
-		return nil, fmt.Errorf("read %s: %w", what, err)
-	}
-
-	if dsa {
-		return nil, fmt.Errorf("%s %w", what, errDSAKey)
-	}
-
 	keyring, err := openpgp.ReadKeyRing(bytes.NewReader(packets))
 	if err != nil {
 		return nil, fmt.Errorf("read %s: %w", what, err)
@@ -247,7 +175,7 @@ func fingerprint(key *openpgp.Entity) string {
 // 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", true)
+	key, err := readOneKey(pubKey, "embedded public key block")
 	if err != nil {
 		return "", err
 	}
@@ -278,40 +206,39 @@ 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 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.
+// 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) {
-	if !bytes.Contains(data, []byte(armorBegin)) {
+	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
 	}
 
-	text := bytes.TrimSpace(data)
-	lastLine := text[bytes.LastIndexByte(text, '\n')+1:]
-
-	if !bytes.HasPrefix(text, []byte(armorBegin)) ||
-		bytes.Count(text, []byte(armorBegin)) != 1 ||
-		!bytes.HasPrefix(lastLine, []byte(armorEnd)) ||
-		bytes.Count(text, []byte(armorEnd)) != 1 {
-		return nil, errNotOneArmoredBlock
-	}
-
-	// armor.Decode passes over a block it cannot read, such as one with a
-	// header line that has no colon, and returns io.EOF on finding no
-	// other.
-	block, err := armor.Decode(bytes.NewReader(text))
-	if err != nil {
-		return nil, errMalformedArmor
-	}
-
-	body, err := io.ReadAll(block.Body)
-	if err != nil {
-		return nil, fmt.Errorf("%w: %w", errMalformedArmor, err)
-	}
-
-	return body, nil
+	return binary, nil
 }
 
 // countPackets returns how many packets in the binary OpenPGP data have
@@ -337,40 +264,3 @@ func countPackets(data []byte, tags ...uint8) (int, error) {
 		}
 	}
 }
-
-// holdsDSAKey reports whether any key or subkey packet in the binary
-// OpenPGP data holds a DSA key. It reads each packet's algorithm octet
-// rather than parsing the packet, since parsing a secret key packet checks
-// its numbers, which for a DSA key with very large numbers is as slow as
-// checking a self-signature.
-func holdsDSAKey(data []byte) (bool, error) {
-	packets := packet.NewOpaqueReader(bytes.NewReader(data))
-
-	for {
-		p, err := packets.Next()
-		if errors.Is(err, io.EOF) {
-			return false, nil
-		}
-
-		if err != nil {
-			return false, err
-		}
-
-		if !slices.Contains([]uint8{
-			secretKeyPacketTag, publicKeyPacketTag,
-			secretSubkeyPacketTag, publicSubkeyPacketTag,
-		}, p.Tag) {
-			continue
-		}
-
-		offset := keyAlgorithmOffset
-		if len(p.Contents) > 0 && p.Contents[0] >= firstKeyVersionWithLength {
-			offset = keyAlgorithmOffsetAfterLength
-		}
-
-		if len(p.Contents) > offset &&
-			packet.PublicKeyAlgorithm(p.Contents[offset]) == packet.PubKeyAlgoDSA {
-			return true, nil
-		}
-	}
-}

mfer/openpgp_test.go

@@ -4,9 +4,7 @@ package mfer
 import (
 	"bytes"
 	"context"
-	"crypto/dsa" //nolint:staticcheck // SA1019: tests need a DSA key to refuse
 	"io"
-	"math/big"
 	"os"
 	"path/filepath"
 	"slices"
@@ -17,7 +15,6 @@ import (
 
 	"github.com/ProtonMail/go-crypto/openpgp"
 	"github.com/ProtonMail/go-crypto/openpgp/armor"
-	"github.com/ProtonMail/go-crypto/openpgp/elgamal"
 	"github.com/ProtonMail/go-crypto/openpgp/packet"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
@@ -287,97 +284,6 @@ func TestSigningKeyFileWithoutSecretKey(t *testing.T) {
 	require.ErrorIs(t, b.Build(context.Background(), io.Discard), errNoSecretKey)
 }
 
-// TestCheckSigningKeyRefusesKeyThatCannotSign checks a key that can sign,
-// and keys that read and need no passphrase but cannot sign now: one that
-// expired in 2020 and one that has been revoked. CheckSigningKey must
-// refuse each of the two, as signing a manifest with it would fail.
-func TestCheckSigningKeyRefusesKeyThatCannotSign(t *testing.T) {
-	t.Parallel()
-
-	require.NoError(t, CheckSigningKey(testSigningOptions(t)))
-
-	made := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
-	expired := newTestKey(t, &packet.Config{
-		Time:            func() time.Time { return made },
-		KeyLifetimeSecs: uint32((24 * time.Hour).Seconds()),
-	})
-
-	revoked := newTestKey(t, nil)
-	require.NoError(t, revoked.RevokeKey(packet.KeyRetired, "", nil))
-
-	for name, key := range map[string]*openpgp.Entity{
-		"expired": expired,
-		"revoked": revoked,
-	} {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			assert.ErrorContains(t, CheckSigningKey(
-				&SigningOptions{SecretKey: armoredSecretKeys(t, key)}),
-				"signing key cannot sign")
-		})
-	}
-}
-
-// TestSigningRefusesVersion6Key signs with an OpenPGP version 6 key, whose
-// fingerprint is 64 hex characters. mfer signs only with version 4 keys.
-func TestSigningRefusesVersion6Key(t *testing.T) {
-	t.Parallel()
-
-	key, err := openpgp.NewEntity("MFER Test Key", "", "test@mfer.test",
-		&packet.Config{V6Keys: true, Algorithm: packet.PubKeyAlgoEd25519})
-	require.NoError(t, err)
-
-	secretKey := armoredSecretKeys(t, key)
-
-	_, err = SecretKeyIsProtected(secretKey)
-	require.ErrorIs(t, err, errNotV4Key)
-
-	b := NewBuilder()
-	b.SetSigningOptions(&SigningOptions{SecretKey: secretKey})
-	require.ErrorIs(t, b.Build(context.Background(), io.Discard), errNotV4Key)
-}
-
-// TestMalformedArmorIsNamed reads a signing key file, a signature and an
-// embedded public key block whose armor is malformed: a header line with
-// no colon, or no blank line after the BEGIN line. Each must fail saying
-// the armor is malformed.
-func TestMalformedArmorIsNamed(t *testing.T) {
-	t.Parallel()
-
-	opts := testSigningOptions(t)
-	manifest := signedTestManifest(t, opts)
-
-	for name, change := range map[string]func([]byte) []byte{
-		"header line with no colon": func(block []byte) []byte {
-			return bytes.Replace(block,
-				[]byte("-----\n"), []byte("-----\nno colon\n"), 1)
-		},
-		"no blank line after the BEGIN line": func(block []byte) []byte {
-			return bytes.Replace(block, []byte("-----\n\n"), []byte("-----\n"), 1)
-		},
-	} {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			_, err := SecretKeyIsProtected(change(opts.SecretKey))
-			require.ErrorIs(t, err, errMalformedArmor)
-
-			for _, changed := range [][]byte{
-				rewriteOuter(t, manifest, func(outer *MFFileOuter) {
-					outer.Signature = change(outer.GetSignature())
-				}),
-				rewriteOuter(t, manifest, func(outer *MFFileOuter) {
-					outer.SigningPubKey = change(outer.GetSigningPubKey())
-				}),
-			} {
-				_, err = NewManifestFromReader(bytes.NewReader(changed))
-				require.ErrorIs(t, err, errMalformedArmor)
-			}
-		})
-	}
-}
-
 func TestVerifySignature(t *testing.T) {
 	t.Parallel()
 
@@ -497,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: 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.
+// 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()
 
@@ -518,9 +424,12 @@ func TestManifestRefusesSecondEmbeddedKey(t *testing.T) {
 	otherAsSubkey.PrimaryKey.IsSubkey = true
 
 	for name, block := range map[string][]byte{
-		"public key":                 armoredPublicKeys(t, other, signer),
-		"secret key without user ID": armoredSecretKeys(t, signer, otherWithoutUserID),
-		"subkey packet first":        armoredPublicKeys(t, otherAsSubkey, signer),
+		"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()
@@ -545,175 +454,31 @@ func TestManifestRefusesSecondEmbeddedKeyWithoutUserID(t *testing.T) {
 	other := newTestKey(t, nil)
 	other.Identities = map[string]*openpgp.Identity{}
 
-	signer := newTestKey(t, nil)
-
-	manifest := rewriteOuter(t, signedTestManifest(t,
-		&SigningOptions{SecretKey: armoredSecretKeys(t, signer)}),
+	manifest := rewriteOuter(t, signedTestManifest(t, testSigningOptions(t)),
 		func(outer *MFFileOuter) {
-			outer.SigningPubKey = armoredPublicKeys(t, other, signer)
+			outer.SigningPubKey = joinArmored(
+				armoredPublicKeys(t, other), outer.GetSigningPubKey())
 		})
 
 	_, err := NewManifestFromReader(bytes.NewReader(manifest))
 	require.ErrorIs(t, err, errKeyCount)
 }
 
-// dsaKeyPacket returns a public key packet holding a DSA key, or a public
-// subkey packet when isSubkey. Its numbers are not a working key: loading
-// must refuse the packet before it uses them.
-func dsaKeyPacket(t *testing.T, isSubkey bool) []byte {
-	t.Helper()
-
-	key := packet.NewDSAPublicKey(time.Now(), &dsa.PublicKey{
-		P: big.NewInt(23), Q: big.NewInt(11), G: big.NewInt(4), Y: big.NewInt(8),
-	})
-	key.IsSubkey = isSubkey
-
-	var buf bytes.Buffer
-
-	require.NoError(t, key.Serialize(&buf))
-
-	return buf.Bytes()
-}
-
-// TestManifestRefusesDSAKey loads manifests whose embedded public key
-// block holds a DSA key: alone, or as a subkey after the key that signed
-// the manifest. openpgp.ReadKeyRing checks a key's self-signatures, which
-// for a DSA key with very large numbers takes minutes each. Loading must
-// refuse each block before that.
-func TestManifestRefusesDSAKey(t *testing.T) {
-	t.Parallel()
-
-	signer := newTestKey(t, nil)
-	manifest := signedTestManifest(t,
-		&SigningOptions{SecretKey: armoredSecretKeys(t, signer)})
-
-	var signerKey bytes.Buffer
-
-	require.NoError(t, signer.Serialize(&signerKey))
-
-	for name, block := range map[string][]byte{
-		"DSA key":    dsaKeyPacket(t, false),
-		"DSA subkey": slices.Concat(signerKey.Bytes(), dsaKeyPacket(t, true)),
-	} {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			embedded := rewriteOuter(t, manifest, func(outer *MFFileOuter) {
-				outer.SigningPubKey = block
-			})
-
-			_, err := NewManifestFromReader(bytes.NewReader(embedded))
-			require.ErrorIs(t, err, errDSAKey)
-		})
-	}
-}
-
-// elGamalSecretSubkeyPacket returns a secret subkey packet holding an
-// ElGamal key. Its numbers are not a working key: loading must refuse the
-// packet before it uses them.
-func elGamalSecretSubkeyPacket(t *testing.T) []byte {
-	t.Helper()
-
-	key := packet.NewElGamalPrivateKey(time.Now(), &elgamal.PrivateKey{
-		P: big.NewInt(23), G: big.NewInt(4), Y: big.NewInt(8), X: big.NewInt(3),
-	})
-	key.IsSubkey = true
-
-	var buf bytes.Buffer
-
-	require.NoError(t, key.Serialize(&buf))
-
-	return buf.Bytes()
-}
-
-// TestManifestRefusesSecretKey loads manifests whose embedded public key
-// block holds a secret key: the key that signed the manifest with its
-// secret key, or its public key followed by an ElGamal secret subkey.
-// openpgp.ReadKeyRing checks the numbers of every secret key it reads,
-// which for an ElGamal key with a very large prime takes minutes. Loading
-// must refuse each block before that.
-func TestManifestRefusesSecretKey(t *testing.T) {
-	t.Parallel()
-
-	signer := newTestKey(t, nil)
-	manifest := signedTestManifest(t,
-		&SigningOptions{SecretKey: armoredSecretKeys(t, signer)})
-
-	var signerKey bytes.Buffer
-
-	require.NoError(t, signer.Serialize(&signerKey))
-
-	for _, block := range [][]byte{
-		armoredSecretKeys(t, signer),
-		slices.Concat(signerKey.Bytes(), elGamalSecretSubkeyPacket(t)),
-	} {
-		embedded := rewriteOuter(t, manifest, func(outer *MFFileOuter) {
-			outer.SigningPubKey = block
-		})
-
-		_, err := NewManifestFromReader(bytes.NewReader(embedded))
-		require.ErrorIs(t, err, errSecretKey)
-	}
-}
-
 // TestManifestRefusesTwoSignatures loads a manifest whose signature field
-// holds its good signature twice, not armored. Loading must refuse it.
+// 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) {
-			sig, err := dearmor(outer.GetSignature())
-			require.NoError(t, err)
-
-			outer.Signature = slices.Concat(sig, sig)
+			outer.Signature = joinArmored(
+				outer.GetSignature(), outer.GetSignature())
 		})
 
 	_, 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.

mfer/scanner.go

@@ -139,15 +139,25 @@ func (s *Scanner) EnumerateFile(filePath string) error {
 	return s.enumerateFileWithInfo(filepath.Base(abs), basePath, info, nil)
 }
 
-// EnumeratePath adds inputPath, a directory or a file, to the scanner as
-// EnumeratePaths adds each of its paths.
+// EnumeratePath walks a directory path and adds all files to the scanner.
 // If progress is non-nil, status updates are sent as files are discovered.
 // The progress channel is closed when the method returns.
 func (s *Scanner) EnumeratePath(
 	inputPath string,
 	progress chan<- EnumerateStatus,
 ) error {
-	return s.EnumeratePaths(progress, inputPath)
+	if progress != nil {
+		defer close(progress)
+	}
+
+	abs, err := filepath.Abs(inputPath)
+	if err != nil {
+		return err
+	}
+
+	afs := afero.NewReadOnlyFs(afero.NewBasePathFs(s.fs, abs))
+
+	return s.enumerateFS(afs, abs, progress)
 }
 
 // EnumeratePaths adds to the scanner the files under each directory path,
@@ -175,14 +185,6 @@ func (s *Scanner) EnumeratePaths(
 		}
 
 		if info.IsDir() {
-			// The walk does not follow a symlink at its top, so a directory
-			// named through one is resolved first. If that fails, the
-			// directory is walked as named and the walk reports the problem.
-			resolved, evalErr := filepath.EvalSymlinks(abs)
-			if evalErr == nil {
-				abs = resolved
-			}
-
 			afs := afero.NewReadOnlyFs(afero.NewBasePathFs(s.fs, abs))
 			err = s.enumerateFS(afs, abs, progress)
 		} else {

mfer/scanner_test.go

@@ -118,27 +118,6 @@ func TestScannerEnumeratePathWithProgress(t *testing.T) {
 	assert.Equal(t, FileSize(6), final.BytesFound)
 }
 
-// TestScannerEnumeratePathFile gives EnumeratePath a file: it is listed
-// by its name, as EnumerateFile lists it, and the manifest can be built.
-func TestScannerEnumeratePathFile(t *testing.T) {
-	t.Parallel()
-
-	fs := afero.NewMemMapFs()
-	require.NoError(t, fs.MkdirAll("/dir", 0o755))
-	require.NoError(t, afero.WriteFile(fs, "/dir/one.txt", []byte("1"), 0o644))
-
-	s := NewScannerWithOptions(&ScannerOptions{Fs: fs})
-	require.NoError(t, s.EnumeratePath("/dir/one.txt", nil))
-
-	var buf bytes.Buffer
-	require.NoError(t, s.ToManifest(context.Background(), &buf, nil))
-
-	m, err := NewManifestFromReader(&buf)
-	require.NoError(t, err)
-	require.Len(t, m.Files(), 1)
-	assert.Equal(t, "one.txt", m.Files()[0].GetPath())
-}
-
 func TestScannerEnumeratePaths(t *testing.T) {
 	t.Parallel()