From 8e272e796276ef5ff91e878d680be94faa76ee5b Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Fri, 21 Feb 2025 14:25:05 +0100 Subject: [PATCH] Only check that message signatures are newer than the key Don't check that binding signatures are newer than the primary key that created them, as some old keys generated by previous versions of OpenPGP.js fail this check. --- openpgp/v2/keys.go | 2 +- openpgp/v2/read.go | 20 +++++++------------- openpgp/v2/subkeys.go | 2 +- openpgp/v2/user.go | 4 ++-- 4 files changed, 11 insertions(+), 17 deletions(-) diff --git a/openpgp/v2/keys.go b/openpgp/v2/keys.go index 246fa6d78..1865fcbe0 100644 --- a/openpgp/v2/keys.go +++ b/openpgp/v2/keys.go @@ -676,7 +676,7 @@ func (e *Entity) LatestValidDirectSignature(date time.Time, config *packet.Confi if sig.Valid == nil { err := e.PrimaryKey.VerifyDirectKeySignature(sig.Packet) if err == nil { - err = checkSignatureDetails(e.PrimaryKey, sig.Packet, date, config) + err = checkSignatureDetails(sig.Packet, date, config) } valid := err == nil sig.Valid = &valid diff --git a/openpgp/v2/read.go b/openpgp/v2/read.go index 5ab9aff53..7cefd37b8 100644 --- a/openpgp/v2/read.go +++ b/openpgp/v2/read.go @@ -732,19 +732,14 @@ func verifyDetachedSignatureReader(keyring KeyRing, signed, signature io.Reader, // It checks the following: // - Hash function should not be invalid according to // config.RejectHashAlgorithms. -// - Verification key must be older than the signature creation time. // - Check signature notations. // - Signature is not expired (unless a zero time is passed to // explicitly ignore expiration). -func checkSignatureDetails(pk *packet.PublicKey, signature *packet.Signature, now time.Time, config *packet.Config) error { +func checkSignatureDetails(signature *packet.Signature, now time.Time, config *packet.Config) error { if config.RejectHashAlgorithm(signature.Hash) { return errors.SignatureError("insecure hash algorithm: " + signature.Hash.String()) } - if pk.CreationTime.Unix() > signature.CreationTime.Unix() { - return errors.ErrSignatureOlderThanKey - } - for _, notation := range signature.Notations { if notation.IsCritical && !config.KnownNotation(notation.Name) { return errors.SignatureError("unknown critical notation: " + notation.Name) @@ -762,30 +757,29 @@ func checkSignatureDetails(pk *packet.PublicKey, signature *packet.Signature, no // signature and all relevant binding signatures. // In addition, the message signature hash algorithm is checked against // config.RejectMessageHashAlgorithms. +// Finally, the signature must be newer than the verification key. func checkMessageSignatureDetails(verifiedKey *Key, signature *packet.Signature, config *packet.Config) error { if config.RejectMessageHashAlgorithm(signature.Hash) { return errors.SignatureError("insecure message hash algorithm: " + signature.Hash.String()) } + if signature.CreationTime.Unix() < verifiedKey.PublicKey.CreationTime.Unix() { + return errors.ErrSignatureOlderThanKey + } + sigsToCheck := []*packet.Signature{signature, verifiedKey.PrimarySelfSignature} if !verifiedKey.IsPrimary() { sigsToCheck = append(sigsToCheck, verifiedKey.SelfSignature, verifiedKey.SelfSignature.EmbeddedSignature) } var errs []error for _, sig := range sigsToCheck { - var pk *packet.PublicKey - if sig == verifiedKey.PrimarySelfSignature || sig == verifiedKey.SelfSignature { - pk = verifiedKey.Entity.PrimaryKey - } else { - pk = verifiedKey.PublicKey - } var time time.Time if sig == signature { time = config.Now() } else { time = signature.CreationTime } - if err := checkSignatureDetails(pk, sig, time, config); err != nil { + if err := checkSignatureDetails(sig, time, config); err != nil { errs = append(errs, err) } } diff --git a/openpgp/v2/subkeys.go b/openpgp/v2/subkeys.go index 3e9fc1890..1b796f8cb 100644 --- a/openpgp/v2/subkeys.go +++ b/openpgp/v2/subkeys.go @@ -187,7 +187,7 @@ func (s *Subkey) LatestValidBindingSignature(date time.Time, config *packet.Conf if sig.Valid == nil { err := s.Primary.PrimaryKey.VerifyKeySignature(s.PublicKey, sig.Packet) if err == nil { - err = checkSignatureDetails(s.Primary.PrimaryKey, sig.Packet, date, config) + err = checkSignatureDetails(sig.Packet, date, config) } valid := err == nil sig.Valid = &valid diff --git a/openpgp/v2/user.go b/openpgp/v2/user.go index 3da03bd7c..4effa6e73 100644 --- a/openpgp/v2/user.go +++ b/openpgp/v2/user.go @@ -121,7 +121,7 @@ func (i *Identity) Revoked(selfCertification *packet.Signature, date time.Time, // Verify revocation signature (not verified yet). err := i.Primary.PrimaryKey.VerifyUserIdSignature(i.Name, i.Primary.PrimaryKey, revocation.Packet) if err == nil { - err = checkSignatureDetails(i.Primary.PrimaryKey, revocation.Packet, date, config) + err = checkSignatureDetails(revocation.Packet, date, config) } valid := err == nil revocation.Valid = &valid @@ -206,7 +206,7 @@ func (i *Identity) LatestValidSelfCertification(date time.Time, config *packet.C // Verify revocation signature (not verified yet). err = i.Primary.PrimaryKey.VerifyUserIdSignature(i.Name, i.Primary.PrimaryKey, sig.Packet) if err == nil { - err = checkSignatureDetails(i.Primary.PrimaryKey, sig.Packet, date, config) + err = checkSignatureDetails(sig.Packet, date, config) } valid := err == nil sig.Valid = &valid