diff --git a/acceptance-tests/acceptance_tests_suite_test.go b/acceptance-tests/acceptance_tests_suite_test.go index 33e195a2..e22c7957 100644 --- a/acceptance-tests/acceptance_tests_suite_test.go +++ b/acceptance-tests/acceptance_tests_suite_test.go @@ -7,9 +7,11 @@ import ( "net" "net/http" "net/url" + "strings" "testing" "time" + "github.com/gorilla/websocket" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/onsi/gomega/gbytes" @@ -33,12 +35,37 @@ var _ = AfterSuite(func() { deleteDeployment(defaultDeploymentName) }) -// Starts a simple test server that returns 200 OK +// Starts a simple test server that returns 200 OK or echoes websocket messages back func startDefaultTestServer() (func(), int) { - By("Starting a local http server to act as a backend") + var upgrader = websocket.Upgrader{} + + By("Starting a local websocket server to act as a backend") closeLocalServer, localPort, err := startLocalHTTPServer(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprintln(w, "Hello cloud foundry") + // if no upgrade requested, act like a normal HTTP server + if strings.ToLower(r.Header.Get("Upgrade")) != "websocket" { + fmt.Fprintln(w, "Hello cloud foundry") + return + } + + conn, err := upgrader.Upgrade(w, r, nil) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + return + } + defer conn.Close() + + for { + messageType, message, err := conn.ReadMessage() + if err != nil { + break + } + err = conn.WriteMessage(messageType, message) + if err != nil { + break + } + } }) + Expect(err).NotTo(HaveOccurred()) return closeLocalServer, localPort } diff --git a/acceptance-tests/go.mod b/acceptance-tests/go.mod index 2d2c4f1e..9c244918 100644 --- a/acceptance-tests/go.mod +++ b/acceptance-tests/go.mod @@ -4,6 +4,7 @@ go 1.16 require ( github.com/bramvdbogaerde/go-scp v0.0.0-20210527193300-acf430e39785 + github.com/gorilla/websocket v1.4.2 github.com/onsi/ginkgo v1.16.2 github.com/onsi/gomega v1.13.0 golang.org/x/crypto v0.0.0-20210513164829-c07d793c2f9a diff --git a/acceptance-tests/go.sum b/acceptance-tests/go.sum index eca55649..d0edeeb4 100644 --- a/acceptance-tests/go.sum +++ b/acceptance-tests/go.sum @@ -19,8 +19,9 @@ github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiu github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc= +github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= @@ -80,7 +81,6 @@ golang.org/x/tools v0.0.0-20201224043029-2b0845dc783e/go.mod h1:emZCQorbCU4vsT4f golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= diff --git a/acceptance-tests/websocket_test.go b/acceptance-tests/websocket_test.go new file mode 100644 index 00000000..3797fdc4 --- /dev/null +++ b/acceptance-tests/websocket_test.go @@ -0,0 +1,162 @@ +package acceptance_tests + +import ( + "context" + "crypto/tls" + "fmt" + "net" + "net/http" + "time" + + "github.com/gorilla/websocket" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/onsi/gomega/gbytes" +) + +var _ = Describe("HTTPS Frontend", func() { + var haproxyInfo haproxyInfo + var closeTunnel func() + var closeLocalServer func() + var enableHTTP2 bool + var disableBackendHttp2Websockets bool + var http1Client *http.Client + var http2Client *http.Client + + haproxyBackendPort := 12000 + opsfileHTTPS := `--- +# Configure HTTP2 +- type: replace + path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/enable_http2? + value: ((enable_http2)) +# Configure Disabling Backend HTTP2 Websockets +- type: replace + path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/disable_backend_http2_websockets? + value: ((disable_backend_http2_websockets)) +# Configure CA and cert chain +- type: replace + path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/crt_list?/- + value: + snifilter: + - haproxy.internal + ssl_pem: + cert_chain: ((https_frontend.certificate))((https_frontend_ca.certificate)) + private_key: ((https_frontend.private_key)) +# Declare certs +- type: replace + path: /variables?/- + value: + name: https_frontend_ca + type: certificate + options: + is_ca: true + common_name: bosh +- type: replace + path: /variables?/- + value: + name: https_frontend + type: certificate + options: + ca: https_frontend_ca + common_name: haproxy.internal + alternative_names: [haproxy.internal] +` + + var creds struct { + HTTPSFrontend struct { + Certificate string `yaml:"certificate"` + PrivateKey string `yaml:"private_key"` + CA string `yaml:"ca"` + } `yaml:"https_frontend"` + } + + JustBeforeEach(func() { + var varsStoreReader varsStoreReader + haproxyInfo, varsStoreReader = deployHAProxy(baseManifestVars{ + haproxyBackendPort: haproxyBackendPort, + haproxyBackendServers: []string{"127.0.0.1"}, + deploymentName: defaultDeploymentName, + }, []string{opsfileHTTPS}, map[string]interface{}{ + "enable_http2": enableHTTP2, + "disable_backend_http2_websockets": disableBackendHttp2Websockets, + }, true) + + err := varsStoreReader(&creds) + Expect(err).NotTo(HaveOccurred()) + + var localPort int + closeLocalServer, localPort = startDefaultTestServer() + closeTunnel = setupTunnelFromHaproxyToTestServer(haproxyInfo, haproxyBackendPort, localPort) + + http1Client = buildHTTPClient( + []string{creds.HTTPSFrontend.CA}, + map[string]string{"haproxy.internal:443": fmt.Sprintf("%s:443", haproxyInfo.PublicIP)}, + []tls.Certificate{}, "", + ) + + http2Client = buildHTTP2Client( + []string{creds.HTTPSFrontend.CA}, + map[string]string{"haproxy.internal:443": fmt.Sprintf("%s:443", haproxyInfo.PublicIP)}, + []tls.Certificate{}, + ) + }) + + AfterEach(func() { + if closeLocalServer != nil { + defer closeLocalServer() + } + if closeTunnel != nil { + defer closeTunnel() + } + }) + + Context("When ha_proxy.disable_backend_http2_websockets is true", func() { + BeforeEach(func() { + enableHTTP2 = true + disableBackendHttp2Websockets = true + }) + + It("succeeds with a websocket", func() { + dialer := websocket.DefaultDialer + dialer.TLSClientConfig = buildTLSConfig([]string{creds.HTTPSFrontend.CA}, []tls.Certificate{}, "") + dialer.NetDialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { + if addr == "haproxy.internal:443" { + addr = fmt.Sprintf("%s:443", haproxyInfo.PublicIP) + } + + return (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + }).DialContext(ctx, network, addr) + } + + By("Sending a request to HAProxy using HTTP 1.1") + resp, err := http1Client.Get("https://haproxy.internal:443") + Expect(err).NotTo(HaveOccurred()) + + Expect(resp.ProtoMajor).To(Equal(1)) + + Expect(resp.StatusCode).To(Equal(http.StatusOK)) + Eventually(gbytes.BufferReader(resp.Body)).Should(gbytes.Say("Hello cloud foundry")) + + By("Sending a request to HAProxy using HTTP 2") + resp, err = http2Client.Get("https://haproxy.internal:443") + Expect(err).NotTo(HaveOccurred()) + Expect(resp.ProtoMajor).To(Equal(2)) + + Expect(resp.StatusCode).To(Equal(http.StatusOK)) + Eventually(gbytes.BufferReader(resp.Body)).Should(gbytes.Say("Hello cloud foundry")) + + By("Sending a request using websockets") + websocketConn, _, err := dialer.Dial("wss://haproxy.internal:443", nil) + Expect(err).NotTo(HaveOccurred()) + defer websocketConn.Close() + + err = websocketConn.WriteMessage(websocket.TextMessage, []byte("hello via websockets")) + Expect(err).NotTo(HaveOccurred()) + _, message, err := websocketConn.ReadMessage() + Expect(err).NotTo(HaveOccurred()) + Expect(string(message)).To(Equal("hello via websockets")) + }) + }) +}) diff --git a/jobs/haproxy/spec b/jobs/haproxy/spec index e62e474a..68e169aa 100644 --- a/jobs/haproxy/spec +++ b/jobs/haproxy/spec @@ -244,6 +244,9 @@ properties: ha_proxy.disable_tls_13: default: false description: "Disable TLS 1.3 in HA Proxy" + ha_proxy.disable_backend_http2_websockets: + default: false + description: "Forward websockets to the backend servers using HTTP/1.1, never HTTP/2. Does not apply to custom routed_backend_servers. Works around https://github.com/cloudfoundry/routing-release/issues/230" ha_proxy.connect_timeout: description: "Timeout (in floating point seconds) used on connections from haproxy to a backend, while waiting for the TCP handshake to complete + connection to establish" diff --git a/jobs/haproxy/templates/haproxy.config.erb b/jobs/haproxy/templates/haproxy.config.erb index 9b82607b..adb1160f 100644 --- a/jobs/haproxy/templates/haproxy.config.erb +++ b/jobs/haproxy/templates/haproxy.config.erb @@ -353,6 +353,14 @@ frontend http-in acl ssl_redirect hdr(host),lower,map_end(/var/vcap/jobs/haproxy/config/ssl_redirect.map,false) -m str true redirect scheme https code 301 if ssl_redirect <%- end -%> + + <%- if p("ha_proxy.disable_backend_http2_websockets") -%> + # Send websockets to a backend that forces HTTP/1.1. This avoids bugs in Go & Gorouter's HTTP/2 websocket support + # https://github.com/cloudfoundry/routing-release/issues/230 + acl is_websocket hdr(Upgrade) -i WebSocket + acl is_websocket hdr_beg(Host) -i ws + use_backend http-routers-ws-http1 if is_websocket + <%- end -%> # }}} <% end -%> @@ -481,6 +489,14 @@ frontend https-in <%- end -%> acl xfp_exists hdr_cnt(X-Forwarded-Proto) gt 0 http-request add-header X-Forwarded-Proto "https" if ! xfp_exists + + <%- if p("ha_proxy.disable_backend_http2_websockets") -%> + # Send websockets to a backend that forces HTTP/1.1. This avoids bugs in Go & Gorouter's HTTP/2 websocket support + # https://github.com/cloudfoundry/routing-release/issues/230 + acl is_websocket hdr(Upgrade) -i WebSocket + acl is_websocket hdr_beg(Host) -i ws + use_backend http-routers-ws-http1 if is_websocket + <%- end -%> # }}} <% end -%> @@ -609,24 +625,18 @@ frontend wss-in <%- end -%> acl xfp_exists hdr_cnt(X-Forwarded-Proto) gt 0 http-request add-header X-Forwarded-Proto "https" if ! xfp_exists + + <%- if p("ha_proxy.disable_backend_http2_websockets") -%> + # Send websockets to a backend that forces HTTP/1.1. This avoids bugs in Go & Gorouter's HTTP/2 websocket support + # https://github.com/cloudfoundry/routing-release/issues/230 + acl is_websocket hdr(Upgrade) -i WebSocket + acl is_websocket hdr_beg(Host) -i ws + use_backend http-routers-ws-http1 if is_websocket + <%- end -%> # }}} <% end -%> -# Default Backend {{{ -backend http-routers - mode http - balance roundrobin - <%- if p("ha_proxy.compress_types") != "" -%> - compression algo gzip - compression type <%= p("ha_proxy.compress_types") %> - <%- end -%> - <%- if properties.ha_proxy.backend_config -%> - <%= p("ha_proxy.backend_config") %> - <%- end -%> - <%- p('ha_proxy.custom_http_error_files', {}).keys.each do |status_code| -%> - errorfile <%= status_code %> /var/vcap/jobs/haproxy/errorfiles/custom<%=status_code%>.http - <%- end -%> - <% +<%- backend_servers = [] backend_servers_local = [] backend_port = nil @@ -649,27 +659,50 @@ backend http-routers if_p("ha_proxy.backend_crt") do backend_crt = "crt /var/vcap/jobs/haproxy/config/backend-crt.pem " end - backend_ssl = "" + backend_ssl = "" if p("ha_proxy.backend_ssl").downcase == "verify" backend_ssl = "ssl verify required ca-file /var/vcap/jobs/haproxy/config/backend-ca-certs.pem " if_p("ha_proxy.backend_ssl_verifyhost") do | verify_hostname | backend_ssl += "verifyhost #{verify_hostname} " end - backend_ssl += alpn_config elsif p("ha_proxy.backend_ssl").downcase == "noverify" backend_ssl = "ssl verify none " - backend_ssl += alpn_config end + + backends = [{ name: "http-routers", backend_ssl: (backend_ssl != "" ? backend_ssl + alpn_config : "") }] + if p("ha_proxy.disable_backend_http2_websockets") + backends += [{ name: "http-routers-ws-http1", backend_ssl: backend_ssl + "alpn http/1.1 " }] + end + + backends.each do |backend| +-%> +# Backend <%= backend[:name] %> {{{ +backend <%= backend[:name] %> + mode http + balance roundrobin + <%- if p("ha_proxy.compress_types") != "" -%> + compression algo gzip + compression type <%= p("ha_proxy.compress_types") %> + <%- end -%> + <%- if properties.ha_proxy.backend_config -%> + <%= p("ha_proxy.backend_config") %> + <%- end -%> + <%- p('ha_proxy.custom_http_error_files', {}).keys.each do |status_code| -%> + errorfile <%= status_code %> /var/vcap/jobs/haproxy/errorfiles/custom<%=status_code%>.http + <%- end -%> + <% -%> <%- if p("ha_proxy.backend_use_http_health") == true -%> option httpchk GET <%= p("ha_proxy.backend_http_health_uri") %> <%- health_check_options = "port " + p("ha_proxy.backend_http_health_port").to_s -%> <%- end -%> <% backend_servers.each_with_index do |ip, index| %> - server node<%= index %> <%= ip %>:<%= backend_port -%> <%= resolvers -%><%= backend_crt -%>check inter 1000 <%= health_check_options %> <%= backend_ssl %><%- if !backend_servers_local.empty? && !backend_servers_local.include?(ip) -%> backup<%- end -%> + server node<%= index %> <%= ip %>:<%= backend_port -%> <%= resolvers -%><%= backend_crt -%>check inter 1000 <%= health_check_options %> <%= backend[:backend_ssl] %><%- if !backend_servers_local.empty? && !backend_servers_local.include?(ip) -%> backup<%- end -%> <% end %> # }}} +<%- end %> + # Routed Backends {{{ <% p('ha_proxy.routed_backend_servers').each do |prefix, data| -%> diff --git a/spec/haproxy/templates/haproxy_config/backend_wss_spec.rb b/spec/haproxy/templates/haproxy_config/backend_wss_spec.rb new file mode 100644 index 00000000..160e6de9 --- /dev/null +++ b/spec/haproxy/templates/haproxy_config/backend_wss_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'rspec' + +describe 'config/haproxy.config backend http-routers-ws-http1' do + let(:haproxy_conf) do + parse_haproxy_config(template.render({ 'ha_proxy' => properties })) + end + + let(:properties) do + { 'backend_servers' => ['10.0.0.1', '10.0.0.2'] } + end + let(:backend_http_routers) { haproxy_conf['backend http-routers'] } + let(:backend_http_routers_ws) { haproxy_conf['backend http-routers-ws-http1'] } + let(:frontend_http) { haproxy_conf['frontend http-in'] } + let(:frontend_https) { haproxy_conf['frontend https-in'] } + let(:frontend_wss_in) { haproxy_conf['frontend wss-in'] } + + it 'does not exist by default' do + expect(backend_http_routers_ws).to be_nil + end + + context 'when ha_proxy.disable_backend_http2_websockets is configured' do + let(:properties) do + super().merge({ 'disable_backend_http2_websockets' => true }) + end + + it 'exists' do + expect(backend_http_routers_ws).not_to be_nil + end + + it 'is the same as the normal http-routers backend except for different server settings' do + http_non_server_entries = backend_http_routers.reject { |l| l =~ /^server / } + http_ws_non_server_entries = backend_http_routers_ws.reject { |l| l =~ /^server / } + expect(http_non_server_entries).to eq(http_ws_non_server_entries) + end + + it 'uses an upstream ALPN of HTTP/1.1 only' do + server_entries = backend_http_routers_ws.select { |l| l =~ /^server / } + expect(server_entries).to contain_exactly( + 'server node0 10.0.0.1:80 check inter 1000 alpn http/1.1', + 'server node1 10.0.0.2:80 check inter 1000 alpn http/1.1' + ) + end + + it 'receives websocket traffic from http-in' do + expect(frontend_http).to include('use_backend http-routers-ws-http1 if is_websocket') + end + + context 'when https is enabled' do + let(:properties) do + super().merge({ 'ssl_pem' => 'ssl pem contents' }) + end + + it 'receives websocket traffic from https-in' do + expect(frontend_http).to include('use_backend http-routers-ws-http1 if is_websocket') + end + + context 'when the port 4443 websocket frontend is enabled' do + let(:properties) do + super().merge({ 'enable_4443' => true }) + end + + it 'receives websocket traffic from wss-in' do + expect(frontend_wss_in).to include('use_backend http-routers-ws-http1 if is_websocket') + end + end + end + end +end