diff --git a/docs/FORMAT.md b/docs/FORMAT.md index 20062a5..6758f88 100644 --- a/docs/FORMAT.md +++ b/docs/FORMAT.md @@ -174,6 +174,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: diff --git a/mfer/openpgp.go b/mfer/openpgp.go index bff1eb5..cff74f1 100644 --- a/mfer/openpgp.go +++ b/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) } diff --git a/mfer/openpgp_test.go b/mfer/openpgp_test.go index 5b571bf..c6673f3 100644 --- a/mfer/openpgp_test.go +++ b/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) {