From 8134e734eab58ba6a554301ee25ffcd6df923563 Mon Sep 17 00:00:00 2001 From: Tamara Boehm Date: Thu, 28 May 2026 11:57:51 +0200 Subject: [PATCH 1/2] fix: Support conn rate limiting when Proxy protocol is enabled --- acceptance-tests/rate_limit_test.go | 55 +++++++++++++++++++ jobs/haproxy/templates/haproxy.config.erb | 13 +++-- .../haproxy_config/rate_limit_spec.rb | 26 ++++++++- 3 files changed, 89 insertions(+), 5 deletions(-) diff --git a/acceptance-tests/rate_limit_test.go b/acceptance-tests/rate_limit_test.go index 18c74765..3415d967 100644 --- a/acceptance-tests/rate_limit_test.go +++ b/acceptance-tests/rate_limit_test.go @@ -165,6 +165,61 @@ var _ = Describe("Rate-Limiting", func() { Expect(successfulRequestCount).To(Equal(connLimit)) }) + It("Connection Based Limiting Works with Proxy Protocol enabled", func() { + connLimit := 5 + opsfileConnRateLimitWithProxyProtocol := fmt.Sprintf(`--- +# 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/connections_rate_limit?/connections + value: %d +- type: replace + path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/connections_rate_limit/window_size? + value: 100s +- type: replace + path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/connections_rate_limit/table_size? + value: 100 +- type: replace + path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/connections_rate_limit/block? + value: true +`, connLimit) + + haproxyBackendPort := 12000 + haproxyInfo, _ := deployHAProxy(baseManifestVars{ + haproxyBackendPort: haproxyBackendPort, + haproxyBackendServers: []string{"127.0.0.1"}, + deploymentName: deploymentNameForTestNode(), + }, []string{opsfileConnRateLimitWithProxyProtocol}, map[string]interface{}{}, true) + + closeLocalServer, localPort := startDefaultTestServer() + defer closeLocalServer() + + closeTunnel := setupTunnelFromHaproxyToTestServer(haproxyInfo, haproxyBackendPort, localPort) + defer closeTunnel() + + By("Sending requests via Proxy Protocol, each on a new TCP connection, expecting connection rate limiting to apply") + testRequestCount := int(float64(connLimit) * 1.5) + firstFailure := -1 + successfulRequestCount := 0 + for i := 0; i < testRequestCount; i++ { + err := performProxyProtocolRequest(haproxyInfo.PublicIP, 80, "/foo") + if err == nil { + successfulRequestCount++ + } else { + if firstFailure == -1 { + firstFailure = i + } + } + } + + By("The first connection should fail after we've sent the amount of connections specified in the Connection Rate Limit") + Expect(firstFailure).To(Equal(connLimit)) + By("The total amount of successful connections per time window should equal the Connection Rate Limit") + Expect(successfulRequestCount).To(Equal(connLimit)) + }) + It("Both types of rate limiting work in parallel", func() { requestLimit := 5 connLimit := 6 // needs to be higher than request limit for this test diff --git a/jobs/haproxy/templates/haproxy.config.erb b/jobs/haproxy/templates/haproxy.config.erb index a7f1a566..e34a8114 100644 --- a/jobs/haproxy/templates/haproxy.config.erb +++ b/jobs/haproxy/templates/haproxy.config.erb @@ -39,6 +39,11 @@ tcp_accept_proxy = accept_proxy if p("ha_proxy.disable_tcp_accept_proxy") tcp_accept_proxy = "" end +# When proxy protocol is active, the real client IP is only known after the +# proxy header is parsed (session phase), so reject must use tcp-request session. +# Without proxy protocol, tcp-request connection is sufficient. +conn_rate_limit_phase = p("ha_proxy.accept_proxy") ? "tcp-request session" : "tcp-request connection" + # }}} # Global SSL Flags {{{ ssl_flags = "" @@ -431,10 +436,10 @@ frontend http-in acl layer4_block src -f /var/vcap/jobs/haproxy/config/blocklist_cidrs_tcp.txt tcp-request connection reject if layer4_block <%- if_p("ha_proxy.connections_rate_limit.table_size", "ha_proxy.connections_rate_limit.window_size") do -%> - tcp-request connection track-sc0 src table st_tcp_conn_rate + <%= conn_rate_limit_phase %> track-sc0 src table st_tcp_conn_rate <%- if_p("ha_proxy.connections_rate_limit.block", "ha_proxy.connections_rate_limit.connections") do |block, connections| -%> <%-if block -%> - tcp-request connection reject if { sc_conn_rate(0) gt <%= connections %> } + <%= conn_rate_limit_phase %> reject if { sc_conn_rate(0) gt <%= connections %> } <%- end -%> <%- end -%> <%- end -%> @@ -565,10 +570,10 @@ frontend https-in acl layer4_block src -f /var/vcap/jobs/haproxy/config/blocklist_cidrs_tcp.txt tcp-request connection reject if layer4_block <%- if_p("ha_proxy.connections_rate_limit.table_size", "ha_proxy.connections_rate_limit.window_size") do -%> - tcp-request connection track-sc0 src table st_tcp_conn_rate + <%= conn_rate_limit_phase %> track-sc0 src table st_tcp_conn_rate <%- if_p("ha_proxy.connections_rate_limit.block", "ha_proxy.connections_rate_limit.connections") do |block, connections| -%> <%-if block -%> - tcp-request connection reject if { sc_conn_rate(0) gt <%= connections %> } + <%= conn_rate_limit_phase %> reject if { sc_conn_rate(0) gt <%= connections %> } <%- 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 5baddf8d..a3bce227 100644 --- a/spec/haproxy/templates/haproxy_config/rate_limit_spec.rb +++ b/spec/haproxy/templates/haproxy_config/rate_limit_spec.rb @@ -88,17 +88,41 @@ expect(frontend_https).to include('tcp-request connection track-sc0 src table st_tcp_conn_rate') end + context 'when proxy protocol used' do + let(:properties) do + temp_properties.deep_merge({ 'accept_proxy' => true }) + end + + it 'tracks connections in stick tables' do + expect(frontend_http).to include('tcp-request session track-sc0 src table st_tcp_conn_rate') + expect(frontend_https).to include('tcp-request session track-sc0 src table st_tcp_conn_rate') + end + end + context 'when "connections" and "block" are also provided' do let(:properties) do temp_properties.deep_merge({ 'connections_rate_limit' => { 'connections' => '5', 'block' => 'true' } }) end - it 'adds http-request deny condition to http-in and https-in frontends' do + it 'adds tcp-request connection reject to http-in and https-in frontends' do expect(frontend_http).to include('tcp-request connection reject if { sc_conn_rate(0) gt 5 }') expect(frontend_http).to include('tcp-request connection track-sc0 src table st_tcp_conn_rate') expect(frontend_https).to include('tcp-request connection reject if { sc_conn_rate(0) gt 5 }') expect(frontend_https).to include('tcp-request connection track-sc0 src table st_tcp_conn_rate') end end + + context 'when proxy protocol used and "connections" and "block" are also provided' do + let(:properties) do + temp_properties.deep_merge({ 'accept_proxy' => true, 'connections_rate_limit' => { 'connections' => '5', 'block' => 'true' } }) + end + + it 'adds tcp-request session reject to http-in and https-in frontends' do + expect(frontend_http).to include('tcp-request session reject if { sc_conn_rate(0) gt 5 }') + expect(frontend_http).to include('tcp-request session track-sc0 src table st_tcp_conn_rate') + expect(frontend_https).to include('tcp-request session reject if { sc_conn_rate(0) gt 5 }') + expect(frontend_https).to include('tcp-request session track-sc0 src table st_tcp_conn_rate') + end + end end end From 1032a97364b25ef6d770850760c4e3e33f3af881 Mon Sep 17 00:00:00 2001 From: Tamara Boehm Date: Fri, 29 May 2026 10:22:59 +0200 Subject: [PATCH 2/2] fix: rejects blocklisted IPs in TCP-layer when proxy protocol used --- acceptance-tests/access_control_test.go | 38 +++++++++++++++++++ acceptance-tests/proxy_protocol_test.go | 6 ++- jobs/haproxy/templates/haproxy.config.erb | 14 +++---- .../haproxy_config/frontend_http_spec.rb | 35 +++++++++++++++++ .../haproxy_config/frontend_https_spec.rb | 35 +++++++++++++++++ 5 files changed, 120 insertions(+), 8 deletions(-) diff --git a/acceptance-tests/access_control_test.go b/acceptance-tests/access_control_test.go index de101914..5cd5aa5f 100644 --- a/acceptance-tests/access_control_test.go +++ b/acceptance-tests/access_control_test.go @@ -140,4 +140,42 @@ var _ = Describe("Access Control", func() { By("Allowing TCP connections from non-blocklisted CIDRs (request from 127.0.0.1 on HAProxy VM)") expectTestServer200(http.Get("http://127.0.0.1:11000")) }) + + It("Rejects IPs in TCP-layer blocklisted CIDRs when proxy protocol is enabled", func() { + haproxyBackendPort := 12000 + + opsfileTCPBlocklistWithProxyProtocol := opsfileTCPBlocklist + ` +# Enable Proxy Protocol +- type: replace + path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/accept_proxy? + value: true +` + haproxyInfo, _ := deployHAProxy(baseManifestVars{ + haproxyBackendPort: haproxyBackendPort, + haproxyBackendServers: []string{"127.0.0.1"}, + deploymentName: deploymentNameForTestNode(), + }, []string{opsfileTCPBlocklistWithProxyProtocol}, map[string]interface{}{ + // traffic from test runner appears to come from this CIDR block + "cidr_blocklist_tcp": []string{"10.1.1.1/32"}, + }, true) + + closeLocalServer, localPort := startDefaultTestServer() + defer closeLocalServer() + + closeBackendTunnel := setupTunnelFromHaproxyToTestServer(haproxyInfo, haproxyBackendPort, localPort) + defer closeBackendTunnel() + + By("Denying TCP connections from blocklisted CIDRs via Proxy Protocol header (source IP 10.1.1.1)") + // Send a request with a Proxy Protocol header advertising a source IP in the blocked 10.0.0.0/8 CIDR. + // With accept_proxy enabled, HAProxy parses the PROXY header and uses the advertised source IP + // for the tcp-request session phase ACL evaluation — so this must be rejected. + err := performProxyProtocolRequestWithSourceIP(haproxyInfo.PublicIP, 80, "/", "10.1.1.1") + Expect(err).To(HaveOccurred()) + Expect(errors.Is(err, syscall.ECONNRESET)).To(BeTrue()) + + By("Allowing TCP connections from non-blocklisted CIDRs via Proxy Protocol header (source IP 192.168.1.1)") + // Send a request with a Proxy Protocol header advertising a source IP outside the blocked CIDR. + err = performProxyProtocolRequestWithSourceIP(haproxyInfo.PublicIP, 80, "/", "192.168.1.1") + Expect(err).NotTo(HaveOccurred()) + }) }) diff --git a/acceptance-tests/proxy_protocol_test.go b/acceptance-tests/proxy_protocol_test.go index df4f3b22..0b8c620f 100644 --- a/acceptance-tests/proxy_protocol_test.go +++ b/acceptance-tests/proxy_protocol_test.go @@ -138,6 +138,10 @@ var _ = Describe("Proxy Protocol", func() { }) func performProxyProtocolRequest(ip string, port int, endpoint string) error { + return performProxyProtocolRequestWithSourceIP(ip, port, endpoint, "10.1.1.1") +} + +func performProxyProtocolRequestWithSourceIP(ip string, port int, endpoint string, sourceIP string) error { // Create a connection to the HAProxy instance conn, err := net.Dial("tcp", net.JoinHostPort(ip, strconv.Itoa(port))) if err != nil { @@ -152,7 +156,7 @@ func performProxyProtocolRequest(ip string, port int, endpoint string) error { Command: proxyproto.PROXY, TransportProtocol: proxyproto.TCPv4, SourceAddr: &net.TCPAddr{ - IP: net.ParseIP("10.1.1.1"), + IP: net.ParseIP(sourceIP), Port: 1000, }, DestinationAddr: &net.TCPAddr{ diff --git a/jobs/haproxy/templates/haproxy.config.erb b/jobs/haproxy/templates/haproxy.config.erb index e34a8114..bd18e5e0 100644 --- a/jobs/haproxy/templates/haproxy.config.erb +++ b/jobs/haproxy/templates/haproxy.config.erb @@ -42,7 +42,7 @@ end # When proxy protocol is active, the real client IP is only known after the # proxy header is parsed (session phase), so reject must use tcp-request session. # Without proxy protocol, tcp-request connection is sufficient. -conn_rate_limit_phase = p("ha_proxy.accept_proxy") ? "tcp-request session" : "tcp-request connection" +tcp_request_phase = p("ha_proxy.accept_proxy") ? "session" : "connection" # }}} # Global SSL Flags {{{ @@ -434,12 +434,12 @@ frontend http-in <%= format_indented_multiline_config(p("ha_proxy.frontend_config")) %> <%- end -%> acl layer4_block src -f /var/vcap/jobs/haproxy/config/blocklist_cidrs_tcp.txt - tcp-request connection reject if layer4_block + tcp-request <%= tcp_request_phase %> reject if layer4_block <%- if_p("ha_proxy.connections_rate_limit.table_size", "ha_proxy.connections_rate_limit.window_size") do -%> - <%= conn_rate_limit_phase %> track-sc0 src table st_tcp_conn_rate + tcp-request <%= tcp_request_phase %> track-sc0 src table st_tcp_conn_rate <%- if_p("ha_proxy.connections_rate_limit.block", "ha_proxy.connections_rate_limit.connections") do |block, connections| -%> <%-if block -%> - <%= conn_rate_limit_phase %> reject if { sc_conn_rate(0) gt <%= connections %> } + tcp-request <%= tcp_request_phase %> reject if { sc_conn_rate(0) gt <%= connections %> } <%- end -%> <%- end -%> <%- end -%> @@ -568,12 +568,12 @@ frontend https-in <%= format_indented_multiline_config(p("ha_proxy.frontend_config")) %> <%- end -%> acl layer4_block src -f /var/vcap/jobs/haproxy/config/blocklist_cidrs_tcp.txt - tcp-request connection reject if layer4_block + tcp-request <%= tcp_request_phase %> reject if layer4_block <%- if_p("ha_proxy.connections_rate_limit.table_size", "ha_proxy.connections_rate_limit.window_size") do -%> - <%= conn_rate_limit_phase %> track-sc0 src table st_tcp_conn_rate + tcp-request <%= tcp_request_phase %> track-sc0 src table st_tcp_conn_rate <%- if_p("ha_proxy.connections_rate_limit.block", "ha_proxy.connections_rate_limit.connections") do |block, connections| -%> <%-if block -%> - <%= conn_rate_limit_phase %> reject if { sc_conn_rate(0) gt <%= connections %> } + tcp-request <%= tcp_request_phase %> reject if { sc_conn_rate(0) gt <%= connections %> } <%- 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 f9cfb3ab..8f64048b 100644 --- a/spec/haproxy/templates/haproxy_config/frontend_http_spec.rb +++ b/spec/haproxy/templates/haproxy_config/frontend_http_spec.rb @@ -153,6 +153,41 @@ expect(frontend_http).to include('tcp-request connection reject if layer4_block') end end + + context 'when ha_proxy.accept_proxy set to true' do + context 'when ha_proxy.cidr_blocklist_tcp is provided' do + let(:properties) do + { 'accept_proxy' => true, 'cidr_blocklist_tcp' => ['172.168.4.1/32', '10.2.0.0/16'] } + end + + it 'sets the correct acl and connection reject rules' do + expect(frontend_http).to include('acl layer4_block src -f /var/vcap/jobs/haproxy/config/blocklist_cidrs_tcp.txt') + expect(frontend_http).to include('tcp-request session reject if layer4_block') + end + end + + context 'when ha_proxy.cidr_blocklist_tcp is not provided' do + let(:properties) do + { 'accept_proxy' => true } + end + + it 'still includes the layer4_block acl and reject rule (always present)' do + expect(frontend_http).to include('acl layer4_block src -f /var/vcap/jobs/haproxy/config/blocklist_cidrs_tcp.txt') + expect(frontend_http).to include('tcp-request session reject if layer4_block') + end + end + + context 'when ha_proxy.cidr_blocklist_tcp is an empty array' do + let(:properties) do + { 'accept_proxy' => true, 'cidr_blocklist_tcp' => [] } + end + + it 'still includes the layer4_block acl and reject rule (always present)' do + expect(frontend_http).to include('acl layer4_block src -f /var/vcap/jobs/haproxy/config/blocklist_cidrs_tcp.txt') + expect(frontend_http).to include('tcp-request session reject if layer4_block') + end + end + end end context 'when ha_proxy.block_all 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 2645fcc3..2c447bb5 100644 --- a/spec/haproxy/templates/haproxy_config/frontend_https_spec.rb +++ b/spec/haproxy/templates/haproxy_config/frontend_https_spec.rb @@ -585,6 +585,41 @@ expect(frontend_https).to include('tcp-request connection reject if layer4_block') end end + + context 'when ha_proxy.accept_proxy set to true' do + context 'when ha_proxy.cidr_blocklist_tcp is provided' do + let(:properties) do + default_properties.merge({ 'accept_proxy' => true, 'cidr_blocklist_tcp' => ['172.168.4.1/32', '10.2.0.0/16'] }) + end + + it 'sets the correct acl and connection reject rules' do + expect(frontend_https).to include('acl layer4_block src -f /var/vcap/jobs/haproxy/config/blocklist_cidrs_tcp.txt') + expect(frontend_https).to include('tcp-request session reject if layer4_block') + end + end + + context 'when ha_proxy.cidr_blocklist_tcp is not provided' do + let(:properties) do + default_properties.merge({ 'accept_proxy' => true }) + end + + it 'still includes the layer4_block acl and reject rule (always present)' do + expect(frontend_https).to include('acl layer4_block src -f /var/vcap/jobs/haproxy/config/blocklist_cidrs_tcp.txt') + expect(frontend_https).to include('tcp-request session reject if layer4_block') + end + end + + context 'when ha_proxy.cidr_blocklist_tcp is an empty array' do + let(:properties) do + default_properties.merge({ 'accept_proxy' => true, 'cidr_blocklist_tcp' => [] }) + end + + it 'still includes the layer4_block acl and reject rule (always present)' do + expect(frontend_https).to include('acl layer4_block src -f /var/vcap/jobs/haproxy/config/blocklist_cidrs_tcp.txt') + expect(frontend_https).to include('tcp-request session reject if layer4_block') + end + end + end end context 'when ha_proxy.block_all is provided' do