Merge pull request #275 from ProtonMail/only-check-msg-sig-newer-than…

…-key Only check that message signatures are newer than the key
ProtonMail · Feb 24, 2025 · 2732e09 · 2732e09
2 parents cfbd6f6 + 8e272e7
commit 2732e09
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 17 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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