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

README.md

@@ -268,8 +268,6 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues).
 
 - `mfer gen` / `mfer gen .`
     - recurses under current directory and writes out an `index.mf`
-    - lists regular files only, and skips a named pipe, socket or device, which
-      it names with `-v`
     - records every file's mode as `0000` unless given `--include-permissions`,
       which records each file's permission bits (`0777` at most)
 - `mfer gen /media/drive`
@@ -296,19 +294,15 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues).
     - rewrites `index.mf` to list the files now under the directory holding it,
       or under the one `--base` names, hashing only the files that are new or
       changed
-    - lists regular files only, as gen does
     - leaves out hidden files unless given `--include-dotfiles`, and symlinks
-      unless given `--follow-symlinks`, which lists each symlink to a regular
-      file under its own name with the contents of the file it points to
+      unless given `--follow-symlinks`, which lists each symlink to a file under
+      its own name with the contents of the file it points to
 - `mfer gen --sign-key key.asc` / `mfer freshen --sign-key key.asc`
     - signs the manifest with the OpenPGP secret key in `key.asc`, armored or
       binary, as `gpg --export-secret-keys` writes it; `MFER_SIGN_KEY` names the
       file too. mfer signs it itself and does not need `gpg`. A file holding
       more than one key is refused, and a key held only on a smartcard cannot
-      sign. The key must be a version 4 key, whose 40-character fingerprint is
-      what `--require-signature` takes, and not a DSA key. A key that cannot
-      sign, because it has expired or been revoked or its passphrase is wrong,
-      stops `gen` and `freshen` before they read any file
+      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
 

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
@@ -1664,119 +1619,20 @@ func TestGenerateValidatesInputPaths(t *testing.T) {
 	})
 }
 
