o Add basic validation for route TLS configuration - checks that

    input is "syntactically" valid.
  o Checkpoint initial code.
  o Add support for validating route tls config.
  o Add option for validating route tls config.
  o Validation fixes.
  o Check private key + cert mismatches.
  o Add tests.
  o Record route rejection.
  o Hook into add route processing + store invalid service alias configs
    in another place - easy to check prior errors on readmission.
  o Remove entry from invalid service alias configs upon route removal.
  o Add generated completions.
  o Bug fixes.
  o Recording rejecting routes is not working completely.

Author: ramr

contrib/completions/bash/openshift

@@ -14891,6 +14891,7 @@ _openshift_infra_router()
     flags+=("--template=")
     flags+=("--token=")
     flags+=("--user=")
+    flags+=("--validate-route-tls-config")
     flags+=("--working-dir=")
     flags+=("--google-json-key=")
     flags+=("--log-flush-frequency=")

pkg/cmd/infra/router/template.go

@@ -54,6 +54,7 @@ type TemplateRouter struct {
 	ReloadInterval         time.Duration
 	DefaultCertificate     string
 	DefaultCertificatePath string
+	ValidateRouteTLSConfig bool
 	RouterService          *ktypes.NamespacedName
 }
 
@@ -77,6 +78,7 @@ func (o *TemplateRouter) Bind(flag *pflag.FlagSet) {
 	flag.StringVar(&o.TemplateFile, "template", util.Env("TEMPLATE_FILE", ""), "The path to the template file to use")
 	flag.StringVar(&o.ReloadScript, "reload", util.Env("RELOAD_SCRIPT", ""), "The path to the reload script to use")
 	flag.DurationVar(&o.ReloadInterval, "interval", reloadInterval(), "Controls how often router reloads are invoked. Mutiple router reload requests are coalesced for the duration of this interval since the last reload time.")
+	flag.BoolVar(&o.ValidateRouteTLSConfig, "validate-route-tls-config", util.Env("VALIDATE_ROUTE_TLS_CONFIG", "") == "true", "If set, then an additional validation step is performed on the TLS configuration for all routes admitted in by this router.")
 }
 
 type RouterStats struct {
@@ -179,6 +181,7 @@ func (o *TemplateRouterOptions) Run() error {
 		StatsPassword:          o.StatsPassword,
 		PeerService:            o.RouterService,
 		IncludeUDP:             o.RouterSelection.IncludeUDP,
+		ValidateRouteTLSConfig: o.ValidateRouteTLSConfig,
 	}
 
 	templatePlugin, err := templateplugin.NewTemplatePlugin(pluginCfg)

pkg/route/api/types.go

@@ -72,6 +72,8 @@ type RouteIngressConditionType string
 const (
 	// RouteAdmitted means the route is able to service requests for the provided Host
 	RouteAdmitted RouteIngressConditionType = "Admitted"
+	// RouteConfigError means the route configuration is invalid.
+	RouteConfigError RouteIngressConditionType = "ConfigError"
 	// TODO: add other route condition types
 )
 

pkg/route/api/validation/validation.go

@@ -1,6 +1,9 @@
 package validation
 
 import (
+	"crypto/tls"
+	"crypto/x509"
+	"encoding/pem"
 	"fmt"
 	"strings"
 
@@ -76,6 +79,64 @@ func ValidateRouteStatusUpdate(route *routeapi.Route, older *routeapi.Route) fie
 	return allErrs
 }
 
+// ValidateRouteTLSConfig checks if the TLS config (certificates and keys) for a route is valid.
+func ValidateRouteTLSConfig(route *routeapi.Route) field.ErrorList {
+	tls := route.Spec.TLS
+	result := field.ErrorList{}
+
+	tlsFieldPath := field.NewPath("spec").Child("tls")
+	if errs := validateTLS(route, tlsFieldPath); len(errs) != 0 {
+		result = append(result, errs...)
+	}
+
+	// TODO: Check if we can be stricter with validating the certificate
+	//       is for the route hostname. Don't want existing routes to
+	//       break, so disable the hostname validation for now.
+	// hostname := route.Spec.Host
+	hostname := ""
+	certPool := x509.NewCertPool()
+
+	if len(tls.CACertificate) > 0 {
+		cacert, err := validateCertificatePEM(tls.CACertificate, nil)
+		if err != nil {
+			result = append(result, field.Invalid(tlsFieldPath.Child("caCertificate"), tls.CACertificate, err.Error()))
+		} else {
+			certPool.AddCert(cacert)
+		}
+	}
+
+	verifyOptions := &x509.VerifyOptions{
+		DNSName: hostname,
+		Roots:   certPool,
+	}
+
+	if len(tls.Certificate) > 0 {
+		if _, err := validateCertificatePEM(tls.Certificate, verifyOptions); err != nil {
+			result = append(result, field.Invalid(tlsFieldPath.Child("certificate"), tls.Certificate, err.Error()))
+		}
+	}
+
+	if len(tls.Key) > 0 {
+		if len(tls.Certificate) > 0 {
+			if err := validatePrivateKeyForCert(tls.Key, tls.Certificate); err != nil {
+				result = append(result, field.Invalid(tlsFieldPath.Child("key"), tls.Key, err.Error()))
+			}
+		} else {
+			if err := validatePrivateKeyPEM(tls.Key); err != nil {
+				result = append(result, field.Invalid(tlsFieldPath.Child("key"), tls.Key, err.Error()))
+			}
+		}
+	}
+
+	if len(tls.DestinationCACertificate) > 0 {
+		if _, err := validateCertificatePEM(tls.DestinationCACertificate, nil); err != nil {
+			result = append(result, field.Invalid(tlsFieldPath.Child("destinationCACertificate"), tls.DestinationCACertificate, err.Error()))
+		}
+	}
+
+	return result
+}
+
 // validateTLS tests fields for different types of TLS combinations are set.  Called
 // by ValidateRoute.
 func validateTLS(route *routeapi.Route, fldPath *field.Path) field.ErrorList {
@@ -179,3 +240,71 @@ func validateInsecureEdgeTerminationPolicy(tls *routeapi.TLSConfig, fldPath *fie
 
 	return nil
 }
+
+// validatePrivateKeyForCert checks if a private key PEM is valid and
+// is valid for the certificate (if any).
+func validatePrivateKeyForCert(keyPEM, certPEM string) error {
+	if err := validatePrivateKeyPEM(keyPEM); err != nil {
+		return err
+	}
+
+	if len(certPEM) > 0 {
+		if _, err := tls.X509KeyPair([]byte(certPEM), []byte(keyPEM)); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// validatePrivateKeyPEM checks if a private key PEM is valid.
+func validatePrivateKeyPEM(keyPEM string) error {
+	if _, err := validatePEMData(keyPEM); err != nil {
+		return err
+	}
+
+	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) {
+	data, err := validatePEMData(certPEM)
+	if err != nil {
+		return nil, err
+	}
+
+	if data == nil || len(data.Bytes) < 1 {
+		return nil, fmt.Errorf("invalid/empty certificate data")
+	}
+
+	cert, err := x509.ParseCertificate(data.Bytes)
+	if err != nil {
+		return nil, fmt.Errorf("error parsing certificate: %s", err.Error())
+	}
+
+	if options != nil {
+		_, err = cert.Verify(*options)
+		if err != nil {
+			return cert, fmt.Errorf("error verifying certificate: %s", err.Error())
+		}
+	}
+
+	return cert, nil
+}
+
+// validatePEMData checks if the PEM data is valid.
+func validatePEMData(pemData string) (*pem.Block, error) {
+	block, rest := pem.Decode([]byte(pemData))
+	if block == nil {
+		return nil, fmt.Errorf("error decoding PEM data")
+	}
+
+	if len(rest) > 0 {
+		// TODO: Need to verify if failing due to extra data is ok
+		//       or if something needs it. For now, ignore it.
+		// return block, fmt.Errorf("extra data")
+	}
+
+	return block, nil
+}