From 3cf48120af0366bf592c99d737b3fc3495345d26 Mon Sep 17 00:00:00 2001 From: Andrew LeFevre Date: Fri, 21 Apr 2023 09:44:17 -0400 Subject: [PATCH] don't rely on info in SSH user cert when connecting to agentless nodes (#24935) After finding now 2 very similar bugs today related to trying to pull connection information out of SSH certificates instead of just passing it directly, opt to pass the information directly to prevent future bugs. --- lib/agentless/agentless.go | 8 ++------ lib/srv/regular/proxy.go | 2 +- lib/web/terminal.go | 7 +++++-- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/agentless/agentless.go b/lib/agentless/agentless.go index 17e0ba9ea8d..34361a6b108 100644 --- a/lib/agentless/agentless.go +++ b/lib/agentless/agentless.go @@ -29,7 +29,6 @@ import ( "github.com/gravitational/teleport/api/utils/sshutils" "github.com/gravitational/teleport/lib/auth/native" "github.com/gravitational/teleport/lib/authz" - "github.com/gravitational/teleport/lib/utils" ) // CertGenerator generates certificates from a certificate request. @@ -40,15 +39,12 @@ type CertGenerator interface { // SignerFromSSHCertificate returns a function that attempts to // create a [ssh.Signer] for the Identity in the provided [ssh.Certificate] // that is signed with the OpenSSH CA and can be used to authenticate to agentless nodes. -func SignerFromSSHCertificate(certificate *ssh.Certificate, generator CertGenerator) func(context.Context) (ssh.Signer, error) { +func SignerFromSSHCertificate(certificate *ssh.Certificate, teleportUser, clusterName string, generator CertGenerator) func(context.Context) (ssh.Signer, error) { return func(ctx context.Context) (ssh.Signer, error) { validBefore := time.Unix(int64(certificate.ValidBefore), 0) ttl := time.Until(validBefore) - clusterName := certificate.Permissions.Extensions[utils.CertTeleportClusterName] - user := certificate.Permissions.Extensions[utils.CertTeleportUser] - - signer, err := createAuthSigner(ctx, user, clusterName, ttl, generator) + signer, err := createAuthSigner(ctx, teleportUser, clusterName, ttl, generator) return signer, trace.Wrap(err) } } diff --git a/lib/srv/regular/proxy.go b/lib/srv/regular/proxy.go index 65bbf9e3d9b..51ecf2a5c3d 100644 --- a/lib/srv/regular/proxy.go +++ b/lib/srv/regular/proxy.go @@ -266,7 +266,7 @@ func (t *proxySubsys) proxyToHost(ctx context.Context, ch ssh.Channel, clientSrc return trace.Wrap(err) } - signer := agentless.SignerFromSSHCertificate(t.ctx.Identity.Certificate, client) + signer := agentless.SignerFromSSHCertificate(t.ctx.Identity.Certificate, t.ctx.Identity.TeleportUser, t.clusterName, client) conn, teleportVersion, err := t.router.DialHost(ctx, clientSrcAddr, clientDstAddr, t.host, t.port, t.clusterName, t.ctx.Identity.AccessChecker, aGetter, signer) if err != nil { return trace.Wrap(err) diff --git a/lib/web/terminal.go b/lib/web/terminal.go index 7dc93f6fb79..959c167778f 100644 --- a/lib/web/terminal.go +++ b/lib/web/terminal.go @@ -96,7 +96,6 @@ type AuthProvider interface { GetSessionTracker(ctx context.Context, sessionID string) (types.SessionTracker, error) IsMFARequired(ctx context.Context, req *authproto.IsMFARequiredRequest) (*authproto.IsMFARequiredResponse, error) GenerateUserSingleUseCerts(ctx context.Context) (authproto.AuthService_GenerateUserSingleUseCertsClient, error) - GenerateOpenSSHCert(ctx context.Context, req *authproto.OpenSSHCertRequest) (*authproto.OpenSSHCert, error) } // NewTerminal creates a web-based terminal based on WebSockets and returns a @@ -660,7 +659,11 @@ func (t *TerminalHandler) connectToHost(ctx context.Context, ws *websocket.Conn, return nil, trace.Wrap(err) } - signer := agentless.SignerFromSSHCertificate(cert, t.authProvider) + authClient, err := t.router.GetSiteClient(ctx, tc.SiteName) + if err != nil { + return nil, trace.Wrap(err) + } + signer := agentless.SignerFromSSHCertificate(cert, tc.Username, tc.SiteName, authClient) type clientRes struct { clt *client.NodeClient