Allow login over plain http in restricted situations (#7835)

This patch will allow the web api to (optionally) fall back from HTTPS
to HTTP under very particular circumstances:

  1. The appropriate `insecure` flag was set, and
  2. The target host is the loopback

If both conditions are met, this patch will allow the WebAPI client to
fall back to plain HTTP when attempting to login to the auth server.

api/client/webclient/webclient.go

@@ -30,10 +30,12 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/gravitational/trace"
-
 	"github.com/gravitational/teleport/api/constants"
 	"github.com/gravitational/teleport/api/defaults"
+	"github.com/gravitational/teleport/api/utils"
+	"github.com/gravitational/trace"
+
+	"log"
 )
 
 // newWebClient creates a new client to the HTTPS web proxy.
@@ -48,6 +50,42 @@ func newWebClient(insecure bool, pool *x509.CertPool) *http.Client {
 	}
 }
 
+// doWithFallback attempts to execute an HTTP request using https, and then
+// fall back to plain HTTP under certain, very specific circumstances.
+//  * The caller must specifically allow it via the allowPlainHTTP parameter, and
+//  * The target host must resolve to the loopback address.
+// If these conditions are not met, then the plain-HTTP fallback is not allowed,
+// and a the HTTPS failure will be considered final.
+func doWithFallback(clt *http.Client, allowPlainHTTP bool, req *http.Request) (*http.Response, error) {
+	// first try https and see how that goes
+	req.URL.Scheme = "https"
+	log.Printf("Attempting %s %s%s", req.Method, req.URL.Host, req.URL.Path)
+	resp, err := clt.Do(req)
+
+	// If the HTTPS succeeds, return that.
+	if err == nil {
+		return resp, nil
+	}
+
+	// If we're not allowed to try plain HTTP, bail out with whatever error we have.
+	// Note that we're only allowed to try plain HTTP on the loopback address, even
+	// if the caller says its OK
+	if !(allowPlainHTTP && utils.IsLoopback(req.URL.Host)) {
+		return nil, trace.Wrap(err)
+	}
+
+	// If we get to here a) the HTTPS attempt failed, and b) we're allowed to try
+	// clear-text HTTP to see if that works.
+	req.URL.Scheme = "http"
+	log.Printf("WARN: Request for %s%s falling back to PLAIN HTTP", req.URL.Host, req.URL.Path)
+	resp, err = clt.Do(req)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	return resp, nil
+}
+
 // Find fetches discovery data by connecting to the given web proxy address.
 // It is designed to fetch proxy public addresses without any inefficiencies.
 func Find(ctx context.Context, proxyAddr string, insecure bool, pool *x509.CertPool) (*PingResponse, error) {
@@ -61,7 +99,7 @@ func Find(ctx context.Context, proxyAddr string, insecure bool, pool *x509.CertP
 		return nil, trace.Wrap(err)
 	}
 
-	resp, err := clt.Do(req)
+	resp, err := doWithFallback(clt, insecure, req)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
@@ -94,7 +132,7 @@ func Ping(ctx context.Context, proxyAddr string, insecure bool, pool *x509.CertP
 		return nil, trace.Wrap(err)
 	}
 
-	resp, err := clt.Do(req)
+	resp, err := doWithFallback(clt, insecure, req)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}

api/client/webclient/webclient_test.go

