Skip to content

Commit

Permalink
Enrich audit events with server information
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 5, 2024
1 parent c9a7fce commit 5d23716
Show file tree
Hide file tree
Showing 8 changed files with 864 additions and 819 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 @@ -1973,6 +1973,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,623 changes: 835 additions & 788 deletions api/types/events/events.pb.go
View file

Large diffs are not rendered by default.

11 changes: 1 addition & 10 deletions lib/srv/ctx.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,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 @@ -1247,15 +1247,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
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 @@ -1459,7 +1459,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 @@ -1469,7 +1469,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 @@ -1480,7 +1480,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 @@ -1490,7 +1490,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 @@ -38,23 +38,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 @@ -79,10 +79,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 @@ -127,13 +127,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 @@ -143,7 +142,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 @@ -153,7 +153,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.")
}
}
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 @@ -2302,7 +2302,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 5d23716

Please sign in to comment.