Refuse an expired signature whose key has expired too (closes #193) #201

Merged
clawbot merged 1 commits from issue-193-expired-signature-expired-key into next 2026-10-13 19:21:53 +02:00
3 changed files with 31 additions and 9 deletions
Showing only changes of commit a6aa83b398 - Show all commits
+4
View File
@@ -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 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. 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 ## Deterministic Serialization
By default, manifests are generated deterministically: By default, manifests are generated deterministically:
+13 -6
View File
@@ -8,6 +8,7 @@ import (
"io" "io"
"slices" "slices"
"strings" "strings"
"time"
"github.com/ProtonMail/go-crypto/openpgp" "github.com/ProtonMail/go-crypto/openpgp"
"github.com/ProtonMail/go-crypto/openpgp/armor" "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)) return strings.ToUpper(hex.EncodeToString(key.PrimaryKey.Fingerprint))
} }
// verifySignature checks that signature is one good OpenPGP signature // verifySignature checks that signature is one good, unexpired OpenPGP
// over data, made by the one primary key in pubKey or one of its subkeys, // signature over data, made by the one primary key in pubKey or one of its
// and returns that primary key's fingerprint. signature and pubKey may each // subkeys, and returns that primary key's fingerprint. signature and pubKey
// be armored or binary. // may each be armored or binary.
func verifySignature(data, signature, pubKey []byte) (string, error) { 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", true)
if err != nil { if err != nil {
@@ -266,10 +267,16 @@ func verifySignature(data, signature, pubKey []byte) (string, error) {
return "", fmt.Errorf("%w, found %d", errNotOneSignature, sigs) 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) bytes.NewReader(data), bytes.NewReader(sigData), nil)
// A manifest outlives its signing key, so a signature by a key that // 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) { if err != nil && !errors.Is(err, pgperrors.ErrKeyExpired) {
return "", fmt.Errorf("verify signature: %w", err) return "", fmt.Errorf("verify signature: %w", err)
} }
+14 -3
View File
@@ -18,6 +18,7 @@ import (
"github.com/ProtonMail/go-crypto/openpgp" "github.com/ProtonMail/go-crypto/openpgp"
"github.com/ProtonMail/go-crypto/openpgp/armor" "github.com/ProtonMail/go-crypto/openpgp/armor"
"github.com/ProtonMail/go-crypto/openpgp/elgamal" "github.com/ProtonMail/go-crypto/openpgp/elgamal"
pgperrors "github.com/ProtonMail/go-crypto/openpgp/errors"
"github.com/ProtonMail/go-crypto/openpgp/packet" "github.com/ProtonMail/go-crypto/openpgp/packet"
"github.com/spf13/afero" "github.com/spf13/afero"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@@ -416,8 +417,10 @@ func TestVerifySignature(t *testing.T) {
assert.Error(t, err) assert.Error(t, err)
} }
// TestVerifySignatureKeyExpiredSince verifies a signature made in 2020 by // TestVerifySignatureKeyExpiredSince verifies two signatures made in 2020
// a key that expired a day after it was made. The signature is still good. // 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) { func TestVerifySignatureKeyExpiredSince(t *testing.T) {
t.Parallel() t.Parallel()
@@ -429,9 +432,14 @@ func TestVerifySignatureKeyExpiredSince(t *testing.T) {
key := newTestKey(t, config) key := newTestKey(t, config)
data := []byte("signed in 2020") 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(&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) pubKey, err := armoredPublicKey(key)
require.NoError(t, err) require.NoError(t, err)
@@ -439,6 +447,9 @@ func TestVerifySignatureKeyExpiredSince(t *testing.T) {
signer, err := verifySignature(data, sig.Bytes(), pubKey) signer, err := verifySignature(data, sig.Bytes(), pubKey)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, fingerprint(key), signer) assert.Equal(t, fingerprint(key), signer)
_, err = verifySignature(data, expiringSig.Bytes(), pubKey)
require.ErrorIs(t, err, pgperrors.ErrSignatureExpired)
} }
func TestManifestSignatureVerification(t *testing.T) { func TestManifestSignatureVerification(t *testing.T) {