From 4b022fcacb07dea61ce519609a330ae852ffbdab Mon Sep 17 00:00:00 2001 From: Gus Luxton Date: Tue, 13 Aug 2019 14:03:22 -0300 Subject: [PATCH] Handle HTTP connections to TLS socket in a more graceful way (#2886) --- lib/defaults/defaults.go | 4 ++-- lib/defaults/defaults_test.go | 5 +++-- lib/multiplexer/multiplexer.go | 34 +++++++++++++++++++++++++++-- lib/multiplexer/multiplexer_test.go | 31 ++++++-------------------- lib/service/cfg.go | 2 +- lib/service/service.go | 4 ++-- 6 files changed, 47 insertions(+), 33 deletions(-) diff --git a/lib/defaults/defaults.go b/lib/defaults/defaults.go index e50e8491678..0068d6c042c 100644 --- a/lib/defaults/defaults.go +++ b/lib/defaults/defaults.go @@ -466,10 +466,10 @@ func SSHServerListenAddr() *utils.NetAddr { return makeAddr(BindIP, SSHServerListenPort) } -// ReverseTunnellListenAddr returns the default listening address for the SSH Proxy service used +// ReverseTunnelListenAddr returns the default listening address for the SSH Proxy service used // by the SSH nodes to establish proxy<->ssh_node connection from behind a firewall which // blocks inbound connecions to ssh_nodes -func ReverseTunnellListenAddr() *utils.NetAddr { +func ReverseTunnelListenAddr() *utils.NetAddr { return makeAddr(BindIP, SSHProxyTunnelListenPort) } diff --git a/lib/defaults/defaults_test.go b/lib/defaults/defaults_test.go index d9e92a219a7..a5759919b92 100644 --- a/lib/defaults/defaults_test.go +++ b/lib/defaults/defaults_test.go @@ -16,8 +16,9 @@ limitations under the License. package defaults import ( - "github.com/gravitational/teleport/lib/utils" "testing" + + "github.com/gravitational/teleport/lib/utils" ) func TestMakeAddr(t *testing.T) { @@ -37,7 +38,7 @@ func TestDefaultAddresses(t *testing.T) { "tcp://0.0.0.0:3023": ProxyListenAddr(), "tcp://0.0.0.0:3080": ProxyWebListenAddr(), "tcp://0.0.0.0:3022": SSHServerListenAddr(), - "tcp://0.0.0.0:3024": ReverseTunnellListenAddr(), + "tcp://0.0.0.0:3024": ReverseTunnelListenAddr(), } for expected, actual := range table { if actual == nil { diff --git a/lib/multiplexer/multiplexer.go b/lib/multiplexer/multiplexer.go index 8b902417df9..3659d725fc5 100644 --- a/lib/multiplexer/multiplexer.go +++ b/lib/multiplexer/multiplexer.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -// package multiplexer implements SSH and TLS multiplexing +// Package multiplexer implements SSH and TLS multiplexing // on the same listener // // mux, _ := multiplexer.New(Config{Listener: listener}) @@ -235,6 +235,9 @@ func (m *Mux) detectAndForward(conn net.Conn) { connWrapper.Close() return } + case ProtoHTTP: + m.Debug("Detected an HTTP request. If this is for a health check, use an HTTPS request instead.") + conn.Close() default: // should not get here, handle this just in case connWrapper.Close() @@ -275,7 +278,7 @@ func detect(conn net.Conn, enableProxyProtocol bool) (*Conn, error) { return nil, trace.Wrap(err) } // repeat the cycle to detect the protocol - case ProtoTLS, ProtoSSH: + case ProtoTLS, ProtoSSH, ProtoHTTP: return &Conn{ protocol: proto, Conn: conn, @@ -297,6 +300,8 @@ const ( ProtoSSH // ProtoProxy is a HAProxy proxy line protocol ProtoProxy + // ProtoHTTP is HTTP protocol + ProtoHTTP ) var ( @@ -305,6 +310,29 @@ var ( tlsPrefix = []byte{0x16} ) +// isHTTP returns true if the first 3 bytes of the prefix indicate +// the use of an HTTP method. +func isHTTP(in []byte) bool { + methods := [...][]byte{ + []byte("GET"), + []byte("POST"), + []byte("PUT"), + []byte("DELETE"), + []byte("HEAD"), + []byte("CONNECT"), + []byte("OPTIONS"), + []byte("TRACE"), + []byte("PATCH"), + } + for _, verb := range methods { + // we only get 3 bytes, so can only compare the first 3 bytes of each verb + if bytes.HasPrefix(verb, in[:3]) { + return true + } + } + return false +} + func detectProto(in []byte) (int, error) { switch { // reader peeks only 3 bytes, slice the longer proxy prefix @@ -314,6 +342,8 @@ func detectProto(in []byte) (int, error) { return ProtoSSH, nil case bytes.HasPrefix(in, tlsPrefix): return ProtoTLS, nil + case isHTTP(in): + return ProtoHTTP, nil default: return ProtoUnknown, trace.BadParameter("failed to detect protocol by prefix: %v", in) } diff --git a/lib/multiplexer/multiplexer_test.go b/lib/multiplexer/multiplexer_test.go index 756f62d46b6..f1d8139c9c3 100644 --- a/lib/multiplexer/multiplexer_test.go +++ b/lib/multiplexer/multiplexer_test.go @@ -58,10 +58,7 @@ func (s *MuxSuite) SetUpSuite(c *check.C) { // TestMultiplexing tests basic use case of multiplexing TLS // and SSH on the same listener socket func (s *MuxSuite) TestMultiplexing(c *check.C) { - ports, err := utils.GetFreeTCPPorts(1) - c.Assert(err, check.IsNil) - - listener, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", ports[0])) + listener, err := net.Listen("tcp", "127.0.0.1:0") c.Assert(err, check.IsNil) mux, err := New(Config{ @@ -130,10 +127,7 @@ func (s *MuxSuite) TestMultiplexing(c *check.C) { // TestProxy tests Proxy line support protocol func (s *MuxSuite) TestProxy(c *check.C) { - ports, err := utils.GetFreeTCPPorts(1) - c.Assert(err, check.IsNil) - - listener, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", ports[0])) + listener, err := net.Listen("tcp", "127.0.0.1:0") c.Assert(err, check.IsNil) mux, err := New(Config{ @@ -185,10 +179,7 @@ func (s *MuxSuite) TestProxy(c *check.C) { // TestDisabledProxy makes sure the connection gets dropped // when Proxy line support protocol is turned off func (s *MuxSuite) TestDisabledProxy(c *check.C) { - ports, err := utils.GetFreeTCPPorts(1) - c.Assert(err, check.IsNil) - - listener, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", ports[0])) + listener, err := net.Listen("tcp", "127.0.0.1:0") c.Assert(err, check.IsNil) mux, err := New(Config{ @@ -238,10 +229,7 @@ func (s *MuxSuite) TestDisabledProxy(c *check.C) { // TestTimeout tests client timeout - client dials, but writes nothing // make sure server hangs up func (s *MuxSuite) TestTimeout(c *check.C) { - ports, err := utils.GetFreeTCPPorts(1) - c.Assert(err, check.IsNil) - - listener, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", ports[0])) + listener, err := net.Listen("tcp", "127.0.0.1:0") c.Assert(err, check.IsNil) config := Config{ @@ -313,10 +301,7 @@ func (s *MuxSuite) TestUnknownProtocol(c *check.C) { // TestDisableSSH disables SSH func (s *MuxSuite) TestDisableSSH(c *check.C) { - ports, err := utils.GetFreeTCPPorts(1) - c.Assert(err, check.IsNil) - - listener, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", ports[0])) + listener, err := net.Listen("tcp", "127.0.0.1:0") c.Assert(err, check.IsNil) mux, err := New(Config{ @@ -365,10 +350,7 @@ func (s *MuxSuite) TestDisableSSH(c *check.C) { // TestDisableTLS tests scenario with disabled TLS func (s *MuxSuite) TestDisableTLS(c *check.C) { - ports, err := utils.GetFreeTCPPorts(1) - c.Assert(err, check.IsNil) - - listener, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", ports[0])) + listener, err := net.Listen("tcp", "127.0.0.1:0") c.Assert(err, check.IsNil) mux, err := New(Config{ @@ -444,6 +426,7 @@ func clientConfig(srv *httptest.Server) *tls.Config { } } +// testClient is a test HTTP client set up for TLS func testClient(srv *httptest.Server) *http.Client { return &http.Client{ Transport: &http.Transport{ diff --git a/lib/service/cfg.go b/lib/service/cfg.go index 1d8df22ebb2..a7f501215a2 100644 --- a/lib/service/cfg.go +++ b/lib/service/cfg.go @@ -470,7 +470,7 @@ func ApplyDefaults(cfg *Config) { cfg.Proxy.Enabled = true cfg.Proxy.SSHAddr = *defaults.ProxyListenAddr() cfg.Proxy.WebAddr = *defaults.ProxyWebListenAddr() - cfg.Proxy.ReverseTunnelListenAddr = *defaults.ReverseTunnellListenAddr() + cfg.Proxy.ReverseTunnelListenAddr = *defaults.ReverseTunnelListenAddr() defaults.ConfigureLimiter(&cfg.Proxy.Limiter) // defaults for the Kubernetes proxy service diff --git a/lib/service/service.go b/lib/service/service.go index 008aec2a43b..8e4f5329092 100644 --- a/lib/service/service.go +++ b/lib/service/service.go @@ -1897,7 +1897,7 @@ func (process *TeleportProcess) setupProxyListeners() (*proxyListeners, error) { go listeners.mux.Serve() return &listeners, nil default: - process.Debugf("Proxy reverse tunnel are listening on the separate ports.") + process.Debugf("Proxy and reverse tunnel are listening on separate ports.") if !cfg.Proxy.DisableReverseTunnel { listeners.reverseTunnel, err = process.importOrCreateListener(teleport.Component(teleport.ComponentProxy, "tunnel"), cfg.Proxy.ReverseTunnelListenAddr.Addr) if err != nil { @@ -1985,7 +1985,7 @@ func (process *TeleportProcess) initProxyEndpoint(conn *Connector) error { if err != nil { return trace.Wrap(err) } - process.RegisterCriticalFunc("proxy.reveresetunnel.server", func() error { + process.RegisterCriticalFunc("proxy.reversetunnel.server", func() error { utils.Consolef(cfg.Console, teleport.ComponentProxy, "Reverse tunnel service is starting on %v.", cfg.Proxy.ReverseTunnelListenAddr.Addr) log.Infof("Starting on %v using %v", cfg.Proxy.ReverseTunnelListenAddr.Addr, process.Config.CachePolicy) if err := tsrv.Start(); err != nil {