lib/kube/proxy: extract auth handling into a separate file

The existing kubeCreds already has tls.Config.
Add a new wrapTransport method to it, which handles added bearer tokens
or basic auth to the request before it goes out.

Use this wrapTransport to inject tokens for both SPDY connections and
catch-all forwarder requests.

Also simplify a few unrelated parts:
- don't manually call serverName verification in SPDY dialer
- have requestCertificate return a finished tls.Config instead of an
  intermediate type

lib/kube/proxy/auth.go

@@ -0,0 +1,103 @@
+package proxy
+
+import (
+	"crypto/tls"
+	"fmt"
+	"io/ioutil"
+	"net"
+	"net/http"
+	"net/url"
+	"os"
+
+	"github.com/gravitational/teleport"
+	kubeutils "github.com/gravitational/teleport/lib/kube/utils"
+	"github.com/gravitational/trace"
+	log "github.com/sirupsen/logrus"
+	"k8s.io/client-go/rest"
+	"k8s.io/client-go/transport"
+
+	// Load kubeconfig auth plugins for gcp and azure.
+	// Without this, users can't provide a kubeconfig using those.
+	//
+	// Note: we don't want to load _all_ plugins. This is a balance between
+	// support for popular hosting providers and minimizing attack surface.
+	_ "k8s.io/client-go/plugin/pkg/client/auth/azure"
+	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
+)
+
+// kubeCreds contain authentication-related fields from kubeconfig.
+type kubeCreds struct {
+	// tlsConfig contains (m)TLS configuration.
+	tlsConfig *tls.Config
+	// transportConfig contains HTTPS-related configuration.
+	// Note: use wrapTransport method if working with http.RoundTrippers.
+	transportConfig *transport.Config
+	// targetAddr is a kubernetes API address.
+	targetAddr string
+}
+
+func getKubeCreds(kubeconfigPath string) (*kubeCreds, error) {
+	var cfg *rest.Config
+	// no kubeconfig is set, assume auth server is running in the cluster
+	if kubeconfigPath == "" {
+		caPEM, err := ioutil.ReadFile(teleport.KubeCAPath)
+		if err != nil {
+			return nil, trace.BadParameter(`auth server assumed that it is
+running in a kubernetes cluster, but %v mounted in pods could not be read: %v,
+set kubeconfig_file if auth server is running outside of the cluster`, teleport.KubeCAPath, err)
+		}
+
+		cfg, err = kubeutils.GetKubeConfig(os.Getenv(teleport.EnvKubeConfig))
+		if err != nil {
+			return nil, trace.BadParameter(`auth server assumed that it is
+running in a kubernetes cluster, but could not init in-cluster kubernetes client: %v`, err)
+		}
+		cfg.CAData = caPEM
+	} else {
+		log.Debugf("Reading configuration from kubeconfig file %v.", kubeconfigPath)
+
+		var err error
+		cfg, err = kubeutils.GetKubeConfig(kubeconfigPath)
+		if err != nil {
+			return nil, trace.Wrap(err)
+		}
+	}
+
+	targetAddr, err := parseKubeHost(cfg.Host)
+	if err != nil {
+		return nil, trace.Wrap(err, "failed to parse kubernetes host")
+	}
+	tlsConfig, err := rest.TLSConfigFor(cfg)
+	if err != nil {
+		return nil, trace.Wrap(err, "failed to generate TLS config from kubeconfig")
+	}
+	transportConfig, err := cfg.TransportConfig()
+	if err != nil {
+		return nil, trace.Wrap(err, "failed to generate transport config from kubeconfig")
+	}
+
+	return &kubeCreds{
+		tlsConfig:       tlsConfig,
+		transportConfig: transportConfig,
+		targetAddr:      targetAddr,
+	}, nil
+}
+
+// parseKubeHost parses and formats kubernetes hostname
+// to host:port format, if no port it set,
+// it assumes default HTTPS port
+func parseKubeHost(host string) (string, error) {
+	u, err := url.Parse(host)
+	if err != nil {
+		return "", trace.Wrap(err, "failed to parse kubernetes host")
+	}
+	if _, _, err := net.SplitHostPort(u.Host); err != nil {
+		// add default HTTPS port
+		return fmt.Sprintf("%v:443", u.Host), nil
+	}
+	return u.Host, nil
+}
+
+func (c *kubeCreds) wrapTransport(rt http.RoundTripper) (http.RoundTripper, error) {
+	return transport.HTTPWrappersForConfig(c.transportConfig, rt)
+}

