From 1327bb83f40a47790570821416fb69f4ff966727 Mon Sep 17 00:00:00 2001 From: Erik Tate Date: Wed, 18 Dec 2024 17:08:21 -0500 Subject: [PATCH] adding additional audit log context around SSH port forwarding --- lib/events/api.go | 11 +- lib/events/dynamic.go | 6 + lib/srv/ctx.go | 8 +- lib/srv/forward/sshserver.go | 4 +- lib/srv/regular/sshserver.go | 114 +++++++++++++----- lib/sshutils/mock.go | 10 -- lib/sshutils/tcpip.go | 68 ----------- lib/sshutils/tcpip_test.go | 63 ---------- .../teleport/src/services/audit/makeEvent.ts | 93 ++++++++++++-- .../teleport/src/services/audit/types.ts | 2 +- 10 files changed, 184 insertions(+), 195 deletions(-) delete mode 100644 lib/sshutils/tcpip_test.go diff --git a/lib/events/api.go b/lib/events/api.go index f9bb4f11beb12..dd0f7dc86982a 100644 --- a/lib/events/api.go +++ b/lib/events/api.go @@ -262,10 +262,13 @@ const ( X11ForwardErr = "error" // Port forwarding event - PortForwardEvent = "port" - PortForwardAddr = "addr" - PortForwardSuccess = "success" - PortForwardErr = "error" + PortForwardEvent = "port" + PortForwardLocalEvent = "port.local" + PortForwardRemoteEvent = "port.remote" + PortForwardRemoteConnEvent = "port.remote_conn" + PortForwardAddr = "addr" + PortForwardSuccess = "success" + PortForwardErr = "error" // AuthAttemptEvent is authentication attempt that either // succeeded or failed based on event status diff --git a/lib/events/dynamic.go b/lib/events/dynamic.go index 06ad99c41f8ec..dbaa535a82368 100644 --- a/lib/events/dynamic.go +++ b/lib/events/dynamic.go @@ -107,6 +107,12 @@ func FromEventFields(fields EventFields) (events.AuditEvent, error) { e = &events.X11Forward{} case PortForwardEvent: e = &events.PortForward{} + case PortForwardLocalEvent: + e = &events.PortForward{} + case PortForwardRemoteEvent: + e = &events.PortForward{} + case PortForwardRemoteConnEvent: + e = &events.PortForward{} case AuthAttemptEvent: e = &events.AuthAttempt{} case SCPEvent: diff --git a/lib/srv/ctx.go b/lib/srv/ctx.go index bb0fc950cfab4..ab54544ee1bdf 100644 --- a/lib/srv/ctx.go +++ b/lib/srv/ctx.go @@ -1401,19 +1401,19 @@ func (c *ServerContext) GetSessionMetadata() apievents.SessionMetadata { } } -func (c *ServerContext) GetPortForwardEvent() apievents.PortForward { +func (c *ServerContext) GetPortForwardEvent(evType, code, addr string) apievents.PortForward { sconn := c.ConnectionContext.ServerConn return apievents.PortForward{ Metadata: apievents.Metadata{ - Type: events.PortForwardEvent, - Code: events.PortForwardCode, + Type: evType, + Code: code, }, UserMetadata: c.Identity.GetUserMetadata(), ConnectionMetadata: apievents.ConnectionMetadata{ LocalAddr: sconn.LocalAddr().String(), RemoteAddr: sconn.RemoteAddr().String(), }, - Addr: c.DstAddr, + Addr: addr, Status: apievents.Status{ Success: true, }, diff --git a/lib/srv/forward/sshserver.go b/lib/srv/forward/sshserver.go index d5f5df7d11f49..86db8ea68ffe5 100644 --- a/lib/srv/forward/sshserver.go +++ b/lib/srv/forward/sshserver.go @@ -930,7 +930,7 @@ func (s *Server) handleForwardedTCPIPRequest(ctx context.Context, nch ssh.NewCha go io.Copy(io.Discard, ch.Stderr()) ch = scx.TrackActivity(ch) - event := scx.GetPortForwardEvent() + event := scx.GetPortForwardEvent(events.PortForwardEvent, events.PortForwardCode, scx.DstAddr) if err := s.EmitAuditEvent(ctx, &event); err != nil { s.log.WithError(err).Error("Failed to emit audit event.") } @@ -1095,7 +1095,7 @@ func (s *Server) handleDirectTCPIPRequest(ctx context.Context, ch ssh.Channel, r } defer conn.Close() - event := scx.GetPortForwardEvent() + event := scx.GetPortForwardEvent(events.PortForwardEvent, events.PortForwardFailureCode, scx.DstAddr) if err := s.EmitAuditEvent(s.closeContext, &event); err != nil { scx.WithError(err).Warn("Failed to emit port forward event.") } diff --git a/lib/srv/regular/sshserver.go b/lib/srv/regular/sshserver.go index 53ade163fe551..ba9a58c95b42b 100644 --- a/lib/srv/regular/sshserver.go +++ b/lib/srv/regular/sshserver.go @@ -1688,27 +1688,17 @@ func (s *Server) handleDirectTCPIPRequest(ctx context.Context, ccx *sshutils.Con return } + startEvent := scx.GetPortForwardEvent(events.PortForwardLocalEvent, events.PortForwardCode, scx.DstAddr) + s.emitAuditEventWithLog(ctx, &startEvent) + if err := utils.ProxyConn(ctx, conn, channel); err != nil && !errors.Is(err, io.EOF) && !errors.Is(err, os.ErrClosed) { - s.Logger.Warnf("Connection problem in direct-tcpip channel: %v %T.", trace.DebugReport(err), err) + errEvent := scx.GetPortForwardEvent(events.PortForwardLocalEvent, events.PortForwardFailureCode, scx.DstAddr) + s.emitAuditEventWithLog(ctx, &errEvent) + slog.WarnContext(ctx, "Connection problem in direct-tcpip channel", "error", err) } - if err := s.EmitAuditEvent(s.ctx, &apievents.PortForward{ - Metadata: apievents.Metadata{ - Type: events.PortForwardEvent, - Code: events.PortForwardCode, - }, - UserMetadata: scx.Identity.GetUserMetadata(), - ConnectionMetadata: apievents.ConnectionMetadata{ - LocalAddr: scx.ServerConn.LocalAddr().String(), - RemoteAddr: scx.ServerConn.RemoteAddr().String(), - }, - Addr: scx.DstAddr, - Status: apievents.Status{ - Success: true, - }, - }); err != nil { - s.Logger.WithError(err).Warn("Failed to emit port forward event.") - } + stopEvent := scx.GetPortForwardEvent(events.PortForwardLocalEvent, events.PortForwardStopCode, scx.DstAddr) + s.emitAuditEventWithLog(ctx, &stopEvent) } // handleSessionRequests handles out of band session requests once the session @@ -2042,9 +2032,7 @@ func (s *Server) handleX11Forward(ch ssh.Channel, req *ssh.Request, ctx *srv.Ser s.replyError(ch, req, err) err = nil } - if err := s.EmitAuditEvent(s.ctx, event); err != nil { - s.Logger.WithError(err).Warn("Failed to emit x11-forward event.") - } + s.emitAuditEventWithLog(s.ctx, event) }() // check if X11 forwarding is disabled, or if xauth can't be handled. @@ -2321,6 +2309,7 @@ func (s *Server) createForwardingContext(ctx context.Context, ccx *sshutils.Conn if err != nil { return nil, nil, trace.Wrap(err) } + listenAddr := sshutils.JoinHostPort(req.Addr, req.Port) scx.IsTestStub = s.isTestStub scx.ExecType = teleport.TCPIPForwardRequest @@ -2359,13 +2348,72 @@ func (s *Server) handleTCPIPForwardRequest(ctx context.Context, ccx *sshutils.Co } scx.SrcAddr = sshutils.JoinHostPort(srcHost, listenPort) - event := scx.GetPortForwardEvent() - if err := s.EmitAuditEvent(ctx, &event); err != nil { - s.Logger.WithError(err).Warn("Failed to emit audit event.") - } - if err := sshutils.StartRemoteListener(ctx, scx.ConnectionContext.ServerConn, scx.SrcAddr, listener); err != nil { - return trace.Wrap(err) - } + event := scx.GetPortForwardEvent(events.PortForwardRemoteEvent, events.PortForwardCode, scx.SrcAddr) + s.emitAuditEventWithLog(ctx, &event) + + // spawn remote forwarding handler to multiplex connections to the forwarded port + go func() { + stopEvent := scx.GetPortForwardEvent(events.PortForwardRemoteEvent, events.PortForwardStopCode, scx.SrcAddr) + defer s.emitAuditEventWithLog(ctx, &stopEvent) + + for { + conn, err := listener.Accept() + if err != nil { + if !utils.IsOKNetworkError(err) { + slog.WarnContext(ctx, "failed to accept connection", "error", err) + } + return + } + logger := slog.With( + "src_addr", scx.SrcAddr, + "remote_addr", conn.RemoteAddr().String(), + ) + + dstHost, dstPort, err := sshutils.SplitHostPort(conn.RemoteAddr().String()) + if err != nil { + conn.Close() + logger.WarnContext(ctx, "failed to parse addr", "error", err) + return + } + + req := sshutils.ForwardedTCPIPRequest{ + Addr: srcHost, + Port: listenPort, + OrigAddr: dstHost, + OrigPort: dstPort, + } + if err := req.CheckAndSetDefaults(); err != nil { + conn.Close() + logger.WarnContext(ctx, "failed to create forwarded tcpip request", "error", err) + return + } + reqBytes := ssh.Marshal(req) + + ch, rch, err := scx.ConnectionContext.ServerConn.OpenChannel(teleport.ChanForwardedTCPIP, reqBytes) + if err != nil { + conn.Close() + logger.WarnContext(ctx, "failed to open channel", "error", err) + continue + } + go ssh.DiscardRequests(rch) + go io.Copy(io.Discard, ch.Stderr()) + go func() { + startEvent := scx.GetPortForwardEvent(events.PortForwardRemoteConnEvent, events.PortForwardCode, scx.SrcAddr) + startEvent.RemoteAddr = conn.RemoteAddr().String() + s.emitAuditEventWithLog(ctx, &startEvent) + + if err := utils.ProxyConn(ctx, conn, ch); err != nil { + errEvent := scx.GetPortForwardEvent(events.PortForwardRemoteConnEvent, events.PortForwardFailureCode, scx.SrcAddr) + errEvent.RemoteAddr = conn.RemoteAddr().String() + s.emitAuditEventWithLog(ctx, &errEvent) + } + + stopEvent := scx.GetPortForwardEvent(events.PortForwardRemoteConnEvent, events.PortForwardStopCode, scx.SrcAddr) + stopEvent.RemoteAddr = conn.RemoteAddr().String() + s.emitAuditEventWithLog(ctx, &stopEvent) + }() + } + }() // Report addr back to the client. if r.WantReply { @@ -2397,7 +2445,6 @@ func (s *Server) handleCancelTCPIPForwardRequest(ctx context.Context, ccx *sshut return trace.Wrap(err) } defer scx.Close() - listener, ok := s.remoteForwardingMap.LoadAndDelete(scx.SrcAddr) if !ok { return trace.NotFound("no remote forwarding listener at %v", scx.SrcAddr) @@ -2405,6 +2452,7 @@ func (s *Server) handleCancelTCPIPForwardRequest(ctx context.Context, ccx *sshut if err := r.Reply(true, nil); err != nil { s.Logger.Warnf("Failed to reply to %q request: %v", r.Type, err) } + return trace.Wrap(listener.Close()) } @@ -2447,7 +2495,7 @@ func (s *Server) parseSubsystemRequest(req *ssh.Request, ctx *srv.ServerContext) case r.Name == teleport.SFTPSubsystem: err := ctx.CheckSFTPAllowed(s.reg) if err != nil { - s.EmitAuditEvent(context.Background(), &apievents.SFTP{ + s.emitAuditEventWithLog(context.Background(), &apievents.SFTP{ Metadata: apievents.Metadata{ Code: events.SFTPDisallowedCode, Type: events.SFTPEvent, @@ -2494,3 +2542,9 @@ func (s *Server) handlePuTTYWinadj(ch ssh.Channel, req *ssh.Request) error { req.WantReply = false return nil } + +func (s *Server) emitAuditEventWithLog(ctx context.Context, event apievents.AuditEvent) { + if err := s.EmitAuditEvent(ctx, event); err != nil { + slog.WarnContext(ctx, "Failed to emit event", "type", event.GetType(), "code", event.GetCode()) + } +} diff --git a/lib/sshutils/mock.go b/lib/sshutils/mock.go index 37cdc2b819f1b..3021a20562719 100644 --- a/lib/sshutils/mock.go +++ b/lib/sshutils/mock.go @@ -21,8 +21,6 @@ package sshutils import ( "errors" "io" - - "golang.org/x/crypto/ssh" ) var ( @@ -59,11 +57,3 @@ func (mc *mockChannel) SendRequest(name string, wantReply bool, payload []byte) func (mc *mockChannel) Stderr() io.ReadWriter { return fakeReaderWriter{} } - -type mockSSHConn struct { - mockChan *mockChannel -} - -func (mc *mockSSHConn) OpenChannel(name string, data []byte) (ssh.Channel, <-chan *ssh.Request, error) { - return mc.mockChan, make(<-chan *ssh.Request), nil -} diff --git a/lib/sshutils/tcpip.go b/lib/sshutils/tcpip.go index 55ac4ea981cf3..2dcdd4e520da9 100644 --- a/lib/sshutils/tcpip.go +++ b/lib/sshutils/tcpip.go @@ -19,16 +19,9 @@ package sshutils import ( - "context" - "io" - "net" - "github.com/gravitational/trace" log "github.com/sirupsen/logrus" "golang.org/x/crypto/ssh" - - "github.com/gravitational/teleport" - "github.com/gravitational/teleport/lib/utils" ) // DirectTCPIPReq represents the payload of an SSH "direct-tcpip" or @@ -72,64 +65,3 @@ func ParseTCPIPForwardReq(data []byte) (*TCPIPForwardReq, error) { } return &r, nil } - -type channelOpener interface { - OpenChannel(name string, data []byte) (ssh.Channel, <-chan *ssh.Request, error) -} - -// StartRemoteListener listens on the given listener and forwards any accepted -// connections over a new "forwarded-tcpip" channel. -func StartRemoteListener(ctx context.Context, sshConn channelOpener, srcAddr string, listener net.Listener) error { - srcHost, srcPort, err := SplitHostPort(srcAddr) - if err != nil { - return trace.Wrap(err) - } - - go func() { - for { - conn, err := listener.Accept() - if err != nil { - if !utils.IsOKNetworkError(err) { - log.WithError(err).Warn("failed to accept connection") - } - return - } - logger := log.WithFields(log.Fields{ - "srcAddr": srcAddr, - "remoteAddr": conn.RemoteAddr().String(), - }) - - dstHost, dstPort, err := SplitHostPort(conn.RemoteAddr().String()) - if err != nil { - conn.Close() - logger.WithError(err).Warn("failed to parse addr") - return - } - - req := ForwardedTCPIPRequest{ - Addr: srcHost, - Port: srcPort, - OrigAddr: dstHost, - OrigPort: dstPort, - } - if err := req.CheckAndSetDefaults(); err != nil { - conn.Close() - logger.WithError(err).Warn("failed to create forwarded tcpip request") - return - } - reqBytes := ssh.Marshal(req) - - ch, rch, err := sshConn.OpenChannel(teleport.ChanForwardedTCPIP, reqBytes) - if err != nil { - conn.Close() - logger.WithError(err).Warn("failed to open channel") - continue - } - go ssh.DiscardRequests(rch) - go io.Copy(io.Discard, ch.Stderr()) - go utils.ProxyConn(ctx, conn, ch) - } - }() - - return nil -} diff --git a/lib/sshutils/tcpip_test.go b/lib/sshutils/tcpip_test.go deleted file mode 100644 index 5a59b5a64e57f..0000000000000 --- a/lib/sshutils/tcpip_test.go +++ /dev/null @@ -1,63 +0,0 @@ -/* - * 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 sshutils - -import ( - "context" - "fmt" - "io" - "net" - "net/http" - "net/http/httptest" - "testing" - "time" - - "github.com/stretchr/testify/require" -) - -func TestStartRemoteListener(t *testing.T) { - // Create a test server to act as the other side of the channel. - tsrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, "Hello, world") - })) - t.Cleanup(tsrv.Close) - testSrvConn, err := net.Dial("tcp", tsrv.Listener.Addr().String()) - require.NoError(t, err) - - sshConn := &mockSSHConn{ - mockChan: &mockChannel{ - ReadWriter: testSrvConn, - }, - } - - // Start the remote listener. - listener, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err) - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) - t.Cleanup(cancel) - require.NoError(t, StartRemoteListener(ctx, sshConn, "127.0.0.1:12345", listener)) - - // Check that dialing listener makes it all the way to the test http server. - resp, err := http.Get("http://" + listener.Addr().String()) - require.NoError(t, err) - defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) - require.NoError(t, err) - require.Equal(t, "Hello, world", string(body)) -} diff --git a/web/packages/teleport/src/services/audit/makeEvent.ts b/web/packages/teleport/src/services/audit/makeEvent.ts index c80c13dd41297..55708524a8302 100644 --- a/web/packages/teleport/src/services/audit/makeEvent.ts +++ b/web/packages/teleport/src/services/audit/makeEvent.ts @@ -20,7 +20,14 @@ import { formatDistanceStrict } from 'date-fns'; import { pluralize } from 'shared/utils/text'; -import { Event, eventCodes, Formatters, RawEvent, RawEvents } from './types'; +import { + Event, + EventCode, + eventCodes, + Formatters, + RawEvent, + RawEvents, +} from './types'; const formatElasticsearchEvent: ( json: @@ -65,6 +72,66 @@ const formatElasticsearchEvent: ( return message; }; +const portForwardEventTypes = [ + 'port', + 'port.local', + 'port.remote', + 'port.remote_conn', +] as const; +type PortForwardEventType = (typeof portForwardEventTypes)[number]; +type PortForwardEvent = + | RawEvents[typeof eventCodes.PORTFORWARD] + | RawEvents[typeof eventCodes.PORTFORWARD_STOP] + | RawEvents[typeof eventCodes.PORTFORWARD_FAILURE]; + +const getPortForwardEventName = (event: string): string => { + let ev = event as PortForwardEventType; + if (!portForwardEventTypes.includes(ev)) { + ev = 'port'; // default to generic 'port' if the event type is unknown + } + + switch (ev) { + case 'port': + return 'Port Forwarding'; + case 'port.local': + return 'Local Port Forwarding'; + case 'port.remote': + return 'Remote Port Forwarding'; + case 'port.remote_conn': + return 'Remote Port Forwarded Connection'; + } +}; + +const formatPortForwardEvent = ({ + user, + code, + event, +}: PortForwardEvent): string => { + const eventName = getPortForwardEventName(event).toLowerCase(); + + switch (code) { + case eventCodes.PORTFORWARD: + return `User [${user}] started ${eventName}`; + case eventCodes.PORTFORWARD_STOP: + return `User [${user}] stopped ${eventName}`; + case eventCodes.PORTFORWARD_FAILURE: + return `User [${user}] failed ${eventName}`; + } +}; + +const describePortForwardEvent = ({ code, event }: PortForwardEvent) => { + const eventName = getPortForwardEventName(event); + + switch (code) { + case eventCodes.PORTFORWARD: + return `${eventName} Start`; + case eventCodes.PORTFORWARD_STOP: + return `${eventName} Stop`; + case eventCodes.PORTFORWARD_FAILURE: + return `${eventName} Failure`; + } +}; + export const formatters: Formatters = { [eventCodes.ACCESS_REQUEST_CREATED]: { type: 'access_request.create', @@ -223,19 +290,18 @@ export const formatters: Formatters = { }, [eventCodes.PORTFORWARD]: { type: 'port', - desc: 'Port Forwarding Started', - format: ({ user }) => `User [${user}] started port forwarding`, + desc: describePortForwardEvent, + format: formatPortForwardEvent, }, [eventCodes.PORTFORWARD_FAILURE]: { type: 'port', - desc: 'Port Forwarding Failed', - format: ({ user, error }) => - `User [${user}] port forwarding request failed: ${error}`, + desc: describePortForwardEvent, + format: formatPortForwardEvent, }, [eventCodes.PORTFORWARD_STOP]: { type: 'port', - desc: 'Port Forwarding Stopped', - format: ({ user }) => `User [${user}] stopped port forwarding`, + desc: describePortForwardEvent, + format: formatPortForwardEvent, }, [eventCodes.SAML_CONNECTOR_CREATED]: { type: 'saml.created', @@ -2067,9 +2133,12 @@ const unknownFormatter = { export default function makeEvent(json: any): Event { // lookup event formatter by code - const formatter = formatters[json.code] || unknownFormatter; - const event = { - codeDesc: formatter.desc, + const formatter = formatters[json.code as EventCode] || unknownFormatter; + return { + codeDesc: + typeof formatter.desc === 'function' + ? formatter.desc(json) + : formatter.desc, message: formatter.format(json as any), id: getId(json), code: json.code, @@ -2077,8 +2146,6 @@ export default function makeEvent(json: any): Event { time: json.time, raw: json, }; - - return event; } // older events might not have an uid field. diff --git a/web/packages/teleport/src/services/audit/types.ts b/web/packages/teleport/src/services/audit/types.ts index 5c5b0448225bc..2aabc46b47aff 100644 --- a/web/packages/teleport/src/services/audit/types.ts +++ b/web/packages/teleport/src/services/audit/types.ts @@ -2014,7 +2014,7 @@ type RawSpannerRPCEvent = RawEvent< export type Formatters = { [key in EventCode]: { type: string; - desc: string; + desc: string | ((json: RawEvents[key]) => string); format: (json: RawEvents[key]) => string; }; };