Skip to content
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.

Sign up for GitHub

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

Implement insecureEdgeTermination options for reencrypt and pasthrough routes #11953

Merged
merged 1 commit into from
Dec 1, 2016
Merged

Implement insecureEdgeTermination options for reencrypt and pasthrough routes #11953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion images/router/haproxy/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ RUN INSTALL_PKGS="haproxy" && \
yum clean all && \
mkdir -p /var/lib/haproxy/router/{certs,cacerts} && \
mkdir -p /var/lib/haproxy/{conf,run,bin,log} && \
touch /var/lib/haproxy/conf/{{os_http_be,os_edge_http_be,os_tcp_be,os_sni_passthrough,os_reencrypt,os_edge_http_expose,os_edge_http_redirect,cert_config,os_wildcard_domain}.map,haproxy.config} && \
touch /var/lib/haproxy/conf/{{os_http_be,os_edge_http_be,os_tcp_be,os_sni_passthrough,os_reencrypt,os_route_http_expose,os_route_http_redirect,cert_config,os_wildcard_domain}.map,haproxy.config} && \
chmod -R 777 /var && \
setcap 'cap_net_bind_service=ep' /usr/sbin/haproxy

Expand Down
44 changes: 26 additions & 18 deletions images/router/haproxy/conf/haproxy-config.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,21 +104,21 @@ frontend public
http-request replace-header Host (.*):.* \1

# check if we need to redirect/force using https.
acl secure_redirect base,map_beg(/var/lib/haproxy/conf/os_edge_http_redirect.map) -m found
acl secure_redirect base,map_beg(/var/lib/haproxy/conf/os_route_http_redirect.map) -m found
redirect scheme https if secure_redirect

{{ if matchPattern "true|TRUE" (env "ROUTER_ALLOW_WILDCARD_ROUTES" "")}}
# Check for wildcard domains with redirected http routes.
acl wildcard_domain hdr(host),map_reg(/var/lib/haproxy/conf/os_wildcard_domain.map) -m found

acl wildcard_secure_redirect base,map_reg(/var/lib/haproxy/conf/os_edge_http_redirect.map) -m found
acl wildcard_secure_redirect base,map_reg(/var/lib/haproxy/conf/os_route_http_redirect.map) -m found
redirect scheme https if wildcard_domain wildcard_secure_redirect

{{ end }}

# Check if it is an edge route exposed insecurely.
acl edge_http_expose base,map_beg(/var/lib/haproxy/conf/os_edge_http_expose.map) -m found
use_backend be_edge_http_%[base,map_beg(/var/lib/haproxy/conf/os_edge_http_expose.map)] if edge_http_expose
# Check if it is an edge or reencrypt route exposed insecurely.
acl route_http_expose base,map_beg(/var/lib/haproxy/conf/os_route_http_expose.map) -m found
use_backend %[base,map_beg(/var/lib/haproxy/conf/os_route_http_expose.map)] if route_http_expose

# map to http backend
# Search from most specific to general path (host case).
Expand All @@ -127,8 +127,8 @@ frontend public

{{ if matchPattern "true|TRUE" (env "ROUTER_ALLOW_WILDCARD_ROUTES" "")}}
# Check for wildcard domains with exposed http routes.
acl wildcard_edge_http_expose base,map_reg(/var/lib/haproxy/conf/os_edge_http_expose.map) -m found
use_backend be_edge_http_%[base,map_reg(/var/lib/haproxy/conf/os_edge_http_expose.map)] if wildcard_domain wildcard_edge_http_expose
acl wildcard_route_http_expose base,map_reg(/var/lib/haproxy/conf/os_route_http_expose.map) -m found
use_backend %[base,map_reg(/var/lib/haproxy/conf/os_route_http_expose.map)] if wildcard_domain wildcard_route_http_expose

# map to http backend
# Search from most specific to general path (host case).
Expand Down Expand Up @@ -576,38 +576,46 @@ backend be_secure_{{$cfgIdx}}
{{ end }}{{/* end edge http host map template */}}