@@ -18,6 +18,10 @@ package webclient
 
 import (
 	"context"
+	"encoding/json"
+	"net"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"testing"
 
@@ -26,6 +30,79 @@ import (
 	"github.com/gravitational/teleport/api/defaults"
 )
 
+func newPingHandler(path string) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		if req.RequestURI != path {
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		json.NewEncoder(w).Encode(PingResponse{ServerVersion: "test"})
+	})
+}
+
+func TestPlainHttpFallback(t *testing.T) {
+	testCases := []struct {
+		desc            string
+		handler         http.Handler
+		actionUnderTest func(addr string, insecure bool) error
+	}{
+		{
+			desc:    "Ping",
+			handler: newPingHandler("/webapi/ping"),
+			actionUnderTest: func(addr string, insecure bool) error {
+				_, err := Ping(context.Background(), addr, insecure, nil /*pool*/, "")
+				return err
+			},
+		}, {
+			desc:    "Find",
+			handler: newPingHandler("/webapi/find"),
+			actionUnderTest: func(addr string, insecure bool) error {
+				_, err := Find(context.Background(), addr, insecure, nil /*pool*/)
+				return err
+			},
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.desc, func(t *testing.T) {
+			t.Run("Allowed on insecure & loopback", func(t *testing.T) {
+				httpSvr := httptest.NewServer(testCase.handler)
+				defer httpSvr.Close()
+
+				err := testCase.actionUnderTest(httpSvr.Listener.Addr().String(), true /* insecure */)
+				require.NoError(t, err)
+			})
+
+			t.Run("Denied on secure", func(t *testing.T) {
+				httpSvr := httptest.NewServer(testCase.handler)
+				defer httpSvr.Close()
+
+				err := testCase.actionUnderTest(httpSvr.Listener.Addr().String(), false /* secure */)
+				require.Error(t, err)
+			})
+
+			t.Run("Denied on non-loopback", func(t *testing.T) {
+				nonLoopbackSvr := httptest.NewUnstartedServer(testCase.handler)
+
+				// replace the test-supplied loopback listener with the first available
+				// non-loopback address
+				nonLoopbackSvr.Listener.Close()
+				l, err := net.Listen("tcp", "0.0.0.0:0")
+				require.NoError(t, err)
+				nonLoopbackSvr.Listener = l
+				nonLoopbackSvr.Start()
+				defer nonLoopbackSvr.Close()
+
+				err = testCase.actionUnderTest(nonLoopbackSvr.Listener.Addr().String(), true /* insecure */)
+				require.Error(t, err)
+			})
+		})
+	}
+}
+
 func TestGetTunnelAddr(t *testing.T) {
 	ctx := context.Background()
 	t.Run("should use TELEPORT_TUNNEL_PUBLIC_ADDR", func(t *testing.T) {

api/utils/addr.go

@@ -0,0 +1,61 @@
+/*
+Copyright 2021 Gravitational, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package utils
+
+import (
+	"context"
+	"net"
+	"strings"
+)
+
+// IsLoopback returns 'true' if a given hostname resolves *only* to the
+// local host's loopback interface
+func IsLoopback(host string) bool {
+	return isLoopbackWithResolver(host, net.DefaultResolver)
+}
+
+type nameResolver interface {
+	LookupIPAddr(ctx context.Context, host string) ([]net.IPAddr, error)
+}
+
+// isLoopbackWithResolver provides the inner implementation of IsLoopback(),
+// but allows the caller to inject a custom resolver for testing purposes.
+func isLoopbackWithResolver(host string, resolver nameResolver) bool {
+	if strings.Contains(host, ":") {
+		var err error
+		host, _, err = net.SplitHostPort(host)
+		if err != nil {
+			return false
+		}
+	}
+	addrs, err := resolver.LookupIPAddr(context.Background(), host)
+	if err != nil {
+		return false
+	}
+
+	if len(addrs) == 0 {
+		return false
+	}
+
+	for _, addr := range addrs {
+		if !addr.IP.IsLoopback() {
+			return false
+		}
+	}
+
+	return true
+}

api/utils/addr_test.go

@@ -0,0 +1,111 @@
+/*
+Copyright 2021 Gravitational, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package utils
+
+import (
+	"context"
+	"errors"
+	"net"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+type mockResolver struct {
+	addresses []net.IPAddr
+	err       error
+}
+
+func (r *mockResolver) LookupIPAddr(ctx context.Context, host string) ([]net.IPAddr, error) {
+	if r.err != nil {
+		return nil, r.err
+	}
+
+	return r.addresses, nil
+}
+
+func TestIsLoopback(t *testing.T) {
+	testCases := []struct {
+		desc     string
+		addr     string
+		resolver nameResolver
+		expect   bool
+	}{
+		{
+			desc:     "localhost should return true",
+			addr:     "localhost",
+			resolver: net.DefaultResolver,
+			expect:   true,
+		}, {
+			desc:     "localhost with port should return true",
+			addr:     "localhost:1234",
+			resolver: net.DefaultResolver,
+			expect:   true,
+		}, {
+			desc: "multiple loopback addresses should return true",
+			addr: "potato.banana.org",
+			resolver: &mockResolver{
+				addresses: []net.IPAddr{
+					{IP: net.IPv6loopback},
+					{IP: []byte{127, 0, 0, 1}},
+				},
+			},
+			expect: true,
+		}, {
+			desc:     "degenerate hostname should return false",
+			addr:     ":1234",
+			resolver: net.DefaultResolver,
+			expect:   false,
+		}, {
+			desc:     "degenerate port should return true",
+			addr:     "localhost:",
+			resolver: net.DefaultResolver,
+			expect:   true,
+		}, {
+			desc:     "DNS failure should return false",
+			addr:     "potato.banana.org",
+			resolver: &mockResolver{err: errors.New("kaboom")},
+			expect:   false,
+		}, {
+			desc: "non-loopback addr should return false",
+			addr: "potato.banana.org",
+			resolver: &mockResolver{
+				addresses: []net.IPAddr{
+					{IP: []byte{192, 168, 0, 1}}, // private, but non-loopback
+				},
+			},
+			expect: false,
+		}, {
+			desc: "Any non-looback address should return false",
+			addr: "potato.banana.org",
+			resolver: &mockResolver{
+				addresses: []net.IPAddr{
+					{IP: net.IPv6loopback},
+					{IP: []byte{192, 168, 0, 1}}, // private, but non-loopback
+					{IP: []byte{127, 0, 0, 1}},
+				},
+			},
+			expect: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			require.Equal(t, tc.expect, isLoopbackWithResolver(tc.addr, tc.resolver))
+		})
+	}
+}

lib/client/api.go

@@ -53,6 +53,7 @@ import (
 	"github.com/gravitational/teleport/api/profile"
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/api/types/wrappers"
+	apiutils "github.com/gravitational/teleport/api/utils"
 	"github.com/gravitational/teleport/api/utils/keypaths"
 	"github.com/gravitational/teleport/lib/auth"
 	"github.com/gravitational/teleport/lib/client/terminal"
@@ -2810,7 +2811,7 @@ func (tc *TeleportClient) EventsChannel() <-chan events.EventFields {
 // loopbackPool reads trusted CAs if it finds it in a predefined location
 // and will work only if target proxy address is loopback
 func loopbackPool(proxyAddr string) *x509.CertPool {
-	if !utils.IsLoopback(proxyAddr) {
+	if !apiutils.IsLoopback(proxyAddr) {
 		log.Debugf("not using loopback pool for remote proxy addr: %v", proxyAddr)
 		return nil
 	}

lib/client/https_client.go

@@ -24,6 +24,7 @@ import (
 	"net/url"
 
 	"github.com/gravitational/teleport"
+	apiutils "github.com/gravitational/teleport/api/utils"
 	"github.com/gravitational/teleport/lib/httplib"
 	"github.com/gravitational/teleport/lib/utils"
 
@@ -72,6 +73,45 @@ type WebClient struct {
 	*roundtrip.Client
 }
 
+// PostJSONWithFallback serializes an object to JSON and attempts to execute a POST
+// using HTTPS, and then fall back to plain HTTP under certain, very specific circumstances.
+//  * The caller must specifically allow it via the allowHTTPFallback parameter, and
+//  * The target host must resolve to the loopback address.
+// If these conditions are not met, then the plain-HTTP fallback is not allowed,
+// and a the HTTPS failure will be considered final.
+//
+func (w *WebClient) PostJSONWithFallback(ctx context.Context, endpoint string, val interface{}, allowHTTPFallback bool) (*roundtrip.Response, error) {
+	// First try HTTPS and see how that goes
+	log.Debugf("Attempting %s", endpoint)
+	resp, httpsErr := w.Client.PostJSON(ctx, endpoint, val)
+	if httpsErr == nil {
+		// If all went well, then we don't need to do anything else - just return
+		// that response
+		return httplib.ConvertResponse(resp, httpsErr)
+	}
+
+	// Parse out the endpoint into its constituent parts. We will need the
+	// hostname to decide if we're allowed to fall back to HTTPS, and we will
+	// re-use this for re-writing the endpoint URL later on anyway.
+	u, err := url.Parse(endpoint)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	// If we're not allowed to try plain HTTP, bail out with whatever error we have.
+	// Note that we're only allowed to try plain HTTP on the loopback address, even
+	// if the caller says its OK
+	if !(allowHTTPFallback && apiutils.IsLoopback(u.Host)) {
+		return nil, trace.Wrap(httpsErr)
+	}
+
+	// re-write the endpoint to try HTTP
+	u.Scheme = "http"
+	endpoint = u.String()
+	log.Warnf("Request for %s/%s falling back to PLAIN HTTP", u.Host, u.Path)
+	return httplib.ConvertResponse(w.Client.PostJSON(ctx, endpoint, val))
+}
+
 func (w *WebClient) PostJSON(ctx context.Context, endpoint string, val interface{}) (*roundtrip.Response, error) {
 	return httplib.ConvertResponse(w.Client.PostJSON(ctx, endpoint, val))
 }

lib/client/weblogin.go

@@ -454,7 +454,7 @@ func HostCredentials(ctx context.Context, proxyAddr string, insecure bool, req a
 		return nil, trace.Wrap(err)
 	}
 
-	resp, err := clt.PostJSON(ctx, clt.Endpoint("webapi", "host", "credentials"), req)
+	resp, err := clt.PostJSONWithFallback(ctx, clt.Endpoint("webapi", "host", "credentials"), req, insecure)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}

lib/client/weblogin_test.go

@@ -0,0 +1,95 @@
+/*
+Copyright 2021 Gravitational, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package client
+
+import (
+	"context"
+	"encoding/json"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gravitational/teleport/api/client/proto"
+	"github.com/gravitational/teleport/lib/auth"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPlainHttpFallback(t *testing.T) {
+	testCases := []struct {
+		desc            string
+		path            string
+		handler         http.HandlerFunc
+		actionUnderTest func(ctx context.Context, addr string, insecure bool) error
+	}{
+		{
+			desc: "HostCredentials",
+			path: "/v1/webapi/host/credentials",
+			handler: func(w http.ResponseWriter, r *http.Request) {
+				if r.RequestURI != "/v1/webapi/host/credentials" {
+					w.WriteHeader(http.StatusNotFound)
+					return
+				}
+				w.Header().Set("Content-Type", "application/json")
+				w.WriteHeader(http.StatusOK)
+				json.NewEncoder(w).Encode(proto.Certs{})
+			},
+			actionUnderTest: func(ctx context.Context, addr string, insecure bool) error {
+				_, err := HostCredentials(ctx, addr, insecure, auth.RegisterUsingTokenRequest{})
+				return err
+			},
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.desc, func(t *testing.T) {
+			ctx := context.Background()
+
+			t.Run("Allowed on insecure & loopback", func(t *testing.T) {
+				httpSvr := httptest.NewServer(testCase.handler)
+				defer httpSvr.Close()
+
+				err := testCase.actionUnderTest(ctx, httpSvr.Listener.Addr().String(), true /* insecure */)
+				require.NoError(t, err)
+			})
+
+			t.Run("Denied on secure", func(t *testing.T) {
+				httpSvr := httptest.NewServer(testCase.handler)
+				defer httpSvr.Close()
+
+				err := testCase.actionUnderTest(ctx, httpSvr.Listener.Addr().String(), false /* secure */)
+				require.Error(t, err)
+			})
+
+			t.Run("Denied on non-loopback", func(t *testing.T) {
+				nonLoopbackSvr := httptest.NewUnstartedServer(testCase.handler)
+
+				// replace the test-supplied loopback listener with the first available
+				// non-loopback address
+				nonLoopbackSvr.Listener.Close()
+				l, err := net.Listen("tcp", "0.0.0.0:0")
+				require.NoError(t, err)
+				nonLoopbackSvr.Listener = l
+				nonLoopbackSvr.Start()
+				defer nonLoopbackSvr.Close()
+
+				err = testCase.actionUnderTest(ctx, nonLoopbackSvr.Listener.Addr().String(), true /* insecure */)
+				require.Error(t, err)
+			})
+		})
+	}
+}

