Skip to content

Commit

Permalink
Enrich audit events with server information (#48475)
Browse files Browse the repository at this point in the history 
Subsystem events did not include ServerMetadata, which results in
any events generated for agentless hosts to contain limited
information. For example, any sftp subsystem requests made in
response to tsh scp for agentless nodes provided information about
the proxy forwarding the requests, but no information about the
target host.

Additionally, the way ServerMetadata information was being populated
by using the ServerContext overrode only a subset of the information
that was available. This lead to SessionData events emitted when
an sftp operation was completed not populating any relevant information
about agentless nodes either. To avoid this going forward, the
GetServerMetadata receiver has been removed from ServerContext and
instead the TargetMetadata must be used from the appropriate SSH
server, which already populates the relevant information without
omitting any.
  • Loading branch information
@rosstimothy
rosstimothy committed Nov 12, 2024
1 parent 6c7415b commit 68374d5
Show file tree
Hide file tree
Showing 11 changed files with 809 additions and 754 deletions.
7 changes: 7 additions & 0 deletions api/proto/teleport/legacy/types/events/events.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1913,6 +1913,13 @@ message Subsystem {

// Error contains error in case of unsucessfull attempt
string Error = 5 [(gogoproto.jsontag) = "exitError"];

// ServerMetadata is a common server metadata
ServerMetadata Server = 6 [
(gogoproto.nullable) = false,
(gogoproto.embed) = true,
(gogoproto.jsontag) = ""
];
}

// ClientDisconnect is emitted when client is disconnected
Expand Down
1,479 changes: 763 additions & 716 deletions api/types/events/events.pb.go
View file

Large diffs are not rendered by default.

6 changes: 5 additions & 1 deletion lib/events/events_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ func TestJSON(t *testing.T) {
},
{
name: "rejected subsystem",
json: `{"ei":0,"cluster_name":"test","addr.local":"127.0.0.1:57518","addr.remote":"127.0.0.1:3022","code":"T3001E","event":"subsystem","exitError":"some error","login":"alice","name":"proxy","time":"2020-04-15T20:28:18Z","uid":"3129a5ae-ee1e-4b39-8d7c-a0a3f218e7dc","user":"[email protected]"}`,
json: `{"ei":0,"cluster_name":"test","addr.local":"127.0.0.1:57518","addr.remote":"127.0.0.1:3022","code":"T3001E","event":"subsystem","exitError":"some error","forwarded_by":"abc","login":"alice","name":"proxy","server_id":"123","time":"2020-04-15T20:28:18Z","uid":"3129a5ae-ee1e-4b39-8d7c-a0a3f218e7dc","user":"[email protected]"}`,
event: apievents.Subsystem{
Metadata: apievents.Metadata{
ID: "3129a5ae-ee1e-4b39-8d7c-a0a3f218e7dc",
Expand All @@ -634,6 +634,10 @@ func TestJSON(t *testing.T) {
LocalAddr: "127.0.0.1:57518",
RemoteAddr: "127.0.0.1:3022",
},
ServerMetadata: apievents.ServerMetadata{
ServerID: "123",
ForwardedBy: "abc",
},
Name: "proxy",
Error: "some error",
},
Expand Down
11 changes: 1 addition & 10 deletions lib/srv/ctx.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1003,7 +1003,7 @@ func (c *ServerContext) reportStats(conn utils.Stater) {
Type: events.SessionDataEvent,
Code: events.SessionDataCode,
},
ServerMetadata: c.GetServerMetadata(),
ServerMetadata: c.srv.TargetMetadata(),
SessionMetadata: c.GetSessionMetadata(),
UserMetadata: c.Identity.GetUserMetadata(),
ConnectionMetadata: apievents.ConnectionMetadata{
Expand Down Expand Up @@ -1387,15 +1387,6 @@ func (c *ServerContext) GetExecRequest() (Exec, error) {
return c.execRequest, nil
}

func (c *ServerContext) GetServerMetadata() apievents.ServerMetadata {
return apievents.ServerMetadata{
ServerVersion: teleport.Version,
ServerID: c.srv.HostUUID(),
ServerHostname: c.srv.GetInfo().GetHostname(),
ServerNamespace: c.srv.GetNamespace(),
}
}

func (c *ServerContext) GetSessionMetadata() apievents.SessionMetadata {
return apievents.SessionMetadata{
SessionID: string(c.SessionID()),
Expand Down
6 changes: 3 additions & 3 deletions lib/srv/exec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ func (e *localExec) transformSecureCopy() error {
Time: time.Now(),
},
UserMetadata: e.Ctx.Identity.GetUserMetadata(),
ServerMetadata: e.Ctx.GetServerMetadata(),
ServerMetadata: e.Ctx.GetServer().TargetMetadata(),
Error: err.Error(),
})
return trace.Wrap(err)
Expand Down Expand Up @@ -367,7 +367,7 @@ func (e *remoteExec) Start(ctx context.Context, ch ssh.Channel) (*ExecResult, er
Time: time.Now(),
},
UserMetadata: e.ctx.Identity.GetUserMetadata(),
ServerMetadata: e.ctx.GetServerMetadata(),
ServerMetadata: e.ctx.GetServer().TargetMetadata(),
Error: err.Error(),
})
return nil, trace.Wrap(err)
Expand Down Expand Up @@ -433,7 +433,7 @@ func (e *remoteExec) PID() int {
// instead of ctx.srv.
func emitExecAuditEvent(ctx *ServerContext, cmd string, execErr error) {
// Create common fields for event.
serverMeta := ctx.GetServerMetadata()
serverMeta := ctx.GetServer().TargetMetadata()
sessionMeta := ctx.GetSessionMetadata()
userMeta := ctx.Identity.GetUserMetadata()

Expand Down
11 changes: 6 additions & 5 deletions lib/srv/exec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,12 @@ func TestEmitExecAuditEvent(t *testing.T) {
rec, ok := scx.session.recorder.(*mockRecorder)
require.True(t, ok)

scx.GetServer().TargetMetadata()

expectedUsr, err := user.Current()
require.NoError(t, err)
expectedHostname, err := os.Hostname()
if err != nil {
expectedHostname = "localhost"
}
expectedHostname := "testHost"

expectedMeta := apievents.UserMetadata{
User: "teleportUser",
Login: expectedUsr.Username,
Expand Down Expand Up @@ -115,7 +115,8 @@ func TestEmitExecAuditEvent(t *testing.T) {
require.Equal(t, tt.outCommand, execEvent.Command)
require.Equal(t, tt.outCode, execEvent.ExitCode)
require.Equal(t, expectedMeta, execEvent.UserMetadata)
require.Equal(t, "testHostUUID", execEvent.ServerID)
require.Equal(t, "123", execEvent.ServerID)
require.Equal(t, "abc", execEvent.ForwardedBy)
require.Equal(t, expectedHostname, execEvent.ServerHostname)
require.Equal(t, "testNamespace", execEvent.ServerNamespace)
require.Equal(t, "xxx", execEvent.SessionID)
Expand Down
8 changes: 4 additions & 4 deletions lib/srv/forward/sshserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1446,7 +1446,7 @@ func (s *Server) handleSubsystem(ctx context.Context, ch ssh.Channel, req *ssh.R
}

// if SFTP was requested, check that
if subsystem.subsytemName == teleport.SFTPSubsystem {
if subsystem.subsystemName == teleport.SFTPSubsystem {
err := serverContext.CheckSFTPAllowed(s.sessionRegistry)
if err != nil {
s.EmitAuditEvent(context.WithoutCancel(ctx), &apievents.SFTP{
Expand All @@ -1456,7 +1456,7 @@ func (s *Server) handleSubsystem(ctx context.Context, ch ssh.Channel, req *ssh.R
Time: time.Now(),
},
UserMetadata: serverContext.Identity.GetUserMetadata(),
ServerMetadata: serverContext.GetServerMetadata(),
ServerMetadata: serverContext.GetServer().TargetMetadata(),
Error: err.Error(),
})
return trace.Wrap(err)
Expand All @@ -1467,7 +1467,7 @@ func (s *Server) handleSubsystem(ctx context.Context, ch ssh.Channel, req *ssh.R
err = subsystem.Start(ctx, ch)
if err != nil {
serverContext.SendSubsystemResult(srv.SubsystemResult{
Name: subsystem.subsytemName,
Name: subsystem.subsystemName,
Err: trace.Wrap(err),
})
return trace.Wrap(err)
Expand All @@ -1477,7 +1477,7 @@ func (s *Server) handleSubsystem(ctx context.Context, ch ssh.Channel, req *ssh.R
go func() {
err := subsystem.Wait()
serverContext.SendSubsystemResult(srv.SubsystemResult{
Name: subsystem.subsytemName,
Name: subsystem.subsystemName,
Err: trace.Wrap(err),
})
}()
Expand Down
22 changes: 11 additions & 11 deletions lib/srv/forward/subsystem.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,23 +37,23 @@ type remoteSubsystem struct {
log *log.Entry

serverContext *srv.ServerContext
subsytemName string
subsystemName string

ctx context.Context
errorCh chan error
}

// parseRemoteSubsystem returns *remoteSubsystem which can be used to run a subsystem on a remote node.
func parseRemoteSubsystem(ctx context.Context, subsytemName string, serverContext *srv.ServerContext) *remoteSubsystem {
func parseRemoteSubsystem(ctx context.Context, subsystemName string, serverContext *srv.ServerContext) *remoteSubsystem {
return &remoteSubsystem{
log: log.WithFields(log.Fields{
teleport.ComponentKey: teleport.ComponentRemoteSubsystem,
teleport.ComponentFields: map[string]string{
"name": subsytemName,
"name": subsystemName,
},
}),
serverContext: serverContext,
subsytemName: subsytemName,
subsystemName: subsystemName,
ctx: ctx,
errorCh: make(chan error, 3),
}
Expand All @@ -78,10 +78,10 @@ func (r *remoteSubsystem) Start(ctx context.Context, channel ssh.Channel) error

// request the subsystem from the remote node. if successful, the user can
// interact with the remote subsystem with stdin, stdout, and stderr.
err = session.RequestSubsystem(ctx, r.subsytemName)
err = session.RequestSubsystem(ctx, r.subsystemName)
if err != nil {
// emit an event to the audit log with the reason remote execution failed
r.emitAuditEvent(err)
r.emitAuditEvent(ctx, err)

return trace.Wrap(err)
}
Expand Down Expand Up @@ -126,13 +126,12 @@ func (r *remoteSubsystem) Wait() error {
}

// emit an event to the audit log with the result of execution
r.emitAuditEvent(lastErr)
r.emitAuditEvent(r.ctx, lastErr)

return lastErr
}

func (r *remoteSubsystem) emitAuditEvent(err error) {
srv := r.serverContext.GetServer()
func (r *remoteSubsystem) emitAuditEvent(ctx context.Context, err error) {
subsystemEvent := &apievents.Subsystem{
Metadata: apievents.Metadata{
Type: events.SubsystemEvent,
Expand All @@ -142,7 +141,8 @@ func (r *remoteSubsystem) emitAuditEvent(err error) {
LocalAddr: r.serverContext.RemoteClient.LocalAddr().String(),
RemoteAddr: r.serverContext.RemoteClient.RemoteAddr().String(),
},
Name: r.subsytemName,
Name: r.subsystemName,
ServerMetadata: r.serverContext.GetServer().TargetMetadata(),
}

if err != nil {
Expand All @@ -152,7 +152,7 @@ func (r *remoteSubsystem) emitAuditEvent(err error) {
subsystemEvent.Code = events.SubsystemCode
}

if err := srv.EmitAuditEvent(srv.Context(), subsystemEvent); err != nil {
if err := r.serverContext.GetServer().EmitAuditEvent(ctx, subsystemEvent); err != nil {
r.log.WithError(err).Warn("Failed to emit subsystem audit event.")
}
}
7 changes: 6 additions & 1 deletion lib/srv/mock.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,12 @@ func (m *mockServer) GetInfo() types.Server {
}

func (m *mockServer) TargetMetadata() apievents.ServerMetadata {
return apievents.ServerMetadata{}
return apievents.ServerMetadata{
ServerID: "123",
ForwardedBy: "abc",
ServerHostname: "testHost",
ServerNamespace: "testNamespace",
}
}

// UseTunnel used to determine if this node has connected to this cluster
Expand Down
4 changes: 2 additions & 2 deletions lib/srv/regular/sftp.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (s *sftpSubsys) Start(ctx context.Context,
Time: time.Now(),
},
UserMetadata: serverCtx.Identity.GetUserMetadata(),
ServerMetadata: serverCtx.GetServerMetadata(),
ServerMetadata: serverCtx.GetServer().TargetMetadata(),
Error: srv.ErrNodeFileCopyingNotPermitted.Error(),
})
return srv.ErrNodeFileCopyingNotPermitted
Expand Down Expand Up @@ -169,7 +169,7 @@ func (s *sftpSubsys) Start(ctx context.Context,
defer auditPipeOut.Close()

// Create common fields for events
serverMeta := serverCtx.GetServerMetadata()
serverMeta := serverCtx.GetServer().TargetMetadata()
sessionMeta := serverCtx.GetSessionMetadata()
userMeta := serverCtx.Identity.GetUserMetadata()
connectionMeta := apievents.ConnectionMetadata{
Expand Down
2 changes: 1 addition & 1 deletion lib/srv/regular/sshserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2475,7 +2475,7 @@ func (s *Server) parseSubsystemRequest(req *ssh.Request, ctx *srv.ServerContext)
Time: time.Now(),
},
UserMetadata: ctx.Identity.GetUserMetadata(),
ServerMetadata: ctx.GetServerMetadata(),
ServerMetadata: ctx.GetServer().TargetMetadata(),
Error: err.Error(),
})
return nil, trace.Wrap(err)
Expand Down

0 comments on commit 68374d5

Please sign in to comment.