-// generateTestDir writes testFile1 and runs gen on testDir, which writes
-// the manifest to testDir/index.mf.
-func generateTestDir(t *testing.T, fs afero.Fs) {
-	t.Helper()
-
-	require.NoError(t, fs.MkdirAll(testDir, 0o755))
-	writeTestFile(t, fs, testFile1, "content")
-
-	opts := testOpts([]string{testApp, cmdGenerate, "-q", testDir}, fs)
-	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
-}
-
-// TestCheckReadsOptionAfterPath runs check on an unsigned manifest with
-// --require-signature after the directory. check must refuse the manifest.
-func TestCheckReadsOptionAfterPath(t *testing.T) {
+// TestFlagAfterArgumentIsArgument asserts that flags are read only before a
+// command's first argument: after it, -v is a path, not the verbose flag.
+func TestFlagAfterArgumentIsArgument(t *testing.T) {
 	t.Parallel()
 
 	fs := afero.NewMemMapFs()
-	generateTestDir(t, fs)
+	require.NoError(t, fs.MkdirAll(testDir, 0o755))
+	writeTestFile(t, fs, testFile1, "content")
 
-	opts := testOpts([]string{
-		testApp, cmdCheck, "-q", testDir, "--" + flagRequireSignature, msgFpA,
-	}, fs)
-	assert.Equal(t, 1, runCLI(opts))
-	assert.Contains(t, testStderr(t, opts), "manifest is not signed")
-}
+	opts := testOpts([]string{testApp, cmdGenerate, testDir, "-v"}, fs)
+	exitCode := runCLI(opts)
 
-// TestFreshenReadsOptionAfterPath runs freshen with --include-permissions
-// after the directory. freshen must record the file's mode.
-func TestFreshenReadsOptionAfterPath(t *testing.T) {
-	t.Parallel()
-
-	fs := afero.NewOsFs()
-	root, manifestPath := setupFreshenDir(t, fs,
-		map[string]string{testFileTxt: "a file"})
-	require.NoError(t, fs.Chmod(filepath.Join(root, testFileTxt), 0o640))
-
-	opts := testOpts([]string{
-		testApp, cmdFreshen, "-q", root, "--" + flagIncludePermissions,
-	}, fs)
-	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
-
-	files := manifestFiles(t, fs, manifestPath)
-	require.Len(t, files, 1)
-	assert.Equal(t, uint32(0o640), files[0].GetMode())
-}
-
-// TestFetchReadsOptionAfterURL runs fetch with --dest after the URL. The
-// file and the manifest must land in that directory and nothing in the
-// current one.
-//
-//nolint:paralleltest // changes the process-global working directory
-func TestFetchReadsOptionAfterURL(t *testing.T) {
-	content := []byte("fetched")
-	manifest := manifestOf(t, map[string][]byte{testFileTxt: content})
-
-	server := httptest.NewServer(fetchTestHandler(manifest,
-		map[string][]byte{testFileTxt: content}))
-	defer server.Close()
-
-	cwd := chdirTemp(t)
-	dest := t.TempDir()
-
-	opts := testOpts([]string{
-		testApp, cmdFetch, "-q", server.URL, "--" + flagDest, dest,
-	}, afero.NewOsFs())
-	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
-
-	assert.Equal(t,
-		map[string][]byte{testFileTxt: content, defaultManifestName: manifest},
-		filesUnder(t, dest))
-	assert.Empty(t, filesUnder(t, cwd), "fetch wrote outside --dest")
-}
-
-// TestOneArgumentCommandsRefuseSecond runs each command that takes one
-// argument with two. It must fail naming the second.
-func TestOneArgumentCommandsRefuseSecond(t *testing.T) {
-	t.Parallel()
-
-	for _, command := range []string{
-		cmdCheck, cmdFreshen, cmdList, cmdExport, cmdFetch,
-	} {
-		t.Run(command, func(t *testing.T) {
-			t.Parallel()
-
-			fs := afero.NewMemMapFs()
-			generateTestDir(t, fs)
-
-			opts := testOpts([]string{testApp, command, testDir, "/second"}, fs)
-			assert.Equal(t, 1, runCLI(opts))
-			assert.Contains(t, testStderr(t, opts),
-				`unexpected argument "/second"`)
-		})
-	}
-}
-
-// TestGenerateReadsPathAfterDoubleDash runs gen -- -x where -x is a
-// directory. -x must be read as the path to list, not as an option.
-//
-//nolint:paralleltest // changes the process-global working directory
-func TestGenerateReadsPathAfterDoubleDash(t *testing.T) {
-	chdirTemp(t)
-
-	fs := afero.NewOsFs()
-	require.NoError(t, fs.Mkdir("-x", 0o755))
-	writeTestFile(t, fs, filepath.Join("-x", testFileTxt), "content")
-
-	opts := testOpts([]string{testApp, cmdGenerate, "-q", "--", "-x"}, fs)
-	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
-
-	files := manifestFiles(t, fs, filepath.Join("-x", defaultManifestName))
-	require.Len(t, files, 1)
-	assert.Equal(t, testFileTxt, files[0].GetPath())
+	assert.Equal(t, 1, exitCode)
+	assert.Contains(t, testStderr(t, opts), "path does not exist: -v")
 }
 
 func TestCheckDetectsManifestCorruption(t *testing.T) {

internal/cli/errmsg_test.go

@@ -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})

internal/cli/freshen.go

@@ -192,13 +192,6 @@ func (s *freshenScanner) walk(path string, info fs.FileInfo, walkErr error) erro
 		info = realInfo
 	}
 
