From e23f8c0e3105b03505f0f2fb5c3cb33eafc7be19 Mon Sep 17 00:00:00 2001 From: ramr Date: Wed, 18 Jul 2018 14:29:51 -0700 Subject: [PATCH] Fix for ip whitelist exceeding max config arguments haproxy allows. fixes bugz #1598738 --- images/router/haproxy/Dockerfile | 2 +- .../haproxy/conf/haproxy-config.template | 14 ++- pkg/router/template/template_helper.go | 24 ++++- pkg/router/template/util/haproxy/whitelist.go | 30 ++++++ .../template/util/haproxy/whitelist_test.go | 92 +++++++++++++++++++ 5 files changed, 159 insertions(+), 3 deletions(-) create mode 100644 pkg/router/template/util/haproxy/whitelist.go create mode 100644 pkg/router/template/util/haproxy/whitelist_test.go diff --git a/images/router/haproxy/Dockerfile b/images/router/haproxy/Dockerfile index b33c3b391267..3503ef2e5b57 100644 --- a/images/router/haproxy/Dockerfile +++ b/images/router/haproxy/Dockerfile @@ -9,7 +9,7 @@ RUN INSTALL_PKGS="haproxy18 rsyslog" && \ yum install -y $INSTALL_PKGS && \ rpm -V $INSTALL_PKGS && \ yum clean all && \ - mkdir -p /var/lib/haproxy/router/{certs,cacerts} && \ + mkdir -p /var/lib/haproxy/router/{certs,cacerts,whitelists} && \ mkdir -p /var/lib/haproxy/{conf/.tmp,run,bin,log} && \ touch /var/lib/haproxy/conf/{{os_http_be,os_edge_reencrypt_be,os_tcp_be,os_sni_passthrough,os_route_http_redirect,cert_config,os_wildcard_domain}.map,haproxy.config} && \ setcap 'cap_net_bind_service=ep' /usr/sbin/haproxy && \ diff --git a/images/router/haproxy/conf/haproxy-config.template b/images/router/haproxy/conf/haproxy-config.template index 90f73694131c..d282c3dde154 100644 --- a/images/router/haproxy/conf/haproxy-config.template +++ b/images/router/haproxy/conf/haproxy-config.template @@ -384,7 +384,13 @@ backend {{genBackendNamePrefix $cfg.TLSTermination}}:{{$cfgIdx}} balance {{ if gt $cfg.ActiveServiceUnits 1 }}roundrobin{{ else }}leastconn{{ end }} {{- end }} {{- with $ip_whiteList := firstMatch $cidrListPattern (index $cfg.Annotations "haproxy.router.openshift.io/ip_whitelist") }} - acl whitelist src {{ $ip_whiteList }} + {{- if validateHAProxyWhiteList $ip_whiteList }} + acl whitelist src {{$ip_whiteList}} + {{- else }} + {{- with $whiteListFileName := generateHAProxyWhiteListFile $workingDir $cfgIdx $ip_whiteList}} + acl whitelist src -f {{$whiteListFileName}} + {{- end }} + {{- end }} tcp-request content reject if !whitelist {{- end }} {{- with $value := firstMatch $timeSpecPattern (index $cfg.Annotations "haproxy.router.openshift.io/timeout")}} @@ -506,7 +512,13 @@ backend {{genBackendNamePrefix $cfg.TLSTermination}}:{{$cfgIdx}} balance {{ if gt $cfg.ActiveServiceUnits 1 }}roundrobin{{ else }}source{{ end }} {{- end }} {{- with $ip_whiteList := firstMatch $cidrListPattern (index $cfg.Annotations "haproxy.router.openshift.io/ip_whitelist") }} + {{- if validateHAProxyWhiteList $ip_whiteList }} acl whitelist src {{$ip_whiteList}} + {{- else }} + {{- with $whiteListFileName := generateHAProxyWhiteListFile $workingDir $cfgIdx $ip_whiteList}} + acl whitelist src -f {{$whiteListFileName}} + {{- end }} + {{- end }} tcp-request content reject if !whitelist {{- end }} {{- with $value := firstMatch $timeSpecPattern (index $cfg.Annotations "haproxy.router.openshift.io/timeout")}} diff --git a/pkg/router/template/template_helper.go b/pkg/router/template/template_helper.go index c2f899f5fabf..88a8f5e7ace4 100644 --- a/pkg/router/template/template_helper.go +++ b/pkg/router/template/template_helper.go @@ -2,6 +2,7 @@ package templaterouter import ( "fmt" + "io/ioutil" "math/rand" "os" "path" @@ -182,6 +183,25 @@ func generateHAProxyCertConfigMap(td templateData) []string { return lines } +// validateHAProxyWhiteList validates a whitelist for use with an haproxy acl. +func validateHAProxyWhiteList(value string) bool { + _, valid := haproxyutil.ValidateWhiteList(value) + return valid +} + +// generateHAProxyWhiteListFile generates a whitelist file for use with an haproxy acl. +func generateHAProxyWhiteListFile(workingDir, id, value string) string { + name := path.Join(workingDir, "whitelists", fmt.Sprintf("%s.txt", id)) + cidrs, _ := haproxyutil.ValidateWhiteList(value) + data := []byte(strings.Join(cidrs, "\n") + "\n") + if err := ioutil.WriteFile(name, data, 0644); err != nil { + glog.Errorf("Error writing haproxy whitelist contents: %v", err) + return "" + } + + return name +} + // getHTTPAliasesGroupedByHost returns HTTP(S) aliases grouped by their host. func getHTTPAliasesGroupedByHost(aliases map[string]ServiceAliasConfig) map[string]map[string]ServiceAliasConfig { result := make(map[string]map[string]ServiceAliasConfig) @@ -270,5 +290,7 @@ var helperFunctions = template.FuncMap{ "getHTTPAliasesGroupedByHost": getHTTPAliasesGroupedByHost, //returns HTTP(S) aliases grouped by their host "getPrimaryAliasKey": getPrimaryAliasKey, //returns the key of the primary alias for a group of aliases - "generateHAProxyMap": generateHAProxyMap, //generates a haproxy map content + "generateHAProxyMap": generateHAProxyMap, //generates a haproxy map content + "validateHAProxyWhiteList": validateHAProxyWhiteList, //validates a haproxy whitelist (acl) content + "generateHAProxyWhiteListFile": generateHAProxyWhiteListFile, //generates a haproxy whitelist file for use in an acl } diff --git a/pkg/router/template/util/haproxy/whitelist.go b/pkg/router/template/util/haproxy/whitelist.go new file mode 100644 index 000000000000..22037154f76b --- /dev/null +++ b/pkg/router/template/util/haproxy/whitelist.go @@ -0,0 +1,30 @@ +package haproxy + +import ( + "strings" +) + +const ( + // HAPROXY_MAX_LINE_ARGS is the maximum number of arguments that haproxy + // supports on a configuration line. + // Ref: https://github.com/haproxy/haproxy/blob/master/include/common/defaults.h#L75 + HAPROXY_MAX_LINE_ARGS = 64 + + // HAPROXY_MAX_WHITELIST_LENGTH is the maximum number of CIDRs allowed + // for an "acl whitelist src []*" config line. + HAPROXY_MAX_WHITELIST_LENGTH = HAPROXY_MAX_LINE_ARGS - 3 +) + +// ValidateWhiteList validates a haproxy acl whitelist from an annotation value. +func ValidateWhiteList(value string) ([]string, bool) { + values := strings.Split(value, " ") + + cidrs := make([]string, 0) + for _, v := range values { + if len(v) > 0 { + cidrs = append(cidrs, v) + } + } + + return cidrs, len(cidrs) <= HAPROXY_MAX_WHITELIST_LENGTH +} diff --git a/pkg/router/template/util/haproxy/whitelist_test.go b/pkg/router/template/util/haproxy/whitelist_test.go new file mode 100644 index 000000000000..220a641d375b --- /dev/null +++ b/pkg/router/template/util/haproxy/whitelist_test.go @@ -0,0 +1,92 @@ +package haproxy + +import ( + "fmt" + "reflect" + "strings" + "testing" + + utilrand "k8s.io/apimachinery/pkg/util/rand" +) + +func generateTestData(n int) []string { + cidrs := make([]string, 0) + prefix := fmt.Sprintf("%d.%d.%d", utilrand.IntnRange(1, 254), utilrand.IntnRange(1, 254), utilrand.IntnRange(1, 254)) + for i := 1; i <= n; i++ { + if i%254 == 0 { + prefix = fmt.Sprintf("%d.%d.%d", utilrand.IntnRange(1, 254), utilrand.IntnRange(1, 254), utilrand.IntnRange(1, 254)) + } + + cidr := fmt.Sprintf("%s.%d", prefix, (i%254)+1) + if i%10 == 0 { + cidr = fmt.Sprintf("%s/24", cidr) + } + cidrs = append(cidrs, cidr) + } + + return cidrs +} + +func TestValidateWhiteList(t *testing.T) { + tests := []struct { + name string + data []string + expectation []string + }{ + { + name: "empty list", + data: []string{}, + expectation: []string{}, + }, + { + name: "blanks", + data: []string{"", " ", "", " ", " "}, + expectation: []string{}, + }, + { + name: "one ip", + data: []string{"1.2.3.4"}, + expectation: []string{"1.2.3.4"}, + }, + { + name: "onesie", + data: []string{"172.16.32.1/24"}, + expectation: []string{"172.16.32.1/24"}, + }, + { + name: "duo", + data: []string{"172.16.32.1/24", "10.1.2.3"}, + expectation: []string{"172.16.32.1/24", "10.1.2.3"}, + }, + { + name: "interleaved blank entries", + data: []string{"172.16.32.1/24", "", "1.2.3.4", "", "5.6.7.8", ""}, + expectation: []string{"172.16.32.1/24", "1.2.3.4", "5.6.7.8"}, + }, + } + + for _, tc := range tests { + values, ok := ValidateWhiteList(strings.Join(tc.data, " ")) + if !reflect.DeepEqual(tc.expectation, values) { + t.Errorf("%s: expected validated data %+v, got %+v", tc.name, tc.expectation, values) + } + flagExpectation := len(tc.expectation) <= 61 + if ok != flagExpectation { + t.Errorf("%s: expected flag %+v, got %+v", tc.name, flagExpectation, ok) + } + } + + limitsTest := []int{9, 10, 16, 32, 60, 61, 62, 63, 64, 128, 253, 254, 255, 256, 512, 1024} + for _, v := range limitsTest { + name := fmt.Sprintf("limits-test-%d", v) + data := generateTestData(v) + values, ok := ValidateWhiteList(strings.Join(data, " ")) + if !reflect.DeepEqual(data, values) { + t.Errorf("%s: expected validated data %+v, got %+v", name, data, values) + } + expectation := len(data) <= 61 + if ok != expectation { + t.Errorf("%s: expected flag %+v, got %+v", name, expectation, ok) + } + } +}