Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add parameter to set a header containing the client ip #759

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 72 additions & 6 deletions acceptance-tests/headers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ package acceptance_tests
import (
"crypto/tls"
"fmt"
"net"
"net/http"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"net/http"
)

var _ = Describe("Headers", func() {
Expand All @@ -18,6 +20,9 @@ var _ = Describe("Headers", func() {
value:
Custom-Header-To-Add: add-value
Custom-Header-To-Replace: replace-value
- type: replace
path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/true_client_ip_header?
value: "X-CF-True-Client-IP"
# Configure CA and cert chain
- type: replace
path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/crt_list?/-
Expand Down Expand Up @@ -47,6 +52,7 @@ var _ = Describe("Headers", func() {
alternative_names: [haproxy.internal]
`
var closeLocalServer func()
var closeTunnel func()
var creds struct {
HTTPSFrontend struct {
Certificate string `yaml:"certificate"`
Expand All @@ -57,8 +63,9 @@ var _ = Describe("Headers", func() {
var client *http.Client
var recordedHeaders http.Header
var request *http.Request
var err error

It("Check correct headers handling", func() {
BeforeEach(func() {
haproxyBackendPort := 12000
var varsStoreReader varsStoreReader
haproxyInfo, varsStoreReader := deployHAProxy(baseManifestVars{
Expand All @@ -67,7 +74,7 @@ var _ = Describe("Headers", func() {
deploymentName: deploymentNameForTestNode(),
}, []string{opsfileHeaders}, map[string]interface{}{}, true)

err := varsStoreReader(&creds)
err = varsStoreReader(&creds)
Expect(err).NotTo(HaveOccurred())

By("Starting a local http server to act as a backend")
Expand All @@ -78,16 +85,21 @@ var _ = Describe("Headers", func() {
_, _ = w.Write([]byte("OK"))
})
Expect(err).NotTo(HaveOccurred())
defer closeLocalServer()

closeTunnel := setupTunnelFromHaproxyToTestServer(haproxyInfo, haproxyBackendPort, localPort)
defer closeTunnel()
closeTunnel = setupTunnelFromHaproxyToTestServer(haproxyInfo, haproxyBackendPort, localPort)
client = buildHTTPClient(
[]string{creds.HTTPSFrontend.CA},
map[string]string{"haproxy.internal:443": fmt.Sprintf("%s:443", haproxyInfo.PublicIP)},
[]tls.Certificate{}, "",
)
})

AfterEach(func() {
closeLocalServer()
closeTunnel()
})

It("Adds, replaces and strips headers correctly", func() {
request, err = http.NewRequest("GET", "https://haproxy.internal:443", nil)
Expect(err).NotTo(HaveOccurred())

Expand Down Expand Up @@ -140,4 +152,58 @@ var _ = Describe("Headers", func() {
}

})

It("Adds a header with the provided name and correct value for the true client ip", func() {
ipAdresses := getAllIpAddresses()
request, err = http.NewRequest("GET", "https://haproxy.internal:443", nil)
Expect(err).NotTo(HaveOccurred())

By("Correctly sets the True-Client-Ip Header")
resp, err := client.Do(request)
expect200(resp, err)
headerKey := "X-Cf-True-Client-Ip"
Expect(recordedHeaders).To(HaveKey(headerKey))
Expect(recordedHeaders[headerKey]).To(HaveLen(1))
Expect(ipAdresses).To(ContainElement(recordedHeaders[headerKey][0]))
hoffmaen marked this conversation as resolved.
Show resolved Hide resolved
})

})

func getAllIpAddresses() (ips []string) {
ifaces, err := net.Interfaces()
if err != nil {
fmt.Printf("Cannot get interface: %s", err)
return
}

for _, iface := range ifaces {
if iface.Flags&net.FlagUp == 0 {
continue
}
if iface.Flags&net.FlagLoopback != 0 {
continue
}
addrs, err := iface.Addrs()
if err != nil {
fmt.Printf("Cannot get addresses for interface %s: %s", iface.Name, err)
continue
}
for _, addr := range addrs {
var ip net.IP
switch v := addr.(type) {
case *net.IPNet:
ip = v.IP
case *net.IPAddr:
ip = v.IP
default:
continue
}
if ip == nil || ip.IsLoopback() {
continue
}

ips = append(ips, ip.String())
}
}
return ips
}
3 changes: 3 additions & 0 deletions jobs/haproxy/spec
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,9 @@ properties:
X-Application-ID: my-custom-header
MyCustomHeader: 3

ha_proxy.true_client_ip_header:
description: "Header to use to store the client's IP address, as seen from HAProxy. The header will be overwritten if it already exists in the request."

ha_proxy.backend_crt:
description: "Provides client certificate to backend server to do mutual ssl. Note this only configures the client cert for HTTP backends configured via the backend_servers property or through BOSH links. It is not used with backend servers configured via routed_backend_servers or TCP backends"
example: |
Expand Down
6 changes: 6 additions & 0 deletions jobs/haproxy/templates/haproxy.config.erb
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,9 @@ frontend http-in
http-response add-header <%= rsp_header.gsub(/(?!:\\)( )/, '\ ') %> "<%= value.to_s.gsub(/(?!:\\) /, '\ ') %>"
<%- end -%>
<%- end -%>
<%- if_p("ha_proxy.true_client_ip_header") do |header| -%>
http-request set-header <%= header %> %[src]
<%- end -%>
<%- if p("ha_proxy.internal_only_domains").size > 0 -%>
acl private src -f /var/vcap/jobs/haproxy/config/trusted_domain_cidrs.txt
<%- p("ha_proxy.internal_only_domains").each do |domain| -%>
Expand Down Expand Up @@ -651,6 +654,9 @@ frontend https-in
http-response add-header <%= rsp_header.gsub(/(?!:\\)( )/, '\ ') %> "<%= value.to_s.gsub(/(?!:\\) /, '\ ') %>"
<%- end -%>
<%- end -%>
<%- if_p("ha_proxy.true_client_ip_header") do |header| -%>
http-request set-header <%= header %> %[src]
<%- end -%>
<%- if p("ha_proxy.internal_only_domains").size > 0 -%>
acl private src -f /var/vcap/jobs/haproxy/config/trusted_domain_cidrs.txt
<%- p("ha_proxy.internal_only_domains").each do |domain| -%>
Expand Down
10 changes: 10 additions & 0 deletions spec/haproxy/templates/haproxy_config/frontend_http_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,16 @@
expect(frontend_http).to include('http-request add-header X-Forwarded-Proto "http" if ! xfp_exists')
end

context 'when ha_proxy.true_client_ip_header is set' do
let(:properties) do
{ 'true_client_ip_header' => 'X-CF-True-Client-IP' }
end

it 'adds the X-CF-True-Client-IP header' do
expect(frontend_http).to include('http-request set-header X-CF-True-Client-IP %[src]')
end
end

context 'when ha_proxy.https_redirect_all is true' do
let(:properties) do
{ 'https_redirect_all' => true }
Expand Down
10 changes: 10 additions & 0 deletions spec/haproxy/templates/haproxy_config/frontend_https_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,16 @@
expect(frontend_https).to include('http-request add-header X-Forwarded-Proto "https" if ! xfp_exists')
end

context 'when ha_proxy.true_client_ip_header is set' do
let(:properties) do
default_properties.merge({ 'true_client_ip_header' => 'X-CF-True-Client-IP' })
end

it 'adds the X-CF-True-Client-IP header' do
expect(frontend_https).to include('http-request set-header X-CF-True-Client-IP %[src]')
end
end

context 'when ha_proxy.enable_http2 is true' do
let(:properties) do
default_properties.merge({ 'enable_http2' => true })
Expand Down