-	// Opening a named pipe can block, and reading a device may never end.
-	if !info.Mode().IsRegular() {
-		log.Verbosef("skipping %s: not a regular file", relPath)
-
-		return nil
-	}
-
 	for _, excluded := range s.excluded {
 		if os.SameFile(info, excluded) {
 			return nil
@@ -388,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,
@@ -407,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/mfer.go

@@ -48,10 +48,6 @@ func manifestTempPath(out string) string {
 // errUnknownCommand indicates an unrecognized command argument.
 var errUnknownCommand = errors.New("unknown command")
 
-// errUnexpectedArgument indicates a second argument to a command that takes
-// one.
-var errUnexpectedArgument = errors.New("unexpected argument")
-
 // CLIApp is the main CLI application container. It holds configuration,
 // I/O streams, and filesystem abstraction to enable testing and flexibility.
 //
@@ -158,14 +154,13 @@ func commonFlags() []cli.Flag {
 	}
 }
 
-// refuseExtraArgs is the ArgValidator of the commands that take one
-// argument: it refuses a second one, naming it.
-func refuseExtraArgs(_ context.Context, cmd *cli.Command) error {
-	if cmd.Args().Len() > 1 {
-		return fmt.Errorf("%w %q", errUnexpectedArgument, cmd.Args().Get(1))
-	}
+// stopOnFirstArg returns the StopOnNthArg setting every command uses: flags
+// are read only before the command's first argument, and everything after it
+// is an argument, as with urfave/cli v2. So `mfer gen d -v` names a path "-v".
+func stopOnFirstArg() *int {
+	n := 1
 
-	return nil
+	return &n
 }
 
 // requireSignatureFlag returns the --require-signature flag taken by the
@@ -191,10 +186,11 @@ func includePermissionsFlag() *cli.BoolFlag {
 
 func (mfa *CLIApp) generateCommand() *cli.Command {
 	return &cli.Command{
-		Name:      cmdGenerate,
-		Aliases:   []string{"gen"},
-		Usage:     "Generate manifest file",
-		ArgsUsage: "[path ...]",
+		Name:         cmdGenerate,
+		Aliases:      []string{"gen"},
+		Usage:        "Generate manifest file",
+		ArgsUsage:    "[path ...]",
+		StopOnNthArg: stopOnFirstArg(),
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -256,7 +252,7 @@ func (mfa *CLIApp) checkCommand() *cli.Command {
 		Name:         cmdCheck,
 		Usage:        "Validate files using manifest file",
 		ArgsUsage:    manifestArgsUsage,
-		ArgValidator: refuseExtraArgs,
+		StopOnNthArg: stopOnFirstArg(),
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -290,7 +286,7 @@ func (mfa *CLIApp) freshenCommand() *cli.Command {
 		Name:         cmdFreshen,
 		Usage:        "Update manifest with changed, new, and removed files",
 		ArgsUsage:    manifestArgsUsage,
-		ArgValidator: refuseExtraArgs,
+		StopOnNthArg: stopOnFirstArg(),
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -341,7 +337,7 @@ func (mfa *CLIApp) exportCommand() *cli.Command {
 		Name:         cmdExport,
 		Usage:        "Export manifest contents as JSON",
 		ArgsUsage:    "[manifest file or URL]",
-		ArgValidator: refuseExtraArgs,
+		StopOnNthArg: stopOnFirstArg(),
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 
@@ -352,8 +348,9 @@ func (mfa *CLIApp) exportCommand() *cli.Command {
 
 func (mfa *CLIApp) versionCommand() *cli.Command {
 	return &cli.Command{
-		Name:  cmdVersion,
-		Usage: "Show version",
+		Name:         cmdVersion,
+		Usage:        "Show version",
+		StopOnNthArg: stopOnFirstArg(),
 		Action: func(context.Context, *cli.Command) error {
 			mfa.printVersion()
 
@@ -368,7 +365,7 @@ func (mfa *CLIApp) listCommand() *cli.Command {
 		Aliases:      []string{"ls"},
 		Usage:        "List files in manifest",
 		ArgsUsage:    manifestArgsUsage,
-		ArgValidator: refuseExtraArgs,
+		StopOnNthArg: stopOnFirstArg(),
 		Action:       mfa.listManifestOperation,
 		Flags: []cli.Flag{
 			&cli.BoolFlag{
@@ -389,7 +386,7 @@ func (mfa *CLIApp) fetchCommand() *cli.Command {
 		Name:         cmdFetch,
 		Usage:        "fetch manifest and referenced files",
 		ArgsUsage:    "URL",
-		ArgValidator: refuseExtraArgs,
+		StopOnNthArg: stopOnFirstArg(),
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -448,8 +445,9 @@ func (mfa *CLIApp) run(args []string) {
 		Writer:                mfa.Stdout,
 		// v3 writes its "Incorrect Usage" line to ErrWriter; v2 wrote it to
 		// stdout, before the help, so it stays on stdout.
-		ErrWriter: mfa.Stdout,
-		Flags:     commonFlags(),
+		ErrWriter:    mfa.Stdout,
+		Flags:        commonFlags(),
+		StopOnNthArg: stopOnFirstArg(),
 		Action: func(_ context.Context, cmd *cli.Command) error {
 			if cmd.Args().Len() > 0 {
 				return fmt.Errorf("%w %q", errUnknownCommand, cmd.Args().First())

internal/cli/pipe_test.go

@@ -1,94 +0,0 @@
-//go:build unix
-
-//nolint:testpackage // white-box tests exercise unexported internals
-package cli
-
-import (
-	"os"
-	"path/filepath"
-	"syscall"
-	"testing"
-	"time"
-
-	"github.com/spf13/afero"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-const (
-	// testPipe is the name of the named pipe the pipe tests make in a tree.
-	testPipe = "pipe"
-
-	// pipeDeadline is how long a test lets a run go before it opens the
-	// test's named pipes for writing.
-	pipeDeadline = 5 * time.Second
-)
-
-// makePipe makes a named pipe at path. Opening a named pipe for reading
-// blocks until it is opened for writing, so once pipeDeadline has passed,
-// unless the test has ended, makePipe opens it for writing and closes it
-// again. A run that wrongly reads the pipe then ends, and the test fails on
-// what it reports instead of hanging. That lets one read through, so each
-// path a wrong run would read gets a pipe of its own.
-func makePipe(t *testing.T, path string) {
-	t.Helper()
-
-	require.NoError(t, syscall.Mkfifo(path, 0o600))
-
-	timer := time.AfterFunc(pipeDeadline, func() {
-		f, err := os.OpenFile(path, os.O_WRONLY, 0) //nolint:gosec // test-controlled path
-		if err == nil {
-			_ = f.Close()
-		}
-	})
-
-	t.Cleanup(func() { timer.Stop() })
-}
-
-// TestGenerateSkipsPipe runs gen -v on a tree holding a named pipe: the
-// manifest lists the regular file only, and gen names the pipe it skipped.
-func TestGenerateSkipsPipe(t *testing.T) {
-	t.Parallel()
-
-	dir := t.TempDir()
-	fs := afero.NewOsFs()
-	writeTestFile(t, fs, filepath.Join(dir, testFileTxt), "regular")
-	makePipe(t, filepath.Join(dir, testPipe))
-
-	opts := testOpts([]string{testApp, cmdGenerate, "-v", dir}, fs)
-	require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
-
-	assert.Equal(t, []string{testFileTxt},
-		manifestPaths(t, fs, filepath.Join(dir, defaultManifestName)))
-	assert.Contains(t, testStderr(t, opts), "skipping pipe: not a regular file")
-}
-
-// TestFreshenSkipsPipes adds a named pipe to a tree after gen made its
-// manifest, and a symlink to another pipe. freshen -v --follow-symlinks
-// leaves the manifest listing the regular file only, and names both entries
-// it skipped.
-func TestFreshenSkipsPipes(t *testing.T) {
-	t.Parallel()
-
-	fs := afero.NewOsFs()
-	root, manifestPath := setupFreshenDir(t, fs,
-		map[string]string{testFileTxt: "regular"})
-	makePipe(t, filepath.Join(root, testPipe))
-
-	// The symlink points outside the tree, so no pipe is read twice.
-	target := filepath.Join(t.TempDir(), testPipe)
-	makePipe(t, target)
-	require.NoError(t, os.Symlink(target, filepath.Join(root, testLink)))
-
-	opts := testOpts([]string{
-		testApp, cmdFreshen, "-v", "--follow-symlinks",
-		testFlagBase, root, manifestPath,
-	}, fs)
-	require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
-
-	assert.Equal(t, []string{testFileTxt}, manifestPaths(t, fs, manifestPath))
-
-	stderr := testStderr(t, opts)
-	assert.Contains(t, stderr, "skipping pipe: not a regular file")
-	assert.Contains(t, stderr, "skipping link: not a regular file")
-}

internal/cli/signing.go

@@ -23,9 +23,9 @@ 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, and must
+// unlock the key.
 func (mfa *CLIApp) signingOptions(path string) (*mfer.SigningOptions, error) {
 	secretKey, err := afero.ReadFile(mfa.Fs, path)
 	if err != nil {
@@ -40,16 +40,17 @@ 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.
+	opts.Passphrase, err = mfa.readPassphrase(path)
+	if err != nil {
+		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)

internal/cli/signing_test.go

@@ -7,9 +7,7 @@ import (
 	"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"
@@ -35,7 +33,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,7 +77,7 @@ 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))
@@ -99,48 +97,29 @@ func TestSignWithProtectedKeyNeedsPassphrase(t *testing.T) {
 	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) {
+// 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")
 
-	wrongPassphrase, _ := testSecretKey(t, []byte("right"), nil)
+	secretKey, _ := testSecretKey(t, []byte("right"))
 
-	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()),
-	})
+	fs := afero.NewMemMapFs()
+	require.NoError(t, afero.WriteFile(fs, testKeyFile, secretKey, 0o600))
 
-	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,
+	for _, args := range [][]string{
+		{
+			testApp, cmdGenerate, "-q", testFlagSignKey, testKeyFile,
+			"-o", testMF, "/missing",
+		},
+		{testApp, cmdFreshen, "-q", testFlagSignKey, testKeyFile, "/missing.mf"},
 	} {
-		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])
-		}
+		opts := testOpts(args, fs)
+		assert.Equal(t, 1, runCLI(opts), args[1])
+		assert.Contains(t, testStderr(t, opts),
+			testKeyFile+": unlock signing key", args[1])
 	}
 }
 
@@ -151,7 +130,7 @@ func TestSignWithKeyThatCannotSignFailsFirst(t *testing.T) {
 func TestGenAsksForPassphraseOnTerminal(t *testing.T) {
 	t.Setenv(envSignKeyPassphrase, "")
 
-	secretKey, fingerprint := testSecretKey(t, []byte("passphrase"), nil)
+	secretKey, fingerprint := testSecretKey(t, []byte("passphrase"))
 
 	fs := afero.NewOsFs()
 	keyFile := filepath.Join(t.TempDir(), "key.asc")

mfer/checker.go

@@ -318,11 +318,6 @@ func (c *Checker) FindExtraFiles(ctx context.Context, results chan<- Result) err
 			return nil
 		}
 
-		// gen never lists a named pipe, a socket or a device.
-		if info.Mode()&(os.ModeNamedPipe|os.ModeSocket|os.ModeDevice) != 0 {
-			return nil
-		}
-
 		// A symlink is compared by what it points to, so a manifest reached
 		// through one is not reported either.
 		if info.Mode()&os.ModeSymlink != 0 {
@@ -376,15 +371,6 @@ func (c *Checker) checkFile(entry *MFFilePath, checkedBytes *FileSize) Result {
 		return Result{Path: relPath, Status: StatusError, Message: err.Error()}
 	}
 
-	// Opening a named pipe can block, and reading a device may never end.
-	if !info.Mode().IsRegular() {
-		return Result{
-			Path:    relPath,
-			Status:  StatusError,
-			Message: "not a regular file",
-		}
-	}
-
 	// Check size
 	if info.Size() != entry.GetSize() {
 		*checkedBytes += FileSize(info.Size())

mfer/openpgp.go

@@ -16,26 +16,12 @@ 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
-
-	// 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
+	// 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
 
 	// armorBegin and armorEnd start the lines that begin and end an
 	// armored block.
@@ -44,19 +30,14 @@ const (
 )
 
 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 +51,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,24 +61,14 @@ 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.
+// 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 {
-	key, err := readSigningKey(opts)
-	if err != nil {
-		return err
-	}
+	_, err := readSigningKey(opts)
 
-	// 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
+	return err
 }
 
 // readSigningKey returns the key in opts.SecretKey, unlocked with
@@ -125,18 +96,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 +111,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 +127,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 +189,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
 	}
@@ -298,20 +240,12 @@ func dearmor(data []byte) ([]byte, error) {
 		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
+		return nil, err
 	}
 
-	body, err := io.ReadAll(block.Body)
-	if err != nil {
-		return nil, fmt.Errorf("%w: %w", errMalformedArmor, err)
-	}
-
-	return body, nil
+	return io.ReadAll(block.Body)
 }
 
 // countPackets returns how many packets in the binary OpenPGP data have
@@ -337,40 +271,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()
 
@@ -557,105 +463,6 @@ func TestManifestRefusesSecondEmbeddedKeyWithoutUserID(t *testing.T) {
 	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.
 func TestManifestRefusesTwoSignatures(t *testing.T) {

mfer/pipe_test.go

@@ -1,123 +0,0 @@
-//go:build unix
-
-//nolint:testpackage // white-box tests exercise unexported internals
-package mfer
-
-import (
-	"bytes"
-	"context"
-	"os"
-	"path/filepath"
-	"syscall"
-	"testing"
-	"time"
-
-	"github.com/spf13/afero"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-// pipeDeadline is how long a test lets a run go before it opens the test's
-// named pipes for writing.
-const pipeDeadline = 5 * time.Second
-
-// makePipe makes a named pipe at path. Opening a named pipe for reading
-// blocks until it is opened for writing, so once pipeDeadline has passed,
-// unless the test has ended, makePipe opens it for writing and closes it
-// again. A run that wrongly reads the pipe then ends, and the test fails on
-// what it reports instead of hanging. That lets one read through, so each
-// path a wrong run would read gets a pipe of its own.
-func makePipe(t *testing.T, path string) {
-	t.Helper()
-
-	require.NoError(t, syscall.Mkfifo(path, 0o600))
-
-	timer := time.AfterFunc(pipeDeadline, func() {
-		f, err := os.OpenFile(path, os.O_WRONLY, 0) //nolint:gosec // test-controlled path
-		if err == nil {
-			_ = f.Close()
-		}
-	})
-
-	t.Cleanup(func() { timer.Stop() })
-}
-
-// TestScannerSkipsPipes scans a tree holding a named pipe and a symlink to
-// another one, following symlinks: the manifest lists the regular file only.
-func TestScannerSkipsPipes(t *testing.T) {
-	t.Parallel()
-
-	dir := t.TempDir()
-	require.NoError(t, os.WriteFile(filepath.Join(dir, testFile1), []byte("a"), 0o600))
-	makePipe(t, filepath.Join(dir, "pipe"))
-
-	// The symlink points outside the tree, so no pipe is read twice.
-	target := filepath.Join(t.TempDir(), "pipe")
-	makePipe(t, target)
-	require.NoError(t, os.Symlink(target, filepath.Join(dir, "link")))
-
-	s := NewScannerWithOptions(&ScannerOptions{FollowSymLinks: true})
-	require.NoError(t, s.EnumeratePath(dir, 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, testFile1, m.Files()[0].GetPath())
-}
-
-// TestCheckPipes checks a tree against a manifest that lists, as an empty
-// file, a path that is a named pipe on disk. The tree holds another pipe the
-// manifest does not list. check reports the listed pipe as an error without
-// reading it, and the search for extra files does not report the other.
-func TestCheckPipes(t *testing.T) {
-	t.Parallel()
-
-	dir := t.TempDir()
-	manifestPath := filepath.Join(dir, "index.mf")
-	content := []byte("in manifest")
-	listedPipe := RelFilePath("listed-pipe")
-
-	fs := afero.NewOsFs()
-	createTestManifest(t, fs, manifestPath, map[string][]byte{
-		testFile1:          content,
-		string(listedPipe): {},
-	})
-	require.NoError(t, afero.WriteFile(fs, filepath.Join(dir, testFile1), content, 0o600))
-	makePipe(t, filepath.Join(dir, string(listedPipe)))
-	makePipe(t, filepath.Join(dir, "unlisted-pipe"))
-
-	chk, err := NewChecker(&CheckerOptions{
-		ManifestPath: manifestPath,
-		BasePath:     dir,
-		Fs:           fs,
-	})
-	require.NoError(t, err)
-
-	results := make(chan Result, 10)
-	require.NoError(t, chk.Check(context.Background(), results, nil))
-
-	byPath := map[RelFilePath]Result{}
-	for r := range results {
-		byPath[r.Path] = r
-	}
-
-	assert.Equal(t, StatusOK, byPath[testFile1].Status)
-	assert.Equal(t, Result{
-		Path:    listedPipe,
-		Status:  StatusError,
-		Message: "not a regular file",
-	}, byPath[listedPipe])
-
-	extras := make(chan Result, 10)
-	require.NoError(t, chk.FindExtraFiles(context.Background(), extras))
-
-	var extraPaths []RelFilePath
-	for r := range extras {
-		extraPaths = append(extraPaths, r.Path)
-	}
-
-	assert.Empty(t, extraPaths)
-}

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 {
@@ -474,13 +476,6 @@ func (s *Scanner) enumerateFileWithInfo(
 		info = realInfo
 	}
 
-	// Opening a named pipe can block, and reading a device may never end.
-	if !info.Mode().IsRegular() {
-		log.Verbosef("skipping %s: not a regular file", cleanPath)
-
-		return nil
-	}
-
 	for _, excluded := range s.excluded {
 		if os.SameFile(info, excluded) {
 			return nil

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()