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 property to force backend websocket connections to use HTTP/1.1 #263

Merged
merged 2 commits into from
Oct 21, 2021
Merged
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
33 changes: 30 additions & 3 deletions acceptance-tests/acceptance_tests_suite_test.go
Original file line number Diff line number Diff line change
@@ -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
}
1 change: 1 addition & 0 deletions acceptance-tests/go.mod
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions acceptance-tests/go.sum
Original file line number Diff line number Diff line change
@@ -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=
162 changes: 162 additions & 0 deletions acceptance-tests/websocket_test.go
Original file line number Diff line number Diff line change
@@ -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"))
})
})
})
6 changes: 6 additions & 0 deletions ci/release_notes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# New Features
- New property `disable_backend_http2_websockets` to force backend websocket connections to use HTTP/1.1 (default `false`) #263 / #261

# Acknowledgements

Thanks @46bit for the PR
3 changes: 3 additions & 0 deletions jobs/haproxy/spec
Original file line number Diff line number Diff line change
@@ -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"
71 changes: 52 additions & 19 deletions jobs/haproxy/templates/haproxy.config.erb
Original file line number Diff line number Diff line change
@@ -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 -%>

@@ -487,6 +495,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 -%>

@@ -621,24 +637,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
@@ -661,27 +671,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| -%>
70 changes: 70 additions & 0 deletions spec/haproxy/templates/haproxy_config/backend_wss_spec.rb
Original file line number Diff line number Diff line change
@@ -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