Donot serve certificate content for Non-SSL routes

By default, when a host does not resolve to a route in a HTTPS or tls
sni request, the default cert is returned to the caller as part of the
503 response. This exposes the default cert and may pose security
concerns. Haproxy strict-sni option to bind suppresses use of the
default cert.

This adds a new environment variable to the router deployment controller,
ROUTER_STRICT_SNI, to control bind processing. When set to "true" or
"TRUE", "strict-sni" is added to the bind. Default is "false".

oc adm router --strict-sni sets ROUTER_STRICT_SNI="true"

bug 1369865
https://bugzilla.redhat.com/show_bug.cgi?id=1369865

Author: pecameron

contrib/completions/bash/oadm

@@ -4856,6 +4856,8 @@ _oadm_router()
     local_nonpersistent_flags+=("--stats-port=")
     flags+=("--stats-user=")
     local_nonpersistent_flags+=("--stats-user=")
+    flags+=("--strict-sni")
+    local_nonpersistent_flags+=("--strict-sni")
     flags+=("--subdomain=")
     local_nonpersistent_flags+=("--subdomain=")
     flags+=("--type=")

contrib/completions/bash/oc

@@ -4857,6 +4857,8 @@ _oc_adm_router()
     local_nonpersistent_flags+=("--stats-port=")
     flags+=("--stats-user=")
     local_nonpersistent_flags+=("--stats-user=")
+    flags+=("--strict-sni")
+    local_nonpersistent_flags+=("--strict-sni")
     flags+=("--subdomain=")
     local_nonpersistent_flags+=("--subdomain=")
     flags+=("--type=")

contrib/completions/bash/openshift

@@ -4856,6 +4856,8 @@ _openshift_admin_router()
     local_nonpersistent_flags+=("--stats-port=")
     flags+=("--stats-user=")
     local_nonpersistent_flags+=("--stats-user=")
+    flags+=("--strict-sni")
+    local_nonpersistent_flags+=("--strict-sni")
     flags+=("--subdomain=")
     local_nonpersistent_flags+=("--subdomain=")
     flags+=("--type=")
@@ -10035,6 +10037,8 @@ _openshift_cli_adm_router()
     local_nonpersistent_flags+=("--stats-port=")
     flags+=("--stats-user=")
     local_nonpersistent_flags+=("--stats-user=")
+    flags+=("--strict-sni")
+    local_nonpersistent_flags+=("--strict-sni")
     flags+=("--subdomain=")
     local_nonpersistent_flags+=("--subdomain=")
     flags+=("--type=")
@@ -23380,6 +23384,8 @@ _openshift_infra_router()
     local_nonpersistent_flags+=("--stats-port=")
     flags+=("--stats-user=")
     local_nonpersistent_flags+=("--stats-user=")
+    flags+=("--strict-sni")
+    local_nonpersistent_flags+=("--strict-sni")
     flags+=("--template=")
     local_nonpersistent_flags+=("--template=")
     flags+=("--token=")

contrib/completions/zsh/oadm

@@ -5005,6 +5005,8 @@ _oadm_router()
     local_nonpersistent_flags+=("--stats-port=")
     flags+=("--stats-user=")
     local_nonpersistent_flags+=("--stats-user=")
+    flags+=("--strict-sni")
+    local_nonpersistent_flags+=("--strict-sni")
     flags+=("--subdomain=")
     local_nonpersistent_flags+=("--subdomain=")
     flags+=("--type=")

contrib/completions/zsh/oc

@@ -5006,6 +5006,8 @@ _oc_adm_router()
     local_nonpersistent_flags+=("--stats-port=")
     flags+=("--stats-user=")
     local_nonpersistent_flags+=("--stats-user=")
+    flags+=("--strict-sni")
+    local_nonpersistent_flags+=("--strict-sni")
     flags+=("--subdomain=")
     local_nonpersistent_flags+=("--subdomain=")
     flags+=("--type=")

contrib/completions/zsh/openshift

@@ -5005,6 +5005,8 @@ _openshift_admin_router()
     local_nonpersistent_flags+=("--stats-port=")
     flags+=("--stats-user=")
     local_nonpersistent_flags+=("--stats-user=")
