Refuse a signature made when its key was not valid (closes #209)

verifySignature lets a key that has expired since verify a signature,
because a manifest outlives its key, but it checked the key only as of
now. It accepted a signature dated after its key expired, after the
key's self-signature or the signing subkey's binding signature expired,
or before the key was made. It now verifies a second time with the
library's clock set to the signature's creation time and refuses the
signature if the key or subkey was not valid then. docs/FORMAT.md
states the rule.

Judgement call: the tests make the refused signatures with the
library's packet API, because openpgp.DetachSign will not sign with a
key that is not valid at its clock's time.

Model: opus-5-5

README.md

@@ -300,9 +300,6 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues).
     - 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
-    - refuses a signed manifest, leaving it as it was and naming the fingerprint
-      of the key that signed it, unless `--sign-key` or `MFER_SIGN_KEY` names a
-      key to sign the new one with
 - `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

docs/FORMAT.md

@@ -75,15 +75,15 @@ After decompressing `innerMessage`, the result is a serialized `MFFile`
 
 Each file entry contains:
 
-| Field      | Number | Type                      | Description                         |
-| ---------- | ------ | ------------------------- | ----------------------------------- |
-| `path`     | 1      | string                    | Relative file path (see Path Rules) |
-| `size`     | 2      | int64                     | File size in bytes                  |
-| `hashes`   | 3      | repeated `MFFileChecksum` | At least one hash required          |
-| `mimeType` | 301    | string (optional)         | MIME type                           |
-| `mtime`    | 302    | Timestamp (optional)      | Modification time                   |
-| `ctime`    | 303    | Timestamp (optional)      | Change time (inode metadata change) |
-| `mode`     | 304    | uint32                    | Permission bits (see File Mode)     |
+| Field      | Number | Type                      | Description                                                   |
+| ---------- | ------ | ------------------------- | ------------------------------------------------------------- |
+| `path`     | 1      | string                    | Relative file path (see Path Rules)                           |
+| `size`     | 2      | int64                     | File size in bytes                                            |
+| `hashes`   | 3      | repeated `MFFileChecksum` | At least one hash required; readers reject an entry with none |
+| `mimeType` | 301    | string (optional)         | MIME type                                                     |
+| `mtime`    | 302    | Timestamp (optional)      | Modification time                                             |
+| `ctime`    | 303    | Timestamp (optional)      | Change time (inode metadata change)                           |
+| `mode`     | 304    | uint32                    | Permission bits (see File Mode)                               |
 
 ## File Mode
 
@@ -174,6 +174,17 @@ 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 signature is good only if the key that made it was valid at the signature's
+creation time. The primary key is judged by the newest self-signature on its
+primary user ID, and a subkey by its newest binding signature. By the
+signature's creation time the primary key and that self-signature must have been
+made and neither may have expired, and when a subkey made the signature the same
+holds of the subkey and its binding signature. A signature made before that
+self-signature or binding signature is not good, even if an older one made the
+key valid at that time. A key that has expired since 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/freshen.go

@@ -28,13 +28,15 @@ const (
 	scanProgressInterval = 100
 )
 
-// errEntryMissingMtime indicates a manifest entry that carries no
-// modification time where one is required to carry it forward unchanged.
-var errEntryMissingMtime = errors.New("manifest entry has no mtime")
+var (
+	// errEntryMissingMtime indicates a manifest entry that carries no
+	// modification time where one is required to carry it forward unchanged.
+	errEntryMissingMtime = errors.New("manifest entry has no mtime")
 
-// errSignKeyNeeded indicates a signed manifest given to freshen with no
-// key to sign the rewritten one.
-var errSignKeyNeeded = errors.New("give --sign-key to keep it signed")
+	// errEntryMissingHash indicates a manifest entry that carries no hash
+	// to carry forward.
+	errEntryMissingHash = errors.New("manifest entry has no hash")
+)
 
 // FreshenStatus contains progress information for the freshen operation.
 type FreshenStatus struct {
@@ -479,10 +481,9 @@ func runFreshenHash(
 }
 
 // loadExistingEntries loads the manifest and indexes its file entries
-// by path. It refuses a signed manifest when no --sign-key is given, as
-// the rewritten manifest would be unsigned.
+// by path.
 func (mfa *CLIApp) loadExistingEntries(
-	cmd *cli.Command, manifestPath string,
+	manifestPath string,
 ) (map[string]*mfer.MFFilePath, error) {
 	log.Infof("loading manifest from %s", manifestPath)
 
@@ -495,10 +496,6 @@ func (mfa *CLIApp) loadExistingEntries(
 		return nil, fmt.Errorf("load manifest: %w", err)
 	}
 
-	if signer := manifest.Signer(); signer != nil && cmd.String("sign-key") == "" {
-		return nil, fmt.Errorf("manifest is signed by %s: %w", signer, errSignKeyNeeded)
-	}
-
 	existingFiles := manifest.Files()
 	log.Infof("manifest contains %d files", len(existingFiles))
 
@@ -529,7 +526,7 @@ func (mfa *CLIApp) freshenManifestOperation(
 		return err
 	}
 
-	existingByPath, err := mfa.loadExistingEntries(cmd, manifestPath)
+	existingByPath, err := mfa.loadExistingEntries(manifestPath)
 	if err != nil {
 		return err
 	}
@@ -653,6 +650,8 @@ func addFileToBuilder(
 // Entries reach this path only when recordEntry classified them as
 // unchanged, which requires a recorded mtime, so an absent mtime here is
 // an error rather than something to paper over with the Unix epoch.
+// Loading refuses an entry with no hash; one that got here anyway is an
+// error too, not a file to leave out of the manifest.
 func addExistingToBuilder(b *mfer.Builder, entry *mfer.MFFilePath) error {
 	mtime, ok := entryMtime(entry)
 	if !ok {
@@ -660,7 +659,7 @@ func addExistingToBuilder(b *mfer.Builder, entry *mfer.MFFilePath) error {
 	}
 
 	if len(entry.GetHashes()) == 0 {
-		return nil
+		return fmt.Errorf("%w: %s", errEntryMissingHash, entry.GetPath())
 	}
 
 	err := b.AddFileWithHash(mfer.RelFilePath(entry.GetPath()),

internal/cli/freshen_test.go

@@ -471,6 +471,24 @@ func TestFreshenAddExistingRejectsMissingMtime(t *testing.T) {
 	assert.Contains(t, err.Error(), "file1.txt")
 }
 
+// TestFreshenAddExistingRejectsMissingHash pins that an entry with no hash
+// is an error rather than a file left out of the rebuilt manifest.
+func TestFreshenAddExistingRejectsMissingHash(t *testing.T) {
+	t.Parallel()
+
+	b := mfer.NewBuilder()
+	entry := &mfer.MFFilePath{
+		Path:  "data.bin",
+		Size:  8,
+		Mtime: &mfer.Timestamp{Seconds: 1_700_000_000},
+	}
+
+	err := addExistingToBuilder(b, entry)
+	require.ErrorIs(t, err, errEntryMissingHash)
+	assert.Contains(t, err.Error(), "data.bin")
+	assert.Zero(t, b.FileCount())
+}
+
 // TestFreshenAddExistingRejectsShortHash pins that an existing manifest
 // entry whose hash the builder refuses is reported as a problem with the
 // manifest, with the command that fixes it.

internal/cli/signing_test.go

@@ -73,56 +73,6 @@ func TestGenAndFreshenSignWithKeyFile(t *testing.T) {
 	}
 }
 
-// TestFreshenSignedManifestNeedsSignKey runs freshen on a signed manifest
-// after a file is added. Without --sign-key it must fail, naming the
-// signer and --sign-key, and leave the manifest as it was; with --sign-key
-// it must rewrite the manifest, signed.
-func TestFreshenSignedManifestNeedsSignKey(t *testing.T) {
-	t.Setenv(envSignKeyPassphrase, "")
-
-	secretKey, fingerprint := testSecretKey(t, nil, 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")
-
-	opts := testOpts([]string{
-		testApp, cmdGenerate, "-q", testFlagSignKey, keyFile,
-		"-o", manifestPath, root,
-	}, fs)
-	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
-
-	before, err := afero.ReadFile(fs, manifestPath)
-	require.NoError(t, err)
-
-	writeTestFile(t, fs, filepath.Join(root, "added.txt"), "added")
-
-	opts = testOpts([]string{testApp, cmdFreshen, "-q", manifestPath}, fs)
-	assert.Equal(t, 1, runCLI(opts))
-	assert.Contains(t, testStderr(t, opts), fingerprint)
-	assert.Contains(t, testStderr(t, opts), testFlagSignKey)
-
-	after, err := afero.ReadFile(fs, manifestPath)
-	require.NoError(t, err)
-	assert.Equal(t, before, after)
-
-	opts = testOpts([]string{
-		testApp, cmdFreshen, "-q", testFlagSignKey, keyFile, manifestPath,
-	}, fs)
-	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
-
-	opts = testOpts([]string{
-		testApp, cmdCheck, "-q",
-		"--" + flagRequireSignature, fingerprint, manifestPath,
-	}, fs)
-	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
-	assert.Len(t, manifestFiles(t, fs, manifestPath), 2)
-}
-
 // TestSignWithProtectedKeyNeedsPassphrase runs gen with a protected key,
 // with MFER_SIGN_KEY_PASSPHRASE empty and no terminal to ask on. gen must
 // fail, naming the variable, and write no manifest.

mfer/deserialize.go

@@ -27,6 +27,7 @@ var (
 	errUUIDMismatch         = errors.New("outer and inner UUID mismatch")
 	errInvalidFileFormat    = errors.New("invalid file format")
 	errInvalidManifestPath  = errors.New("invalid file entry")
+	errEntryMissingHash     = errors.New("file entry has no hash")
 	errDecodedTooLarge      = errors.New("too much memory needed")
 	errSignerNotSigningKey  = errors.New(
 		"signer is not the fingerprint of the key that made the signature")
@@ -293,7 +294,8 @@ func (m *manifest) deserializeInner() error {
 	// untrusted .mf. Reject loudly on the first offender rather than
 	// dropping entries, which would let a hostile manifest hide files from a
 	// check. A path listed twice is refused too: check would check the one
-	// file against both entries.
+	// file against both entries. So is an entry with no hash: there is
+	// nothing to check the file against.
 	seen := make(map[string]bool, len(m.pbInner.GetFiles()))
 
 	for _, f := range m.pbInner.GetFiles() {
@@ -302,6 +304,10 @@ func (m *manifest) deserializeInner() error {
 			return fmt.Errorf("%w: %w", errInvalidManifestPath, err)
 		}
 
+		if len(f.GetHashes()) == 0 {
+			return fmt.Errorf("%w: %q", errEntryMissingHash, f.GetPath())
+		}
+
 		if seen[f.GetPath()] {
 			return fmt.Errorf("%w %q", errDuplicatePath, f.GetPath())
 		}

mfer/deserialize_path_test.go

@@ -165,6 +165,24 @@ func TestDeserializeRefusesPathListedTwice(t *testing.T) {
 	}
 }
 
+// A manifest holding a file entry with no hash is refused as it is loaded,
+// naming the path: there is nothing to check the file against.
+func TestDeserializeRefusesEntryWithNoHash(t *testing.T) {
+	t.Parallel()
+
+	id := uuid.NewV4()
+	inner, err := proto.Marshal(&MFFile{
+		Version: MFFile_VERSION_ONE,
+		Files:   []*MFFilePath{{Path: "x", Size: 1}},
+		Uuid:    id[:],
+	})
+	require.NoError(t, err)
+
+	_, err = NewManifestFromReader(bytes.NewReader(wrapInner(t, id, inner)))
+	require.ErrorIs(t, err, errEntryMissingHash)
+	require.EqualError(t, err, `file entry has no hash: "x"`)
+}
+
 // Entries of a path, an empty hash, an empty MIME type and empty modification
 // and change times are counted at 432 bytes each (176 + 112 + 16 + 64 + 64)
 // and take 16 bytes plus the path to encode. A 37-character path makes that
@@ -229,6 +247,8 @@ func TestDeserializeDropsUnknownFields(t *testing.T) {
 
 	entry := protowire.AppendTag(nil, 1, protowire.BytesType) // MFFilePath.path
 	entry = protowire.AppendString(entry, "a")
+	entry = protowire.AppendTag(entry, 3, protowire.BytesType) // MFFilePath.hashes
+	entry = protowire.AppendBytes(entry, nil)
 	entry = append(entry, unknown...)
 
 	id := uuid.NewV4()

mfer/manifest.go

@@ -47,17 +47,6 @@ func (m *manifest) Files() []*MFFilePath {
 	return m.pbInner.GetFiles()
 }
 
-// Signer returns the fingerprint of the key that made a loaded manifest's
-// signature, which loading the manifest checked, or nil if the manifest is
-// not signed.
-func (m *manifest) Signer() []byte {
-	if len(m.pbOuter.GetSignature()) == 0 {
-		return nil
-	}
-
-	return m.pbOuter.GetSigner()
-}
-
 // signatureString generates the canonical string used for signing/verification.
 // Format: MAGIC-UUID-MULTIHASH where UUID and multihash are hex-encoded.
 // Requires pbOuter to be set with Uuid and Sha256 fields.

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"
@@ -56,7 +57,9 @@ var (
 		"signature must hold exactly one signature")
 	errNotOneArmoredBlock = errors.New(
 		"must be exactly one armored block and nothing else")
-	errMalformedArmor = errors.New("armor is malformed")
+	errMalformedArmor        = errors.New("armor is malformed")
+	errKeyNotValidWhenSigned = errors.New(
+		"signature was made when its key was not valid")
 )
 
 // SigningOptions holds the key a manifest is signed with.
@@ -242,10 +245,11 @@ 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 while that key was valid, and returns that primary key's
+// fingerprint. The key may have expired since. 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,14 +270,39 @@ 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)
 	}
 
+	// The library checks whether the key is valid at the time its config
+	// gives, which above is now. Verifying again as of the signature's
+	// creation time refuses a signature made before the key or subkey that
+	// made it was made, after it expired, or after its self-signature or
+	// binding signature expired. The library judges the primary key by the
+	// newest self-signature on its primary user ID and a subkey by its
+	// newest binding signature, so this also refuses a signature made
+	// before either of those, even if an older one was valid then.
+	signedAt := &packet.Config{
+		Time: func() time.Time { return sig.CreationTime },
+	}
+
+	_, _, err = openpgp.VerifyDetachedSignature(openpgp.EntityList{key},
+		bytes.NewReader(data), bytes.NewReader(sigData), signedAt)
+	if err != nil {
+		return "", fmt.Errorf("verify signature: %w: %w",
+			errKeyNotValidWhenSigned, err)
+	}
+
 	return fingerprint(key), nil
 }
 

mfer/openpgp_test.go

@@ -4,6 +4,7 @@ package mfer
 import (
 	"bytes"
 	"context"
+	"crypto"
 	"crypto/dsa" //nolint:staticcheck // SA1019: tests need a DSA key to refuse
 	"io"
 	"math/big"
@@ -18,6 +19,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"
@@ -175,25 +177,6 @@ func TestBuilderWithSigning(t *testing.T) {
 		"-----BEGIN PGP PUBLIC KEY BLOCK-----")
 }
 
-// TestManifestSigner checks that Signer gives the signing key's
-// fingerprint for a signed manifest, and nil once the signature is taken
-// out, although the signer field is still there.
-func TestManifestSigner(t *testing.T) {
-	t.Parallel()
-
-	key := newTestKey(t, nil)
-	data := signedTestManifest(t, &SigningOptions{SecretKey: armoredSecretKeys(t, key)})
-
-	signed, err := NewManifestFromReader(bytes.NewReader(data))
-	require.NoError(t, err)
-	assert.Equal(t, fingerprint(key), string(signed.Signer()))
-
-	unsigned, err := NewManifestFromReader(bytes.NewReader(
-		rewriteOuter(t, data, func(outer *MFFileOuter) { outer.Signature = nil })))
-	require.NoError(t, err)
-	assert.Nil(t, unsigned.Signer())
-}
-
 func TestScannerWithSigning(t *testing.T) {
 	t.Parallel()
 
@@ -435,8 +418,11 @@ 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, as is one made then by a signing subkey that
+// expired a day after it was made. One that expired an hour after it was
+// made is refused, although the key has expired too.
 func TestVerifySignatureKeyExpiredSince(t *testing.T) {
 	t.Parallel()
 
@@ -448,9 +434,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)
@@ -458,6 +449,131 @@ func TestVerifySignatureKeyExpiredSince(t *testing.T) {
 	signer, err := verifySignature(data, sig.Bytes(), pubKey)
 	require.NoError(t, err)
 	assert.Equal(t, fingerprint(key), signer)
+
+	withSubkey := newTestKey(t, &packet.Config{Time: config.Time})
+	require.NoError(t, withSubkey.AddSigningSubkey(&packet.Config{
+		Algorithm: packet.PubKeyAlgoEdDSA, Time: config.Time,
+		KeyLifetimeSecs: config.KeyLifetimeSecs,
+	}))
+
+	var subkeySig bytes.Buffer
+
+	require.NoError(t, openpgp.ArmoredDetachSign(&subkeySig, withSubkey,
+		bytes.NewReader(data), config))
+
+	subkeyPubKey, err := armoredPublicKey(withSubkey)
+	require.NoError(t, err)
+
+	_, err = verifySignature(data, subkeySig.Bytes(), subkeyPubKey)
+	require.NoError(t, err)
+
+	_, err = verifySignature(data, expiringSig.Bytes(), pubKey)
+	require.ErrorIs(t, err, pgperrors.ErrSignatureExpired)
+}
+
+// signDated returns a detached signature over data by signer, dated when.
+// openpgp.DetachSign signs only with a key that is valid at the time its
+// config gives, so this makes the signature packet without that check.
+func signDated(t *testing.T, signer *packet.PrivateKey, data []byte,
+	when time.Time,
+) []byte {
+	t.Helper()
+
+	sig := &packet.Signature{
+		SigType:      packet.SigTypeBinary,
+		PubKeyAlgo:   signer.PubKeyAlgo,
+		Hash:         crypto.SHA256,
+		CreationTime: when,
+		IssuerKeyId:  &signer.KeyId,
+	}
+
+	h, err := sig.PrepareSign(nil)
+	require.NoError(t, err)
+
+	_, err = h.Write(data)
+	require.NoError(t, err)
+
+	require.NoError(t, sig.Sign(h, signer, nil))
+
+	var buf bytes.Buffer
+
+	require.NoError(t, sig.Serialize(&buf))
+
+	return buf.Bytes()
+}
+
+// TestVerifySignatureRefusesKeyNotValidWhenSigned verifies signatures
+// dated when the key or subkey that made them was not valid: after the key
+// expired; after the key's self-signature, or the signing subkey's binding
+// signature, expired, the key or subkey having expired since as well; and
+// before the key was made, also by a key made in 2100. Each must be
+// refused.
+func TestVerifySignatureRefusesKeyNotValidWhenSigned(t *testing.T) {
+	t.Parallel()
+
+	made := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
+	twoDaysLater := made.Add(48 * time.Hour)
+	day := uint32((24 * time.Hour).Seconds())
+	clock := func(when time.Time) func() time.Time {
+		return func() time.Time { return when }
+	}
+
+	expiredAfterADay := newTestKey(t, &packet.Config{
+		Time: clock(made), KeyLifetimeSecs: day,
+	})
+	selfSignatureExpiredAfterADay := newTestKey(t, &packet.Config{
+		Time: clock(made), KeyLifetimeSecs: 30 * day, SigLifetimeSecs: day,
+	})
+
+	withSubkey := newTestKey(t, &packet.Config{Time: clock(made)})
+	require.NoError(t, withSubkey.AddSigningSubkey(&packet.Config{
+		Algorithm: packet.PubKeyAlgoEdDSA, Time: clock(made),
+		KeyLifetimeSecs: 30 * day, SigLifetimeSecs: day,
+	}))
+	subkey := withSubkey.Subkeys[len(withSubkey.Subkeys)-1].PrivateKey
+
+	madeADayLater := newTestKey(t, &packet.Config{
+		Time: clock(made.Add(24 * time.Hour)),
+	})
+	madeIn2100 := newTestKey(t, &packet.Config{
+		Time: clock(time.Date(2100, 1, 1, 0, 0, 0, 0, time.UTC)),
+	})
+
+	for name, test := range map[string]struct {
+		key    *openpgp.Entity
+		signer *packet.PrivateKey
+		when   time.Time
+	}{
+		"after the key expired": {
+			expiredAfterADay, expiredAfterADay.PrivateKey, twoDaysLater,
+		},
+		"after the self-signature expired": {
+			selfSignatureExpiredAfterADay,
+			selfSignatureExpiredAfterADay.PrivateKey, twoDaysLater,
+		},
+		"after the binding signature expired": {
+			withSubkey, subkey, twoDaysLater,
+		},
+		"before the key was made": {
+			madeADayLater, madeADayLater.PrivateKey, made,
+		},
+		"before a key made in 2100": {
+			madeIn2100, madeIn2100.PrivateKey, made,
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			data := []byte("signed in 2020")
+
+			pubKey, err := armoredPublicKey(test.key)
+			require.NoError(t, err)
+
+			_, err = verifySignature(data,
+				signDated(t, test.signer, data, test.when), pubKey)
+			require.ErrorIs(t, err, errKeyNotValidWhenSigned)
+		})
+	}
 }
 
 func TestManifestSignatureVerification(t *testing.T) {