diff --git a/acceptance-tests/rate_limit_test.go b/acceptance-tests/rate_limit_test.go index 2400d173..1d02009c 100644 --- a/acceptance-tests/rate_limit_test.go +++ b/acceptance-tests/rate_limit_test.go @@ -165,6 +165,54 @@ var _ = Describe("Rate-Limiting", func() { Expect(successfulRequestCount).To(Equal(connLimit)) }) + It("Excluded CIDRs are never connection rate-limited", func() { + connLimit := 5 + // exclude_cidrs covers all IPv4 sources so the test runner's egress IP is + // guaranteed to match, proving the negated reject rule lets excluded sources through + opsfileConnRateLimitExclude := fmt.Sprintf(`--- +- 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 +- type: replace + path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/connections_rate_limit/exclude_cidrs? + value: + - 0.0.0.0/0 +`, connLimit) + haproxyBackendPort := 12000 + haproxyInfo, _ := deployHAProxy(baseManifestVars{ + haproxyBackendPort: haproxyBackendPort, + haproxyBackendServers: []string{"127.0.0.1"}, + deploymentName: deploymentNameForTestNode(), + }, []string{opsfileConnRateLimitExclude}, map[string]interface{}{}, true) + + closeLocalServer, localPort := startDefaultTestServer() + defer closeLocalServer() + + closeTunnel := setupTunnelFromHaproxyToTestServer(haproxyInfo, haproxyBackendPort, localPort) + defer closeTunnel() + + By("Sending more connections than the limit, expecting none to be blocked because the source is excluded") + testRequestCount := connLimit * 3 + for i := 0; i < testRequestCount; i++ { + rt := &http.Transport{ + DisableKeepAlives: true, + } + client := &http.Client{Transport: rt} + resp, err := client.Get(fmt.Sprintf("http://%s/foo", haproxyInfo.PublicIP)) + Expect(err).NotTo(HaveOccurred()) + Expect(resp.StatusCode).To(Equal(http.StatusOK)) + } + }) + It("Connection Based Limiting Works with Proxy Protocol enabled", func() { connLimit := 5 opsfileConnRateLimitWithProxyProtocol := fmt.Sprintf(`--- diff --git a/jobs/haproxy/spec b/jobs/haproxy/spec index ef407dc2..7da59dee 100644 --- a/jobs/haproxy/spec +++ b/jobs/haproxy/spec @@ -26,6 +26,7 @@ templates: 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 + rate_limit_exclusion_cidrs.txt.erb: config/rate_limit_exclusion_cidrs.txt consumes: - name: http_backend @@ -827,3 +828,12 @@ properties: ha_proxy.connections_rate_limit.block: description: Whether or not to block connections. See docs/rate_limiting.md default: false + ha_proxy.connections_rate_limit.exclude_cidrs: + description: "List of CIDRs (e.g. private/NAT ranges) to exclude from connection based rate-limiting. Excluded sources are still tracked in the stick-table but are never rejected. Format is string array of CIDRs or single string of base64 encoded gzip. See docs/rate_limiting.md" + default: ~ + example: + connections_rate_limit: + exclude_cidrs: + - 10.0.0.0/8 + - 192.168.0.0/16 + - 2001:db8::/32 diff --git a/jobs/haproxy/templates/haproxy.config.erb b/jobs/haproxy/templates/haproxy.config.erb index 12ef57d7..03d334e0 100644 --- a/jobs/haproxy/templates/haproxy.config.erb +++ b/jobs/haproxy/templates/haproxy.config.erb @@ -451,9 +451,10 @@ frontend http-in acl layer4_block src -f /var/vcap/jobs/haproxy/config/blocklist_cidrs_tcp.txt 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 -%> + acl rate_limit_exclude src -f /var/vcap/jobs/haproxy/config/rate_limit_exclusion_cidrs.txt tcp-request <%= tcp_request_phase %> track-sc0 src table st_tcp_conn_rate # use sub() converter as variable references are only accepted as arguments to converters - tcp-request <%= tcp_request_phase %> reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 } + tcp-request <%= tcp_request_phase %> reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 } !rate_limit_exclude <%- end -%> <%- if_p("ha_proxy.requests_rate_limit.table_size", "ha_proxy.requests_rate_limit.window_size") do -%> http-request track-sc1 src table st_http_req_rate @@ -582,9 +583,10 @@ frontend https-in acl layer4_block src -f /var/vcap/jobs/haproxy/config/blocklist_cidrs_tcp.txt 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 -%> + acl rate_limit_exclude src -f /var/vcap/jobs/haproxy/config/rate_limit_exclusion_cidrs.txt tcp-request <%= tcp_request_phase %> track-sc0 src table st_tcp_conn_rate # use sub() converter as variable references are only accepted as arguments to converters - tcp-request <%= tcp_request_phase %> reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 } + tcp-request <%= tcp_request_phase %> reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 } !rate_limit_exclude <%- end -%> <%- if_p("ha_proxy.requests_rate_limit.table_size", "ha_proxy.requests_rate_limit.window_size") do -%> http-request track-sc1 src table st_http_req_rate diff --git a/jobs/haproxy/templates/rate_limit_exclusion_cidrs.txt.erb b/jobs/haproxy/templates/rate_limit_exclusion_cidrs.txt.erb new file mode 100644 index 00000000..35911f84 --- /dev/null +++ b/jobs/haproxy/templates/rate_limit_exclusion_cidrs.txt.erb @@ -0,0 +1,21 @@ +# generated from rate_limit_exclusion_cidrs.txt.erb +<% +require "base64" +require 'zlib' +require 'stringio' + +cidrs = p("ha_proxy.connections_rate_limit.exclude_cidrs", []) +uncompressed = '' +if cidrs.is_a?(Array) && cidrs.any? + uncompressed << "\# detected cidrs provided as array in cleartext format\n" + cidrs.each do |cidr| + uncompressed << cidr << "\n" + end +elsif cidrs.is_a?(String) + gzplain = Base64.decode64(cidrs) + gz = Zlib::GzipReader.new(StringIO.new(gzplain)) + uncompressed = gz.read +end +%> +# This list contains CIDRs excluded from connection based rate-limiting (tracked but never rejected). +<%= uncompressed %> diff --git a/spec/haproxy/templates/haproxy_config/rate_limit_spec.rb b/spec/haproxy/templates/haproxy_config/rate_limit_spec.rb index f041cf90..0d14b1d1 100644 --- a/spec/haproxy/templates/haproxy_config/rate_limit_spec.rb +++ b/spec/haproxy/templates/haproxy_config/rate_limit_spec.rb @@ -104,8 +104,8 @@ end it 'always emits the reject rule (even without connections or block set in manifest)' do - expect(frontend_http).to include('tcp-request connection reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 }') - expect(frontend_https).to include('tcp-request connection reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 }') + expect(frontend_http).to include('tcp-request connection reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 } !rate_limit_exclude') + expect(frontend_https).to include('tcp-request connection reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 } !rate_limit_exclude') end it 'always sets proc.connections_rate_limit_block to false in global when block is not configured in manifest' do @@ -113,6 +113,34 @@ expect(haproxy_conf['global']).not_to include('set-var proc.connections_rate_limit_connections') end + it 'always adds the exclusion acl and negates the reject rule (empty exclusion file is a no-op by default)' do + expect(frontend_http).to include('acl rate_limit_exclude src -f /var/vcap/jobs/haproxy/config/rate_limit_exclusion_cidrs.txt') + expect(frontend_https).to include('acl rate_limit_exclude src -f /var/vcap/jobs/haproxy/config/rate_limit_exclusion_cidrs.txt') + expect(frontend_http).to include('tcp-request connection reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 } !rate_limit_exclude') + expect(frontend_https).to include('tcp-request connection reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 } !rate_limit_exclude') + end + + context 'when connections_rate_limit.exclude_cidrs is provided' do + let(:properties) do + temp_properties.deep_merge({ 'connections_rate_limit' => { 'exclude_cidrs' => ['10.0.0.0/8', '192.168.0.0/16'] } }) + end + + it 'adds the exclusion acl to http-in and https-in frontends' do + expect(frontend_http).to include('acl rate_limit_exclude src -f /var/vcap/jobs/haproxy/config/rate_limit_exclusion_cidrs.txt') + expect(frontend_https).to include('acl rate_limit_exclude src -f /var/vcap/jobs/haproxy/config/rate_limit_exclusion_cidrs.txt') + end + + it 'still emits the track-sc0 directive so excluded sources remain tracked (not skipped)' do + expect(frontend_http).to include('tcp-request connection track-sc0 src table st_tcp_conn_rate') + expect(frontend_https).to include('tcp-request connection track-sc0 src table st_tcp_conn_rate') + end + + it 'negates the reject rule with the exclusion acl so excluded sources are never rejected' do + expect(frontend_http).to include('tcp-request connection reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 } !rate_limit_exclude') + expect(frontend_https).to include('tcp-request connection reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 } !rate_limit_exclude') + end + end + context 'when proxy protocol used' do let(:properties) do temp_properties.deep_merge({ 'accept_proxy' => true }) @@ -130,9 +158,9 @@ end it 'adds tcp-request connection reject using process variables to http-in and https-in frontends' do - expect(frontend_http).to include('tcp-request connection reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 }') + expect(frontend_http).to include('tcp-request connection reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 } !rate_limit_exclude') 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 { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 }') + expect(frontend_https).to include('tcp-request connection reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 } !rate_limit_exclude') expect(frontend_https).to include('tcp-request connection track-sc0 src table st_tcp_conn_rate') end end @@ -148,8 +176,8 @@ end it 'still emits reject rule (rejection controlled at runtime via proc.connections_rate_limit_block variable)' do - expect(frontend_http).to include('tcp-request connection reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 }') - expect(frontend_https).to include('tcp-request connection reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 }') + expect(frontend_http).to include('tcp-request connection reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 } !rate_limit_exclude') + expect(frontend_https).to include('tcp-request connection reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 } !rate_limit_exclude') end end @@ -169,9 +197,9 @@ end it 'adds tcp-request session reject using process variables to http-in and https-in frontends' do - expect(frontend_http).to include('tcp-request session reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 }') + expect(frontend_http).to include('tcp-request session reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 } !rate_limit_exclude') 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 { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 }') + expect(frontend_https).to include('tcp-request session reject if { var(proc.connections_rate_limit_block) -m bool } { var(proc.connections_rate_limit_connections) -m int gt 0 } { sc_conn_rate(0),sub(proc.connections_rate_limit_connections) gt 0 } !rate_limit_exclude') expect(frontend_https).to include('tcp-request session track-sc0 src table st_tcp_conn_rate') end end diff --git a/spec/haproxy/templates/rate_limit_exclusion_cidrs.txt_spec.rb b/spec/haproxy/templates/rate_limit_exclusion_cidrs.txt_spec.rb new file mode 100644 index 00000000..62284e47 --- /dev/null +++ b/spec/haproxy/templates/rate_limit_exclusion_cidrs.txt_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'rspec' + +describe 'config/rate_limit_exclusion_cidrs.txt' do + let(:template) { haproxy_job.template('config/rate_limit_exclusion_cidrs.txt') } + + context 'when ha_proxy.connections_rate_limit.exclude_cidrs is provided' do + context 'when an array of cidrs is provided' do + it 'has the correct contents' do + expect(template.render({ + 'ha_proxy' => { + 'connections_rate_limit' => { + 'exclude_cidrs' => [ + '10.0.0.0/8', + '192.168.2.0/24' + ] + } + } + })).to eq(<<~EXPECTED) + # generated from rate_limit_exclusion_cidrs.txt.erb + + # This list contains CIDRs excluded from connection based rate-limiting (tracked but never rejected). + # detected cidrs provided as array in cleartext format + 10.0.0.0/8 + 192.168.2.0/24 + + EXPECTED + end + end + + context 'when a base64-encoded, gzipped config is provided' do + it 'has the correct contents' do + expect(template.render({ + 'ha_proxy' => { + 'connections_rate_limit' => { + 'exclude_cidrs' => gzip_and_b64_encode(<<~INPUT) + 10.0.0.0/8 + 192.168.2.0/24 + INPUT + } + } + })).to eq(<<~EXPECTED) + # generated from rate_limit_exclusion_cidrs.txt.erb + + # This list contains CIDRs excluded from connection based rate-limiting (tracked but never rejected). + 10.0.0.0/8 + 192.168.2.0/24 + + EXPECTED + end + end + end + + context 'when ha_proxy.connections_rate_limit.exclude_cidrs is not provided' do + it 'renders only the header comment (empty exclusion list is a no-op)' do + expect(template.render({})).to eq(<<~EXPECTED) + # generated from rate_limit_exclusion_cidrs.txt.erb + + # This list contains CIDRs excluded from connection based rate-limiting (tracked but never rejected). + + EXPECTED + end + end +end