Skip to content

Commit 41db25b

Browse files
committed
OCPBUGS-45290: Block Upgrades for CA-Signed Certs Using SHA1
Previously, upgrades from 4.15 to 4.16 were only blocked for leaf certs using SHA1. However, in 4.16, any SHA1 cert that is CA-signed (not self-signed) is unsupported. As a result, we were incorrectly allowing upgrades for SHA1 intermediate certificates, while blocking upgrades for self-signed SHA1 leaf certificates. This update refactors the upgrade validation plugin to address these issues by checking all certificates (server, CA, destinationCA) for SHA1 while excluding self-signed certificates from the validation.
1 parent dc38fbd commit 41db25b

File tree

2 files changed

+440
-50
lines changed

2 files changed

+440
-50
lines changed

pkg/router/routeapihelpers/validation.go

Lines changed: 93 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"encoding/pem"
1010
"fmt"
1111

12+
kerrors "k8s.io/apimachinery/pkg/util/errors"
1213
"k8s.io/apimachinery/pkg/util/validation/field"
1314
"k8s.io/client-go/util/cert"
1415

@@ -387,29 +388,100 @@ func UpgradeRouteValidation(route *routev1.Route) field.ErrorList {
387388
return result
388389
}
389390

390-
// Verify the route for incompatible SHA1 certificates within Spec.TLS.Certificate
391-
// as it will prevent HaProxy from starting.
392-
// There's no need to verify SHA1 certificates within Spec.TLS.CACertificate
393-
// as they will be rejected by the ExtendedValidator plugin.
394-
// Similarly, verifying SHA1 certificates within Spec.TLS.DestinationCACertificate
395-
// is unnecessary as it will NOT prevent HaProxy from starting.
396-
if len(tlsConfig.Certificate) > 0 {
397-
certs, err := cert.ParseCertsPEM([]byte(tlsConfig.Certificate))
398-
if err != nil {
399-
// Handling cert parsing errors, like malformed or invalid certs, isn't necessary here,
400-
// as the ExtendedValidator plugin is responsible for handling these errors.
401-
return result
402-
}
391+
// Verify the route for incompatible SHA1 CA-Signed certs within Spec.TLS.Certificate, Spec.TLS.CACertificates,
392+
// and Spec.TLS.DestinationCACertificate as they will be rejected in 4.16. Self-signed certificates using SHA1,
393+
// including root CA certificates, remain upgradeSupported in 4.16.
394+
if err := validateCertSignatureAlgorithmsForUpgrade(tlsConfig.Certificate); err != nil {
395+
tlsCertFieldPath := field.NewPath("spec").Child("tls").Child("certificate")
396+
result = append(result, field.Invalid(tlsCertFieldPath, "redacted certificate data", err.Error()))
397+
}
398+
if err := validateCertSignatureAlgorithmsForUpgrade(tlsConfig.CACertificate); err != nil {
399+
tlsCertFieldPath := field.NewPath("spec").Child("tls").Child("caCertificate")
400+
result = append(result, field.Invalid(tlsCertFieldPath, "redacted certificate data", err.Error()))
401+
}
402+
if err := validateCertSignatureAlgorithmsForUpgrade(tlsConfig.DestinationCACertificate); err != nil {
403+
tlsCertFieldPath := field.NewPath("spec").Child("tls").Child("destinationCACertificate")
404+
result = append(result, field.Invalid(tlsCertFieldPath, "redacted certificate data", err.Error()))
405+
}
403406

404-
if len(certs) < 1 {
405-
return result
406-
}
407+
return result
408+
}
407409