lib/utils/addr.go

@@ -23,6 +23,7 @@ import (
 	"strconv"
 	"strings"
 
+	apiutils "github.com/gravitational/teleport/api/utils"
 	"github.com/gravitational/trace"
 	log "github.com/sirupsen/logrus"
 )
@@ -76,7 +77,7 @@ func (a *NetAddr) IsLocal() bool {
 
 // IsLoopback returns true if this is a loopback address
 func (a *NetAddr) IsLoopback() bool {
-	return IsLoopback(a.Addr)
+	return apiutils.IsLoopback(a.Addr)
 }
 
 // IsEmpty returns true if address is empty
@@ -266,28 +267,6 @@ func IsLocalhost(host string) bool {
 	return ip.IsLoopback() || ip.IsUnspecified()
 }
 
-// IsLoopback returns 'true' if a given hostname resolves to local
-// host's loopback interface
-func IsLoopback(host string) bool {
-	if strings.Contains(host, ":") {
-		var err error
-		host, _, err = net.SplitHostPort(host)
-		if err != nil {
-			return false
-		}
-	}
-	ips, err := net.LookupIP(host)
-	if err != nil {
-		return false
-	}
-	for _, ip := range ips {
-		if ip.IsLoopback() {
-			return true
-		}
-	}
-	return false
-}
-
 // GuessIP tries to guess an IP address this machine is reachable at on the
 // internal network, always picking IPv4 from the internal address space
 //

lib/utils/addr_test.go

@@ -161,24 +161,6 @@ func (s *AddrTestSuite) TestLocalAddrs(c *C) {
 	}
 }
 
-func (s *AddrTestSuite) TestLoopbackAddrs(c *C) {
-	testCases := []struct {
-		in       string
-		expected bool
-	}{
-		{in: "localhost", expected: true},
-		{in: "localhost:5000", expected: true},
-		{in: "127.0.0.2:4003", expected: true},
-		{in: "", expected: false},
-		{in: "bad-host.example.com", expected: false},
-		{in: "bad-host.example.com:443", expected: false},
-	}
-	for i, testCase := range testCases {
-		c.Assert(IsLoopback(testCase.in), Equals, testCase.expected,
-			Commentf("test case %v, %v should be loopback(%v)", i, testCase.in, testCase.expected))
-	}
-}
-
 func (s *AddrTestSuite) TestGuessesIPAddress(c *C) {
 	var testCases = []struct {
 		addrs    []net.Addr