Merge pull request #214 from ProtonMail/update-go-crypto

Update go crypto, and improve signature verification

CHANGELOG.md

@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+# Changed
+- Update `github.com/ProtonMail/go-crypto` to the latest version
+- More strictly verify detached signatures: reject detached signatures from revoked and expired keys.
+- In `GetVerifiedSignatureTimestamp`, use the new `VerifyDetachedSignatureAndHash` function to get the verified signature, instead of parsing the signature packets manually to get the timestamp.
+
 ## [2.5.2] 2022-01-25
 # Changed
 - Update `github.com/ProtonMail/go-crypto` to the latest version

crypto/keyring_message.go

@@ -84,12 +84,13 @@ func (keyRing *KeyRing) SignDetached(message *PlainMessage) (*PGPSignature, erro
 // VerifyDetached verifies a PlainMessage with a detached PGPSignature
 // and returns a SignatureVerificationError if fails.
 func (keyRing *KeyRing) VerifyDetached(message *PlainMessage, signature *PGPSignature, verifyTime int64) error {
-	return verifySignature(
+	_, err := verifySignature(
 		keyRing.entities,
 		message.NewReader(),
 		signature.GetBinary(),
 		verifyTime,
 	)
+	return err
 }
 
 // SignDetachedEncrypted generates and returns a PGPMessage
@@ -126,38 +127,16 @@ func (keyRing *KeyRing) VerifyDetachedEncrypted(message *PlainMessage, encrypted
 // returns the creation time of the signature if it succeeds
 // and returns a SignatureVerificationError if fails.
 func (keyRing *KeyRing) GetVerifiedSignatureTimestamp(message *PlainMessage, signature *PGPSignature, verifyTime int64) (int64, error) {
-	packets := packet.NewReader(bytes.NewReader(signature.Data))
+	sigPacket, err := verifySignature(
-	var err error
+		keyRing.entities,
-	var p packet.Packet
+		message.NewReader(),
-	for {
+		signature.GetBinary(),
-		p, err = packets.Next()
+		verifyTime,
-		if errors.Is(err, io.EOF) {
+	)
-			break
+	if err != nil {
-		}
+		return 0, err
-		if err != nil {
-			continue
-		}
-		sigPacket, ok := p.(*packet.Signature)
-		if !ok {
-			continue
-		}
-		var outBuf bytes.Buffer
-		err = sigPacket.Serialize(&outBuf)
-		if err != nil {
-			continue
-		}
-		err = verifySignature(
-			keyRing.entities,
-			message.NewReader(),
-			outBuf.Bytes(),
-			verifyTime,
-		)
-		if err != nil {
-			continue
-		}
-		return sigPacket.CreationTime.Unix(), nil
 	}
-	return 0, errors.Wrap(err, "gopenpgp: can't verify any signature packets")
+	return sigPacket.CreationTime.Unix(), nil
 }
 
 // ------ INTERNAL FUNCTIONS -------

crypto/keyring_streaming.go

@@ -324,12 +324,13 @@ func (keyRing *KeyRing) VerifyDetachedStream(
 	signature *PGPSignature,
 	verifyTime int64,
 ) error {
-	return verifySignature(
+	_, err := verifySignature(
 		keyRing.entities,
 		message,
 		signature.GetBinary(),
 		verifyTime,
 	)
+	return err
 }
 
 // SignDetachedEncryptedStream generates and returns a PGPMessage

crypto/signature.go

@@ -119,7 +119,7 @@ func verifyDetailsSignature(md *openpgp.MessageDetails, verifierKey *KeyRing) er
 }
 
 // verifySignature verifies if a signature is valid with the entity list.
-func verifySignature(pubKeyEntries openpgp.EntityList, origText io.Reader, signature []byte, verifyTime int64) error {
+func verifySignature(pubKeyEntries openpgp.EntityList, origText io.Reader, signature []byte, verifyTime int64) (*packet.Signature, error) {
 	config := &packet.Config{}
 	if verifyTime == 0 {
 		config.Time = func() time.Time {
@@ -132,10 +132,13 @@ func verifySignature(pubKeyEntries openpgp.EntityList, origText io.Reader, signa
 	}
 	signatureReader := bytes.NewReader(signature)
 
-	signer, err := openpgp.CheckDetachedSignatureAndHash(pubKeyEntries, origText, signatureReader, allowedHashes, config)
+	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 verifyTime == 0 { // Expiration check disabled
+			return sig, nil
+		}
 
-	if errors.Is(err, pgpErrors.ErrSignatureExpired) && signer != nil && verifyTime > 0 {
-		// if verifyTime = 0: time check disabled, everything is okay
 		// Maybe the creation time offset pushed it over the edge
 		// Retry with the actual verification time
 		config.Time = func() time.Time {
@@ -144,18 +147,15 @@ func verifySignature(pubKeyEntries openpgp.EntityList, origText io.Reader, signa
 
 		_, err = signatureReader.Seek(0, io.SeekStart)
 		if err != nil {
-			return newSignatureFailed()
+			return nil, newSignatureFailed()
 		}
 
-		signer, err = openpgp.CheckDetachedSignatureAndHash(pubKeyEntries, origText, signatureReader, allowedHashes, config)
+		sig, signer, err = openpgp.VerifyDetachedSignatureAndHash(pubKeyEntries, origText, signatureReader, allowedHashes, config)
-		if err != nil {
-			return newSignatureFailed()
-		}
 	}
 
-	if signer == nil {
+	if err != nil || sig == nil || signer == nil {
-		return newSignatureFailed()
+		return nil, newSignatureFailed()
 	}
 
-	return nil
+	return sig, nil
 }

go.mod

@@ -3,7 +3,7 @@ module github.com/ProtonMail/gopenpgp/v2
 go 1.15
 
 require (
-	github.com/ProtonMail/go-crypto v0.0.0-20230124153114-0acdc8ae009b
+	github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8
 	github.com/ProtonMail/go-mime v0.0.0-20221031134845-8fd9bc37cf08
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/pkg/errors v0.9.1

go.sum

@@ -1,6 +1,6 @@
 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
-github.com/ProtonMail/go-crypto v0.0.0-20230124153114-0acdc8ae009b h1:1DHH9haxfhaVM8owXQjLdn7UP4AkDfzSdiRoLdcSCqE=
+github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8 h1:wPbRQzjjwFc0ih8puEVAOFGELsn1zoIIYdxvML7mDxA=
-github.com/ProtonMail/go-crypto v0.0.0-20230124153114-0acdc8ae009b/go.mod h1:I0gYDMZ6Z5GRU7l58bNFSkPTFN6Yl12dsUlAZ8xy98g=
+github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8/go.mod h1:I0gYDMZ6Z5GRU7l58bNFSkPTFN6Yl12dsUlAZ8xy98g=
 github.com/ProtonMail/go-mime v0.0.0-20221031134845-8fd9bc37cf08 h1:dS7r5z4iGS0qCjM7UwWdsEMzQesUQbGcXdSm2/tWboA=
 github.com/ProtonMail/go-mime v0.0.0-20221031134845-8fd9bc37cf08/go.mod h1:qRZgbeASl2a9OwmsV85aWwRqic0NHPh+9ewGAzb4cgM=
 github.com/ProtonMail/go-mobile v0.0.0-20210326110230-f181c70e4e2b h1:XVeh08xp93T+xK6rzpCSQTZ+LwEo+ASHvOifrQ5ZgEE=