Skip to content

Commit

Permalink
Adjust misordered chain logic
Browse files Browse the repository at this point in the history
Switch from considering all signature verification errors as
a confirmed identification of a misordered cert chain to
explicitly looking for `x509.InsecureAlgorithmError` and ignoring
those. In that scenario we rely exclusively on the issuer/subject
equality check.

If any other signature verification error occurs we consider
that scenario an indicator of a misordered cert chain.

No signature verification error for current/next certificate is
interpreted as the certificates being in the correct order.

refs GH-72
  • Loading branch information
atc0005 committed Dec 11, 2024
1 parent fa95a7e commit 0b603f3
Showing 1 changed file with 23 additions and 1 deletion.
24 changes: 23 additions & 1 deletion format/internal/shared/shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ package shared
import (
"crypto/x509"
"crypto/x509/pkix"
"errors"
"fmt"
"math"
"strings"
Expand Down Expand Up @@ -345,9 +346,30 @@ func HasMisorderedCerts(certChain []*x509.Certificate) bool {

// Verify the current certificate is signed by the next certificate's
// public key.
if err := currentCert.CheckSignatureFrom(nextCert); err != nil {
sigVerifyErr := nextCert.CheckSignature(
currentCert.SignatureAlgorithm,
currentCert.RawTBSCertificate,
currentCert.Signature,
)

switch {
case errors.Is(sigVerifyErr, x509.InsecureAlgorithmError(currentCert.SignatureAlgorithm)):
// NOTE: We ignore x509.InsecureAlgorithmError errors and instead
// rely solely on issuer/subject mismatches as we could be
// evaluating a certificate with a deprecated signature algorithm
// that current versions of Go object to.
//
// https://github.com/atc0005/cert-payload/issues/72
continue

case sigVerifyErr != nil:
// return fmt.Errorf("signature verification failed between certificate at index %d and %d: %w", i, i+1, err)
return true

default:
// Current certificate is signed by the next certificate's public
// key. Check the next cert.
continue
}
}

Expand Down

0 comments on commit 0b603f3

Please sign in to comment.