{{/*
os_edge_http_expose.map: contains a mapping of www.example.com -> <service name>.
Map is used to also expose edge terminated routes via an insecure scheme
os_route_http_expose.map: contains a mapping of www.example.com -> <service name>.
Map is used to also expose edge terminated and reencrypt routes via an insecure scheme
(http) if acls match for routes with insecure option set to expose.
*/}}
{{ define "/var/lib/haproxy/conf/os_edge_http_expose.map" }}
{{ define "/var/lib/haproxy/conf/os_route_http_expose.map" }}
{{ range $idx, $cfg := .State }}
{{ if and (ne $cfg.Host "") (and (eq $cfg.TLSTermination "edge") (eq $cfg.InsecureEdgeTerminationPolicy "Allow"))}}
{{ if and (ne $cfg.Host "") (and (or (eq $cfg.TLSTermination "edge") (eq $cfg.TLSTermination "reencrypt")) (eq $cfg.InsecureEdgeTerminationPolicy "Allow"))}}
{{ if $cfg.IsWildcard }}
{{genSubdomainWildcardRegexp $cfg.Host $cfg.Path false}} {{$idx}}
{{ if (eq $cfg.TLSTermination "edge") }}
{{genSubdomainWildcardRegexp $cfg.Host $cfg.Path false}} be_edge_http_{{$idx}}
{{ else }}
{{genSubdomainWildcardRegexp $cfg.Host $cfg.Path false}} be_secure_{{$idx}}
{{ end }}
{{ else }}
{{$cfg.Host}}{{$cfg.Path}} {{$idx}}
{{ if (eq $cfg.TLSTermination "edge") }}
{{$cfg.Host}}{{$cfg.Path}} be_edge_http_{{$idx}}
{{ else }}
{{$cfg.Host}}{{$cfg.Path}} be_secure_{{$idx}}
{{ end }}
{{ end }}
{{ end }}
{{ end }}
{{ end }}{{/* end edge insecure expose http host map template */}}
{{ end }}{{/* end edge and reencrypt expose http host map template */}}

{{/*
os_edge_http_redirect.map: contains a mapping of www.example.com -> <service name>.
os_route_http_redirect.map: contains a mapping of www.example.com -> <service name>.
Map is used to redirect insecure traffic to use a secure scheme (https)
if acls match for routes that have the insecure option set to redirect.
*/}}
{{ define "/var/lib/haproxy/conf/os_edge_http_redirect.map" }}
{{ define "/var/lib/haproxy/conf/os_route_http_redirect.map" }}
{{ range $idx, $cfg := .State }}
{{ if and (ne $cfg.Host "") (and (eq $cfg.TLSTermination "edge") (eq $cfg.InsecureEdgeTerminationPolicy "Redirect"))}}
{{ if and (ne $cfg.Host "") (eq $cfg.InsecureEdgeTerminationPolicy "Redirect")}}
{{ if $cfg.IsWildcard }}
{{genSubdomainWildcardRegexp $cfg.Host $cfg.Path false}} {{$idx}}
{{ else }}
{{$cfg.Host}}{{$cfg.Path}} {{$idx}}
{{ end }}
{{ end }}
{{ end }}
{{ end }}{{/* end edge insecure redirect http host map template */}}
{{ end }}{{/* end redirect http host map template */}}


