Enable PROXY protocol for specific CIDRs in HAProxy

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"))
+		})
 	})
 })
 

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)"
+    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

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
+%>

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") do -%>
+      <%- 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") do -%>
+    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") do -%>
+    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") do -%>
+    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

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

spec/haproxy/templates/haproxy_config/frontend_http_spec.rb

@@ -42,6 +42,41 @@
         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.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

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

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