diff --git a/acceptance-tests/acceptance_tests_suite_test.go b/acceptance-tests/acceptance_tests_suite_test.go index b621b7db..04aa2bfb 100644 --- a/acceptance-tests/acceptance_tests_suite_test.go +++ b/acceptance-tests/acceptance_tests_suite_test.go @@ -229,6 +229,10 @@ func expectConnectionRefusedErr(err error) { checkNetOpErr(err, "connect: connection refused") } +func expectConnectionResetErr(err error) { + checkNetOpErr(err, "read: connection reset by peer") +} + func checkNetOpErr(err error, expectString string) { Expect(err).To(HaveOccurred()) urlErr, ok := err.(*url.Error) diff --git a/acceptance-tests/go.mod b/acceptance-tests/go.mod index 82f79a85..3877eb69 100644 --- a/acceptance-tests/go.mod +++ b/acceptance-tests/go.mod @@ -1,12 +1,13 @@ module github.com/cloudfoundry/haproxy-boshrelease/acceptance-tests -go 1.18 +go 1.21 require ( github.com/bramvdbogaerde/go-scp v1.4.0 github.com/gorilla/websocket v1.5.1 github.com/onsi/ginkgo/v2 v2.17.2 github.com/onsi/gomega v1.33.1 + github.com/pires/go-proxyproto v0.7.0 golang.org/x/crypto v0.22.0 golang.org/x/net v0.24.0 gopkg.in/yaml.v2 v2.4.0 diff --git a/acceptance-tests/go.sum b/acceptance-tests/go.sum index e6047d97..64a94e52 100644 --- a/acceptance-tests/go.sum +++ b/acceptance-tests/go.sum @@ -1,6 +1,7 @@ github.com/bramvdbogaerde/go-scp v1.4.0 h1:jKMwpwCbcX1KyvDbm/PDJuXcMuNVlLGi0Q0reuzjyKY= github.com/bramvdbogaerde/go-scp v1.4.0/go.mod h1:on2aH5AxaFb2G0N5Vsdy6B0Ml7k9HuHSwfo1y0QzAbQ= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ= github.com/go-logr/logr v1.4.1/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= @@ -15,8 +16,12 @@ github.com/onsi/ginkgo/v2 v2.17.2 h1:7eMhcy3GimbsA3hEnVKdw/PQM9XN9krpKVXsZdph0/g github.com/onsi/ginkgo/v2 v2.17.2/go.mod h1:nP2DPOQoNsQmsVyv5rDA8JkXQoCs6goXIvr/PRJ1eCc= github.com/onsi/gomega v1.33.1 h1:dsYjIxxSR755MDmKVsaFQTE22ChNBcuuTWgkUDSubOk= github.com/onsi/gomega v1.33.1/go.mod h1:U4R44UsT+9eLIaYRB2a5qajjtQYn0hauxvRm16AVYg0= +github.com/pires/go-proxyproto v0.7.0 h1:IukmRewDQFWC7kfnb66CSomk2q/seBuilHBYFwyq0Hs= +github.com/pires/go-proxyproto v0.7.0/go.mod h1:Vz/1JPY/OACxWGQNIRY2BeyDmpoaWmEP40O9LbuiFR4= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= golang.org/x/crypto v0.22.0 h1:g1v0xeRhjcugydODzvb3mEM9SQ0HGp9s/nh3COQ/C30= golang.org/x/crypto v0.22.0/go.mod h1:vr6Su+7cTlO45qkww3VDJlzDn0ctJvRgYbC2NvXHt+M= golang.org/x/net v0.24.0 h1:1PcaxkF854Fu3+lvBIx5SYn9wRlBzzcnHZSiaFFAb0w= @@ -24,11 +29,13 @@ golang.org/x/net v0.24.0/go.mod h1:2Q7sJY5mzlzWjKtYUEXSlBWCdyaioyXzRB2RtU8KVE8= golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o= golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.19.0 h1:+ThwsDv+tYfnJFhF4L8jITxu1tdTWRTZpdsWgEgjL6Q= +golang.org/x/term v0.19.0/go.mod h1:2CuTdWZ7KHSQwUzKva0cbMg6q2DMI3Mmxp+gKJbskEk= golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/tools v0.20.0 h1:hz/CVckiOxybQvFw6h7b/q80NTr9IUQb4s1IIzW7KNY= golang.org/x/tools v0.20.0/go.mod h1:WvitBU7JJf6A4jOdg4S1tviW9bhUxkgeCui/0JHctQg= google.golang.org/protobuf v1.33.0 h1:uNO2rsAINq/JlFpSdYEKIZ0uKD/R9cpdv0T+yoGwGmI= +google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= diff --git a/acceptance-tests/proxy_protocol_test.go b/acceptance-tests/proxy_protocol_test.go new file mode 100644 index 00000000..3b8a049b --- /dev/null +++ b/acceptance-tests/proxy_protocol_test.go @@ -0,0 +1,120 @@ +package acceptance_tests + +import ( + "fmt" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + proxyproto "github.com/pires/go-proxyproto" + "net" + "net/http" + "time" +) + +var _ = Describe("Proxy Protocol", func() { + opsfileProxyProtocol := `--- +# Enable Proxy Protocol +- type: replace + path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/accept_proxy? + value: true +- type: replace + path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/enable_health_check_http? + value: true +- type: replace + path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/disable_health_check_proxy? + value: false +` + + It("Correctly proxies Proxy Protocol requests", func() { + haproxyBackendPort := 12000 + haproxyInfo, _ := deployHAProxy(baseManifestVars{ + haproxyBackendPort: haproxyBackendPort, + haproxyBackendServers: []string{"127.0.0.1"}, + deploymentName: deploymentNameForTestNode(), + }, []string{opsfileProxyProtocol}, map[string]interface{}{}, false) + + closeLocalServer, localPort := startDefaultTestServer() + defer closeLocalServer() + + closeTunnel := setupTunnelFromHaproxyToTestServer(haproxyInfo, haproxyBackendPort, localPort) + defer closeTunnel() + + By("Waiting for monit to report that HAProxy is healthy") + // Since the backend is now listening, HAProxy healthcheck should start returning healthy + // and monit should in turn start reporting a healthy process + // We will wait up to one minute for the status to stabilise + Eventually(func() string { + return boshInstances(deploymentNameForTestNode())[0].ProcessState + }, time.Minute, time.Second).Should(Equal("running")) + + By("Sending a request with Proxy Protocol Header to HAProxy traffic port") + err := performProxyProtocolRequest(haproxyInfo.PublicIP, 80, "/") + Expect(err).NotTo(HaveOccurred()) + + By("Sending a request without Proxy Protocol Header to HAProxy") + _, err = http.Get(fmt.Sprintf("http://%s", haproxyInfo.PublicIP)) + expectConnectionResetErr(err) + + By("Sending a request with Proxy Protocol Header to HAProxy health check port") + err = performProxyProtocolRequest(haproxyInfo.PublicIP, 8080, "/health") + Expect(err).NotTo(HaveOccurred()) + + By("Sending a request without Proxy Protocol Header to HAProxy health check port") + _, err = http.Get(fmt.Sprintf("http://%s:8080/health", haproxyInfo.PublicIP)) + expectConnectionResetErr(err) + }) +}) + +func performProxyProtocolRequest(ip string, port int, endpoint string) error { + // Create a connection to the HAProxy instance + conn, err := net.Dial("tcp", fmt.Sprintf("%s:%d", ip, port)) + if err != nil { + return err + } + + defer conn.Close() + + // Create proxy protocol header + header := &proxyproto.Header{ + Version: 1, + Command: proxyproto.PROXY, + TransportProtocol: proxyproto.TCPv4, + SourceAddr: &net.TCPAddr{ + IP: net.ParseIP("10.1.1.1"), + Port: 1000, + }, + DestinationAddr: &net.TCPAddr{ + IP: net.ParseIP(ip), + Port: port, + }, + } + + // Write header to the connection + _, err = header.WriteTo(conn) + if err != nil { + return err + } + + // Send HTTP Request + request := fmt.Sprintf("GET %s HTTP/1.1\r\n"+ + "Host: %s\r\n"+ + "Content-Length: 0\r\n"+ + "Content-Type: text/plain\r\n"+ + "\r\n", endpoint, ip) + _, err = conn.Write([]byte(request)) + if err != nil { + return err + } + + // Read the response + buf := make([]byte, 1024) + _, err = conn.Read(buf) + if err != nil { + return err + } + + // Get HTTP status code + if string(buf[9:12]) != "200" { + return fmt.Errorf("expected HTTP status code 200, got %s", string(buf)) + } + return nil +} diff --git a/acceptance-tests/run-local.sh b/acceptance-tests/run-local.sh index 3b0b139c..abf39f49 100755 --- a/acceptance-tests/run-local.sh +++ b/acceptance-tests/run-local.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash -set -e +set -eu SCRIPT_DIR="$(cd "$(dirname "$0")/" && pwd)" REPO_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" @@ -29,6 +29,46 @@ docker_mac_check_cgroupsv1() { fi } +check_required_files() { + PIDS="" + REQUIRED_FILE_PATTERNS=( + ../ci/scripts/bpm/bpm-release-*.tgz!https://bosh.io/d/github.com/cloudfoundry/bpm-release + ../ci/scripts/stemcell/bosh-stemcell-*-ubuntu-jammy-*.tgz!https://bosh.io/d/stemcells/bosh-warden-boshlite-ubuntu-jammy-go_agent + ../ci/scripts/stemcell-bionic/bosh-stemcell-*-ubuntu-bionic-*.tgz!https://bosh.io/d/stemcells/bosh-warden-boshlite-ubuntu-bionic-go_agent + ) + + for entry in "${REQUIRED_FILE_PATTERNS[@]}"; do + pattern=$(cut -f1 -d! <<<"$entry") + url=$(cut -f2 -d! <<<"$entry") + folder=$(realpath "$(dirname "$SCRIPT_DIR/$pattern")") + filepattern=$(basename "$pattern") + pattern=$folder/$filepattern + + # shellcheck disable=SC2086 + # glob resolution is desired here. + if [ -f $pattern ]; then + continue + fi + + ( + echo "$filepattern not found, downloading latest." + cd "$folder" && \ + resolved=$(curl -s --write-out '\n%{redirect_url}' "$url" | tail -n1) && \ + curl -s --remote-name --remote-header-name --location "$resolved" && \ + echo "Downloaded '$url' successfully." && \ + ls -1lh "$folder/"$filepattern + )& + + PIDS="$PIDS $!" + + done + # shellcheck disable=SC2086 + # expansion is desired, as $PIDS is a list of PIDs. Wait on all of those PIDs. + wait $PIDS +} + +check_required_files + if [ "$(uname)" == "Darwin" ]; then docker_mac_check_cgroupsv1 fi diff --git a/docs/rate_limiting.md b/docs/rate_limiting.md index 57b7f7a4..2a948bef 100644 --- a/docs/rate_limiting.md +++ b/docs/rate_limiting.md @@ -47,7 +47,7 @@ config: #### Resulting `haproxy.config` ```ini backend st_http_req_rate - stick-table type ip size 1m expire 10s store http_req_rate(10s) + stick-table type ipv6 size 1m expire 10s store http_req_rate(10s) # [...] frontend http-in http-request track-sc1 src table st_http_req_rate @@ -68,7 +68,7 @@ config: #### Resulting `haproxy.config` ```ini backend st_http_req_rate - stick-table type ip size 1m expire 10s store http_req_rate(10s) + stick-table type ipv6 size 1m expire 10s store http_req_rate(10s) # [...] frontend http-in http-request track-sc1 src table st_http_req_rate @@ -95,10 +95,10 @@ config: #### Resulting `haproxy.config` ```ini backend st_http_req_rate - stick-table type ip size 1m expire 10s store http_req_rate(10s) + stick-table type ipv6 size 1m expire 10s store http_req_rate(10s) backend st_tcp_conn_rate - stick-table type ip size 1m expire 10s store conn_rate(10s) + stick-table type ipv6 size 1m expire 10s store conn_rate(10s) # [...] frontend http-in # [...] diff --git a/jobs/haproxy/spec b/jobs/haproxy/spec index 0df72fe7..255e84fe 100644 --- a/jobs/haproxy/spec +++ b/jobs/haproxy/spec @@ -509,11 +509,14 @@ properties: description: "Number of dns queries to send to resolve a server name before giving up" default: 3 ha_proxy.accept_proxy: - description: "Turned off by default. Enforces the use of the PROXY protocol for all incoming connections to all frontends. When enabled standard tcp connections to these port no longer work." + description: "Turned off by default. Enforces the use of the PROXY protocol for all incoming connections to all frontends, with the exception of local requests to the health check endpoint, since Monit does not support PROXY protocol. When enabled, standard TCP connections to these ports no longer work." default: false ha_proxy.disable_tcp_accept_proxy: description: "Disables the PROXY protocol on tcp backends. Only applies if `ha_proxy.accept_proxy` is enabled." default: false + ha_proxy.disable_health_check_proxy: + description: "Disables the use of the PROXY protocol for health checks. Only applies if `ha_proxy.accept_proxy` is enabled." + default: false ha_proxy.binding_ip: description: "If there are multiple ethernet interfaces, specify which one to bind. Set to `::` to bind to all IPv6 interfaces (no IPv4). IPv6 must be enabled on the HAProxy VM in the deployment manifest." default: "" diff --git a/jobs/haproxy/templates/haproxy.config.erb b/jobs/haproxy/templates/haproxy.config.erb index 9018b04e..f78d5014 100644 --- a/jobs/haproxy/templates/haproxy.config.erb +++ b/jobs/haproxy/templates/haproxy.config.erb @@ -343,6 +343,9 @@ listen health_check_http_url mode http option httpclose monitor-uri /health + <% if p("ha_proxy.accept_proxy") && !p("ha_proxy.disable_health_check_proxy") -%> + tcp-request connection expect-proxy layer4 unless LOCALHOST + <%- end -%> acl http-routers_down nbsrv(<%= backends.first[:name] %>) eq 0 monitor fail if http-routers_down <% end -%> @@ -359,12 +362,12 @@ resolvers default <% if_p("ha_proxy.requests_rate_limit.table_size", "ha_proxy.requests_rate_limit.window_size") do |table_size, window_size| %> backend st_http_req_rate - stick-table type ip size <%= table_size %> expire <%= window_size %> store http_req_rate(<%= window_size %>) + stick-table type ipv6 size <%= table_size %> expire <%= window_size %> store http_req_rate(<%= window_size %>) <% end %> <% if_p("ha_proxy.connections_rate_limit.table_size", "ha_proxy.connections_rate_limit.window_size") do |table_size, window_size| %> backend st_tcp_conn_rate - stick-table type ip size <%= table_size %> expire <%= window_size %> store conn_rate(<%= window_size %>) + stick-table type ipv6 size <%= table_size %> expire <%= window_size %> store conn_rate(<%= window_size %>) <% end %> <% unless p("ha_proxy.disable_http") -%> diff --git a/spec/haproxy/templates/haproxy_config/healthcheck_listener_spec.rb b/spec/haproxy/templates/haproxy_config/healthcheck_listener_spec.rb index da9f3e5c..f85ac41f 100644 --- a/spec/haproxy/templates/haproxy_config/healthcheck_listener_spec.rb +++ b/spec/haproxy/templates/haproxy_config/healthcheck_listener_spec.rb @@ -58,5 +58,32 @@ expect(healthcheck_listener).to include('bind :1234') end end + + context 'when ha_proxy.accept_proxy is true' do + let(:properties) do + { + 'enable_health_check_http' => true, + 'accept_proxy' => true + } + end + + it 'sets expect-proxy with exception for LOCALHOST' do + expect(healthcheck_listener).to include('tcp-request connection expect-proxy layer4 unless LOCALHOST') + end + + context 'when ha_proxy.disable_health_check_proxy is also true' do + let(:properties) do + { + 'enable_health_check_http' => true, + 'accept_proxy' => true, + 'disable_health_check_proxy' => true + } + end + + it 'does not set expect-proxy for the healthcheck' do + expect(healthcheck_listener).not_to include('tcp-request connection expect-proxy layer4 unless LOCALHOST') + end + end + end end end diff --git a/spec/haproxy/templates/haproxy_config/rate_limit_spec.rb b/spec/haproxy/templates/haproxy_config/rate_limit_spec.rb index 344cbbc2..f001a8be 100644 --- a/spec/haproxy/templates/haproxy_config/rate_limit_spec.rb +++ b/spec/haproxy/templates/haproxy_config/rate_limit_spec.rb @@ -37,7 +37,7 @@ let(:properties) { temp_properties } it 'sets up stick-tables' do - expect(backend_req_rate).to include('stick-table type ip size 10m expire 10s store http_req_rate(10s)') + expect(backend_req_rate).to include('stick-table type ipv6 size 10m expire 10s store http_req_rate(10s)') end it 'tracks requests in stick tables' do @@ -78,7 +78,7 @@ let(:properties) { temp_properties } it 'sets up stick-tables' do - expect(backend_conn_rate).to include('stick-table type ip size 10m expire 10s store conn_rate(10s)') + expect(backend_conn_rate).to include('stick-table type ipv6 size 10m expire 10s store conn_rate(10s)') end it 'tracks connections in stick tables' do