+    flags+=("--strict-sni")
+    local_nonpersistent_flags+=("--strict-sni")
     flags+=("--subdomain=")
     local_nonpersistent_flags+=("--subdomain=")
     flags+=("--type=")
@@ -10184,6 +10186,8 @@ _openshift_cli_adm_router()
     local_nonpersistent_flags+=("--stats-port=")
     flags+=("--stats-user=")
     local_nonpersistent_flags+=("--stats-user=")
+    flags+=("--strict-sni")
+    local_nonpersistent_flags+=("--strict-sni")
     flags+=("--subdomain=")
     local_nonpersistent_flags+=("--subdomain=")
     flags+=("--type=")
@@ -23529,6 +23533,8 @@ _openshift_infra_router()
     local_nonpersistent_flags+=("--stats-port=")
     flags+=("--stats-user=")
     local_nonpersistent_flags+=("--stats-user=")
+    flags+=("--strict-sni")
+    local_nonpersistent_flags+=("--strict-sni")
     flags+=("--template=")
     local_nonpersistent_flags+=("--template=")
     flags+=("--token=")

images/router/haproxy/conf/haproxy-config.template

@@ -204,7 +204,10 @@ backend be_sni
 
 frontend fe_sni
   # terminate ssl on edge
-  bind 127.0.0.1:{{env "ROUTER_SERVICE_SNI_PORT" "10444"}} ssl no-sslv3 {{ if gt (len .DefaultCertificate) 0 }}crt {{.DefaultCertificate}}{{ else }}crt /var/lib/haproxy/conf/default_pub_keys.pem{{ end }} crt-list /var/lib/haproxy/conf/cert_config.map accept-proxy
+  bind 127.0.0.1:{{env "ROUTER_SERVICE_SNI_PORT" "10444"}} ssl no-sslv3
+    {{- if matchPattern "true|TRUE" (env "ROUTER_STRICT_SNI" "") }} strict-sni {{ end }}
+    {{- if gt (len .DefaultCertificate) 0 }} crt {{.DefaultCertificate}}{{ else }} crt /var/lib/haproxy/conf/default_pub_keys.pem{{ end }}
+    {{- ""}} crt-list /var/lib/haproxy/conf/cert_config.map accept-proxy
   mode http
 
   # check re-encrypt backends first - from most specific to general path.

pkg/cmd/admin/router/router.go

