From cdb9722becf1aaeeaa1e9529dac19f3d5281f0a1 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Mon, 7 Nov 2022 13:46:53 +0400 Subject: [PATCH] feat: add support for +-5 min clock skew By default PGP library considers any key which was generated "in the future" as expired without any allowed clock skew. This fixes that, and adds some tests. Signed-off-by: Andrey Smirnov --- pkg/pgp/key.go | 35 ++++++++++++------- pkg/pgp/key_test.go | 83 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 12 deletions(-) diff --git a/pkg/pgp/key.go b/pkg/pgp/key.go index fc94527..321f91a 100644 --- a/pkg/pgp/key.go +++ b/pkg/pgp/key.go @@ -16,8 +16,10 @@ import ( pgpcrypto "github.com/ProtonMail/gopenpgp/v2/crypto" ) +// Time-related key settings. const ( - maxAllowedLifetime = 8 * time.Hour + MaxAllowedLifetime = 8 * time.Hour + AllowedClockSkew = 5 * time.Minute ) // Key represents a PGP key. It can be a public key or a private & public key pair. @@ -100,16 +102,26 @@ func (p *Key) ArmorPublic() (string, error) { return p.key.GetArmoredPublicKey() } +// IsExpired returns true if the key is expired with clock skew. +func (p *Key) IsExpired(clockSkew time.Duration) bool { + now := time.Now() + + i := p.key.GetEntity().PrimaryIdentity() + + expired := func(t time.Time) bool { + return p.key.GetEntity().PrimaryKey.KeyExpired(i.SelfSignature, t) || // primary key has expired + i.SelfSignature.SigExpired(t) // user ID self-signature has expired + } + + return expired(now.Add(clockSkew)) && expired(now.Add(-clockSkew)) +} + // Validate validates the key. func (p *Key) Validate() error { if p.key.IsRevoked() { return fmt.Errorf("key is revoked") } - if p.key.IsExpired() { - return fmt.Errorf("key is expired") - } - entity := p.key.GetEntity() if entity == nil { return fmt.Errorf("key does not contain an entity") @@ -120,6 +132,10 @@ func (p *Key) Validate() error { return fmt.Errorf("key does not contain a primary identity") } + if p.IsExpired(AllowedClockSkew) { + return fmt.Errorf("key expired") + } + _, err := mail.ParseAddress(identity.Name) if err != nil { return fmt.Errorf("key does not contain a valid email address: %w: %s", err, identity.Name) @@ -137,7 +153,7 @@ func (p *Key) validateLifetime() error { return fmt.Errorf("key does not contain a valid key lifetime") } - expiration := time.Now().Add(maxAllowedLifetime) + expiration := time.Now().Add(MaxAllowedLifetime) if !entity.PrimaryKey.KeyExpired(sig, expiration) { return fmt.Errorf("key lifetime is too long: %s", time.Duration(*sig.KeyLifetimeSecs)*time.Second) @@ -158,10 +174,5 @@ func generateEntity(name, comment, email string, lifetimeSecs uint32) (*openpgp. SigLifetimeSecs: lifetimeSecs, } - newEntity, err := openpgp.NewEntity(name, comment, email, cfg) - if err != nil { - return nil, err - } - - return newEntity, nil + return openpgp.NewEntity(name, comment, email, cfg) } diff --git a/pkg/pgp/key_test.go b/pkg/pgp/key_test.go index 686e580..1a4dbe5 100644 --- a/pkg/pgp/key_test.go +++ b/pkg/pgp/key_test.go @@ -5,9 +5,13 @@ package pgp_test import ( + "crypto" "testing" "time" + "github.com/ProtonMail/go-crypto/openpgp" + "github.com/ProtonMail/go-crypto/openpgp/packet" + pgpcrypto "github.com/ProtonMail/gopenpgp/v2/crypto" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -30,3 +34,82 @@ func TestKeyFlow(t *testing.T) { assert.Error(t, key.Verify(message[:len(message)-1], signature)) assert.Error(t, key.Verify(message, signature[:len(signature)-1])) } + +func genKey(t *testing.T, lifetimeSecs uint32, now func() time.Time) *pgp.Key { + cfg := &packet.Config{ + Algorithm: packet.PubKeyAlgoEdDSA, + DefaultHash: crypto.SHA256, + DefaultCipher: packet.CipherAES256, + DefaultCompressionAlgo: packet.CompressionZLIB, + KeyLifetimeSecs: lifetimeSecs, + SigLifetimeSecs: lifetimeSecs, + Time: now, + } + + entity, err := openpgp.NewEntity("test", "test", "keytest@example.com", cfg) + require.NoError(t, err) + + key, err := pgpcrypto.NewKeyFromEntity(entity) + require.NoError(t, err) + + pgpKey, err := pgp.NewKey(key) + require.NoError(t, err) + + return pgpKey +} + +func TestKeyExpiration(t *testing.T) { + for _, tt := range []struct { //nolint:govet + name string + lifetime time.Duration + shift time.Duration + expectedError string + }{ + { + name: "no expiration", + expectedError: "key does not contain a valid key lifetime", + }, + { + name: "expiration too long", + lifetime: pgp.MaxAllowedLifetime + 1*time.Hour, + expectedError: "key lifetime is too long: 9h0m0s", + }, + { + name: "generated in the future", + lifetime: pgp.MaxAllowedLifetime / 2, + shift: pgp.AllowedClockSkew * 2, + expectedError: "key expired", + }, + { + name: "already expired", + lifetime: pgp.MaxAllowedLifetime / 2, + shift: -pgp.AllowedClockSkew*2 - pgp.MaxAllowedLifetime/2, + expectedError: "key expired", + }, + { + name: "within clock skew -", + lifetime: pgp.MaxAllowedLifetime / 2, + shift: -pgp.AllowedClockSkew / 2, + }, + { + name: "within clock skew +", + lifetime: pgp.MaxAllowedLifetime / 2, + shift: pgp.AllowedClockSkew / 2, + }, + } { + t.Run(tt.name, func(t *testing.T) { + key := genKey(t, uint32(tt.lifetime/time.Second), func() time.Time { + return time.Now().Add(tt.shift) + }) + + err := key.Validate() + + if tt.expectedError != "" { + assert.Error(t, err) + assert.EqualError(t, err, tt.expectedError) + } else { + assert.NoError(t, err) + } + }) + } +}