Improving SSH port forwarding audit logs #50932
Conversation
| fmt.Fprint(w, "Hello, world") | ||
| })) | ||
| t.Cleanup(tsrv.Close) | ||
| testSrvConn, err := net.Dial("tcp", tsrv.Listener.Addr().String()) |
There was a problem hiding this comment.
is this a conn that needs to be closed on cleanup as well?
| func (c *ServerContext) GetPortForwardEvent() apievents.PortForward { | ||
| sconn := c.ConnectionContext.ServerConn | ||
| func (c *ServerContext) GetPortForwardEvent(evType, code string) apievents.PortForward { | ||
| sconn := c.ServerConn |
There was a problem hiding this comment.
Suggestion: Prefer being explicit over relying on the fact that ConnectionContext is embedded in ServerContext here
| sconn := c.ServerConn | |
| sconn := c. ConnectionContext.ServerConn |
| @@ -648,6 +648,47 @@ func (s *Server) Serve() { | |||
|
|
|||
| succeeded = true | |||
|
|
|||
| if err := s.EmitAuditEvent(ctx, &apievents.PortForward{ | |||
There was a problem hiding this comment.
Is this the right place to emit this event? Isn't this the entrypoint for all connections established to the forward server? Won't this cause the event to be emitted any time a user connects to an agentless host or a host in proxy recording mode?
| CloseCancel: func() { | ||
| s.connectionContext.Close() | ||
| }, |
There was a problem hiding this comment.
Suggestion: revert changes to preserve the original blame here
| } | ||
|
|
||
| if err := utils.ProxyConn(ctx, client, server); err != nil { | ||
| s.logger.ErrorContext(ctx, "PROXY CONN FAILURE", "error", err) |
There was a problem hiding this comment.
Suggestion: remove the CAPS?
| } | ||
| } | ||
|
|
||
| s.logger.ErrorContext(ctx, "PROXY CONN DONE") |
There was a problem hiding this comment.
Suggestion: remove the CAPS?
This PR emits some additional events for SSH port forwarding and adds some context around local/remote when possible. The emitted events should look like the following:
Every event should contain the remote, local, and target address where applicable. For remote forwarding, session-level events will show the requester as the remote address whereas connection-level events should show the client address initiating the connection.