From b006a4aa719f2dfd99d4d0d1e92d9e8874e90f8d Mon Sep 17 00:00:00 2001 From: "STeve (Xin) Huang" Date: Tue, 9 May 2023 09:11:12 -0400 Subject: [PATCH] TLS Routing behind ALB Connect support for SSH and Database access. (#25495) * TLS Routing behind ALB Connect support for SSH and Database access. * update/add comments --- integration/proxy/proxy_helpers.go | 5 ++++ integration/proxy/teleterm_test.go | 20 ++++++++++---- integration/teleterm_test.go | 3 +-- lib/client/client.go | 1 + lib/teleterm/clusters/cluster_auth.go | 6 +++++ lib/teleterm/clusters/cluster_gateways.go | 32 ++++++++++++----------- lib/teleterm/clusters/storage.go | 6 +++++ lib/teleterm/gateway/config.go | 7 +++++ lib/teleterm/gateway/gateway.go | 31 ++++++++-------------- 9 files changed, 69 insertions(+), 42 deletions(-) diff --git a/integration/proxy/proxy_helpers.go b/integration/proxy/proxy_helpers.go index 81e6f82059f..513417cd445 100644 --- a/integration/proxy/proxy_helpers.go +++ b/integration/proxy/proxy_helpers.go @@ -639,6 +639,10 @@ func (m *mockAWSALBProxy) serve(ctx context.Context) { conn, err := m.Accept() if err != nil { + if utils.IsUseOfClosedNetworkError(err) { + continue + } + logrus.WithError(err).Debugf("Failed to accept conn.") return } @@ -649,6 +653,7 @@ func (m *mockAWSALBProxy) serve(ctx context.Context) { // Handshake with incoming client and drops ALPN. downstreamConn := tls.Server(conn, &tls.Config{ Certificates: []tls.Certificate{m.cert}, + ClientAuth: tls.NoClientCert, }) // api.Client may try different connection methods. Just close the diff --git a/integration/proxy/teleterm_test.go b/integration/proxy/teleterm_test.go index 99705b78363..94b1e9afe26 100644 --- a/integration/proxy/teleterm_test.go +++ b/integration/proxy/teleterm_test.go @@ -53,7 +53,7 @@ func testTeletermGatewaysCertRenewal(t *testing.T, pack *dbhelpers.DatabasePack) databaseURI := uri.NewClusterURI(rootClusterName). AppendDB(pack.Root.MysqlService.Name) - testGatewayCertRenewal(t, pack, creds, databaseURI) + testGatewayCertRenewal(t, pack, "", creds, databaseURI) }) t.Run("leaf cluster", func(t *testing.T) { leafClusterName := pack.Leaf.Cluster.Secrets.SiteName @@ -61,18 +61,28 @@ func testTeletermGatewaysCertRenewal(t *testing.T, pack *dbhelpers.DatabasePack) AppendLeafCluster(leafClusterName). AppendDB(pack.Leaf.MysqlService.Name) - testGatewayCertRenewal(t, pack, creds, databaseURI) + testGatewayCertRenewal(t, pack, "", creds, databaseURI) + }) + t.Run("ALPN connection upgrade", func(t *testing.T) { + // Make a mock ALB which points to the Teleport Proxy Service. Then + // ALPN local proxies will point to this ALB instead. + albProxy := mustStartMockALBProxy(t, pack.Root.Cluster.Web) + + databaseURI := uri.NewClusterURI(rootClusterName). + AppendDB(pack.Root.MysqlService.Name) + + testGatewayCertRenewal(t, pack, albProxy.Addr().String(), creds, databaseURI) }) } -func testGatewayCertRenewal(t *testing.T, pack *dbhelpers.DatabasePack, creds *helpers.UserCreds, databaseURI uri.ResourceURI) { +func testGatewayCertRenewal(t *testing.T, pack *dbhelpers.DatabasePack, albAddr string, creds *helpers.UserCreds, databaseURI uri.ResourceURI) { tc, err := pack.Root.Cluster.NewClientWithCreds(helpers.ClientConfig{ Login: pack.Root.User.GetName(), Cluster: pack.Root.Cluster.Secrets.SiteName, + ALBAddr: albAddr, }, *creds) require.NoError(t, err) - // The profile on disk created by NewClientWithCreds doesn't have WebProxyAddr set. - tc.WebProxyAddr = pack.Root.Cluster.Web + // Save the profile yaml file to disk as NewClientWithCreds doesn't do that by itself. tc.SaveProfile(false /* makeCurrent */) fakeClock := clockwork.NewFakeClockAt(time.Now()) diff --git a/integration/teleterm_test.go b/integration/teleterm_test.go index df3338ec228..21d7aadffa9 100644 --- a/integration/teleterm_test.go +++ b/integration/teleterm_test.go @@ -222,8 +222,7 @@ func mustLogin(t *testing.T, userName string, pack *dbhelpers.DatabasePack, cred Cluster: pack.Root.Cluster.Secrets.SiteName, }, *creds) require.NoError(t, err) - // The profile on disk created by NewClientWithCreds doesn't have WebProxyAddr set. - tc.WebProxyAddr = pack.Root.Cluster.Web + // Save the profile yaml file to disk as NewClientWithCreds doesn't do that by itself. tc.SaveProfile(false /* makeCurrent */) return tc } diff --git a/lib/client/client.go b/lib/client/client.go index 18806994aa3..678cc67b34f 100644 --- a/lib/client/client.go +++ b/lib/client/client.go @@ -1134,6 +1134,7 @@ func (proxy *ProxyClient) ConnectToAuthServiceThroughALPNSNIProxy(ctx context.Co CircuitBreakerConfig: breaker.NoopBreakerConfig(), ALPNConnUpgradeRequired: proxy.teleportClient.IsALPNConnUpgradeRequiredForWebProxy(proxyAddr), PROXYHeaderGetter: CreatePROXYHeaderGetter(ctx, proxy.teleportClient.PROXYSigner), + InsecureAddressDiscovery: proxy.teleportClient.InsecureSkipVerify, }) if err != nil { return nil, trace.Wrap(err) diff --git a/lib/teleterm/clusters/cluster_auth.go b/lib/teleterm/clusters/cluster_auth.go index e54556eb178..d83ed99a81c 100644 --- a/lib/teleterm/clusters/cluster_auth.go +++ b/lib/teleterm/clusters/cluster_auth.go @@ -24,6 +24,7 @@ import ( "github.com/gravitational/trace" "github.com/sirupsen/logrus" + apiclient "github.com/gravitational/teleport/api/client" "github.com/gravitational/teleport/api/client/webclient" "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/utils/keys" @@ -42,6 +43,11 @@ func (c *Cluster) SyncAuthPreference(ctx context.Context) (*webclient.WebConfigA return nil, trace.Wrap(err) } + // Do the ALPN handshake test to decide if connection upgrades are required + // for TLS Routing. Only do the test once Ping verifies the cluster is + // reachable. + c.clusterClient.TLSRoutingConnUpgradeRequired = apiclient.IsALPNConnUpgradeRequired(c.clusterClient.WebProxyAddr, c.clusterClient.InsecureSkipVerify) + if err := c.clusterClient.SaveProfile(false); err != nil { return nil, trace.Wrap(err) } diff --git a/lib/teleterm/clusters/cluster_gateways.go b/lib/teleterm/clusters/cluster_gateways.go index ce2da150d4d..d901b779cc4 100644 --- a/lib/teleterm/clusters/cluster_gateways.go +++ b/lib/teleterm/clusters/cluster_gateways.go @@ -58,21 +58,23 @@ func (c *Cluster) CreateGateway(ctx context.Context, params CreateGatewayParams) } gw, err := gateway.New(gateway.Config{ - LocalPort: params.LocalPort, - TargetURI: params.TargetURI, - TargetUser: params.TargetUser, - TargetName: db.GetName(), - TargetSubresourceName: params.TargetSubresourceName, - Protocol: db.GetProtocol(), - KeyPath: c.status.KeyPath(), - CertPath: c.status.DatabaseCertPathForCluster(c.clusterClient.SiteName, db.GetName()), - Insecure: c.clusterClient.InsecureSkipVerify, - WebProxyAddr: c.clusterClient.WebProxyAddr, - Log: c.Log, - CLICommandProvider: params.CLICommandProvider, - TCPPortAllocator: params.TCPPortAllocator, - OnExpiredCert: params.OnExpiredCert, - Clock: c.clock, + LocalPort: params.LocalPort, + TargetURI: params.TargetURI, + TargetUser: params.TargetUser, + TargetName: db.GetName(), + TargetSubresourceName: params.TargetSubresourceName, + Protocol: db.GetProtocol(), + KeyPath: c.status.KeyPath(), + CertPath: c.status.DatabaseCertPathForCluster(c.clusterClient.SiteName, db.GetName()), + Insecure: c.clusterClient.InsecureSkipVerify, + WebProxyAddr: c.clusterClient.WebProxyAddr, + Log: c.Log, + CLICommandProvider: params.CLICommandProvider, + TCPPortAllocator: params.TCPPortAllocator, + OnExpiredCert: params.OnExpiredCert, + Clock: c.clock, + TLSRoutingConnUpgradeRequired: c.clusterClient.TLSRoutingConnUpgradeRequired, + RootClusterCACertPoolFunc: c.clusterClient.RootClusterCACertPool, }) if err != nil { return nil, trace.Wrap(err) diff --git a/lib/teleterm/clusters/storage.go b/lib/teleterm/clusters/storage.go index 157b087b8ee..4b40a849126 100644 --- a/lib/teleterm/clusters/storage.go +++ b/lib/teleterm/clusters/storage.go @@ -22,6 +22,7 @@ import ( "github.com/gravitational/trace" + apiclient "github.com/gravitational/teleport/api/client" "github.com/gravitational/teleport/api/profile" "github.com/gravitational/teleport/lib/client" "github.com/gravitational/teleport/lib/teleterm/api/uri" @@ -159,6 +160,11 @@ func (s *Storage) addCluster(ctx context.Context, dir, webProxyAddress string) ( return nil, trace.Wrap(err) } + // Do the ALPN handshake test to decide if connection upgrades are required + // for TLS Routing. Only do the test once Ping verifies the cluster is + // reachable. + clusterClient.TLSRoutingConnUpgradeRequired = apiclient.IsALPNConnUpgradeRequired(webProxyAddress, s.InsecureSkipVerify) + if err := clusterClient.SaveProfile(false); err != nil { return nil, trace.Wrap(err) } diff --git a/lib/teleterm/gateway/config.go b/lib/teleterm/gateway/config.go index 60fc022462c..448850d33fc 100644 --- a/lib/teleterm/gateway/config.go +++ b/lib/teleterm/gateway/config.go @@ -27,6 +27,7 @@ import ( "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/lib/defaults" + "github.com/gravitational/teleport/lib/srv/alpnproxy" "github.com/gravitational/teleport/lib/teleterm/api/uri" "github.com/gravitational/teleport/lib/tlsca" ) @@ -73,6 +74,12 @@ type Config struct { // // Handling of the connection is blocked until OnExpiredCert returns. OnExpiredCert OnExpiredCertFunc + // TLSRoutingConnUpgradeRequired indicates that ALPN connection upgrades + // are required for making TLS routing requests. + TLSRoutingConnUpgradeRequired bool + // RootClusterCACertPoolFunc is callback function to fetch Root cluster CAs + // when ALPN connection upgrade is required. + RootClusterCACertPoolFunc alpnproxy.GetClusterCACertPoolFunc } // OnExpiredCertFunc is the type of a function that is called when a new downstream connection is diff --git a/lib/teleterm/gateway/gateway.go b/lib/teleterm/gateway/gateway.go index 6220ae6825c..6c7d33d177d 100644 --- a/lib/teleterm/gateway/gateway.go +++ b/lib/teleterm/gateway/gateway.go @@ -28,7 +28,6 @@ import ( "github.com/gravitational/teleport/api/utils/keys" alpn "github.com/gravitational/teleport/lib/srv/alpnproxy" - alpncommon "github.com/gravitational/teleport/lib/srv/alpnproxy/common" "github.com/gravitational/teleport/lib/teleterm/api/uri" "github.com/gravitational/teleport/lib/tlsca" "github.com/gravitational/teleport/lib/utils" @@ -68,16 +67,6 @@ func New(cfg Config) (*Gateway, error) { cfg.LocalPort = port - protocol, err := alpncommon.ToALPNProtocol(cfg.Protocol) - if err != nil { - return nil, trace.Wrap(err) - } - - address, err := utils.ParseAddr(cfg.WebProxyAddr) - if err != nil { - return nil, trace.Wrap(err) - } - tlsCert, err := keys.LoadX509KeyPair(cfg.CertPath, cfg.KeyPath) if err != nil { return nil, trace.Wrap(err) @@ -89,14 +78,13 @@ func New(cfg Config) (*Gateway, error) { } localProxyConfig := alpn.LocalProxyConfig{ - InsecureSkipVerify: cfg.Insecure, - RemoteProxyAddr: cfg.WebProxyAddr, - Protocols: []alpncommon.Protocol{protocol}, - Listener: listener, - ParentContext: closeContext, - SNI: address.Host(), - Certs: []tls.Certificate{tlsCert}, - Clock: cfg.Clock, + InsecureSkipVerify: cfg.Insecure, + RemoteProxyAddr: cfg.WebProxyAddr, + Listener: listener, + ParentContext: closeContext, + Certs: []tls.Certificate{tlsCert}, + Clock: cfg.Clock, + ALPNConnUpgradeRequired: cfg.TLSRoutingConnUpgradeRequired, } localProxyMiddleware := &localProxyMiddleware{ @@ -108,7 +96,10 @@ func New(cfg Config) (*Gateway, error) { localProxyConfig.Middleware = localProxyMiddleware } - localProxy, err := alpn.NewLocalProxy(localProxyConfig) + localProxy, err := alpn.NewLocalProxy(localProxyConfig, + alpn.WithDatabaseProtocol(cfg.Protocol), + alpn.WithClusterCAsIfConnUpgrade(closeContext, cfg.RootClusterCACertPoolFunc), + ) if err != nil { return nil, trace.Wrap(err) }