Refuse a path with a . segment (closes #208)

ValidatePath refused a .. segment but accepted a . segment, so a
manifest could list ./a.txt. check then verified a.txt and also
reported it as an extra file, and one file could be listed twice under
two spellings despite the listed-once rule. A . segment is now refused
when a manifest is written and when it is loaded, and the Path Rules
in docs/FORMAT.md list it.

Two fetch tests listed ./ paths to check name clashes; such manifests
can no longer be built, so those cases are removed.

Model: opus-5-5

docs/FORMAT.md

@@ -107,6 +107,7 @@ All `path` values must satisfy these invariants:
 - **Forward slashes**: use `/` as the path separator (never `\`)
 - **Relative only**: no leading `/`
 - **No parent traversal**: no `..` path segments
+- **No current-directory segments**: no `.` path segments
 - **No empty segments**: no `//` sequences
 - **No trailing slash**: paths refer to files, not directories
 - **Listed once**: each path appears at most once in a manifest, compared byte
@@ -174,6 +175,10 @@ 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.
 
+A signing key that has expired does not make the signature bad, because a
+manifest outlives its key. A signature past its own expiry time is not good,
+whether or not the key has expired too.
+
 ## Deterministic Serialization
 
 By default, manifests are generated deterministically:

internal/cli/fetch_test.go

@@ -1243,10 +1243,10 @@ func TestFetchRequireSignature(t *testing.T) {
 
 // TestFetchRefusesListedManifestName fetches manifests that list, at the
 // top of the tree, the name fetch saves the manifest under or that name's
-// temp file: as a file, as a directory, in capitals, and with a leading
-// "./". Saving the manifest would replace or remove what is listed there,
-// so fetch must refuse the manifest before it creates the destination or
-// requests any file.
+// temp file: as a file, as a directory and in capitals. Saving the
+// manifest would replace or remove what is listed there, so fetch must
+// refuse the manifest before it creates the destination or requests any
+// file.
 func TestFetchRefusesListedManifestName(t *testing.T) {
 	t.Parallel()
 
@@ -1255,7 +1255,6 @@ func TestFetchRefusesListedManifestName(t *testing.T) {
 		tempPathFor(defaultManifestName),
 		defaultManifestName + "/" + testFileTxt,
 		"INDEX.MF",
-		"./" + defaultManifestName,
 	} {
 		t.Run(listed, func(t *testing.T) {
 			t.Parallel()
@@ -1331,9 +1330,9 @@ func TestFetchRefusesListedTempName(t *testing.T) {
 // directory, one of them under a spelling the manifest does not list,
 // and check reports that file as not in the manifest. So fetch must
 // refuse each on every filesystem before it creates the destination or
-// requests any file. A file and a directory with the same name, and a
-// file listed twice, are refused the same way. A manifest whose names
-// differ in more than letter case is fetched in full.
+// requests any file. A file and a directory with the same name are
+// refused the same way. A manifest whose names differ in more than
+// letter case is fetched in full.
 func TestFetchRefusesNamesEqualIgnoringCase(t *testing.T) {
 	t.Parallel()
 
@@ -1343,7 +1342,6 @@ func TestFetchRefusesNamesEqualIgnoringCase(t *testing.T) {
 		{"Dir", "dir/x", "dir/x (the file Dir)"},
 		{"Dir/a.txt", "dir/b.txt", "dir/b.txt (the directory Dir)"},
 		{"dir", "dir/x", "dir/x (the file dir)"},
-		{"./x.txt", "x.txt", "x.txt (the file ./x.txt)"},
 	} {
 		t.Run(tc.first+" and "+tc.second, func(t *testing.T) {
 			t.Parallel()
@@ -1573,8 +1571,7 @@ func manifestWithMode(t *testing.T, path string, content []byte, mode uint32) []
 }
 
 // builtManifest returns a manifest of files, built directly rather than
-// scanned, since a scan lists no hidden files and never a path starting
-// with "./".
+// scanned, since a scan lists no hidden files.
 func builtManifest(t *testing.T, files map[string][]byte) []byte {
 	t.Helper()
 

mfer/builder.go

@@ -34,6 +34,7 @@ var (
 	errPathAbsolute     = errors.New("is absolute; must be relative")
 	errPathEmptySegment = errors.New("contains empty segment")
 	errPathDotDot       = errors.New("contains '..' segment")
+	errPathDot          = errors.New("contains '.' segment")
 	errSizeMismatch     = errors.New("size mismatch")
 	errNegativeSize     = errors.New("size cannot be negative")
 	errHashNotMultihash = errors.New("hash is not a valid multihash")
@@ -46,6 +47,7 @@ var (
 //   - Must use forward slashes only (no backslashes)
 //   - Must be relative (no leading /)
 //   - Must not contain ".." segments
+//   - Must not contain "." segments
 //   - Must not contain empty segments (no "//")
 //   - Must not be empty
 func ValidatePath(p string) error {
@@ -73,6 +75,10 @@ func ValidatePath(p string) error {
 		if seg == ".." {
 			return fmt.Errorf("path %q %w", p, errPathDotDot)
 		}
+
+		if seg == "." {
+			return fmt.Errorf("path %q %w", p, errPathDot)
+		}
 	}
 
 	return nil

mfer/builder_test.go

@@ -286,6 +286,9 @@ func TestValidatePath(t *testing.T) {
 		{"has/../traversal", "dot-dot segment"},
 		{"has//double", "empty segment"},
 		{"..", "just dot-dot"},
+		{"./a.txt", "leading dot segment"},
+		{"has/./dot", "interior dot segment"},
+		{".", "just dot"},
 		{string([]byte{0xff, 0xfe}), "invalid UTF-8"},
 	}
 	for _, tt := range invalid {

mfer/deserialize_path_test.go

@@ -80,6 +80,8 @@ func TestDeserializeRejectsInvalidEntryPaths(t *testing.T) {
 	}{
 		{"parent traversal", "../escape"},
 		{"interior traversal", "a/../../escape"},
+		{"leading dot segment", "./a.txt"},
+		{"interior dot segment", "dir/./a.txt"},
 		{"absolute path", "/etc/passwd"},
 		{"backslash path", `a\b`},
 		{"double slash", "a//b"},

mfer/errmsg_test.go

@@ -62,6 +62,12 @@ func TestValidatePathMessagesVerbatim(t *testing.T) {
 			want: `path "a/../b" contains '..' segment`,
 			is:   errPathDotDot,
 		},
+		{
+			name: "dot segment",
+			path: "a/./b",
+			want: `path "a/./b" contains '.' segment`,
+			is:   errPathDot,
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()

mfer/openpgp.go

@@ -8,6 +8,7 @@ import (
 	"io"
 	"slices"
 	"strings"
+	"time"
 
 	"github.com/ProtonMail/go-crypto/openpgp"
 	"github.com/ProtonMail/go-crypto/openpgp/armor"
@@ -242,10 +243,10 @@ func fingerprint(key *openpgp.Entity) string {
 	return strings.ToUpper(hex.EncodeToString(key.PrimaryKey.Fingerprint))
 }
 
-// verifySignature checks that signature is one good OpenPGP signature
-// over data, made by the one primary key in pubKey or one of its subkeys,
-// and returns that primary key's fingerprint. signature and pubKey may each
-// be armored or binary.
+// verifySignature checks that signature is one good, unexpired OpenPGP
+// signature over data, made by the one primary key in pubKey or one of its
+// subkeys, and returns that primary key's fingerprint. signature and pubKey
+// may each be armored or binary.
 func verifySignature(data, signature, pubKey []byte) (string, error) {
 	key, err := readOneKey(pubKey, "embedded public key block", true)
 	if err != nil {
@@ -266,10 +267,16 @@ func verifySignature(data, signature, pubKey []byte) (string, error) {
 		return "", fmt.Errorf("%w, found %d", errNotOneSignature, sigs)
 	}
 
-	_, err = openpgp.CheckDetachedSignature(openpgp.EntityList{key},
+	sig, _, err := openpgp.VerifyDetachedSignature(openpgp.EntityList{key},
 		bytes.NewReader(data), bytes.NewReader(sigData), nil)
 	// A manifest outlives its signing key, so a signature by a key that
-	// has expired since is still good.
+	// has expired since is still good, unless the signature has expired
+	// itself. The library does not check that once it finds the key
+	// expired.
+	if errors.Is(err, pgperrors.ErrKeyExpired) && sig.SigExpired(time.Now()) {
+		err = pgperrors.ErrSignatureExpired
+	}
+
 	if err != nil && !errors.Is(err, pgperrors.ErrKeyExpired) {
 		return "", fmt.Errorf("verify signature: %w", err)
 	}

mfer/openpgp_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/ProtonMail/go-crypto/openpgp"
 	"github.com/ProtonMail/go-crypto/openpgp/armor"
 	"github.com/ProtonMail/go-crypto/openpgp/elgamal"
+	pgperrors "github.com/ProtonMail/go-crypto/openpgp/errors"
 	"github.com/ProtonMail/go-crypto/openpgp/packet"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
@@ -416,8 +417,10 @@ func TestVerifySignature(t *testing.T) {
 	assert.Error(t, err)
 }
 
-// TestVerifySignatureKeyExpiredSince verifies a signature made in 2020 by
-// a key that expired a day after it was made. The signature is still good.
+// TestVerifySignatureKeyExpiredSince verifies two signatures made in 2020
+// by a key that expired a day after it was made. A signature with no
+// expiry time is still good. One that expired an hour after it was made is
+// refused, although the key has expired too.
 func TestVerifySignatureKeyExpiredSince(t *testing.T) {
 	t.Parallel()
 
@@ -429,9 +432,14 @@ func TestVerifySignatureKeyExpiredSince(t *testing.T) {
 	key := newTestKey(t, config)
 	data := []byte("signed in 2020")
 
-	var sig bytes.Buffer
+	var sig, expiringSig bytes.Buffer
 
 	require.NoError(t, openpgp.ArmoredDetachSign(&sig, key, bytes.NewReader(data), config))
+	require.NoError(t, openpgp.ArmoredDetachSign(&expiringSig, key,
+		bytes.NewReader(data), &packet.Config{
+			Time:            config.Time,
+			SigLifetimeSecs: uint32(time.Hour.Seconds()),
+		}))
 
 	pubKey, err := armoredPublicKey(key)
 	require.NoError(t, err)
@@ -439,6 +447,9 @@ func TestVerifySignatureKeyExpiredSince(t *testing.T) {
 	signer, err := verifySignature(data, sig.Bytes(), pubKey)
 	require.NoError(t, err)
 	assert.Equal(t, fingerprint(key), signer)
+
+	_, err = verifySignature(data, expiringSig.Bytes(), pubKey)
+	require.ErrorIs(t, err, pgperrors.ErrSignatureExpired)
 }
 
 func TestManifestSignatureVerification(t *testing.T) {