lib/kube/proxy/forwarder.go

@@ -24,11 +24,8 @@ import (
 	"crypto/x509/pkix"
 	"encoding/pem"
 	"fmt"
-	"io/ioutil"
 	"net"
 	"net/http"
-	"net/url"
-	"os"
 	"path/filepath"
 	"strings"
 	"sync"
@@ -55,14 +52,9 @@ import (
 	log "github.com/sirupsen/logrus"
 	"golang.org/x/crypto/ssh"
 	"k8s.io/apimachinery/pkg/util/httpstream"
-	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/remotecommand"
 	"k8s.io/client-go/transport/spdy"
 	utilexec "k8s.io/client-go/util/exec"
-
-	// Load all kubeconfig auth plugins (gcp/azure/etc).
-	// Without this, users can't provide a kubeconfig using those.
-	_ "k8s.io/client-go/plugin/pkg/client/auth"
 )
 
 // ForwarderConfig specifies configuration for proxy forwarder
@@ -659,7 +651,7 @@ const (
 )
 
 func (f *Forwarder) setupForwardingHeaders(ctx *authContext, sess *clusterSession, req *http.Request) error {
-	if err := setupImpersonationHeaders(f.Entry, ctx, req.Header, f.creds.cfg.BearerToken); err != nil {
+	if err := setupImpersonationHeaders(f.Entry, ctx, req.Header); err != nil {
 		return trace.Wrap(err)
 	}
 
@@ -678,7 +670,7 @@ func (f *Forwarder) setupForwardingHeaders(ctx *authContext, sess *clusterSessio
 }
 
 // setupImpersonationHeaders sets up Impersonate-User and Impersonate-Group headers
-func setupImpersonationHeaders(log log.FieldLogger, ctx *authContext, headers http.Header, bearerToken string) error {
+func setupImpersonationHeaders(log log.FieldLogger, ctx *authContext, headers http.Header) error {
 	var impersonateUser string
 	var impersonateGroups []string
 	for header, values := range headers {
@@ -744,6 +736,7 @@ func setupImpersonationHeaders(log log.FieldLogger, ctx *authContext, headers ht
 				"please select a user to impersonate, refusing to select a user due to several kuberenetes_users set up for this user")
 		}
 	}
+
 	if len(impersonateGroups) == 0 {
 		for group := range ctx.kubeGroups {
 			impersonateGroups = append(impersonateGroups, group)
@@ -751,12 +744,13 @@ func setupImpersonationHeaders(log log.FieldLogger, ctx *authContext, headers ht
 	}
 
 	if !ctx.cluster.isRemote {
-		headers.Add("Impersonate-User", impersonateUser)
+		headers.Set(ImpersonateUserHeader, impersonateUser)
+
+		// Make sure to overwrite the exiting headers, instead of appending to
+		// them.
+		headers[ImpersonateGroupHeader] = nil
 		for _, group := range impersonateGroups {
-			headers.Add("Impersonate-Group", group)
-		}
-		if bearerToken != "" {
-			headers.Set("Authorization", fmt.Sprintf("Bearer %v", bearerToken))
+			headers.Add(ImpersonateGroupHeader, group)
 		}
 	}
 	return nil
@@ -788,9 +782,12 @@ func (f *Forwarder) getExecutor(ctx authContext, sess *clusterSession, req *http
 		dial:            sess.DialWithContext,
 		tlsConfig:       sess.tlsConfig,
 		followRedirects: true,
-		bearerToken:     f.creds.cfg.BearerToken,
 	})
-	return remotecommand.NewSPDYExecutorForTransports(upgradeRoundTripper, upgradeRoundTripper, req.Method, req.URL)
+	rt, err := f.creds.wrapTransport(upgradeRoundTripper)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	return remotecommand.NewSPDYExecutorForTransports(rt, upgradeRoundTripper, req.Method, req.URL)
 }
 
 func (f *Forwarder) getDialer(ctx authContext, sess *clusterSession, req *http.Request) (httpstream.Dialer, error) {
@@ -800,10 +797,13 @@ func (f *Forwarder) getDialer(ctx authContext, sess *clusterSession, req *http.R
 		dial:            sess.DialWithContext,
 		tlsConfig:       sess.tlsConfig,
 		followRedirects: true,
-		bearerToken:     f.creds.cfg.BearerToken,
 	})
+	rt, err := f.creds.wrapTransport(upgradeRoundTripper)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
 	client := &http.Client{
-		Transport: upgradeRoundTripper,
+		Transport: rt,
 	}
 
 	return spdy.NewDialer(upgradeRoundTripper, client, req.Method, req.URL), nil
@@ -939,34 +939,19 @@ func (f *Forwarder) serializedNewClusterSession(authContext authContext) (*clust
 }
 
 func (f *Forwarder) newClusterSession(ctx authContext) (*clusterSession, error) {
-	var tlsConfig *tls.Config
-	// for remote clusters, retrieve a client certificate
+	tlsConfig := f.creds.tlsConfig
+
+	// For remote (trusted) clusters, generate a new teleport TLS client
+	// certificate for the user via auth server. Effectively, impersonate the
+	// user to the remote proxy.
 	if ctx.cluster.isRemote {
-		response, err := f.requestCertificate(ctx)
+		var err error
+		tlsConfig, err = f.requestCertificate(ctx)
 		if err != nil {
 			f.Warningf("Failed to get certificate for %v: %v.", ctx, err)
 			return nil, trace.AccessDenied("access denied: failed to authenticate with auth server")
 		}
-		cert, err := tls.X509KeyPair(response.cert, response.key)
-		if err != nil {
-			return nil, trace.Wrap(err)
-		}
-		pool := x509.NewCertPool()
-		for _, certAuthority := range response.certAuthorities {
-			ok := pool.AppendCertsFromPEM(certAuthority)
-			if !ok {
-				return nil, trace.BadParameter("failed to append certificates, check that kubeconfig has correctly encoded certificate authority data")
-			}
-		}
-		tlsConfig = &tls.Config{
-			RootCAs:      pool,
-			Certificates: []tls.Certificate{cert},
-			ClientAuth:   tls.RequireAndVerifyClientCert,
-		}
-	} else {
-		tlsConfig = f.creds.tlsConfig
 	}
-	tlsConfig.BuildNameToCertificate()
 
 	// remote clusters use special hardcoded URL,
 	// and use a special dialer
@@ -984,14 +969,18 @@ func (f *Forwarder) newClusterSession(ctx authContext) (*clusterSession, error)
 
 	var transport http.RoundTripper = f.newTransport(sess.Dial, tlsConfig)
 
-	// when running inside Kubernetes cluster, kubeconfig provides a
-	// transport wrapper that adds service account token to requests
+	// When running inside Kubernetes cluster or using auth/exec providers,
+	// kubeconfig provides a transport wrapper that adds a bearer token to
+	// requests
 	//
-	// when forwarding request to a remote cluster, this is not needed
-	// as the proxy uses client cert auth to reach out to remote proxy
-	// which will then use its own transport wrapper
-	if !ctx.cluster.isRemote && f.creds.cfg.WrapTransport != nil {
-		transport = f.creds.cfg.WrapTransport(transport)
+	// When forwarding request to a remote cluster, this is not needed
+	// as the proxy uses client cert auth to reach out to remote proxy.
+	if !ctx.cluster.isRemote {
+		var err error
+		transport, err = f.creds.wrapTransport(transport)
+		if err != nil {
+			return nil, trace.Wrap(err)
+		}
 	}
 
 	fwd, err := forward.New(
@@ -1037,13 +1026,6 @@ func (f *Forwarder) newTransport(dial DialFunc, tlsConfig *tls.Config) *http.Tra
 	}
 }
 
-type bundle struct {
-	cert            []byte
-	key             []byte
-	certAuthorities [][]byte
-	targetAddr      string
-}
-
 // getOrCreateRequestContext creates a new certificate request for a given context,
 // if there is no active CSR request in progress, or returns an existing one.
 // if the new context has been created, cancel function is returned as a
@@ -1066,7 +1048,7 @@ func (f *Forwarder) getOrCreateRequestContext(key string) (context.Context, cont
 	}
 }
 
-func (f *Forwarder) requestCertificate(ctx authContext) (*bundle, error) {
+func (f *Forwarder) requestCertificate(ctx authContext) (*tls.Config, error) {
 	f.Debugf("Requesting K8s cert for %v.", ctx)
 	keyPEM, _, err := f.Keygen.GenerateKeyPair("")
 	if err != nil {
@@ -1098,80 +1080,25 @@ func (f *Forwarder) requestCertificate(ctx authContext) (*bundle, error) {
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
+
 	f.Debugf("Received valid K8s cert for %v.", ctx)
-	return &bundle{
-		cert:            response.Cert,
-		certAuthorities: response.CertAuthorities,
-		targetAddr:      response.TargetAddr,
-		key:             keyPEM,
-	}, nil
-}
 
-type kubeCreds struct {
-	// cfg is a raw k8s config loaded from kubeconfig.
-	cfg *rest.Config
-	// tlsConfig is generated from cfg with all authentication fields set as
-	// needed.
-	tlsConfig *tls.Config
-	// targetAddr is a target address of the kubernetes cluster read from
-	// config
-	targetAddr string
-}
-
-func getKubeCreds(kubeconfigPath string) (*kubeCreds, error) {
-	var cfg *rest.Config
-	// no kubeconfig is set, assume auth server is running in the cluster
-	if kubeconfigPath == "" {
-		caPEM, err := ioutil.ReadFile(teleport.KubeCAPath)
-		if err != nil {
-			return nil, trace.BadParameter(`auth server assumed that it is
-running in a kubernetes cluster, but %v mounted in pods could not be read: %v,
-set kubeconfig_file if auth server is running outside of the cluster`, teleport.KubeCAPath, err)
-		}
-
-		cfg, err = kubeutils.GetKubeConfig(os.Getenv(teleport.EnvKubeConfig))
-		if err != nil {
-			return nil, trace.BadParameter(`auth server assumed that it is
-running in a kubernetes cluster, but could not init in-cluster kubernetes client: %v`, err)
-		}
-		cfg.CAData = caPEM
-	} else {
-		log.Debugf("Reading configuration from kubeconfig file %v.", kubeconfigPath)
-
-		var err error
-		cfg, err = kubeutils.GetKubeConfig(kubeconfigPath)
-		if err != nil {
-			return nil, trace.Wrap(err)
+	cert, err := tls.X509KeyPair(response.Cert, keyPEM)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	pool := x509.NewCertPool()
+	for _, certAuthority := range response.CertAuthorities {
+		ok := pool.AppendCertsFromPEM(certAuthority)
+		if !ok {
+			return nil, trace.BadParameter("failed to append certificates, check that kubeconfig has correctly encoded certificate authority data")
 		}
 	}
-
-	targetAddr, err := parseKubeHost(cfg.Host)
-	if err != nil {
-		return nil, trace.Wrap(err, "failed to parse kubernetes host")
-	}
-	tlsConfig, err := rest.TLSConfigFor(cfg)
-	if err != nil {
-		return nil, trace.Wrap(err, "failed to generate TLS config from kubeconfig")
+	tlsConfig := &tls.Config{
+		RootCAs:      pool,
+		Certificates: []tls.Certificate{cert},
 	}
+	tlsConfig.BuildNameToCertificate()
 
-	return &kubeCreds{
-		cfg:        cfg,
-		tlsConfig:  tlsConfig,
-		targetAddr: targetAddr,
-	}, nil
-}
-
-// parseKubeHost parses and formats kubernetes hostname
-// to host:port format, if no port it set,
-// it assumes default HTTPS port
-func parseKubeHost(host string) (string, error) {
-	u, err := url.Parse(host)
-	if err != nil {
-		return "", trace.Wrap(err, "failed to parse kubernetes host")
-	}
-	if _, _, err := net.SplitHostPort(u.Host); err != nil {
-		// add default HTTPS port
-		return fmt.Sprintf("%v:443", u.Host), nil
-	}
-	return u.Host, nil
+	return tlsConfig, nil
 }

lib/kube/proxy/roundtrip.go

@@ -51,8 +51,7 @@ type SpdyRoundTripper struct {
 	//to the remote server.
 	tlsConfig *tls.Config
 
-	authCtx     authContext
-	bearerToken string
+	authCtx authContext
 
 	/* TODO according to http://golang.org/pkg/net/http/#RoundTripper, a RoundTripper
 	   must be safe for use by multiple concurrent goroutines. If this is absolutely
@@ -84,7 +83,6 @@ type DialWithContext func(context context.Context, network, address string) (net
 type roundTripperConfig struct {
 	ctx             context.Context
 	authCtx         authContext
-	bearerToken     string
 	dial            DialWithContext
 	tlsConfig       *tls.Config
 	followRedirects bool
@@ -93,7 +91,7 @@ type roundTripperConfig struct {
 // NewSpdyRoundTripperWithDialer creates a new SpdyRoundTripper that will use
 // the specified tlsConfig. This function is mostly meant for unit tests.
 func NewSpdyRoundTripperWithDialer(cfg roundTripperConfig) *SpdyRoundTripper {
-	return &SpdyRoundTripper{tlsConfig: cfg.tlsConfig, followRedirects: cfg.followRedirects, dialWithContext: cfg.dial, ctx: cfg.ctx, authCtx: cfg.authCtx, bearerToken: cfg.bearerToken}
+	return &SpdyRoundTripper{tlsConfig: cfg.tlsConfig, followRedirects: cfg.followRedirects, dialWithContext: cfg.dial, ctx: cfg.ctx, authCtx: cfg.authCtx}
 }
 
 // TLSClientConfig implements pkg/util/net.TLSClientConfigHolder for proper TLS checking during
@@ -104,7 +102,7 @@ func (s *SpdyRoundTripper) TLSClientConfig() *tls.Config {
 
 // Dial implements k8s.io/apimachinery/pkg/util/net.Dialer.
 func (s *SpdyRoundTripper) Dial(req *http.Request) (net.Conn, error) {
-	conn, err := s.dial(req)
+	conn, err := s.dial(req.URL)
 	if err != nil {
 		return nil, err
 	}
@@ -117,13 +115,8 @@ func (s *SpdyRoundTripper) Dial(req *http.Request) (net.Conn, error) {
 	return conn, nil
 }
 
-// dial dials the host specified by req
-func (s *SpdyRoundTripper) dial(req *http.Request) (net.Conn, error) {
-	return s.dialWithoutProxy(req.URL)
-}
-
-// dialWithoutProxy dials the host specified by url, using TLS if appropriate.
-func (s *SpdyRoundTripper) dialWithoutProxy(url *url.URL) (net.Conn, error) {
+// dial dials the host specified by url, using TLS if appropriate.
+func (s *SpdyRoundTripper) dial(url *url.URL) (net.Conn, error) {
 	dialAddr := netutil.CanonicalAddr(url)
 
 	if url.Scheme == "http" {
@@ -144,24 +137,13 @@ func (s *SpdyRoundTripper) dialWithoutProxy(url *url.URL) (net.Conn, error) {
 		conn, err = utils.TLSDial(s.ctx, s.dialWithContext, "tcp", dialAddr, s.tlsConfig)
 	}
 	if err != nil {
-		return nil, err
+		return nil, trace.Wrap(err)
 	}
 
-	// Return if we were configured to skip validation
-	if s.tlsConfig != nil && s.tlsConfig.InsecureSkipVerify {
-		return conn, nil
-	}
-
-	host, _, err := net.SplitHostPort(dialAddr)
-	if err != nil {
-		return nil, err
-	}
-	if s.tlsConfig != nil && len(s.tlsConfig.ServerName) > 0 {
-		host = s.tlsConfig.ServerName
-	}
-	err = conn.VerifyHostname(host)
-	if err != nil {
-		return nil, err
+	// Client handshake will verify the server hostname and cert chain. That
+	// way we can err our before first read/write.
+	if err := conn.Handshake(); err != nil {
+		return nil, trace.Wrap(err)
 	}
 
 	return conn, nil
@@ -175,7 +157,7 @@ func (s *SpdyRoundTripper) RoundTrip(req *http.Request) (*http.Response, error)
 	header.Add(httpstream.HeaderConnection, httpstream.HeaderUpgrade)
 	header.Add(httpstream.HeaderUpgrade, streamspdy.HeaderSpdy31)
 
-	if err := setupImpersonationHeaders(log.StandardLogger(), &s.authCtx, header, s.bearerToken); err != nil {
+	if err := setupImpersonationHeaders(log.StandardLogger(), &s.authCtx, header); err != nil {
 		return nil, trace.Wrap(err)
 	}