Skip to content

Commit

Permalink
OCPBUGS-45290: Reject All CA-Signed Certs Using SHA1
Browse files Browse the repository at this point in the history
Previously, only SHA1 leaf certs were rejected. However, in 4.16, any
SHA1 cert that is CA-signed (not self-signed) is unsupported. This led
to cases were routes with SHA1 intermediate CA certs were accepted, but
HAProxy rejects them. Self-signed SHA1 certificates (i.e. root CA)
remain supported since they are not subject to verification.

This update ensures all route certs, including the server, CA, and
destination CA certs, are inspected, and any SHA1 cert that is not
self-signed is rejected.
  • Loading branch information
gcs278 committed Dec 24, 2024
1 parent 4d9b8c4 commit 660cacd
Show file tree
Hide file tree
Showing 2 changed files with 420 additions and 38 deletions.
53 changes: 45 additions & 8 deletions pkg/router/routeapihelpers/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,11 @@ func ExtendedValidateRoute(route *routev1.Route) field.ErrorList {
} else {
tlsConfig.CACertificate = string(data)
}
// HAProxy will fail to start if intermediate CA certs use unsupported signature algorithms.
// However, root CAs can still use unsupported algorithms since they are self-signed.
if err := validateCertSignatureAlgorithms(certs); err != nil {
result = append(result, field.Invalid(tlsFieldPath.Child("caCertificate"), "redacted ca certificate data", err.Error()))
}
}

verifyOptions = &x509.VerifyOptions{
Expand Down Expand Up @@ -254,7 +259,7 @@ func ExtendedValidateRoute(route *routev1.Route) field.ErrorList {
}

if len(tlsConfig.DestinationCACertificate) > 0 {
if _, err := cert.ParseCertsPEM([]byte(tlsConfig.DestinationCACertificate)); err != nil {
if certs, err := cert.ParseCertsPEM([]byte(tlsConfig.DestinationCACertificate)); err != nil {
errmsg := fmt.Sprintf("failed to parse destination CA certificate: %v", err)
result = append(result, field.Invalid(tlsFieldPath.Child("destinationCACertificate"), "redacted destination ca certificate data", errmsg))
} else {
Expand All @@ -263,6 +268,11 @@ func ExtendedValidateRoute(route *routev1.Route) field.ErrorList {
} else {
tlsConfig.DestinationCACertificate = string(data)
}
// Unsupported destinationCACertificates algorithms won't prevent HAProxy from starting.
// However, HAProxy will quietly refuse to use them at runtime. Rejecting here improves UX.
if err := validateCertSignatureAlgorithms(certs); err != nil {
result = append(result, field.Invalid(tlsFieldPath.Child("destinationCACertificate"), "redacted ca certificate data", err.Error()))
}
}
}

Expand Down Expand Up @@ -353,6 +363,38 @@ func validateInsecureEdgeTerminationPolicy(tls *routev1.TLSConfig, fldPath *fiel
return nil
}

// isSelfSignedCert checks if a certificate is self-signed by comparing the issuer with the subject
// and the authority key identifier with the subject key identifier. This logic mirrors the approach
// that OpenSSL uses to set the EXFLAG_SS flag, which indicates a certificate is self-signed.
// Ref: https://man.openbsd.org/X509_get_extension_flags.3#EXFLAG_SS
func isSelfSignedCert(cert *x509.Certificate) bool {
isIssuerEqualToSubject := bytes.Equal(cert.RawIssuer, cert.RawSubject)
isAuthorityKeyEqualToSubjectKey := bytes.Equal(cert.AuthorityKeyId, cert.SubjectKeyId)
return isIssuerEqualToSubject && isAuthorityKeyEqualToSubjectKey
}

// validateCertSignatureAlgorithms checks if the certificate list has any certs that use a
// signature algorithm that the router no longer supports. If an unsupported cert is present, HAProxy
// may refuse to start (server & CA certs) or may start but reject connections (destination CA certs).
func validateCertSignatureAlgorithms(certs []*x509.Certificate) error {
for _, cert := range certs {
// Verify the signature algorithms only for certs signed by a CA.
// Since OpenSSL doesn't validate self-signed certificates, the signature algorithm check can be skipped.
// It's important that we do NOT reject self-signed certificates, as many root CAs still utilize SHA1.
if !isSelfSignedCert(cert) {
switch certs[0].SignatureAlgorithm {
case x509.SHA1WithRSA, x509.ECDSAWithSHA1:
return fmt.Errorf("router does not support CA-signed certs using SHA1")
case x509.MD5WithRSA:
return fmt.Errorf("router does not support CA-signed certs using MD5")
default:
// Acceptable algorithm
}
}
}
return nil
}

// validateCertificatePEM checks if a certificate PEM is valid and
// optionally verifies the certificate using the options.
func validateCertificatePEM(certPEM string, options *x509.VerifyOptions) ([]*x509.Certificate, error) {
Expand All @@ -366,13 +408,8 @@ func validateCertificatePEM(certPEM string, options *x509.VerifyOptions) ([]*x50
}

// Reject any unsupported cert algorithms as HaProxy will refuse to start with them.
switch certs[0].SignatureAlgorithm {
case x509.SHA1WithRSA, x509.ECDSAWithSHA1:
return certs, fmt.Errorf("router does not support certs using SHA1")
case x509.MD5WithRSA:
return certs, fmt.Errorf("router does not support certs using MD5")
default:
// Acceptable algorithm
if err := validateCertSignatureAlgorithms(certs); err != nil {
return certs, err
}

if options != nil {
Expand Down
Loading

0 comments on commit 660cacd

Please sign in to comment.