@@ -230,6 +230,9 @@ type RouterConfig struct {
 	// Ciphers is the set of ciphers to use with bind
 	// modern | intermediate | old | set of cihers
 	Ciphers string
+
+	// Strict SNI (do not use default cert)
+	StrictSNI bool
 }
 
 const (
@@ -310,6 +313,7 @@ func NewCmdRouter(f *clientcmd.Factory, parentName, name string, out, errout io.
 	cmd.Flags().BoolVar(&cfg.DisableNamespaceOwnershipCheck, "disable-namespace-ownership-check", cfg.DisableNamespaceOwnershipCheck, "Disables the namespace ownership check and allows different namespaces to claim either different paths to a route host or overlapping host names in case of a wildcard route. The default behavior (false) to restrict claims to the oldest namespace that has claimed either the host or the subdomain. Please be aware that if namespace ownership checks are disabled, routes in a different namespace can use this mechanism to 'steal' sub-paths for existing domains. This is only safe if route creation privileges are restricted, or if all the users can be trusted.")
 	cmd.Flags().StringVar(&cfg.MaxConnections, "max-connections", cfg.MaxConnections, "Specifies the maximum number of concurrent connections. Not supported for F5.")
 	cmd.Flags().StringVar(&cfg.Ciphers, "ciphers", cfg.Ciphers, "Specifies the cipher suites to use. You can choose a predefined cipher set ('modern', 'intermediate', or 'old') or specify exact cipher suites by passing a : separated list. Not supported for F5.")
+	cmd.Flags().BoolVar(&cfg.StrictSNI, "strict-sni", cfg.StrictSNI, "Use strict-sni bind processing (do not use default cert). Not supported for F5.")
 
 	cfg.Action.BindForOutput(cmd.Flags())
 	cmd.Flags().String("output-version", "", "The preferred API versions of the output objects")
@@ -664,6 +668,9 @@ func RunCmdRouter(f *clientcmd.Factory, cmd *cobra.Command, out, errout io.Write
 	if cfg.DisableNamespaceOwnershipCheck {
 		env["ROUTER_DISABLE_NAMESPACE_OWNERSHIP_CHECK"] = "true"
 	}
+	if cfg.StrictSNI {
+		env["ROUTER_STRICT_SNI"] = "true"
+	}
 	if len(cfg.RouterCanonicalHostname) > 0 {
 		if errs := validation.IsDNS1123Subdomain(cfg.RouterCanonicalHostname); len(errs) != 0 {
 			return fmt.Errorf("invalid canonical hostname (RFC 1123): %s", cfg.RouterCanonicalHostname)

pkg/cmd/infra/router/template.go

@@ -72,6 +72,7 @@ type TemplateRouter struct {
 	BindPortsAfterSync       bool
 	MaxConnections           string
 	Ciphers                  string
+	StrictSNI                bool
 	MetricsType              string
 }
 
@@ -102,6 +103,7 @@ func (o *TemplateRouter) Bind(flag *pflag.FlagSet) {
 	flag.BoolVar(&o.BindPortsAfterSync, "bind-ports-after-sync", util.Env("ROUTER_BIND_PORTS_AFTER_SYNC", "") == "true", "Bind ports only after route state has been synchronized")
 	flag.StringVar(&o.MaxConnections, "max-connections", util.Env("ROUTER_MAX_CONNECTIONS", ""), "Specifies the maximum number of concurrent connections.")
 	flag.StringVar(&o.Ciphers, "ciphers", util.Env("ROUTER_CIPHERS", ""), "Specifies the cipher suites to use. You can choose a predefined cipher set ('modern', 'intermediate', or 'old') or specify exact cipher suites by passing a : separated list.")
+	flag.BoolVar(&o.StrictSNI, "strict-sni", util.Env("ROUTER_STRICT_SNI", "") == "true", "Use strict-sni bind processing (do not use default cert).")
 	flag.StringVar(&o.MetricsType, "metrics-type", util.Env("ROUTER_METRICS_TYPE", ""), "Specifies the type of metrics to gather. Supports 'haproxy'.")
 }
 
@@ -302,6 +304,7 @@ func (o *TemplateRouterOptions) Run() error {
 		AllowWildcardRoutes:      o.RouterSelection.AllowWildcardRoutes,
 		MaxConnections:           o.MaxConnections,
 		Ciphers:                  o.Ciphers,
+		StrictSNI:                o.StrictSNI,
 	}
 
 	oc, kc, err := o.Config.Clients()

pkg/router/template/plugin.go

@@ -55,6 +55,7 @@ type TemplatePluginConfig struct {
 	BindPortsAfterSync       bool
 	MaxConnections           string
 	Ciphers                  string
+	StrictSNI                bool
 }
 
 // routerInterface controls the interaction of the plugin with the underlying router implementation

test/cmd/router.sh

@@ -44,6 +44,8 @@ os::cmd::expect_failure_and_text 'oadm router --dry-run --host-network=false --h
 os::cmd::expect_success_and_text 'oadm router --dry-run --host-network=false --host-ports=false --max-connections=14583 -o yaml' '14583'
 # ciphers 
 os::cmd::expect_success_and_text 'oadm router --dry-run --host-network=false --host-ports=false --ciphers=modern -o yaml' 'modern'
+# strict-sni
+os::cmd::expect_success_and_text 'oadm router --dry-run --host-network=false --host-ports=false --strict-sni -o yaml' 'ROUTER_STRICT_SNI'
 
 # mount tls crt as secret
 os::cmd::expect_success_and_not_text 'oadm router --dry-run --host-network=false --host-ports=false -o yaml' 'value: /etc/pki/tls/private/tls.crt'