From ba8a8468375ddc1443285fdb9b80d8c1553dd7dc Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Fri, 17 Feb 2023 13:46:16 +0100 Subject: [PATCH 1/4] Update go-crypto --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index bfca986..be26c51 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 115d161..51ebe1d 100644 --- a/go.sum +++ b/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-20230124153114-0acdc8ae009b/go.mod h1:I0gYDMZ6Z5GRU7l58bNFSkPTFN6Yl12dsUlAZ8xy98g= +github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8 h1:wPbRQzjjwFc0ih8puEVAOFGELsn1zoIIYdxvML7mDxA= +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= From 379e4814e057cb626d6b7e93e0bde09222b19d96 Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Wed, 15 Feb 2023 17:54:37 +0100 Subject: [PATCH 2/4] More strictly verify detached signatures Reject detached signatures from revoked and expired keys. --- crypto/signature.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crypto/signature.go b/crypto/signature.go index 67c1d7c..3b15ba3 100644 --- a/crypto/signature.go +++ b/crypto/signature.go @@ -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 signer != nil && (errors.Is(err, pgpErrors.ErrSignatureExpired) || errors.Is(err, pgpErrors.ErrKeyExpired)) { + if verifyTime == 0 { // Expiration check disabled + return 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 { @@ -147,13 +150,10 @@ func verifySignature(pubKeyEntries openpgp.EntityList, origText io.Reader, signa return newSignatureFailed() } - signer, err = openpgp.CheckDetachedSignatureAndHash(pubKeyEntries, origText, signatureReader, allowedHashes, config) - if err != nil { - return newSignatureFailed() - } + sig, signer, err = openpgp.VerifyDetachedSignatureAndHash(pubKeyEntries, origText, signatureReader, allowedHashes, config) } - if signer == nil { + if err != nil || sig == nil || signer == nil { return newSignatureFailed() } From 9d05b3e9b68ae5dde7912e0a774c8a7bd5dce2d7 Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Wed, 15 Feb 2023 18:04:47 +0100 Subject: [PATCH 3/4] Use returned signature in GetVerifiedSignatureTimestamp Instead of parsing the signature packets manually, use the signature packet returned by VerifyDetachedSignatureAndHash to get the signature creation time. --- crypto/keyring_message.go | 43 ++++++++++--------------------------- crypto/keyring_streaming.go | 3 ++- crypto/signature.go | 12 +++++------ 3 files changed, 19 insertions(+), 39 deletions(-) diff --git a/crypto/keyring_message.go b/crypto/keyring_message.go index 1c429eb..999ed73 100644 --- a/crypto/keyring_message.go +++ b/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)) - var err error - var p packet.Packet - for { - p, err = packets.Next() - if errors.Is(err, io.EOF) { - break - } - 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 + sigPacket, err := verifySignature( + keyRing.entities, + message.NewReader(), + signature.GetBinary(), + verifyTime, + ) + if err != nil { + return 0, err } - return 0, errors.Wrap(err, "gopenpgp: can't verify any signature packets") + return sigPacket.CreationTime.Unix(), nil } // ------ INTERNAL FUNCTIONS ------- diff --git a/crypto/keyring_streaming.go b/crypto/keyring_streaming.go index f742a9b..51ef80d 100644 --- a/crypto/keyring_streaming.go +++ b/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 diff --git a/crypto/signature.go b/crypto/signature.go index 3b15ba3..609b347 100644 --- a/crypto/signature.go +++ b/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 { @@ -134,9 +134,9 @@ func verifySignature(pubKeyEntries openpgp.EntityList, origText io.Reader, signa sig, signer, err := openpgp.VerifyDetachedSignatureAndHash(pubKeyEntries, origText, signatureReader, allowedHashes, config) - if 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)) { if verifyTime == 0 { // Expiration check disabled - return nil + return sig, nil } // Maybe the creation time offset pushed it over the edge @@ -147,15 +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() } sig, signer, err = openpgp.VerifyDetachedSignatureAndHash(pubKeyEntries, origText, signatureReader, allowedHashes, config) } if err != nil || sig == nil || signer == nil { - return newSignatureFailed() + return nil, newSignatureFailed() } - return nil + return sig, nil } From ff75643943dfbe0ed0a902a4c8550cf243f7f37c Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Wed, 1 Mar 2023 17:40:52 +0100 Subject: [PATCH 4/4] Update CHANGELOG --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 882507f..c92f1ba 100644 --- a/CHANGELOG.md +++ b/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