408-
if certs[0].SignatureAlgorithm == x509.SHA1WithRSA || certs[0].SignatureAlgorithm == x509.ECDSAWithSHA1 {
409-
tlsCertFieldPath := field.NewPath("spec").Child("tls").Child("certificate")
410-
message := "OpenShift 4.16 does not support certificates using SHA1 signature algorithms. This route will be rejected in OpenShift 4.16. To maintain functionality in OpenShift 4.16, generate a new certificate using a supported signature algorithm such as SHA256, SHA384, or SHA512, and update this route accordingly."
411-
result = append(result, field.Invalid(tlsCertFieldPath, "redacted certificate data", message))
410+
// isSelfSignedCert determines if a certificate is self-signed by verifying that the issuer matches the subject,
411+
// the authority key identifier matches the subject key identifier, and the public key algorithm matches the
412+
// signature algorithm. This logic mirrors the approach that OpenSSL uses to set the EXFLAG_SS flag, which
413+
// indicates a certificate is self-signed.
414+
// Ref: https://github.com/openssl/openssl/blob/b85e6f534906f0bf9114386d227e481d2336a0ff/crypto/x509/v3_purp.c#L557
415+
func isSelfSignedCert(cert *x509.Certificate) bool {
416+
issuerIsEqualToSubject := bytes.Equal(cert.RawIssuer, cert.RawSubject)
417+
authorityKeyIsEqualToSubjectKey := bytes.Equal(cert.AuthorityKeyId, cert.SubjectKeyId)
418+
algorithmIsConsistent := signatureAlgorithmToPublicKeyAlgorithm(cert.SignatureAlgorithm) == cert.PublicKeyAlgorithm
419+
return issuerIsEqualToSubject &&
420+
(cert.AuthorityKeyId == nil || authorityKeyIsEqualToSubjectKey) &&
421+
algorithmIsConsistent
422+
}
423+
424+
// signatureAlgorithmToPublicKeyAlgorithm maps a SignatureAlgorithm to its corresponding PublicKeyAlgorithm.
425+
// Unfortunately, the x509 library does not expose a public mapping function for this.
426+
// Returns UnknownPublicKeyAlgorithm if the mapping is not recognized.
427+
func signatureAlgorithmToPublicKeyAlgorithm(sigAlgo x509.SignatureAlgorithm) x509.PublicKeyAlgorithm {
428+
switch sigAlgo {
429+
case x509.MD2WithRSA,
430+
x509.MD5WithRSA,
431+
x509.SHA1WithRSA,
432+
x509.SHA256WithRSA,
433+
x509.SHA384WithRSA,
434+
x509.SHA512WithRSA,
435+
x509.SHA256WithRSAPSS,
436+
x509.SHA384WithRSAPSS,
437+
x509.SHA512WithRSAPSS:
438+
return x509.RSA
439+
case x509.DSAWithSHA1,
440+
x509.DSAWithSHA256:
441+
return x509.DSA
442+
case x509.ECDSAWithSHA1,
443+
x509.ECDSAWithSHA256,
444+
x509.ECDSAWithSHA384,
445+
x509.ECDSAWithSHA512:
446+
return x509.ECDSA
447+
case x509.PureEd25519:
448+
return x509.Ed25519
449+
default:
450+
return x509.UnknownPublicKeyAlgorithm
451+
}
452+
}
453+
454+
// validateCertSignatureAlgorithmsForUpgrade checks if the certificate list has any certs that use a
455+
// signature algorithm that the router will not support in the next OpenShift version. If an
456+
// unsupported cert is present, HAProxy may refuse to start (server & CA certs) or may start but
457+
// reject connections (destination CA certs).
458+
func validateCertSignatureAlgorithmsForUpgrade(pemCerts string) error {
459+
var errs []error
460+
if len(pemCerts) == 0 {
461+
return nil
462+
}
463+
certs, err := cert.ParseCertsPEM([]byte(pemCerts))
464+
if err != nil {
465+
// Handling cert parsing errors, like malformed or invalid certs, isn't necessary here,
466+
// as the ExtendedValidator plugin is responsible for handling these errors.
467+
return nil
468+
}
469+
470+
for _, cert := range certs {
471+
// Verify the signature algorithms only for certs signed by a CA.
472+
// Since OpenSSL doesn't validate self-signed certificates, the signature algorithm check can be skipped.
473+
// It's important that we do NOT reject self-signed certificates, as many root CAs still utilize SHA1.
474+
if !isSelfSignedCert(cert) {
475+
switch cert.SignatureAlgorithm {
476+
case x509.SHA1WithRSA, x509.ECDSAWithSHA1:
477+
sha1UnsupportedMsg := "OpenShift 4.16 does not support CA-signed certificates using SHA1 signature algorithms. This route " +
478+
"will be rejected in OpenShift 4.16. To maintain functionality in OpenShift 4.16, generate a new certificate " +
479+
"using a supported signature algorithm such as SHA256, SHA384, or SHA512, and update this route accordingly."
480+
errs = append(errs, fmt.Errorf(sha1UnsupportedMsg))
481+
default:
482+
// Acceptable algorithm
483+
}
412484
}
413485
}
414-
return result
486+
return kerrors.NewAggregate(errs)
415487
}

0 commit comments

Comments
 (0)