Skip to content

Commit

Permalink
Only check that message signatures are newer than the key
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
twiss committed Feb 21, 2025
1 parent cfbd6f6 commit 8e272e7
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 17 deletions.
2 changes: 1 addition & 1 deletion openpgp/v2/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 7 additions & 13 deletions openpgp/v2/read.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}
Expand Down
2 changes: 1 addition & 1 deletion openpgp/v2/subkeys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions openpgp/v2/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 8e272e7

Please sign in to comment.