Skip to content

Commit

Permalink
Reduce tsh binary size (#41743) (#41976)
Browse files Browse the repository at this point in the history
* Remove lib/web dependency from tsh

Refactors the terminal stream code in lib/web into a subpackage
so that tsh no longer depends on the entire lib/web package.

* Remove lib/auth dependency from tsh

Refactors invalid credentials errors and helpers into lib/auth/authclient
so that tsh no longer imports lib/auth directly.

* fix find-replace typos
  • Loading branch information
rosstimothy authored May 24, 2024
1 parent d3d3f98 commit 25497b9
Show file tree
Hide file tree
Showing 24 changed files with 778 additions and 661 deletions.
2 changes: 1 addition & 1 deletion buf.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ lint:
- api/proto/teleport/legacy/types/types.proto
- api/proto/teleport/legacy/types/wrappers/wrappers.proto
- proto/teleport/lib/multiplexer/test/ping.proto
- proto/teleport/lib/web/envelope.proto
- proto/teleport/lib/web/terminal/envelope.proto
ignore_only:
COMMENT_MESSAGE:
- proto/prehog
Expand Down
4 changes: 2 additions & 2 deletions build.assets/genproto.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ main() {
--path=api/proto/teleport/legacy/ \
--path=api/proto/teleport/attestation/ \
--path=api/proto/teleport/usageevents/ \
--path=proto/teleport/lib/web/envelope.proto \
--path=proto/teleport/lib/web/terminal/envelope.proto \
--exclude-path=api/proto/teleport/legacy/client/proto/event.proto
cp -r gogogen/github.com/gravitational/teleport/. .
# error out if there's anything outside of github.com/gravitational/teleport
Expand All @@ -58,7 +58,7 @@ main() {
--exclude-path=api/proto/teleport/legacy/ \
--exclude-path=api/proto/teleport/attestation/ \
--exclude-path=api/proto/teleport/usageevents/ \
--exclude-path=proto/teleport/lib/web/envelope.proto \
--exclude-path=proto/teleport/lib/web/terminal/envelope.proto \
--exclude-path=proto/prehog/

# Generate event.proto separately because we only want to run it on this
Expand Down
7 changes: 4 additions & 3 deletions integration/assist/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import (
"github.com/gravitational/teleport/lib/sshutils"
"github.com/gravitational/teleport/lib/utils"
"github.com/gravitational/teleport/lib/web"
"github.com/gravitational/teleport/lib/web/terminal"
)

const (
Expand Down Expand Up @@ -452,12 +453,12 @@ type executionWebsocketReader struct {
*websocket.Conn
}

func (r executionWebsocketReader) Read() (web.Envelope, error) {
func (r executionWebsocketReader) Read() (terminal.Envelope, error) {
_, data, err := r.ReadMessage()
if err != nil {
return web.Envelope{}, trace.Wrap(err)
return terminal.Envelope{}, trace.Wrap(err)
}
var envelope web.Envelope
var envelope terminal.Envelope
return envelope, trace.Wrap(proto.Unmarshal(data, &envelope))
}

Expand Down
7 changes: 4 additions & 3 deletions integration/helpers/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ import (
"github.com/gravitational/teleport/lib/utils"
"github.com/gravitational/teleport/lib/web"
websession "github.com/gravitational/teleport/lib/web/session"
"github.com/gravitational/teleport/lib/web/terminal"
)

const (
Expand Down Expand Up @@ -1532,7 +1533,7 @@ func CreateWebSession(proxyHost, user, password string) (*web.CreateSessionRespo
// SSH establishes an SSH connection via the web api in the same manner that
// the web UI does. The returned [web.TerminalStream] should be used as stdin/stdout
// for the session.
func (w *WebClient) SSH(termReq web.TerminalRequest) (*web.TerminalStream, error) {
func (w *WebClient) SSH(termReq web.TerminalRequest) (*terminal.Stream, error) {
u := url.URL{
Host: w.i.Web,
Scheme: client.WSS,
Expand Down Expand Up @@ -1574,7 +1575,7 @@ func (w *WebClient) SSH(termReq web.TerminalRequest) (*web.TerminalStream, error
return nil, trace.BadParameter("unexpected websocket message; got %d want %d", ty, websocket.BinaryMessage)
}

var env web.Envelope
var env terminal.Envelope
err = proto.Unmarshal(raw, &env)
if err != nil {
return nil, trace.Wrap(err)
Expand All @@ -1590,7 +1591,7 @@ func (w *WebClient) SSH(termReq web.TerminalRequest) (*web.TerminalStream, error
return nil, trace.Wrap(err)
}

stream := web.NewTerminalStream(context.Background(), ws, utils.NewLoggerForTests())
stream := terminal.NewStream(context.Background(), terminal.StreamConfig{WS: ws})
return stream, nil
}

Expand Down
23 changes: 19 additions & 4 deletions lib/auth/authclient/clt.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package authclient

import (
"context"
"errors"
"fmt"
"net"
"net/url"
Expand Down Expand Up @@ -68,10 +69,24 @@ const (
MissingNamespaceError = "missing required parameter: namespace"
)

// ErrNoMFADevices is returned when an MFA ceremony is performed without possible devices to
// complete the challenge with.
var ErrNoMFADevices = &trace.AccessDeniedError{
Message: "MFA is required to access this resource but user has no MFA devices; use 'tsh mfa add' to register MFA devices",
var (
// ErrNoMFADevices is returned when an MFA ceremony is performed without possible devices to
// complete the challenge with.
ErrNoMFADevices = &trace.AccessDeniedError{
Message: "MFA is required to access this resource but user has no MFA devices; use 'tsh mfa add' to register MFA devices",
}
// InvalidUserPassError is the error for when either the provided username or
// password is incorrect.
InvalidUserPassError = &trace.AccessDeniedError{Message: "invalid username or password"}
// InvalidUserPass2FError is the error for when either the provided username,
// password, or second factor is incorrect.
InvalidUserPass2FError = &trace.AccessDeniedError{Message: "invalid username, password or second factor"}
)

// IsInvalidLocalCredentialError checks if an error resulted from an incorrect username,
// password, or second factor.
func IsInvalidLocalCredentialError(err error) bool {
return errors.Is(err, InvalidUserPassError) || errors.Is(err, InvalidUserPass2FError)
}

// HostFQDN consists of host UUID and cluster name joined via '.'.
Expand Down
21 changes: 4 additions & 17 deletions lib/auth/methods.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,30 +191,17 @@ func (a *Server) emitAuthAuditEvent(ctx context.Context, props authAuditProps) e
var (
// authenticateHeadlessError is the generic error returned for failed headless
// authentication attempts.
authenticateHeadlessError = trace.AccessDenied("headless authentication failed")
authenticateHeadlessError = &trace.AccessDeniedError{Message: "headless authentication failed"}
// authenticateWebauthnError is the generic error returned for failed WebAuthn
// authentication attempts.
authenticateWebauthnError = trace.AccessDenied("invalid Webauthn response")
// invalidUserPassError is the error for when either the provided username or
// password is incorrect.
invalidUserPassError = trace.AccessDenied("invalid username or password")
// invalidUserpass2FError is the error for when either the provided username,
// password, or second factor is incorrect.
invalidUserPass2FError = trace.AccessDenied("invalid username, password or second factor")

authenticateWebauthnError = &trace.AccessDeniedError{Message: "invalid Webauthn response"}
// errSSOUserLocalAuth is issued for SSO users attempting local authentication
// or related actions (like trying to set a password)
// Kept purposefully vague, as such actions don't happen during normal
// utilization of the system.
errSSOUserLocalAuth = &trace.AccessDeniedError{Message: "invalid credentials"}
)

// IsInvalidLocalCredentialError checks if an error resulted from an incorrect username,
// password, or second factor.
func IsInvalidLocalCredentialError(err error) bool {
return errors.Is(err, invalidUserPassError) || errors.Is(err, invalidUserPass2FError)
}

type verifyMFADeviceLocksParams struct {
// Checker used to verify locks.
// Optional, created via a [UserState] fetch if nil.
Expand Down Expand Up @@ -352,7 +339,7 @@ func (a *Server) authenticateUserInternal(ctx context.Context, req authclient.Au
}
return res.mfaDev, nil
}
authErr = invalidUserPass2FError
authErr = authclient.InvalidUserPass2FError
}
if authenticateFn != nil {
err := a.WithUserLock(user, func() error {
Expand Down Expand Up @@ -420,7 +407,7 @@ func (a *Server) authenticateUserInternal(ctx context.Context, req authclient.Au
// provide obscure message on purpose, while logging the real
// error server side
log.Debugf("User %v failed to authenticate: %v.", user, err)
return nil, "", trace.Wrap(invalidUserPassError)
return nil, "", trace.Wrap(authclient.InvalidUserPassError)
}
return nil, user, nil
}
Expand Down
19 changes: 16 additions & 3 deletions lib/benchmark/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import (
"github.com/gravitational/teleport/lib/client"
"github.com/gravitational/teleport/lib/session"
"github.com/gravitational/teleport/lib/utils"
"github.com/gravitational/teleport/lib/web"
"github.com/gravitational/teleport/lib/web/terminal"
)

// WebSSHBenchmark is a benchmark suite that connects to the configured
Expand Down Expand Up @@ -172,9 +172,22 @@ func getServers(ctx context.Context, tc *client.TeleportClient) ([]types.Server,
return resources, nil
}

// TerminalRequest describes a request to create a web-based terminal
// to a remote SSH server.
type TerminalRequest struct {
// Server describes a server to connect to (serverId|hostname[:port]).
Server string `json:"server_id"`

// Login is Linux username to connect as.
Login string `json:"login"`

// Term is the initial PTY size.
Term session.TerminalParams `json:"term"`
}

// connectToHost opens an SSH session to the target host via the Proxy web api.
func connectToHost(ctx context.Context, tc *client.TeleportClient, webSession *webSession, host string) (io.ReadWriteCloser, error) {
req := web.TerminalRequest{
req := TerminalRequest{
Server: host,
Login: tc.HostLogin,
Term: session.TerminalParams{
Expand Down Expand Up @@ -220,7 +233,7 @@ func connectToHost(ctx context.Context, tc *client.TeleportClient, webSession *w
return nil, trace.BadParameter("unexpected websocket message received %d", ty)
}

stream := web.NewTerminalStream(ctx, ws, utils.NewLogger())
stream := terminal.NewStream(ctx, terminal.StreamConfig{WS: ws})
return stream, trace.Wrap(err)
}

Expand Down
3 changes: 2 additions & 1 deletion lib/web/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ import (
"github.com/gravitational/teleport/lib/utils"
"github.com/gravitational/teleport/lib/web/app"
websession "github.com/gravitational/teleport/lib/web/session"
"github.com/gravitational/teleport/lib/web/terminal"
"github.com/gravitational/teleport/lib/web/ui"
)

Expand Down Expand Up @@ -3807,7 +3808,7 @@ func (h *Handler) writeErrToWebSocket(ws *websocket.Conn, err error) {
if err == nil {
return
}
errEnvelope := Envelope{
errEnvelope := terminal.Envelope{
Type: defaults.WebsocketError,
Payload: trace.UserMessage(err),
}
Expand Down
Loading

0 comments on commit 25497b9

Please sign in to comment.