Merge pull request #233 from DmitriyMV/fix-verify-retry

Reset origText in verifySignature before the retry

crypto/signature.go

@@ -251,7 +251,7 @@ func verifySignature(
 
 	sig, signer, err := openpgp.VerifyDetachedSignatureAndHash(pubKeyEntries, origText, signatureReader, allowedHashes, config)
 
-	if sig != nil && signer != nil && (errors.Is(err, pgpErrors.ErrSignatureExpired) || errors.Is(err, pgpErrors.ErrKeyExpired)) {
+	if sig != nil && signer != nil && (errors.Is(err, pgpErrors.ErrSignatureExpired) || errors.Is(err, pgpErrors.ErrKeyExpired)) { //nolint:nestif
 		if verifyTime == 0 { // Expiration check disabled
 			err = nil
 		} else {
@@ -261,12 +261,22 @@ func verifySignature(
 				return time.Unix(verifyTime, 0)
 			}
 
+			seeker, ok := origText.(io.ReadSeeker)
+			if !ok {
+				return nil, errors.Wrap(err, "gopenpgp: message reader do not support seeking, cannot retry signature verification")
+			}
+
+			_, err = seeker.Seek(0, io.SeekStart)
+			if err != nil {
+				return nil, newSignatureFailed(errors.Wrap(err, "gopenpgp: could not rewind the data reader."))
+			}
+
 			_, err = signatureReader.Seek(0, io.SeekStart)
 			if err != nil {
 				return nil, newSignatureFailed(err)
 			}
 
-			sig, signer, err = openpgp.VerifyDetachedSignatureAndHash(pubKeyEntries, origText, signatureReader, allowedHashes, config)
+			sig, signer, err = openpgp.VerifyDetachedSignatureAndHash(pubKeyEntries, seeker, signatureReader, allowedHashes, config)
 		}
 	}
 

crypto/signature_test.go

@@ -2,15 +2,19 @@ package crypto
 
 import (
 	"bytes"
+	"crypto"
 	"errors"
 	"io"
 	"io/ioutil"
 	"regexp"
 	"testing"
+	"time"
 
+	"github.com/ProtonMail/go-crypto/openpgp"
 	"github.com/ProtonMail/go-crypto/openpgp/packet"
-	"github.com/ProtonMail/gopenpgp/v2/constants"
 	"github.com/stretchr/testify/assert"
+
+	"github.com/ProtonMail/gopenpgp/v2/constants"
 )
 
 const testMessage = "Hello world!"
@@ -610,3 +614,52 @@ func Test_VerifyDetachedWithDoubleContext(t *testing.T) {
 	// then
 	checkVerificationError(t, err, constants.SIGNATURE_BAD_CONTEXT)
 }
+
+func Test_verifySignaturExpire(t *testing.T) {
+	defer func(t int64) { pgp.latestServerTime = t }(pgp.latestServerTime)
+	pgp.latestServerTime = 0
+
+	const lifetime = uint32(time.Hour / time.Second)
+
+	cfg := &packet.Config{
+		Algorithm:              packet.PubKeyAlgoEdDSA,
+		DefaultHash:            crypto.SHA256,
+		DefaultCipher:          packet.CipherAES256,
+		DefaultCompressionAlgo: packet.CompressionZLIB,
+		KeyLifetimeSecs:        lifetime,
+		SigLifetimeSecs:        lifetime,
+	}
+
+	entity, err := openpgp.NewEntity("John Smith", "Linux", "john.smith@example.com", cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	key, err := NewKeyFromEntity(entity)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	keyRing, err := NewKeyRing(key)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	data := []byte("Hello, World!")
+	message := NewPlainMessage(data)
+
+	signature, err := keyRing.SignDetached(message)
+	if err != nil {
+		t.Fatalf("%#+v", err)
+	}
+
+	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)
+	if err != nil {
+		t.Fatal(err)
+	}
+}