diff --git a/acceptance-tests/proxy_protocol_test.go b/acceptance-tests/proxy_protocol_test.go index 3b8a049b..a30f790c 100644 --- a/acceptance-tests/proxy_protocol_test.go +++ b/acceptance-tests/proxy_protocol_test.go @@ -11,7 +11,9 @@ import ( ) var _ = Describe("Proxy Protocol", func() { - opsfileProxyProtocol := `--- + + Context("accept_proxy", func() { + opsfileProxyProtocol := `--- # Enable Proxy Protocol - type: replace path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/accept_proxy? @@ -23,44 +25,90 @@ var _ = Describe("Proxy Protocol", func() { 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) + + }) + }) - 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) + Context("expect_proxy_cidrs", func() { + opsfileExpectProxyProtocol := `--- +# Enable Proxy Protocol +- type: replace + path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/accept_proxy? + value: false +- type: replace + path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/expect_proxy_cidrs? + value: + - 10.0.0.0/8 # Bosh Network CIDR +` + It("Correctly proxies request with Expect Proxy CIDRs list", func() { + haproxyBackendPort := 12000 + haproxyInfo, _ := deployHAProxy(baseManifestVars{ + haproxyBackendPort: haproxyBackendPort, + haproxyBackendServers: []string{"127.0.0.1"}, + deploymentName: deploymentNameForTestNode(), + }, []string{opsfileExpectProxyProtocol}, map[string]interface{}{}, true) + + closeLocalServer, localPort := startDefaultTestServer() + defer closeLocalServer() + + closeBackendTunnel := setupTunnelFromHaproxyToTestServer(haproxyInfo, haproxyBackendPort, localPort) + defer closeBackendTunnel() + + By("Checking that Proxy Protocol is expected for requests from IPs in the list") + // Requests without Proxy Protocol Header should be rejected + _, err := http.Get(fmt.Sprintf("http://%s", haproxyInfo.PublicIP)) + expectConnectionResetErr(err) + + // Requests with Proxy Protocol Header should succeed + err = performProxyProtocolRequest(haproxyInfo.PublicIP, 80, "/") + Expect(err).NotTo(HaveOccurred()) + + By("Checking that Proxy Protocol is NOT expected for requests from IPs NOT in the list") + // Set up a tunnel so that requests to localhost:11000 appear to come from localhost + // on the HAProxy VM as Proxy Protocol is not expected for requests from localhost (see ops file above) + closeFrontendTunnel := setupTunnelFromLocalMachineToHAProxy(haproxyInfo, 11000, 80) + defer closeFrontendTunnel() + + // Requests without Proxy Protocol Header should succeed + expectTestServer200(http.Get("http://127.0.0.1:11000")) + }) }) }) diff --git a/jobs/haproxy/spec b/jobs/haproxy/spec index 9a44561c..3fa40c10 100644 --- a/jobs/haproxy/spec +++ b/jobs/haproxy/spec @@ -23,6 +23,7 @@ templates: client-revocation-list.erb: config/client-revocation-list.pem blacklist_cidrs.txt.erb: config/blacklist_cidrs.txt whitelist_cidrs.txt.erb: config/whitelist_cidrs.txt + expect_proxy_cidrs.txt.erb: config/expect_proxy_cidrs.txt trusted_domain_cidrs.txt.erb: config/trusted_domain_cidrs.txt consumes: @@ -580,6 +581,13 @@ properties: cidr_whitelist: - 172.168.4.1/32 - 10.2.0.0/16 + ha_proxy.expect_proxy_cidrs: + description: "List of CIDRs to enable proxy protocol for. This enables forwarding of the client source IP for hyperscalers not supporting IP dual stack (v4 & v6). This property is mutually exclusive with the accept_proxy." + default: ~ + example: + expect_proxy_cidrs: + - 10.6.7.8/27 + - 2001:db8::/32 ha_proxy.block_all: description: "Optionally block all incoming traffic to http(s). Use in conjunction with whitelist." default: false diff --git a/jobs/haproxy/templates/expect_proxy_cidrs.txt.erb b/jobs/haproxy/templates/expect_proxy_cidrs.txt.erb new file mode 100644 index 00000000..9ad207e9 --- /dev/null +++ b/jobs/haproxy/templates/expect_proxy_cidrs.txt.erb @@ -0,0 +1,14 @@ +<% +require 'zlib' +require 'stringio' + +if_p("ha_proxy.expect_proxy_cidrs") do |cidrs| +-%> +# generated from expect_proxy_cidrs.txt.erb + +# BEGIN expect_proxy_cidrs +<%= cidrs.join("\n") %> +# END expect_proxy_cidrs +<% +end +%> \ No newline at end of file diff --git a/jobs/haproxy/templates/haproxy.config.erb b/jobs/haproxy/templates/haproxy.config.erb index 199e172d..e6cb4c98 100644 --- a/jobs/haproxy/templates/haproxy.config.erb +++ b/jobs/haproxy/templates/haproxy.config.erb @@ -169,6 +169,9 @@ end # }}} # Error checking + if p("ha_proxy.accept_proxy", false) && p("ha_proxy.expect_proxy_cidrs", nil) + abort "Conflicting configuration: accept_proxy and expect_proxy_cidrs are mutually exclusive" + end if !p("ha_proxy.drain_enable", false) && p("ha_proxy.drain_frontend_grace_time") > 0 abort "Conflicting configuration: drain_enable must be true to use drain_frontend_grace_time" end @@ -359,9 +362,14 @@ 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") -%> + <%- if p("ha_proxy.accept_proxy") && !p("ha_proxy.disable_health_check_proxy") -%> tcp-request connection expect-proxy layer4 unless LOCALHOST <%- end -%> + <%- if p("ha_proxy.expect_proxy_cidrs", []).size > 0 -%> + <%- if !p("ha_proxy.disable_health_check_proxy") -%> + tcp-request connection expect-proxy layer4 if { src -f /var/vcap/jobs/haproxy/config/expect_proxy_cidrs.txt } + <%- end -%> + <%- end -%> acl http-routers_down nbsrv(<%= backends.first[:name] %>) eq 0 monitor fail if http-routers_down <% end -%> @@ -410,6 +418,9 @@ frontend http-in <%- end -%> <%- end -%> <%- end -%> + <%- if p("ha_proxy.expect_proxy_cidrs", []).size > 0 -%> + tcp-request connection expect-proxy layer4 if { src -f /var/vcap/jobs/haproxy/config/expect_proxy_cidrs.txt } + <%- end -%> <%- if_p("ha_proxy.cidr_whitelist") do -%> acl whitelist src -f /var/vcap/jobs/haproxy/config/whitelist_cidrs.txt tcp-request content accept if whitelist @@ -528,6 +539,9 @@ frontend https-in <%- end -%> <%- end -%> <%- end -%> + <%- if p("ha_proxy.expect_proxy_cidrs", []).size > 0 -%> + tcp-request connection expect-proxy layer4 if { src -f /var/vcap/jobs/haproxy/config/expect_proxy_cidrs.txt } + <%- end -%> <%- if_p("ha_proxy.cidr_whitelist") do -%> acl whitelist src -f /var/vcap/jobs/haproxy/config/whitelist_cidrs.txt tcp-request content accept if whitelist @@ -691,6 +705,9 @@ frontend wss-in <%- if properties.ha_proxy.frontend_config -%> <%= format_indented_multiline_config(p("ha_proxy.frontend_config")) %> <%- end -%> + <%- if p("ha_proxy.expect_proxy_cidrs", []).size > 0 -%> + tcp-request connection expect-proxy layer4 if { src -f /var/vcap/jobs/haproxy/config/expect_proxy_cidrs.txt } + <%- end -%> <%- if_p("ha_proxy.cidr_whitelist") do -%> acl whitelist src -f /var/vcap/jobs/haproxy/config/whitelist_cidrs.txt tcp-request content accept if whitelist diff --git a/spec/haproxy/templates/expect_proxy_cidrs.txt_spec.rb b/spec/haproxy/templates/expect_proxy_cidrs.txt_spec.rb new file mode 100644 index 00000000..d321259a --- /dev/null +++ b/spec/haproxy/templates/expect_proxy_cidrs.txt_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'rspec' + +describe 'config/expect_proxy_cidrs.txt' do + let(:template) { haproxy_job.template('config/expect_proxy_cidrs.txt') } + + context 'when ha_proxy.expect_proxy_cidrs' do + context 'when a list of cidrs is provided' do + it 'has the correct contents' do + expect(template.render({ + 'ha_proxy' => { + 'expect_proxy_cidrs' => ['10.5.6.7/27', + '2001:db8::/32'] + } + })).to eq(<<~EXPECTED) + # generated from expect_proxy_cidrs.txt.erb + + # BEGIN expect_proxy_cidrs + 10.5.6.7/27 + 2001:db8::/32 + # END expect_proxy_cidrs + EXPECTED + end + end + end + + context 'when ha_proxy.expect_proxy_cidrs is not provided' do + it 'is empty' do + expect(template.render({})).to be_a_blank_string + end + end +end diff --git a/spec/haproxy/templates/haproxy_config/frontend_http_spec.rb b/spec/haproxy/templates/haproxy_config/frontend_http_spec.rb index 07af539d..d9d4ead9 100644 --- a/spec/haproxy/templates/haproxy_config/frontend_http_spec.rb +++ b/spec/haproxy/templates/haproxy_config/frontend_http_spec.rb @@ -42,6 +42,52 @@ expect(frontend_http).to include('bind :80 accept-proxy') end end + + context 'when ha_proxy.expect_proxy_cidrs is not empty/nil and ha_proxy.accept_proxy is false' do + let(:properties) do + { 'accept_proxy' => false, + 'expect_proxy_cidrs' => ['127.0.0.1/8'] } + end + + it 'sets expect-proxy of tcp connection to the file proxies_cidrs.txt contents' do + expect(frontend_http).to include('tcp-request connection expect-proxy layer4 if { src -f /var/vcap/jobs/haproxy/config/expect_proxy_cidrs.txt }') + end + end + + context 'when ha_proxy.expect_proxy_cidrs is empty and not nil and ha_proxy.accept_proxy is false' do + let(:properties) do + { 'accept_proxy' => false, + 'expect_proxy_cidrs' => [] } + end + + it 'does not contain expect-proxy of tcp connection directive' do + expect(frontend_http).not_to include('tcp-request connection expect-proxy layer4 if { src -f /var/vcap/jobs/haproxy/config/expect_proxy_cidrs.txt }') + end + end + + context 'when ha_proxy.expect_proxy_cidrs is nil and ha_proxy.accept_proxy is false' do + let(:properties) do + { 'accept_proxy' => false, + 'expect_proxy_cidrs' => nil } + end + + it 'does not contain expect-proxy of tcp connection directive' do + expect(frontend_http).not_to include('tcp-request connection expect-proxy layer4 if { src -f /var/vcap/jobs/haproxy/config/expect_proxy_cidrs.txt }') + end + end + + context 'when ha_proxy.accept_proxy is true and ha_proxy.expect_proxy_cidrs is not empty/nil' do + let(:properties) do + { 'accept_proxy' => true, + 'expect_proxy_cidrs' => ['127.0.0.1/8'] } + end + + it 'aborts with a meaningful error message' do + expect do + frontend_http + end.to raise_error(/Conflicting configuration: accept_proxy and expect_proxy_cidrs are mutually exclusive/) + end + end end context 'when a custom ha_proxy.frontend_config is provided' do diff --git a/spec/haproxy/templates/haproxy_config/frontend_https_spec.rb b/spec/haproxy/templates/haproxy_config/frontend_https_spec.rb index 6c63e1e0..0723632c 100644 --- a/spec/haproxy/templates/haproxy_config/frontend_https_spec.rb +++ b/spec/haproxy/templates/haproxy_config/frontend_https_spec.rb @@ -48,6 +48,30 @@ expect(frontend_https).to include('bind :443 accept-proxy ssl crt /var/vcap/jobs/haproxy/config/ssl') end end + + context 'when ha_proxy.expect_proxy_cidrs is not empty/nil and ha_proxy.accept_proxy is false' do + let(:properties) do + default_properties.merge({ 'accept_proxy' => false, + 'expect_proxy_cidrs' => ['127.0.0.1/8'] }) + end + + it 'sets expect-proxy of tcp connection to the file proxies_cidrs.txt contents' do + expect(frontend_https).to include('tcp-request connection expect-proxy layer4 if { src -f /var/vcap/jobs/haproxy/config/expect_proxy_cidrs.txt }') + end + end + + context 'when ha_proxy.accept_proxy is true and ha_proxy.expect_proxy_cidrs is not empty/nil' do + let(:properties) do + default_properties.merge({ 'accept_proxy' => true, + 'expect_proxy_cidrs' => ['127.0.0.1/8'] }) + end + + it 'aborts with a meaningful error message' do + expect do + frontend_https + end.to raise_error(/Conflicting configuration: accept_proxy and expect_proxy_cidrs are mutually exclusive/) + end + end end context 'when ha_proxy.disable_domain_fronting is true' do diff --git a/spec/haproxy/templates/haproxy_config/frontend_wss_spec.rb b/spec/haproxy/templates/haproxy_config/frontend_wss_spec.rb index 36f8ac5a..23efef00 100644 --- a/spec/haproxy/templates/haproxy_config/frontend_wss_spec.rb +++ b/spec/haproxy/templates/haproxy_config/frontend_wss_spec.rb @@ -50,6 +50,30 @@ expect(frontend_wss).to include('bind :4443 accept-proxy ssl crt /var/vcap/jobs/haproxy/config/ssl') end end + + context 'when ha_proxy.expect_proxy_cidrs is not empty/nil and ha_proxy.accept_proxy is false' do + let(:properties) do + default_properties.merge({ 'accept_proxy' => false, + 'expect_proxy_cidrs' => ['127.0.0.1/8'] }) + end + + it 'sets expect-proxy of tcp connection to the file proxies_cidrs.txt contents' do + expect(frontend_wss).to include('tcp-request connection expect-proxy layer4 if { src -f /var/vcap/jobs/haproxy/config/expect_proxy_cidrs.txt }') + end + end + + context 'when ha_proxy.accept_proxy is true and ha_proxy.expect_proxy_cidrs is not empty/nil' do + let(:properties) do + default_properties.merge({ 'accept_proxy' => true, + 'expect_proxy_cidrs' => ['127.0.0.1/8'] }) + end + + it 'aborts with a meaningful error message' do + expect do + frontend_wss + end.to raise_error(/Conflicting configuration: accept_proxy and expect_proxy_cidrs are mutually exclusive/) + end + end end context 'when ha_proxy.disable_domain_fronting is true' do