Cleanup fixed time and allow using time offset for crypto operations

ProtonMail · May 9, 2023 · 056fbc6 · 056fbc6
1 parent d37f4eb
commit 056fbc6
Show file tree

Hide file tree

Showing 11 changed files with 85 additions and 67 deletions.
diff --git a/crypto/attachment_manual_test.go b/crypto/attachment_manual_test.go
@@ -9,8 +9,8 @@ import (
 )
 
 func TestManualAttachmentProcessor(t *testing.T) {
-	pgp.latestServerTime = 1615394034
-	defer func() { pgp.latestServerTime = testTime }()
+	pgp.fixedTime = 1615394034
+	defer func() { pgp.fixedTime = testTime }()
 	passphrase := []byte("wUMuF/lkDPYWH/0ZqqY8kJKw7YJg6kS")
 	pk, err := NewKeyFromArmored(readTestFile("att_key", false))
 	if err != nil {
@@ -86,8 +86,8 @@ func TestManualAttachmentProcessor(t *testing.T) {
 }
 
 func TestManualAttachmentProcessorNotEnoughBuffer(t *testing.T) {
-	pgp.latestServerTime = 1615394034
-	defer func() { pgp.latestServerTime = testTime }()
+	pgp.fixedTime = 1615394034
+	defer func() { pgp.fixedTime = testTime }()
 	passphrase := []byte("wUMuF/lkDPYWH/0ZqqY8kJKw7YJg6kS")
 	pk, err := NewKeyFromArmored(readTestFile("att_key", false))
 	if err != nil {
@@ -141,8 +141,8 @@ func TestManualAttachmentProcessorNotEnoughBuffer(t *testing.T) {
 }
 
 func TestManualAttachmentProcessorEmptyBuffer(t *testing.T) {
-	pgp.latestServerTime = 1615394034
-	defer func() { pgp.latestServerTime = testTime }()
+	pgp.fixedTime = 1615394034
+	defer func() { pgp.fixedTime = testTime }()
 	passphrase := []byte("wUMuF/lkDPYWH/0ZqqY8kJKw7YJg6kS")
 	pk, err := NewKeyFromArmored(readTestFile("att_key", false))
 	if err != nil {

diff --git a/crypto/base_test.go b/crypto/base_test.go
@@ -27,7 +27,7 @@ func readTestFile(name string, trimNewlines bool) string {
 }
 
 func init() {
-	UpdateTime(testTime) // 2019-05-13T13:37:07+00:00
+	SetFixedTime(testTime) // 2019-05-13T13:37:07+00:00
 
 	initGenerateKeys()
 	initArmoredKeys()

diff --git a/crypto/gopenpgp.go b/crypto/gopenpgp.go
@@ -6,15 +6,15 @@ import "sync"
 // GopenPGP is used as a "namespace" for many of the functions in this package.
 // It is a struct that keeps track of time skew between server and client.
 type GopenPGP struct {
-	latestServerTime int64
-	generationOffset int64
-	lock             *sync.RWMutex
+	fixedTime  int64
+	timeOffset int64
+	lock       *sync.RWMutex
 }
 
 var pgp = GopenPGP{
-	latestServerTime: 0,
-	generationOffset: 0,
-	lock:             &sync.RWMutex{},
+	fixedTime:  0,
+	timeOffset: 0,
+	lock:       &sync.RWMutex{},
 }
 
 // clone returns a clone of the byte slice. Internal function used to make sure

diff --git a/crypto/key.go b/crypto/key.go
@@ -447,7 +447,7 @@ func generateKey(
 	cfg := &packet.Config{
 		Algorithm:              packet.PubKeyAlgoRSA,
 		RSABits:                bits,
-		Time:                   getKeyGenerationTimeGenerator(),
+		Time:                   getTimeGenerator(),
 		DefaultHash:            crypto.SHA256,
 		DefaultCipher:          packet.CipherAES256,
 		DefaultCompressionAlgo: packet.CompressionZLIB,

diff --git a/crypto/key_test.go b/crypto/key_test.go
@@ -221,6 +221,25 @@ func TestIsExpired(t *testing.T) {
 	assert.Exactly(t, true, futureKey.IsExpired())
 }
 
+func TestGeneratedWithOffset(t *testing.T) {
+	defer SetFixedTime(testTime)
+	SetFixedTime(0)
+	defer SetTimeOffset(0)
+	SetTimeOffset(30)
+
+	// generate key with offset
+	keyTestRSA, err := GenerateKey(keyTestName, keyTestDomain, "rsa", 1024)
+	if err != nil {
+		panic("Cannot generate RSA key:" + err.Error())
+	}
+
+	// Bring back offset to zero
+	SetTimeOffset(0)
+
+	// Verify if key was generated with offset
+	assert.GreaterOrEqual(t, keyTestRSA.entity.PrimaryKey.CreationTime.Unix(), GetUnixTime()+30)
+}
+
 func TestGenerateKeyWithPrimes(t *testing.T) {
 	prime1, _ := base64.StdEncoding.DecodeString(
 		"/thF8zjjk6fFx/y9NId35NFx8JTA7jvHEl+gI0dp9dIl9trmeZb+ESZ8f7bNXUmTI8j271kyenlrVJiqwqk80Q==")
@@ -418,9 +437,9 @@ func TestKeyCapabilities(t *testing.T) {
 }
 
 func TestRevokedKeyCapabilities(t *testing.T) {
-	pgp.latestServerTime = 1632219895
+	pgp.fixedTime = 1632219895
 	defer func() {
-		pgp.latestServerTime = testTime
+		pgp.fixedTime = testTime
 	}()
 
 	revokedKey, err := NewKeyFromArmored(readTestFile("key_revoked", false))

diff --git a/crypto/keyring_test.go b/crypto/keyring_test.go
@@ -237,9 +237,9 @@ func TestKeyringCapabilities(t *testing.T) {
 
 func TestVerificationTime(t *testing.T) {
 	message := NewPlainMessageFromString("Hello")
-	pgp.latestServerTime = 1632312383
+	pgp.fixedTime = 1632312383
 	defer func() {
-		pgp.latestServerTime = testTime
+		pgp.fixedTime = testTime
 	}()
 	enc, err := keyRingTestPublic.Encrypt(
 		message,

diff --git a/crypto/message_test.go b/crypto/message_test.go
@@ -211,9 +211,9 @@ func TestBinaryMessageEncryption(t *testing.T) {
 }
 
 func TestIssue11(t *testing.T) {
-	pgp.latestServerTime = 1559655272
+	pgp.fixedTime = 1559655272
 	defer func() {
-		pgp.latestServerTime = testTime
+		pgp.fixedTime = testTime
 	}()
 
 	var issue11Password = []byte("1234")
@@ -258,8 +258,8 @@ func TestIssue11(t *testing.T) {
 }
 
 func TestDummy(t *testing.T) {
-	pgp.latestServerTime = 1636644417
-	defer func() { pgp.latestServerTime = testTime }()
+	pgp.fixedTime = 1636644417
+	defer func() { pgp.fixedTime = testTime }()
 
 	dummyKey, err := NewKeyFromArmored(readTestFile("key_dummy", false))
 	if err != nil {

diff --git a/crypto/signature_test.go b/crypto/signature_test.go
@@ -141,9 +141,9 @@ func TestVerifyBinDetachedSig(t *testing.T) {
 func Test_KeyRing_GetVerifiedSignatureTimestampSuccess(t *testing.T) {
 	message := NewPlainMessageFromString(testMessage)
 	var time int64 = 1600000000
-	pgp.latestServerTime = time
+	pgp.fixedTime = time
 	defer func() {
-		pgp.latestServerTime = testTime
+		pgp.fixedTime = testTime
 	}()
 	signature, err := keyRingTestPrivate.SignDetached(message)
 	if err != nil {
@@ -161,9 +161,9 @@ func Test_KeyRing_GetVerifiedSignatureTimestampSuccess(t *testing.T) {
 func Test_KeyRing_GetVerifiedSignatureTimestampWithContext(t *testing.T) {
 	message := NewPlainMessageFromString(testMessage)
 	var time int64 = 1600000000
-	pgp.latestServerTime = time
+	pgp.fixedTime = time
 	defer func() {
-		pgp.latestServerTime = testTime
+		pgp.fixedTime = testTime
 	}()
 	var testContext = "test-context"
 	signature, err := keyRingTestPrivate.SignDetachedWithContext(message, NewSigningContext(testContext, true))
@@ -261,9 +261,9 @@ func getTimestampOfIssuer(signature *PGPSignature, keyID uint64) int64 {
 func Test_KeyRing_GetVerifiedSignatureTimestampError(t *testing.T) {
 	message := NewPlainMessageFromString(testMessage)
 	var time int64 = 1600000000
-	pgp.latestServerTime = time
+	pgp.fixedTime = time
 	defer func() {
-		pgp.latestServerTime = testTime
+		pgp.fixedTime = testTime
 	}()
 	signature, err := keyRingTestPrivate.SignDetached(message)
 	if err != nil {
@@ -616,12 +616,13 @@ func Test_VerifyDetachedWithDoubleContext(t *testing.T) {
 }
 
 func Test_verifySignaturExpire(t *testing.T) {
-	defer func(t int64) { pgp.latestServerTime = t }(pgp.latestServerTime)
-	pgp.latestServerTime = 0
+	defer func(t int64) { pgp.fixedTime = t }(pgp.fixedTime)
+	pgp.fixedTime = 0
 
 	const lifetime = uint32(time.Hour / time.Second)
 
 	cfg := &packet.Config{
+		Time:                   GetTime,
 		Algorithm:              packet.PubKeyAlgoEdDSA,
 		DefaultHash:            crypto.SHA256,
 		DefaultCipher:          packet.CipherAES256,
@@ -655,10 +656,7 @@ func Test_verifySignaturExpire(t *testing.T) {
 
 	sig := NewPGPSignature(signature.GetBinary())
 
-	// packet.PublicKey.KeyExpired will return false here because PublicKey CreationTime has
-	// nanosecond precision, while pgpcrypto.GetUnixTime() has only second precision.
-	// Adjust the check time to be in the future to ensure that the key is not expired.
-	err = keyRing.VerifyDetached(message, sig, GetUnixTime()+1)
+	err = keyRing.VerifyDetached(message, sig, GetUnixTime())
 	if err != nil {
 		t.Fatal(err)
 	}

diff --git a/crypto/time.go b/crypto/time.go
@@ -4,22 +4,22 @@ import (
 	"time"
 )
 
-// UpdateTime updates cached time.
-func UpdateTime(newTime int64) {
+// SetFixedTime updates fixed time used for crypto operations.
+// Setting 0 will cause system time to be used.
+func SetFixedTime(newTime int64) {
 	pgp.lock.Lock()
 	defer pgp.lock.Unlock()
 
-	if newTime > pgp.latestServerTime {
-		pgp.latestServerTime = newTime
-	}
+	pgp.fixedTime = newTime
 }
 
-// SetKeyGenerationOffset updates the offset when generating keys.
-func SetKeyGenerationOffset(offset int64) {
+// SetTimeOffset updates time offset used for crypto operations.
+// Offset will be applied to all crypto operations unless fixed time is used.
+func SetTimeOffset(newOffset int64) {
 	pgp.lock.Lock()
 	defer pgp.lock.Unlock()
 
-	pgp.generationOffset = offset
+	pgp.timeOffset = newOffset
 }
 
 // GetUnixTime gets latest cached time.
@@ -39,31 +39,14 @@ func getNow() time.Time {
 	pgp.lock.RLock()
 	defer pgp.lock.RUnlock()
 
-	if pgp.latestServerTime == 0 {
-		return time.Now()
+	if pgp.fixedTime == 0 {
+		return time.Unix(time.Now().Unix()+pgp.timeOffset, 0)
 	}
 
-	return time.Unix(pgp.latestServerTime, 0)
+	return time.Unix(pgp.fixedTime, 0)
 }
 
 // getTimeGenerator Returns a time generator function.
 func getTimeGenerator() func() time.Time {
 	return getNow
 }
-
-// getNowKeyGenerationOffset returns the current time with the key generation offset.
-func getNowKeyGenerationOffset() time.Time {
-	pgp.lock.RLock()
-	defer pgp.lock.RUnlock()
-
-	if pgp.latestServerTime == 0 {
-		return time.Unix(time.Now().Unix()+pgp.generationOffset, 0)
-	}
-
-	return time.Unix(pgp.latestServerTime+pgp.generationOffset, 0)
-}
-
-// getKeyGenerationTimeGenerator Returns a time generator function with the key generation offset.
-func getKeyGenerationTimeGenerator() func() time.Time {
-	return getNowKeyGenerationOffset
-}
diff --git a/crypto/time_test.go b/crypto/time_test.go
@@ -7,11 +7,29 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestTime(t *testing.T) {
-	UpdateTime(1571072494)
+func Test_SetFixedTime(t *testing.T) {
+	defer SetFixedTime(testTime)
+	SetFixedTime(1571072494)
 	time.Sleep(1 * time.Second)
 	now := GetUnixTime()
 
-	assert.Exactly(t, int64(1571072494), now) // Use latest server time
-	UpdateTime(testTime)
+	assert.Exactly(t, int64(1571072494), now) // Use fixed time
+}
+
+func Test_TimeOffset(t *testing.T) {
+	defer SetFixedTime(testTime)
+	defer SetTimeOffset(0)
+	SetFixedTime(testTime)
+	SetTimeOffset(30)
+	time.Sleep(1 * time.Second)
+	now := GetUnixTime()
+
+	assert.Exactly(t, int64(testTime), now) // Use fixed time without offset
+
+	SetFixedTime(0)
+	SetTimeOffset(0)
+	now = GetUnixTime()
+	SetTimeOffset(30)
+
+	assert.GreaterOrEqual(t, GetUnixTime(), now+30) // Use offset with no fixed time
 }
diff --git a/helper/base_test.go b/helper/base_test.go
@@ -24,5 +24,5 @@ func readTestFile(name string, trimNewlines bool) string {
 var testMailboxPassword = []byte("apple")
 
 func init() {
-	crypto.UpdateTime(testTime) // 2019-05-13T13:37:07+00:00
+	crypto.SetFixedTime(testTime) // 2019-05-13T13:37:07+00:00
 }