{{/*
Expand Down
24 changes: 14 additions & 10 deletions pkg/route/api/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,23 +264,27 @@ func validateInsecureEdgeTerminationPolicy(tls *routeapi.TLSConfig, fldPath *fie
return nil
}

// Ensure insecure is set only for edge terminated routes.
if routeapi.TLSTerminationEdge != tls.Termination {
// tls.InsecureEdgeTerminationPolicy option is not supported for a non edge-terminated routes.
return field.Invalid(fldPath, tls.InsecureEdgeTerminationPolicy, "InsecureEdgeTerminationPolicy is only allowed for edge-terminated routes")
}

// It is an edge-terminated route, check insecure option value is
// It is an edge-terminated or reencrypt route, check insecure option value is
// one of None(for disable), Allow or Redirect.
allowedValues := map[routeapi.InsecureEdgeTerminationPolicyType]struct{}{
routeapi.InsecureEdgeTerminationPolicyNone: {},
routeapi.InsecureEdgeTerminationPolicyAllow: {},
routeapi.InsecureEdgeTerminationPolicyRedirect: {},
}

if _, ok := allowedValues[tls.InsecureEdgeTerminationPolicy]; !ok {
msg := fmt.Sprintf("invalid value for InsecureEdgeTerminationPolicy option, acceptable values are %s, %s, %s, or empty", routeapi.InsecureEdgeTerminationPolicyNone, routeapi.InsecureEdgeTerminationPolicyAllow, routeapi.InsecureEdgeTerminationPolicyRedirect)
return field.Invalid(fldPath, tls.InsecureEdgeTerminationPolicy, msg)
switch tls.Termination {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider updating the unit test for this function to ensure against regression.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marun updated the unit test

case routeapi.TLSTerminationReencrypt:
fallthrough
case routeapi.TLSTerminationEdge:
if _, ok := allowedValues[tls.InsecureEdgeTerminationPolicy]; !ok {
msg := fmt.Sprintf("invalid value for InsecureEdgeTerminationPolicy option, acceptable values are %s, %s, %s, or empty", routeapi.InsecureEdgeTerminationPolicyNone, routeapi.InsecureEdgeTerminationPolicyAllow, routeapi.InsecureEdgeTerminationPolicyRedirect)
return field.Invalid(fldPath, tls.InsecureEdgeTerminationPolicy, msg)
}
case routeapi.TLSTerminationPassthrough:
if routeapi.InsecureEdgeTerminationPolicyNone != tls.InsecureEdgeTerminationPolicy && routeapi.InsecureEdgeTerminationPolicyRedirect != tls.InsecureEdgeTerminationPolicy {
msg := fmt.Sprintf("invalid value for InsecureEdgeTerminationPolicy option, acceptable values are %s, %s, or empty", routeapi.InsecureEdgeTerminationPolicyNone, routeapi.InsecureEdgeTerminationPolicyRedirect)
return field.Invalid(fldPath, tls.InsecureEdgeTerminationPolicy, msg)
}
}

return nil
Expand Down
150 changes: 73 additions & 77 deletions pkg/route/api/validation/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1055,71 +1055,34 @@ func TestValidateTLS(t *testing.T) {
}
}

func TestValidateTLSInsecureEdgeTerminationPolicy(t *testing.T) {
tests := []struct {
name string
route *api.Route
}{
{
name: "Passthrough termination",
route: &api.Route{
Spec: api.RouteSpec{
TLS: &api.TLSConfig{
Termination: api.TLSTerminationPassthrough,
},
},
},
},
{
name: "Reencrypt termination",
route: &api.Route{
Spec: api.RouteSpec{
TLS: &api.TLSConfig{
Termination: api.TLSTerminationReencrypt,
DestinationCACertificate: "dca",
},
},
},
},
{
name: "Reencrypt termination DestCACert",
route: &api.Route{
Spec: api.RouteSpec{
TLS: &api.TLSConfig{
Termination: api.TLSTerminationReencrypt,
DestinationCACertificate: testDestinationCACertificate,
},
},
},
},
}
func TestValidatePassthroughInsecureEdgeTerminationPolicy(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think your intent is reasonable here, but I think it is confusing to use a test table to define the single route used in each iteration of the test loop. Consider creating the route in the loop instead and getting ride of tests.


insecureTypes := []api.InsecureEdgeTerminationPolicyType{
api.InsecureEdgeTerminationPolicyNone,
api.InsecureEdgeTerminationPolicyAllow,
api.InsecureEdgeTerminationPolicyRedirect,
"support HTTPsec",
"or maybe HSTS",
insecureTypes := map[api.InsecureEdgeTerminationPolicyType]bool{
"": false,
api.InsecureEdgeTerminationPolicyNone: false,
api.InsecureEdgeTerminationPolicyAllow: true,
api.InsecureEdgeTerminationPolicyRedirect: false,
"support HTTPsec": true,
"or maybe HSTS": true,
}

for _, tc := range tests {
if errs := validateTLS(tc.route, nil); len(errs) != 0 {
t.Errorf("Test case %s got %d errors where none were expected. %v",
tc.name, len(errs), errs)
for key, expected := range insecureTypes {
route := &api.Route{
Spec: api.RouteSpec{
TLS: &api.TLSConfig{
Termination: api.TLSTerminationPassthrough,
InsecureEdgeTerminationPolicy: key,
},
},
}

tc.route.Spec.TLS.InsecureEdgeTerminationPolicy = ""
if errs := validateTLS(tc.route, nil); len(errs) != 0 {
t.Errorf("Test case %s got %d errors where none were expected. %v",
tc.name, len(errs), errs)
route.Spec.TLS.InsecureEdgeTerminationPolicy = key
errs := validateTLS(route, nil)
if !expected && len(errs) != 0 {
t.Errorf("Test case for Passthrough termination with insecure=%s got %d errors where none where expected. %v",
key, len(errs), errs)
}

for _, val := range insecureTypes {
tc.route.Spec.TLS.InsecureEdgeTerminationPolicy = val
if errs := validateTLS(tc.route, nil); len(errs) != 1 {
t.Errorf("Test case %s with insecure=%q got %d errors where one was expected. %v",
tc.name, val, len(errs), errs)
}
if expected && len(errs) == 0 {
t.Errorf("Test case for Passthrough termination with insecure=%s got no errors where some where expected.", key)
}
}
}
Expand Down Expand Up @@ -1258,7 +1221,45 @@ func TestValidateInsecureEdgeTerminationPolicy(t *testing.T) {
}
}

func TestValidateNoTLSInsecureEdgeTerminationPolicy(t *testing.T) {
func TestValidateEdgeReencryptInsecureEdgeTerminationPolicy(t *testing.T) {
tests := []struct {
name string
route *api.Route
}{
{
name: "Reencrypt termination",
route: &api.Route{
Spec: api.RouteSpec{
TLS: &api.TLSConfig{
Termination: api.TLSTerminationReencrypt,
DestinationCACertificate: "dca",
},
},
},
},
{
name: "Reencrypt termination DestCACert",
route: &api.Route{
Spec: api.RouteSpec{
TLS: &api.TLSConfig{
Termination: api.TLSTerminationReencrypt,
DestinationCACertificate: testDestinationCACertificate,
},
},
},
},
{
name: "Edge termination",
route: &api.Route{
Spec: api.RouteSpec{
TLS: &api.TLSConfig{
Termination: api.TLSTerminationEdge,
},
},
},
},
}

insecureTypes := map[api.InsecureEdgeTerminationPolicyType]bool{
api.InsecureEdgeTerminationPolicyNone: false,
api.InsecureEdgeTerminationPolicyAllow: false,
Expand All @@ -1267,22 +1268,17 @@ func TestValidateNoTLSInsecureEdgeTerminationPolicy(t *testing.T) {
"or maybe HSTS": true,
}

for key, expected := range insecureTypes {
route := &api.Route{
Spec: api.RouteSpec{
TLS: &api.TLSConfig{
Termination: api.TLSTerminationEdge,
InsecureEdgeTerminationPolicy: key,
},
},
}
errs := validateTLS(route, nil)
if !expected && len(errs) != 0 {
t.Errorf("Test case for edge termination with insecure=%s got %d errors where none were expected. %v",
key, len(errs), errs)
}
if expected && len(errs) == 0 {
t.Errorf("Test case for edge termination with insecure=%s got no errors where some were expected.", key)
for _, tc := range tests {
for key, expected := range insecureTypes {
tc.route.Spec.TLS.InsecureEdgeTerminationPolicy = key
errs := validateTLS(tc.route, nil)
if !expected && len(errs) != 0 {
t.Errorf("Test case %s with insecure=%s got %d errors where none were expected. %v",
tc.name, key, len(errs), errs)
}
if expected && len(errs) == 0 {
t.Errorf("Test case %s with insecure=%s got no errors where some were expected.", tc.name, key)
}
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions pkg/router/template/router.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,9 +572,7 @@ func (r *templateRouter) AddRoute(serviceID string, weight int32, route *routeap
if tls != nil && len(tls.Termination) > 0 {
config.TLSTermination = tls.Termination

if tls.Termination == routeapi.TLSTerminationEdge {
config.InsecureEdgeTerminationPolicy = tls.InsecureEdgeTerminationPolicy
}
config.InsecureEdgeTerminationPolicy = tls.InsecureEdgeTerminationPolicy

if tls.Termination != routeapi.TLSTerminationPassthrough {
if config.Certificates == nil {
Expand Down
19 changes: 19 additions & 0 deletions test/integration/router_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,25 @@ func TestRouter(t *testing.T) {
},
routerUrl: "0.0.0.0",
},
{
name: "reencrypt-InsecureEdgePolicy",
serviceName: "example-reencrypt",
endpoints: []kapi.EndpointSubset{httpsEndpoint},
routeAlias: "www.example.com",
endpointEventType: watch.Added,
routeEventType: watch.Added,
protocol: "http",
expectedResponse: tr.HelloPodSecure,
routeTLS: &routeapi.TLSConfig{
Termination: routeapi.TLSTerminationReencrypt,
Certificate: tr.ExampleCert,
Key: tr.ExampleKey,
CACertificate: tr.ExampleCACert,
DestinationCACertificate: tr.ExampleCACert,
InsecureEdgeTerminationPolicy: routeapi.InsecureEdgeTerminationPolicyAllow,
},
routerUrl: "0.0.0.0",
},
{
name: "reencrypt-destcacert",
serviceName: "example-reencrypt-destcacert",
Expand Down