Skip to content

Add basic validation for route TLS configuration - checks that input is "syntactically" valid. #8366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 17, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions contrib/completions/bash/openshift
Original file line number Diff line number Diff line change
Expand Up @@ -15651,6 +15651,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")
Expand Down
9 changes: 8 additions & 1 deletion pkg/cmd/infra/router/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -54,6 +55,7 @@ type TemplateRouter struct {
ReloadInterval time.Duration
DefaultCertificate string
DefaultCertificatePath string
ExtendedValidation bool
RouterService *ktypes.NamespacedName
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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, controller.RejectionRecorder(statusPlugin))
}
plugin := controller.NewUniqueHost(nextPlugin, o.RouteSelectionFunc(), controller.RejectionRecorder(statusPlugin))

factory := o.RouterSelection.NewFactory(oc, kc)
controller := factory.Create(plugin)
Expand Down
2 changes: 2 additions & 0 deletions pkg/route/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ type RouteIngressConditionType string
const (
// RouteAdmitted means the route is able to service requests for the provided Host
RouteAdmitted RouteIngressConditionType = "Admitted"
// RouteExtendedValidationFailed means the route configuration failed an extended validation check.
RouteExtendedValidationFailed RouteIngressConditionType = "ExtendedValidationFailed"
// TODO: add other route condition types
)

Expand Down
99 changes: 99 additions & 0 deletions pkg/route/api/validation/validation.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package validation

import (
"crypto/tls"
"crypto/x509"
"encoding/pem"
"fmt"
"strings"

Expand Down Expand Up @@ -76,6 +79,67 @@ 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{}

if tlsConfig == nil {
return result
}

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()))
}

certKeyBytes := []byte{}
certKeyBytes = append(certKeyBytes, []byte(tlsConfig.Certificate)...)
if len(tlsConfig.Key) > 0 {
certKeyBytes = append(certKeyBytes, byte('\n'))
certKeyBytes = append(certKeyBytes, []byte(tlsConfig.Key)...)
}

if _, err := tls.X509KeyPair(certKeyBytes, certKeyBytes); 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 {
Expand Down Expand Up @@ -158,3 +222,38 @@ 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
break
}
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
}
Loading