diff --git a/integration/appaccess/pack.go b/integration/appaccess/pack.go index fd67fbb3ff921..38e402712fda1 100644 --- a/integration/appaccess/pack.go +++ b/integration/appaccess/pack.go @@ -156,6 +156,10 @@ func (p *Pack) RootWebAddr() string { return p.rootCluster.Web } +func (p *Pack) RootAppName() string { + return p.rootAppName +} + func (p *Pack) RootAppClusterName() string { return p.rootAppClusterName } @@ -164,6 +168,10 @@ func (p *Pack) RootAppPublicAddr() string { return p.rootAppPublicAddr } +func (p *Pack) LeafAppName() string { + return p.leafAppName +} + func (p *Pack) LeafAppClusterName() string { return p.leafAppClusterName } @@ -278,6 +286,22 @@ func (p *Pack) MakeTeleportClient(t *testing.T, user string) *client.TeleportCli return tc } +// GenerateAndSetupUserCreds is useful in situations where we need to manually manipulate user +// certs, for example when we want to force a TeleportClient to operate using expired certs. +// +// ttl equals to 0 means that the certs will have the default TTL used by helpers.GenerateUserCreds. +func (p *Pack) GenerateAndSetupUserCreds(t *testing.T, tc *client.TeleportClient, ttl time.Duration) { + creds, err := helpers.GenerateUserCreds(helpers.UserCredsRequest{ + Process: p.rootCluster.Process, + Username: tc.Username, + TTL: ttl, + }) + require.NoError(t, err) + + err = helpers.SetupUserCreds(tc, p.rootCluster.Process.Config.Proxy.SSHAddr.Addr, *creds) + require.NoError(t, err) +} + // CreateAppSession creates an application session with the root cluster. The // application that the user connects to may be running in a leaf cluster. func (p *Pack) CreateAppSession(t *testing.T, publicAddr, clusterName string) []*http.Cookie { diff --git a/integration/proxy/proxy_helpers.go b/integration/proxy/proxy_helpers.go index 26246ff412cbb..89a639cf684f8 100644 --- a/integration/proxy/proxy_helpers.go +++ b/integration/proxy/proxy_helpers.go @@ -717,6 +717,25 @@ func mustConnectDatabaseGateway(t *testing.T, _ *daemon.Service, gw gateway.Gate require.NoError(t, client.Close()) } +// mustConnectAppGateway verifies that the gateway acts as an unauthenticated proxy that forwards +// requests to the app behind it. +func mustConnectAppGateway(t *testing.T, _ *daemon.Service, gw gateway.Gateway) { + t.Helper() + + appGw, err := gateway.AsApp(gw) + require.NoError(t, err) + + req, err := http.NewRequest(http.MethodGet, appGw.LocalProxyURL(), nil) + require.NoError(t, err) + + client := &http.Client{} + resp, err := client.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusOK, resp.StatusCode) +} + func kubeClientForLocalProxy(t *testing.T, kubeconfigPath, teleportCluster, kubeCluster string) *kubernetes.Clientset { t.Helper() diff --git a/integration/proxy/proxy_test.go b/integration/proxy/proxy_test.go index 84c2aa75cf34c..a8bb5acb3c9be 100644 --- a/integration/proxy/proxy_test.go +++ b/integration/proxy/proxy_test.go @@ -1215,8 +1215,8 @@ func TestALPNSNIProxyDatabaseAccess(t *testing.T) { require.NoError(t, client.Close()) }) - t.Run("teleterm gateways cert renewal", func(t *testing.T) { - testTeletermGatewaysCertRenewal(t, pack) + t.Run("teleterm db gateways cert renewal", func(t *testing.T) { + testTeletermDbGatewaysCertRenewal(t, pack) }) } @@ -1271,6 +1271,10 @@ func TestALPNSNIProxyAppAccess(t *testing.T) { resp.Body.Close() require.Equal(t, http.StatusOK, resp.StatusCode) }) + + t.Run("teleterm app gateways cert renewal", func(t *testing.T) { + testTeletermAppGateway(t, pack) + }) } // TestALPNProxyRootLeafAuthDial tests dialing local/remote auth service based on ALPN diff --git a/integration/proxy/teleterm_test.go b/integration/proxy/teleterm_test.go index 86265e3d55901..05a97450c2a65 100644 --- a/integration/proxy/teleterm_test.go +++ b/integration/proxy/teleterm_test.go @@ -38,6 +38,7 @@ import ( "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/types" api "github.com/gravitational/teleport/gen/proto/go/teleport/lib/teleterm/v1" + "github.com/gravitational/teleport/integration/appaccess" dbhelpers "github.com/gravitational/teleport/integration/db" "github.com/gravitational/teleport/integration/helpers" "github.com/gravitational/teleport/integration/kube" @@ -57,9 +58,9 @@ import ( "github.com/gravitational/teleport/lib/utils" ) -// testTeletermGatewaysCertRenewal is run from within TestALPNSNIProxyDatabaseAccess to amortize the +// testTeletermDbGatewaysCertRenewal is run from within TestALPNSNIProxyDatabaseAccess to amortize the // cost of setting up clusters in tests. -func testTeletermGatewaysCertRenewal(t *testing.T, pack *dbhelpers.DatabasePack) { +func testTeletermDbGatewaysCertRenewal(t *testing.T, pack *dbhelpers.DatabasePack) { ctx := context.Background() t.Run("root cluster", func(t *testing.T) { @@ -97,48 +98,62 @@ func testTeletermGatewaysCertRenewal(t *testing.T, pack *dbhelpers.DatabasePack) func testDBGatewayCertRenewal(ctx context.Context, t *testing.T, pack *dbhelpers.DatabasePack, albAddr string, databaseURI uri.ResourceURI) { t.Helper() + tc, err := pack.Root.Cluster.NewClient(helpers.ClientConfig{ + Login: pack.Root.User.GetName(), + Cluster: pack.Root.Cluster.Secrets.SiteName, + ALBAddr: albAddr, + }) + require.NoError(t, err) + testGatewayCertRenewal( ctx, t, gatewayCertRenewalParams{ - inst: pack.Root.Cluster, - username: pack.Root.User.GetName(), - albAddr: albAddr, + tc: tc, + albAddr: albAddr, createGatewayParams: daemon.CreateGatewayParams{ TargetURI: databaseURI.String(), TargetUser: pack.Root.User.GetName(), }, testGatewayConnectionFunc: mustConnectDatabaseGateway, + generateAndSetupUserCreds: func(t *testing.T, tc *libclient.TeleportClient, ttl time.Duration) { + creds, err := helpers.GenerateUserCreds(helpers.UserCredsRequest{ + Process: pack.Root.Cluster.Process, + Username: tc.Username, + TTL: ttl, + }) + require.NoError(t, err) + err = helpers.SetupUserCreds(tc, pack.Root.Cluster.Process.Config.Proxy.SSHAddr.Addr, *creds) + require.NoError(t, err) + }, }, ) } type testGatewayConnectionFunc func(*testing.T, *daemon.Service, gateway.Gateway) +type generateAndSetupUserCredsFunc func(t *testing.T, tc *libclient.TeleportClient, ttl time.Duration) + type gatewayCertRenewalParams struct { - inst *helpers.TeleInstance - username string + tc *libclient.TeleportClient albAddr string createGatewayParams daemon.CreateGatewayParams testGatewayConnectionFunc testGatewayConnectionFunc webauthnLogin libclient.WebauthnLoginFunc + generateAndSetupUserCreds generateAndSetupUserCredsFunc } func testGatewayCertRenewal(ctx context.Context, t *testing.T, params gatewayCertRenewalParams) { t.Helper() - tc, err := params.inst.NewClient(helpers.ClientConfig{ - Login: params.username, - Cluster: params.inst.Secrets.SiteName, - ALBAddr: params.albAddr, - }) - require.NoError(t, err) + tc := params.tc - // Save the profile yaml file to disk as NewClientWithCreds doesn't do that by itself. - err = tc.SaveProfile(false /* makeCurrent */) + // Save the profile yaml file to disk as test helpers like helpers.NewClientWithCreds don't do + // that by themselves. + err := tc.SaveProfile(false /* makeCurrent */) require.NoError(t, err) - tshdEventsService := newMockTSHDEventsServiceServer(t, tc, params.inst, params.username) + tshdEventsService := newMockTSHDEventsServiceServer(t, tc, params.generateAndSetupUserCreds) var webauthLoginCalls atomic.Uint32 webauthnLogin := func(ctx context.Context, origin string, assertion *wantypes.CredentialAssertion, prompt wancli.LoginPrompt, opts *wancli.LoginOpts) (*proto.MFAAuthenticateResponse, string, error) { @@ -204,14 +219,7 @@ func testGatewayCertRenewal(ctx context.Context, t *testing.T, params gatewayCer fakeClock.Advance(time.Hour * 48) // Overwrite user certs with expired ones to simulate the user cert expiry. - expiredCreds, err := helpers.GenerateUserCreds(helpers.UserCredsRequest{ - Process: params.inst.Process, - Username: params.username, - TTL: -time.Hour, - }) - require.NoError(t, err) - err = helpers.SetupUserCreds(tc, params.inst.Config.Proxy.SSHAddr.Addr, *expiredCreds) - require.NoError(t, err) + params.generateAndSetupUserCreds(t, tc, -time.Hour) // Open a new connection. // This should trigger the relogin flow. The middleware will notice that the cert has expired @@ -235,26 +243,26 @@ func testGatewayCertRenewal(ctx context.Context, t *testing.T, params gatewayCer type mockTSHDEventsService struct { *api.UnimplementedTshdEventsServiceServer + t *testing.T tc *libclient.TeleportClient - inst *helpers.TeleInstance - username string addr string reloginCallCount atomic.Uint32 sendNotificationCallCount atomic.Uint32 promptMFACallCount atomic.Uint32 + generateAndSetupUserCreds generateAndSetupUserCredsFunc } -func newMockTSHDEventsServiceServer(t *testing.T, tc *libclient.TeleportClient, inst *helpers.TeleInstance, username string) (service *mockTSHDEventsService) { +func newMockTSHDEventsServiceServer(t *testing.T, tc *libclient.TeleportClient, generateAndSetupUserCreds generateAndSetupUserCredsFunc) (service *mockTSHDEventsService) { t.Helper() ls, err := net.Listen("tcp", "127.0.0.1:0") require.NoError(t, err) tshdEventsService := &mockTSHDEventsService{ - tc: tc, - inst: inst, - username: username, - addr: ls.Addr().String(), + t: t, + tc: tc, + addr: ls.Addr().String(), + generateAndSetupUserCreds: generateAndSetupUserCreds, } grpcServer := grpc.NewServer() @@ -284,17 +292,9 @@ func newMockTSHDEventsServiceServer(t *testing.T, tc *libclient.TeleportClient, // cert on disk with a valid one. func (c *mockTSHDEventsService) Relogin(context.Context, *api.ReloginRequest) (*api.ReloginResponse, error) { c.reloginCallCount.Add(1) - creds, err := helpers.GenerateUserCreds(helpers.UserCredsRequest{ - Process: c.inst.Process, - Username: c.username, - }) - if err != nil { - return nil, trace.Wrap(err) - } - err = helpers.SetupUserCreds(c.tc, c.inst.Config.Proxy.SSHAddr.Addr, *creds) - if err != nil { - return nil, trace.Wrap(err) - } + + // Generate valid certs with the default TTL. + c.generateAndSetupUserCreds(c.t, c.tc, 0 /* ttl */) return &api.ReloginResponse{}, nil } @@ -460,6 +460,13 @@ func testKubeGatewayCertRenewal(ctx context.Context, t *testing.T, params kubeGa teleportCluster = params.kubeURI.GetLeafClusterName() } + tc, err := params.suite.root.NewClient(helpers.ClientConfig{ + Login: params.suite.username, + Cluster: params.suite.root.Secrets.SiteName, + ALBAddr: params.albAddr, + }) + require.NoError(t, err) + testKubeConnection := func(t *testing.T, daemonService *daemon.Service, gw gateway.Gateway) { t.Helper() @@ -480,14 +487,23 @@ func testKubeGatewayCertRenewal(ctx context.Context, t *testing.T, params kubeGa ctx, t, gatewayCertRenewalParams{ - inst: params.suite.root, - username: params.suite.username, - albAddr: params.albAddr, + albAddr: params.albAddr, + tc: tc, createGatewayParams: daemon.CreateGatewayParams{ TargetURI: params.kubeURI.String(), }, testGatewayConnectionFunc: testKubeConnection, webauthnLogin: params.webauthnLogin, + generateAndSetupUserCreds: func(t *testing.T, tc *libclient.TeleportClient, ttl time.Duration) { + creds, err := helpers.GenerateUserCreds(helpers.UserCredsRequest{ + Process: params.suite.root.Process, + Username: tc.Username, + TTL: ttl, + }) + require.NoError(t, err) + err = helpers.SetupUserCreds(tc, params.suite.root.Process.Config.Proxy.SSHAddr.Addr, *creds) + require.NoError(t, err) + }, }, ) } @@ -576,3 +592,43 @@ func setupUserMFA(ctx context.Context, t *testing.T, authServer *auth.Server, ro return webauthnLogin } + +func testTeletermAppGateway(t *testing.T, pack *appaccess.Pack) { + ctx := context.Background() + + t.Run("root cluster", func(t *testing.T) { + profileName := mustGetProfileName(t, pack.RootWebAddr()) + appURI := uri.NewClusterURI(profileName).AppendApp(pack.RootAppName()) + + testAppGatewayCertRenewal(ctx, t, pack, appURI) + }) + + t.Run("leaf cluster", func(t *testing.T) { + profileName := mustGetProfileName(t, pack.RootWebAddr()) + appURI := uri.NewClusterURI(profileName). + AppendLeafCluster(pack.LeafAppClusterName()). + AppendApp(pack.LeafAppName()) + + testAppGatewayCertRenewal(ctx, t, pack, appURI) + }) +} + +func testAppGatewayCertRenewal(ctx context.Context, t *testing.T, pack *appaccess.Pack, appURI uri.ResourceURI) { + t.Helper() + + user, _ := pack.CreateUser(t) + tc := pack.MakeTeleportClient(t, user.GetName()) + + testGatewayCertRenewal( + ctx, + t, + gatewayCertRenewalParams{ + tc: tc, + createGatewayParams: daemon.CreateGatewayParams{ + TargetURI: appURI.String(), + }, + testGatewayConnectionFunc: mustConnectAppGateway, + generateAndSetupUserCreds: pack.GenerateAndSetupUserCreds, + }, + ) +} diff --git a/lib/srv/alpnproxy/local_proxy.go b/lib/srv/alpnproxy/local_proxy.go index f17c6c88c6bc6..aa98c7026fd0b 100644 --- a/lib/srv/alpnproxy/local_proxy.go +++ b/lib/srv/alpnproxy/local_proxy.go @@ -398,6 +398,22 @@ func (l *LocalProxy) CheckDBCerts(dbRoute tlsca.RouteToDatabase) error { return trace.Wrap(CheckCertSubject(cert, dbRoute)) } +// CheckCertExpiry checks the proxy certificates for expiration. +func (l *LocalProxy) CheckCertExpiry() error { + l.cfg.Log.Debug("checking local proxy certs") + l.certsMu.RLock() + defer l.certsMu.RUnlock() + if len(l.cfg.Certs) == 0 { + return trace.NotFound("local proxy has no TLS certificates configured") + } + cert, err := utils.TLSCertLeaf(l.cfg.Certs[0]) + if err != nil { + return trace.Wrap(err) + } + + return trace.Wrap(utils.VerifyCertificateExpiry(cert, l.cfg.Clock)) +} + // CheckCertSubject checks if the route to the database from the cert matches the provided route in // terms of username and database (if present). func CheckCertSubject(cert *x509.Certificate, dbRoute tlsca.RouteToDatabase) error { diff --git a/lib/teleterm/clusters/cluster_gateways.go b/lib/teleterm/clusters/cluster_gateways.go index 6823c3f8f1359..fa9faaa330ef7 100644 --- a/lib/teleterm/clusters/cluster_gateways.go +++ b/lib/teleterm/clusters/cluster_gateways.go @@ -207,6 +207,8 @@ func (c *Cluster) ReissueGatewayCerts(ctx context.Context, g gateway.Gateway) (t } // db gateways still store certs on disk, so they need to load it after reissue. + // Unlike other gateway types, the custom middleware for db proxies does not set the cert on the + // local proxy. err = g.ReloadCert() if err != nil { return tls.Certificate{}, trace.Wrap(err) @@ -220,12 +222,13 @@ func (c *Cluster) ReissueGatewayCerts(ctx context.Context, g gateway.Gateway) (t return cert, trace.Wrap(err) case g.TargetURI().IsApp(): appName := g.TargetURI().GetAppName() - app, err := c.getApp(ctx, appName) if err != nil { return tls.Certificate{}, trace.Wrap(err) } + // The cert is saved and then loaded from disk, then returned from this function and finally set + // on LocalProxy by the middleware. cert, err := c.reissueAppCert(ctx, app) return cert, trace.Wrap(err) default: diff --git a/lib/teleterm/gateway/app.go b/lib/teleterm/gateway/app.go index 56361d07613a8..38e9c0333e827 100644 --- a/lib/teleterm/gateway/app.go +++ b/lib/teleterm/gateway/app.go @@ -18,7 +18,7 @@ package gateway import ( "context" - "net" + "crypto/tls" "net/url" "strings" @@ -55,11 +55,29 @@ func makeAppGateway(cfg Config) (Gateway, error) { return nil, trace.Wrap(err) } + middleware := &appMiddleware{ + log: a.cfg.Log, + onExpiredCert: func(ctx context.Context) (tls.Certificate, error) { + cert, err := a.cfg.OnExpiredCert(ctx, a) + return cert, trace.Wrap(err) + }, + } + + localProxyConfig := alpnproxy.LocalProxyConfig{ + InsecureSkipVerify: a.cfg.Insecure, + RemoteProxyAddr: a.cfg.WebProxyAddr, + Listener: listener, + ParentContext: a.closeContext, + Clock: a.cfg.Clock, + ALPNConnUpgradeRequired: a.cfg.TLSRoutingConnUpgradeRequired, + } + lp, err := alpnproxy.NewLocalProxy( - makeBasicLocalProxyConfig(a.closeContext, a.cfg, listener), + localProxyConfig, alpnproxy.WithALPNProtocol(alpnProtocolForApp(a.cfg.Protocol)), alpnproxy.WithClientCerts(a.cfg.Cert), alpnproxy.WithClusterCAsIfConnUpgrade(a.closeContext, a.cfg.RootClusterCACertPoolFunc), + alpnproxy.WithMiddleware(middleware), ) if err != nil { return nil, trace.NewAggregate(err, listener.Close()) @@ -69,16 +87,6 @@ func makeAppGateway(cfg Config) (Gateway, error) { return a, nil } -func makeBasicLocalProxyConfig(ctx context.Context, cfg *Config, listener net.Listener) alpnproxy.LocalProxyConfig { - return alpnproxy.LocalProxyConfig{ - RemoteProxyAddr: cfg.WebProxyAddr, - InsecureSkipVerify: cfg.Insecure, - ParentContext: ctx, - Listener: listener, - ALPNConnUpgradeRequired: cfg.TLSRoutingConnUpgradeRequired, - } -} - func alpnProtocolForApp(protocol string) alpncommon.Protocol { if protocol == types.ApplicationProtocolTCP { return alpncommon.ProtocolTCP diff --git a/lib/teleterm/gateway/app_middleware.go b/lib/teleterm/gateway/app_middleware.go new file mode 100644 index 0000000000000..cfb616f110652 --- /dev/null +++ b/lib/teleterm/gateway/app_middleware.go @@ -0,0 +1,67 @@ +// Teleport +// Copyright (C) 2024 Gravitational, Inc. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package gateway + +import ( + "context" + "crypto/tls" + "crypto/x509" + "errors" + "net" + + "github.com/gravitational/trace" + "github.com/sirupsen/logrus" + + alpn "github.com/gravitational/teleport/lib/srv/alpnproxy" +) + +type appMiddleware struct { + onExpiredCert func(context.Context) (tls.Certificate, error) + log *logrus.Entry +} + +// OnNewConnection calls m.onExpiredCert to get a fresh cert if the cert has expired and then sets +// it on the local proxy. +// Other middlewares typically also handle MFA here. App access doesn't support per-session MFA yet, +// so detecting expired certs is all this middleware can do. +func (m *appMiddleware) OnNewConnection(ctx context.Context, lp *alpn.LocalProxy, conn net.Conn) error { + err := lp.CheckCertExpiry() + if err == nil { + return nil + } + + // Return early and don't fire onExpiredCert if certs are invalid but not due to expiry. + if !errors.As(err, &x509.CertificateInvalidError{}) { + return trace.Wrap(err) + } + + m.log.WithError(err).Debug("Gateway certificates have expired") + + cert, err := m.onExpiredCert(ctx) + if err != nil { + return trace.Wrap(err) + } + + lp.SetCerts([]tls.Certificate{cert}) + return nil +} + +// OnStart is a noop. Middlewares used by tsh check cert validity on start. However, in Connect +// there's no flow which would allow the user to create a local proxy without valid certs. +func (m *appMiddleware) OnStart(context.Context, *alpn.LocalProxy) error { + return nil +} diff --git a/lib/teleterm/gateway/config.go b/lib/teleterm/gateway/config.go index 872fe679066ed..d927894ed941b 100644 --- a/lib/teleterm/gateway/config.go +++ b/lib/teleterm/gateway/config.go @@ -90,6 +90,8 @@ type Config struct { // OnExpiredCert is called when a new downstream connection is accepted by the // gateway but cannot be proxied because the cert used by the gateway has expired. // + // Returns a fresh valid cert. + // // Handling of the connection is blocked until OnExpiredCert returns. OnExpiredCert OnExpiredCertFunc // TLSRoutingConnUpgradeRequired indicates that ALPN connection upgrades diff --git a/lib/teleterm/gateway/db_middleware.go b/lib/teleterm/gateway/db_middleware.go index 12d6e31561f06..b22cca304582d 100644 --- a/lib/teleterm/gateway/db_middleware.go +++ b/lib/teleterm/gateway/db_middleware.go @@ -59,9 +59,8 @@ func (m *dbMiddleware) OnNewConnection(ctx context.Context, lp *alpn.LocalProxy, return trace.Wrap(m.onExpiredCert(ctx)) } -// OnStart is a noop. client.DBCertChecker.OnStart checks cert validity too. However in Connect -// there's no flow which would allow the user to create a local proxy without valid -// certs. +// OnStart is a noop. client.DBCertChecker.OnStart checks cert validity. However in Connect there's +// no flow which would allow the user to create a local proxy without valid certs. func (m *dbMiddleware) OnStart(context.Context, *alpn.LocalProxy) error { return nil }