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.
  o Fix status update problem - we should set the status to admitted
    only if we had no errors handling a route.
  o Rework to use a new controller - extended_validator as per
    @smarterclayton comments.
  o Cleanup validation as per @liggitt comments.
  o Update bash completions.
  o Fixup older validation unit tests.

Author: ramr

contrib/completions/bash/openshift

@@ -14868,6 +14868,7 @@ _openshift_infra_router()
     flags+=("--context=")
     flags+=("--default-certificate=")
     flags+=("--default-certificate-path=")
+    flags+=("--extended-validation")
     flags+=("--fields=")
     flags+=("--hostname-template=")
     flags+=("--include-udp-endpoints")

pkg/cmd/infra/router/template.go

@@ -15,6 +15,7 @@ import (
 
 	"github.com/openshift/origin/pkg/cmd/util"
 	"github.com/openshift/origin/pkg/cmd/util/clientcmd"
+	"github.com/openshift/origin/pkg/router"
 	"github.com/openshift/origin/pkg/router/controller"
 	templateplugin "github.com/openshift/origin/pkg/router/template"
 	"github.com/openshift/origin/pkg/util/proc"
@@ -54,6 +55,7 @@ type TemplateRouter struct {
 	ReloadInterval         time.Duration
 	DefaultCertificate     string
 	DefaultCertificatePath string
+	ExtendedValidation     bool
 	RouterService          *ktypes.NamespacedName
 }
 
@@ -77,6 +79,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.ExtendedValidation, "extended-validation", util.Env("EXTENDED_VALIDATION", "") == "true", "If set, then an additional extended validation step is performed on all routes admitted in by this router.")
 }
 
 type RouterStats struct {
@@ -192,7 +195,11 @@ func (o *TemplateRouterOptions) Run() error {
 	}
 
 	statusPlugin := controller.NewStatusAdmitter(templatePlugin, oc, o.RouterName)
-	plugin := controller.NewUniqueHost(statusPlugin, o.RouteSelectionFunc(), statusPlugin)
+	var nextPlugin router.Plugin = statusPlugin
+	if o.ExtendedValidation {
+		nextPlugin = controller.NewExtendedValidator(nextPlugin, statusPlugin)
+	}
+	plugin := controller.NewUniqueHost(nextPlugin, o.RouteSelectionFunc(), statusPlugin)
 
 	factory := o.RouterSelection.NewFactory(oc, kc)
 	controller := factory.Create(plugin)

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,60 @@ func ValidateRouteStatusUpdate(route *routeapi.Route, older *routeapi.Route) fie
 	return allErrs
 }
 
+// ExtendedValidateRoute performs an extended validation on the route
+// including checking that the TLS config is valid.
+func ExtendedValidateRoute(route *routeapi.Route) field.ErrorList {
+	tlsConfig := 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 := ""
+	var certPool *x509.CertPool
+
+	if len(tlsConfig.CACertificate) > 0 {
+		certPool = x509.NewCertPool()
+		if ok := certPool.AppendCertsFromPEM([]byte(tlsConfig.CACertificate)); !ok {
+			result = append(result, field.Invalid(tlsFieldPath.Child("caCertificate"), tlsConfig.CACertificate, "failed to parse CA certificate"))
+		}
+	}
+
+	verifyOptions := &x509.VerifyOptions{
+		DNSName: hostname,
+		Roots:   certPool,
+	}
+
+	if len(tlsConfig.Certificate) > 0 {
+		if _, err := validateCertificatePEM(tlsConfig.Certificate, verifyOptions); err != nil {
+			result = append(result, field.Invalid(tlsFieldPath.Child("certificate"), tlsConfig.Certificate, err.Error()))
+		}
+	}
+
+	if len(tlsConfig.Key) > 0 {
+		if len(tlsConfig.Certificate) > 0 {
+			if _, err := tls.X509KeyPair([]byte(tlsConfig.Certificate), []byte(tlsConfig.Key)); err != nil {
+				result = append(result, field.Invalid(tlsFieldPath.Child("key"), tlsConfig.Key, err.Error()))
+			}
+		}
+	}
+
+	if len(tlsConfig.DestinationCACertificate) > 0 {
+		roots := x509.NewCertPool()
+		if ok := roots.AppendCertsFromPEM([]byte(tlsConfig.DestinationCACertificate)); !ok {
+			result = append(result, field.Invalid(tlsFieldPath.Child("destinationCACertificate"), tlsConfig.DestinationCACertificate, "failed to parse destination CA certificate"))
+		}
+	}
+
+	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 +236,37 @@ func validateInsecureEdgeTerminationPolicy(tls *routeapi.TLSConfig, fldPath *fie
 
 	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) {
+	var data *pem.Block
+	for remaining := []byte(certPEM); len(remaining) > 0; {
+		block, rest := pem.Decode(remaining)
+		if block == nil {
+			return nil, fmt.Errorf("error decoding certificate data")
+		}
+		if block.Type == "CERTIFICATE" {
+			data = block
+		}
+		remaining = rest
+	}
+
+	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
+}