diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 1e64d0efef..ad0411fc66 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -18995,11 +18995,11 @@ const docTemplate = `{ "type": "string" }, "status_code": { - "description": "StatusCode is the HTTP status code of the request.", + "description": "StatusCode is the HTTP status code or tunnel authorization outcome.", "type": "integer" }, "user": { - "description": "User is omitted if the connection event was from an unauthenticated user.", + "description": "User is omitted if the connection event was unauthenticated.", "allOf": [ { "$ref": "#/definitions/codersdk.User" diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 27e026aee9..e7b75e54fa 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -17173,11 +17173,11 @@ "type": "string" }, "status_code": { - "description": "StatusCode is the HTTP status code of the request.", + "description": "StatusCode is the HTTP status code or tunnel authorization outcome.", "type": "integer" }, "user": { - "description": "User is omitted if the connection event was from an unauthenticated user.", + "description": "User is omitted if the connection event was unauthenticated.", "allOf": [ { "$ref": "#/definitions/codersdk.User" diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index a257b9360a..6bbee110cb 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -1369,20 +1369,20 @@ func (api *API) workspaceAgentClientCoordinate(rw http.ResponseWriter, r *http.R return } - api.logTunnelConnection(ctx, r, waws) - ctx, wsNetConn := codersdk.WebsocketNetConn(ctx, conn, websocket.MessageBinary) defer wsNetConn.Close() ctx = api.wsWatcher.Watch(ctx, api.Logger, conn) defer conn.Close(websocket.StatusNormalClosure, "") + auth := tailnet.ClientCoordinateeAuth{ + AgentID: waws.WorkspaceAgent.ID, + Auditor: api.tunnelAuditor(r), + } err = api.TailnetClientService.ServeClient(ctx, version, wsNetConn, tailnet.StreamID{ Name: "client", ID: peerID, - Auth: tailnet.ClientCoordinateeAuth{ - AgentID: waws.WorkspaceAgent.ID, - }, + Auth: auth, }) if err != nil && !xerrors.Is(err, io.EOF) && !xerrors.Is(err, context.Canceled) { _ = conn.Close(websocket.StatusInternalError, err.Error()) @@ -1390,64 +1390,110 @@ func (api *API) workspaceAgentClientCoordinate(rw http.ResponseWriter, r *http.R } } -// logTunnelConnection records a connection log entry attributing a -// tunnel to the authenticated user who opened it. Agent-reported rows -// cannot identify the user (see coderd/agentapi/connectionlog.go), and -// workspace-proxy-authenticated requests carry no API key and are -// skipped. -func (api *API) logTunnelConnection(ctx context.Context, r *http.Request, waws database.GetWorkspaceAgentAndWorkspaceByIDRow) { - apiKey, ok := httpmw.APIKeyOptional(r) +type connectionLogTunnelAuditor struct { + api *API + userID uuid.UUID + ip string + userAgent string +} + +var _ tailnet.TunnelAuditor = connectionLogTunnelAuditor{} + +func (a connectionLogTunnelAuditor) Audit(agentID uuid.UUID, authorizationErr error) { + statusCode := int32(http.StatusSwitchingProtocols) + if authorizationErr != nil { + statusCode = http.StatusForbidden + } + // Authorization runs under the in-memory coordinator mutex, so the + // database write cannot happen inline. + go a.api.logTunnelConnection(agentID, statusCode, a.userID, a.ip, a.userAgent) +} + +func (api *API) tunnelAuditor(r *http.Request) tailnet.TunnelAuditor { + subject, ok := dbauthz.ActorFromContext(r.Context()) if !ok { + api.Logger.Error(r.Context(), "tunnel auditor missing authorization subject") + return nil + } + if subject.Type != rbac.SubjectTypeUser { + api.Logger.Debug(r.Context(), "skip tunnel audit for non-user subject", + slog.F("subject_type", subject.Type), + ) + return nil + } + userID, err := uuid.Parse(subject.ID) + if err != nil { + api.Logger.Error(r.Context(), "parse tunnel auditor user ID", slog.Error(err)) + return nil + } + return connectionLogTunnelAuditor{ + api: api, + userID: userID, + ip: r.RemoteAddr, + userAgent: r.UserAgent(), + } +} + +func (api *API) logTunnelConnection(agentID uuid.UUID, statusCode int32, userID uuid.UUID, ip, userAgent string) { + ctx, cancel := context.WithTimeout(api.ctx, 3*time.Second) + defer cancel() + + // nolint:gocritic // System context is needed to attribute denied tunnel decisions. + waws, err := api.Database.GetWorkspaceAgentAndWorkspaceByID(dbauthz.AsSystemRestricted(ctx), agentID) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + api.Logger.Warn(ctx, "skip tunnel connection log for unknown agent", + slog.F("agent_id", agentID), + slog.F("user_id", userID), + ) + return + } + api.Logger.Error(ctx, "resolve tunnel connection log agent", + slog.F("agent_id", agentID), + slog.F("user_id", userID), + slog.Error(err), + ) return } - // Bounded so log backpressure cannot stall tunnel establishment. - writeCtx, writeCancel := context.WithTimeout(ctx, 3*time.Second) - defer writeCancel() - userAgent := r.UserAgent() - now := dbtime.Now() - // Clients re-dial automatically, so dedupe reconnects through the - // same audit session mechanism as workspace apps, keyed on - // (agent, user, IP, user agent). Status 101 and the empty slug - // keep tunnel sessions from ever colliding with app or - // port-forwarding sessions. staleInterval := api.Options.WorkspaceAppAuditSessionTimeout if staleInterval == 0 { staleInterval = time.Hour } - // nolint:gocritic // System context is needed to write audit sessions. - newSession, err := api.Database.UpsertWorkspaceAppAuditSession(dbauthz.AsSystemRestricted(writeCtx), database.UpsertWorkspaceAppAuditSessionParams{ - // Config. + now := dbtime.Now() + // nolint:gocritic // System context is needed to write connection-log dedupe sessions. + newSession, err := api.Database.UpsertWorkspaceAppAuditSession(dbauthz.AsSystemRestricted(ctx), database.UpsertWorkspaceAppAuditSessionParams{ StaleIntervalMS: staleInterval.Milliseconds(), - - // Data. - ID: uuid.New(), - AgentID: waws.WorkspaceAgent.ID, - AppID: uuid.Nil, // Tunnels are not associated with an app. - UserID: apiKey.UserID, - Ip: r.RemoteAddr, - UserAgent: userAgent, - SlugOrPort: "", - StatusCode: http.StatusSwitchingProtocols, - StartedAt: now, - UpdatedAt: now, + ID: uuid.New(), + AgentID: agentID, + AppID: uuid.Nil, + UserID: userID, + Ip: ip, + UserAgent: userAgent, + SlugOrPort: "", + StatusCode: statusCode, + StartedAt: now, + UpdatedAt: now, }) if err != nil { - // Skip logging rather than risk spamming the connection log. api.Logger.Error(ctx, "upsert tunnel audit session", slog.F("workspace_id", waws.WorkspaceTable.ID), - slog.F("user_id", apiKey.UserID), + slog.F("agent_id", waws.WorkspaceAgent.ID), + slog.F("user_id", userID), + slog.F("user_agent", userAgent), slog.Error(err), ) return } if !newSession { - // Reconnection of an already-logged session. return } - connLogger := *api.ConnectionLogger.Load() - err = connLogger.Upsert(writeCtx, database.UpsertConnectionLogParams{ + logger := api.ConnectionLogger.Load() + if logger == nil { + return + } + err = (*logger).Upsert(ctx, database.UpsertConnectionLogParams{ ID: uuid.New(), Time: now, OrganizationID: waws.WorkspaceTable.OrganizationID, @@ -1456,25 +1502,21 @@ func (api *API) logTunnelConnection(ctx context.Context, r *http.Request, waws d WorkspaceName: waws.WorkspaceTable.Name, AgentName: waws.WorkspaceAgent.Name, Type: database.ConnectionTypeTunnel, - IP: database.ParseIP(r.RemoteAddr), - Code: sql.NullInt32{ - Int32: http.StatusSwitchingProtocols, - Valid: true, - }, - UserAgent: sql.NullString{String: userAgent, Valid: userAgent != ""}, - UserID: uuid.NullUUID{UUID: apiKey.UserID, Valid: true}, - // Left unset so each session gets its own row; reusing peerID - // would make resume_token reconnects upsert into a stale row. - ConnectionID: uuid.NullUUID{}, - ConnectionStatus: database.ConnectionStatusConnected, - // N/A + IP: database.ParseIP(ip), + Code: sql.NullInt32{Int32: statusCode, Valid: true}, + UserAgent: sql.NullString{String: userAgent, Valid: userAgent != ""}, + UserID: uuid.NullUUID{UUID: userID, Valid: userID != uuid.Nil}, SlugOrPort: sql.NullString{}, + ConnectionID: uuid.NullUUID{}, DisconnectReason: sql.NullString{}, + ConnectionStatus: database.ConnectionStatusConnected, }) if err != nil { api.Logger.Error(ctx, "upsert tunnel connection log", slog.F("workspace_id", waws.WorkspaceTable.ID), - slog.F("user_id", apiKey.UserID), + slog.F("agent_id", waws.WorkspaceAgent.ID), + slog.F("user_id", userID), + slog.F("user_agent", userAgent), slog.Error(err), ) } @@ -2542,15 +2584,17 @@ func (api *API) tailnetRPCConn(rw http.ResponseWriter, r *http.Request) { ctx, cancel := context.WithCancel(ctx) defer cancel() ctx = api.wsWatcher.Watch(ctx, api.Logger, conn) + auth := tailnet.ClientUserCoordinateeAuth{ + Auth: &rbacAuthorizer{ + sshPrep: sshPrep, + db: api.Database, + }, + Auditor: api.tunnelAuditor(r), + } err = api.TailnetClientService.ServeClient(ctx, version, wsNetConn, tailnet.StreamID{ Name: "client", ID: peerID, - Auth: tailnet.ClientUserCoordinateeAuth{ - Auth: &rbacAuthorizer{ - sshPrep: sshPrep, - db: api.Database, - }, - }, + Auth: auth, }) if err != nil && !xerrors.Is(err, io.EOF) && !xerrors.Is(err, context.Canceled) { _ = conn.Close(websocket.StatusInternalError, err.Error()) diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go index 53792cf78a..c16371fb8a 100644 --- a/coderd/workspaceagents_test.go +++ b/coderd/workspaceagents_test.go @@ -967,28 +967,6 @@ func TestWorkspaceAgentClientCoordinate_ConnectionLog(t *testing.T) { }, testutil.WaitShort, testutil.IntervalFast) err = conn.Close() require.NoError(t, err) - - // A second handshake within the audit session stale interval is a - // reconnection and must be deduplicated rather than producing a - // second row. - conn2, err := workspacesdk.New(client). - DialAgent(ctx, resources[0].Agents[0].ID, &workspacesdk.DialAgentOptions{ - Logger: testutil.Logger(t).Named("client2"), - }) - require.NoError(t, err) - defer conn2.Close() - // The connection log write happens in the coordinate handler - // before any coordination traffic is served, so once the tunnel is - // reachable the second handshake has already been processed. - require.True(t, conn2.AwaitReachable(ctx)) - - tunnelRows := 0 - for _, cl := range connLogger.ConnectionLogs() { - if cl.Type == database.ConnectionTypeTunnel { - tunnelRows++ - } - } - require.Equal(t, 1, tunnelRows) } func TestWorkspaceAgentClientCoordinate_BadVersion(t *testing.T) { @@ -3104,6 +3082,87 @@ func TestOwnedWorkspacesCoordinate(t *testing.T) { }) } +func TestUserTailnetConnectionLog(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + connLogger := connectionlog.NewFake() + firstClient, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{ + ConnectionLogger: connLogger, + Coordinator: tailnet.NewCoordinator(logger), + Logger: &logger, + }) + firstUser := coderdtest.CreateFirstUser(t, firstClient) + member, memberUser := coderdtest.CreateAnotherUser(t, firstClient, firstUser.OrganizationID) + + allowedWorkspace := buildWorkspaceWithAgent(t, member, firstUser.OrganizationID, memberUser.ID, api.Database, api.Pubsub) + allowedSDKWorkspace, err := member.Workspace(ctx, allowedWorkspace.ID) + require.NoError(t, err) + allowedAgentID := allowedSDKWorkspace.LatestBuild.Resources[0].Agents[0].ID + + deniedWorkspace := buildWorkspaceWithAgent(t, firstClient, firstUser.OrganizationID, firstUser.UserID, api.Database, api.Pubsub) + deniedSDKWorkspace, err := firstClient.Workspace(ctx, deniedWorkspace.ID) + require.NoError(t, err) + deniedAgentID := deniedSDKWorkspace.LatestBuild.Resources[0].Agents[0].ID + + dial := func() (*websocket.Conn, tailnetproto.DRPCTailnet_CoordinateClient) { + u, err := member.URL.Parse("/api/v2/tailnet?version=2.0") + require.NoError(t, err) + //nolint:bodyclose // websocket.Dial owns the HTTP response body on success. + wsConn, response, err := websocket.Dial(ctx, u.String(), &websocket.DialOptions{ + HTTPHeader: http.Header{ + "Coder-Session-Token": []string{member.SessionToken()}, + }, + }) + if err != nil && response != nil { + err = codersdk.ReadBodyAsError(response) + } + require.NoError(t, err) + rpcClient, err := tailnet.NewDRPCClient( + websocket.NetConn(ctx, wsConn, websocket.MessageBinary), + logger, + ) + require.NoError(t, err) + stream, err := rpcClient.Coordinate(ctx) + require.NoError(t, err) + return wsConn, stream + } + + acceptedConn, acceptedStream := dial() + require.NoError(t, acceptedStream.Send(&tailnetproto.CoordinateRequest{ + AddTunnel: &tailnetproto.CoordinateRequest_Tunnel{Id: tailnet.UUIDToByteSlice(allowedAgentID)}, + })) + require.Eventually(t, func() bool { + return connLogger.Contains(t, database.UpsertConnectionLogParams{ + WorkspaceID: allowedWorkspace.ID, + AgentName: allowedSDKWorkspace.LatestBuild.Resources[0].Agents[0].Name, + Type: database.ConnectionTypeTunnel, + Code: sql.NullInt32{Int32: http.StatusSwitchingProtocols, Valid: true}, + UserID: uuid.NullUUID{UUID: memberUser.ID, Valid: true}, + UserAgent: sql.NullString{}, + ConnectionStatus: database.ConnectionStatusConnected, + }) + }, testutil.WaitShort, testutil.IntervalFast) + require.NoError(t, acceptedConn.Close(websocket.StatusNormalClosure, "done")) + + deniedConn, deniedStream := dial() + require.NoError(t, deniedStream.Send(&tailnetproto.CoordinateRequest{ + AddTunnel: &tailnetproto.CoordinateRequest_Tunnel{Id: tailnet.UUIDToByteSlice(deniedAgentID)}, + })) + require.Eventually(t, func() bool { + return connLogger.Contains(t, database.UpsertConnectionLogParams{ + WorkspaceID: deniedWorkspace.ID, + AgentName: deniedSDKWorkspace.LatestBuild.Resources[0].Agents[0].Name, + Type: database.ConnectionTypeTunnel, + Code: sql.NullInt32{Int32: http.StatusForbidden, Valid: true}, + UserID: uuid.NullUUID{UUID: memberUser.ID, Valid: true}, + ConnectionStatus: database.ConnectionStatusConnected, + }) + }, testutil.WaitShort, testutil.IntervalFast) + require.NoError(t, deniedConn.Close(websocket.StatusNormalClosure, "done")) +} + func TestUserTailnetTelemetry(t *testing.T) { t.Parallel() diff --git a/codersdk/connectionlog.go b/codersdk/connectionlog.go index 72cf6e3d18..19b79ae24c 100644 --- a/codersdk/connectionlog.go +++ b/codersdk/connectionlog.go @@ -46,10 +46,8 @@ const ( ConnectionTypeReconnectingPTY ConnectionType = "reconnecting_pty" ConnectionTypeWorkspaceApp ConnectionType = "workspace_app" ConnectionTypePortForwarding ConnectionType = "port_forwarding" - // ConnectionTypeTunnel is recorded by coderd when a client - // establishes a tailnet tunnel to a workspace agent, and carries - // the authenticated user's identity. Tunnels via the user-scoped - // tailnet API (e.g. Coder Desktop) are not currently recorded. + // ConnectionTypeTunnel records accepted and denied tailnet tunnel + // requests made by authenticated users. ConnectionTypeTunnel ConnectionType = "tunnel" ) @@ -73,10 +71,10 @@ func (s ConnectionLogStatus) Valid() bool { type ConnectionLogWebInfo struct { UserAgent string `json:"user_agent"` - // User is omitted if the connection event was from an unauthenticated user. + // User is omitted if the connection event was unauthenticated. User *User `json:"user"` SlugOrPort string `json:"slug_or_port"` - // StatusCode is the HTTP status code of the request. + // StatusCode is the HTTP status code or tunnel authorization outcome. StatusCode int32 `json:"status_code"` } diff --git a/docs/admin/monitoring/connection-logs.md b/docs/admin/monitoring/connection-logs.md index 6f023cf747..01c4ede6b4 100644 --- a/docs/admin/monitoring/connection-logs.md +++ b/docs/admin/monitoring/connection-logs.md @@ -30,30 +30,20 @@ events for the same workspace and agent. ## Tunnel Connections -The connection log records a tunnel event each time a client -establishes a tunnel to a workspace agent, carrying the identity, IP -address, and user agent of the authenticated user who opened it. Tunnels -carry SSH and IDE traffic, so these events provide the user attribution -that agent-reported events lack. +The connection log records the authorization decision for each request to add a tunnel to a workspace agent. +Accepted requests have status code `101`, and denied requests have status code `403`. +Tunnel events include the authenticated user's identity, IP address, and user agent. Keep the following in mind when interpreting tunnel events: -- A tunnel event records that a tunnel was established, not what it was - used for. Any client that dials a workspace agent produces one, - including `coder ssh`, `coder port-forward`, `coder ping`, - `coder speedtest`, and IDE extensions. One tunnel may carry many - sessions, or none. -- Tunnel events are deduplicated per user, workspace agent, IP address, - and client. Clients automatically re-establish tunnels after network - interruptions or server restarts; reconnections do not produce new - events while a session is active. A new event is recorded when a - session has been idle for one hour, or when the user connects from a - new IP address or client. -- Connections made through Coder Desktop (Coder Connect) do not - currently produce tunnel events. -- Like workspace app connections, tunnel events are point-in-time - records: they have no close time and are excluded from `status:` - filter results. +- A tunnel event records an authorization decision, not how the tunnel was used. + Clients such as `coder ssh`, `coder port-forward`, `coder ping`, `coder speedtest`, Coder Desktop, and IDE extensions can request tunnels. +- Tunnel events are deduplicated per workspace agent, actor, IP address, client, and authorization result. + Clients automatically re-request tunnels after network interruptions or server restarts. + These requests do not produce new events while a session is active. + A new event is recorded after one hour of inactivity, or when the actor, IP address, client, or result changes. +- Like workspace app connections, tunnel events are point-in-time records. + They have no close time and are excluded from `status:` filter results. ## How to Filter Connection Logs @@ -67,9 +57,9 @@ You can filter connection logs by the following parameters: For more connection types, refer to the [CoderSDK documentation](https://pkg.go.dev/github.com/coder/coder/v2/codersdk#ConnectionType). - `username`: The name of the user who initiated the connection. - Results will not include agent-reported SSH or IDE sessions. + Results do not include agent-reported SSH or IDE sessions. - `user_email`: The email of the user who initiated the connection. - Results will not include agent-reported SSH or IDE sessions. + Results do not include agent-reported SSH or IDE sessions. - `connected_after`: The time after which the connection started. Uses the RFC3339Nano format. - `connected_before`: The time before which the connection started. diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 2c20028c82..6c5ad23480 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -4514,12 +4514,12 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in ### Properties -| Name | Type | Required | Restrictions | Description | -|----------------|--------------------------------|----------|--------------|---------------------------------------------------------------------------| -| `slug_or_port` | string | false | | | -| `status_code` | integer | false | | Status code is the HTTP status code of the request. | -| `user` | [codersdk.User](#codersdkuser) | false | | User is omitted if the connection event was from an unauthenticated user. | -| `user_agent` | string | false | | | +| Name | Type | Required | Restrictions | Description | +|----------------|--------------------------------|----------|--------------|----------------------------------------------------------------------| +| `slug_or_port` | string | false | | | +| `status_code` | integer | false | | Status code is the HTTP status code or tunnel authorization outcome. | +| `user` | [codersdk.User](#codersdkuser) | false | | User is omitted if the connection event was unauthenticated. | +| `user_agent` | string | false | | | ## codersdk.ConnectionType diff --git a/enterprise/coderd/connectionlog_test.go b/enterprise/coderd/connectionlog_test.go index 6f8f66599e..5b7b85d1ad 100644 --- a/enterprise/coderd/connectionlog_test.go +++ b/enterprise/coderd/connectionlog_test.go @@ -5,6 +5,7 @@ import ( "database/sql" "fmt" "net" + "net/http" "testing" "time" @@ -203,6 +204,7 @@ func TestConnectionLogs(t *testing.T) { OrganizationID: ws.OrganizationID, WorkspaceOwnerID: ws.OwnerID, UserAgent: sql.NullString{String: "coder-cli/2.0.0", Valid: true}, + Code: sql.NullInt32{Int32: http.StatusSwitchingProtocols, Valid: true}, UserID: uuid.NullUUID{UUID: ws.OwnerID, Valid: true}, }) @@ -216,6 +218,7 @@ func TestConnectionLogs(t *testing.T) { require.Equal(t, clog.UserAgent.String, logs.ConnectionLogs[0].WebInfo.UserAgent) require.NotNil(t, logs.ConnectionLogs[0].WebInfo.User) require.Equal(t, ws.OwnerID, logs.ConnectionLogs[0].WebInfo.User.ID) + require.EqualValues(t, http.StatusSwitchingProtocols, logs.ConnectionLogs[0].WebInfo.StatusCode) }) t.Run("SSHInfo", func(t *testing.T) { diff --git a/enterprise/tailnet/workspaceproxy.go b/enterprise/tailnet/workspaceproxy.go index c2510db0aa..df90bed4fa 100644 --- a/enterprise/tailnet/workspaceproxy.go +++ b/enterprise/tailnet/workspaceproxy.go @@ -33,11 +33,10 @@ func (s *ClientService) ServeMultiAgentClient(ctx context.Context, version strin } switch major { case 2: - auth := agpl.SingleTailnetCoordinateeAuth{} streamID := agpl.StreamID{ Name: id.String(), ID: id, - Auth: auth, + Auth: agpl.SingleTailnetCoordinateeAuth{}, } return s.ServeConnV2(ctx, conn, streamID) default: diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 74d8e009c6..b9c456118b 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -3727,12 +3727,12 @@ export const ConnectionLogStatuses: ConnectionLogStatus[] = [ export interface ConnectionLogWebInfo { readonly user_agent: string; /** - * User is omitted if the connection event was from an unauthenticated user. + * User is omitted if the connection event was unauthenticated. */ readonly user: User | null; readonly slug_or_port: string; /** - * StatusCode is the HTTP status code of the request. + * StatusCode is the HTTP status code or tunnel authorization outcome. */ readonly status_code: number; } diff --git a/site/src/pages/ConnectionLogPage/ConnectionLogRow/ConnectionLogDescription/ConnectionLogDescription.stories.tsx b/site/src/pages/ConnectionLogPage/ConnectionLogRow/ConnectionLogDescription/ConnectionLogDescription.stories.tsx index 4df6d91476..a3d9f20021 100644 --- a/site/src/pages/ConnectionLogPage/ConnectionLogRow/ConnectionLogDescription/ConnectionLogDescription.stories.tsx +++ b/site/src/pages/ConnectionLogPage/ConnectionLogRow/ConnectionLogDescription/ConnectionLogDescription.stories.tsx @@ -1,6 +1,9 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; +import { expect, within } from "storybook/test"; import { MockConnectedSSHConnectionLog, + MockDeniedTunnelConnectionLog, + MockTunnelConnectionLog, MockWebConnectionLog, } from "#/testHelpers/entities"; import { ConnectionLogDescription } from "./ConnectionLogDescription"; @@ -97,10 +100,11 @@ export const JetBrains: Story = { export const Tunnel: Story = { args: { - connectionLog: { - ...MockWebConnectionLog, - type: "tunnel", - }, + connectionLog: MockTunnelConnectionLog, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect(canvas.getByText(/established a tunnel to/)).toBeVisible(); }, }; @@ -109,13 +113,22 @@ export const Tunnel: Story = { export const TunnelOtherUser: Story = { args: { connectionLog: { - ...MockWebConnectionLog, - type: "tunnel", + ...MockTunnelConnectionLog, workspace_owner_username: "some-other-user", }, }, }; +export const TunnelDenied: Story = { + args: { + connectionLog: MockDeniedTunnelConnectionLog, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect(canvas.getByText(/was denied a tunnel to/)).toBeVisible(); + }, +}; + export const WebTerminal: Story = { args: { connectionLog: { diff --git a/site/src/pages/ConnectionLogPage/ConnectionLogRow/ConnectionLogDescription/ConnectionLogDescription.tsx b/site/src/pages/ConnectionLogPage/ConnectionLogRow/ConnectionLogDescription/ConnectionLogDescription.tsx index 8676b0f9d7..d759b18a30 100644 --- a/site/src/pages/ConnectionLogPage/ConnectionLogRow/ConnectionLogDescription/ConnectionLogDescription.tsx +++ b/site/src/pages/ConnectionLogPage/ConnectionLogRow/ConnectionLogDescription/ConnectionLogDescription.tsx @@ -92,13 +92,16 @@ export const ConnectionLogDescription: FC = ({ case "tunnel": { if (!web_info) return null; - const { user } = web_info; + const { user, status_code } = web_info; + const actor = user?.username ?? "Unknown user"; + const action = + status_code >= 400 + ? "was denied a tunnel to" + : "established a tunnel to"; const isOwnWorkspace = workspace_owner_username === user?.username; return ( - {/* Tunnel rows are only written for authenticated requests, - so user should always be present. */} - {user?.username ?? "Unknown user"} established a tunnel to{" "} + {actor} {action}{" "} {isOwnWorkspace ? "their" : `${workspace_owner_username}'s`}{" "} diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 65a1ff3bbe..bdcd17f4dc 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -3068,6 +3068,28 @@ export const MockWebConnectionLog: TypesGen.ConnectionLog = { }, }; +const MockTunnelWebInfo: TypesGen.ConnectionLogWebInfo = { + user_agent: "coder-cli/2.0.0", + user: MockUserMember, + slug_or_port: "", + status_code: 101, +}; + +export const MockTunnelConnectionLog: TypesGen.ConnectionLog = { + ...MockWebConnectionLog, + type: "tunnel", + web_info: MockTunnelWebInfo, +}; + +export const MockDeniedTunnelConnectionLog: TypesGen.ConnectionLog = { + ...MockTunnelConnectionLog, + id: "09747acf-207f-4f53-a875-fde339924f60", + web_info: { + ...MockTunnelWebInfo, + status_code: 403, + }, +}; + export const MockConnectedSSHConnectionLog: TypesGen.ConnectionLog = { id: "7884a866-4ae1-4945-9fba-b2b8d2b7c5a9", connect_time: "2022-05-19T16:45:57.122Z", diff --git a/tailnet/controllers_test.go b/tailnet/controllers_test.go index 899ffac7eb..f585d82e40 100644 --- a/tailnet/controllers_test.go +++ b/tailnet/controllers_test.go @@ -76,7 +76,7 @@ func TestTunnelSrcCoordController_Mainline(t *testing.T) { reqs := make(chan *proto.CoordinateRequest, 100) resps := make(chan *proto.CoordinateResponse, 100) - mCoord.EXPECT().Coordinate(gomock.Any(), clientID, gomock.Any(), tailnet.ClientCoordinateeAuth{agentID}). + mCoord.EXPECT().Coordinate(gomock.Any(), clientID, gomock.Any(), tailnet.ClientCoordinateeAuth{AgentID: agentID}). Times(1).Return(reqs, resps) var coord tailnet.Coordinator = mCoord @@ -415,7 +415,7 @@ func TestAgentCoordinationController_SendsReadyForHandshake(t *testing.T) { reqs := make(chan *proto.CoordinateRequest, 100) resps := make(chan *proto.CoordinateResponse, 100) - mCoord.EXPECT().Coordinate(gomock.Any(), clientID, gomock.Any(), tailnet.ClientCoordinateeAuth{agentID}). + mCoord.EXPECT().Coordinate(gomock.Any(), clientID, gomock.Any(), tailnet.ClientCoordinateeAuth{AgentID: agentID}). Times(1).Return(reqs, resps) var coord tailnet.Coordinator = mCoord diff --git a/tailnet/service.go b/tailnet/service.go index 6ae02876a4..0515ece954 100644 --- a/tailnet/service.go +++ b/tailnet/service.go @@ -204,7 +204,7 @@ func (s *DRPCService) Coordinate(stream proto.DRPCTailnet_CoordinateStream) erro _ = stream.Close() return xerrors.New("no Stream ID") } - logger := s.Logger.With(slog.F("peer_id", streamID), slog.F("name", streamID.Name)) + logger := s.Logger.With(slog.F("peer_id", streamID.ID.String()), slog.F("name", streamID.Name)) logger.Debug(ctx, "starting tailnet Coordinate") coord := *(s.CoordPtr.Load()) reqs, resps := coord.Coordinate(ctx, streamID.ID, streamID.Name, streamID.Auth) diff --git a/tailnet/service_test.go b/tailnet/service_test.go index 0c268b05ed..a34f7b6581 100644 --- a/tailnet/service_test.go +++ b/tailnet/service_test.go @@ -238,7 +238,8 @@ func TestClientUserCoordinateeAuth(t *testing.T) { ctrl := gomock.NewController(t) updatesProvider := tailnettest.NewMockWorkspaceUpdatesProvider(ctrl) - fCoord, client := createUpdateService(t, ctx, clientID, updatesProvider) + auditor := &recordingTunnelAuditor{} + fCoord, client := createUpdateService(t, ctx, clientID, updatesProvider, auditor) // Coordinate stream, err := client.Coordinate(ctx) @@ -261,9 +262,62 @@ func TestClientUserCoordinateeAuth(t *testing.T) { require.NoError(t, call.Auth.Authorize(ctx, &proto.CoordinateRequest{ AddTunnel: &proto.CoordinateRequest_Tunnel{Id: tailnet.UUIDToByteSlice(agentID)}, })) - require.Error(t, call.Auth.Authorize(ctx, &proto.CoordinateRequest{ + require.Len(t, auditor.decisions, 1) + require.Equal(t, agentID, auditor.decisions[0].agentID) + require.NoError(t, auditor.decisions[0].authorizationErr) + err = call.Auth.Authorize(ctx, &proto.CoordinateRequest{ AddTunnel: &proto.CoordinateRequest_Tunnel{Id: tailnet.UUIDToByteSlice(agentID2)}, - })) + }) + require.EqualError(t, err, "workspace agent not found or you do not have permission") + require.Len(t, auditor.decisions, 2) + require.Equal(t, agentID2, auditor.decisions[1].agentID) + require.Error(t, auditor.decisions[1].authorizationErr) + + err = call.Auth.Authorize(ctx, &proto.CoordinateRequest{ + AddTunnel: &proto.CoordinateRequest_Tunnel{Id: tailnet.UUIDToByteSlice(agentID)}, + UpdateSelf: &proto.CoordinateRequest_UpdateSelf{ + Node: &proto.Node{Addresses: []string{"not-an-address"}}, + }, + }) + require.ErrorContains(t, err, "parse node address") + require.Len(t, auditor.decisions, 3) + require.Equal(t, agentID, auditor.decisions[2].agentID) + require.NoError(t, auditor.decisions[2].authorizationErr) + + err = call.Auth.Authorize(ctx, &proto.CoordinateRequest{ + AddTunnel: &proto.CoordinateRequest_Tunnel{Id: []byte("invalid")}, + }) + require.ErrorContains(t, err, "parse add tunnel id") + require.Len(t, auditor.decisions, 3) +} + +func TestClientCoordinateeAuthTunnelAuditor(t *testing.T) { + t.Parallel() + + agentID := uuid.New() + otherAgentID := uuid.New() + auditor := &recordingTunnelAuditor{} + auth := tailnet.ClientCoordinateeAuth{ + AgentID: agentID, + Auditor: auditor, + } + + err := auth.Authorize(t.Context(), &proto.CoordinateRequest{ + AddTunnel: &proto.CoordinateRequest_Tunnel{Id: tailnet.UUIDToByteSlice(agentID)}, + ReadyForHandshake: []*proto.CoordinateRequest_ReadyForHandshake{{}}, + }) + require.ErrorContains(t, err, "clients may not send ready_for_handshake") + require.Len(t, auditor.decisions, 1) + require.Equal(t, agentID, auditor.decisions[0].agentID) + require.NoError(t, auditor.decisions[0].authorizationErr) + + err = auth.Authorize(t.Context(), &proto.CoordinateRequest{ + AddTunnel: &proto.CoordinateRequest_Tunnel{Id: tailnet.UUIDToByteSlice(otherAgentID)}, + }) + require.ErrorContains(t, err, "invalid agent id") + require.Len(t, auditor.decisions, 2) + require.Equal(t, otherAgentID, auditor.decisions[1].agentID) + require.Error(t, auditor.decisions[1].authorizationErr) } func TestWorkspaceUpdates(t *testing.T) { @@ -278,7 +332,7 @@ func TestWorkspaceUpdates(t *testing.T) { clientID := uuid.UUID{0x03} wsID := uuid.UUID{0x04} - _, client := createUpdateService(t, ctx, clientID, updatesProvider) + _, client := createUpdateService(t, ctx, clientID, updatesProvider, nil) // Workspace updates expected := &proto.WorkspaceUpdate{ @@ -315,7 +369,7 @@ func TestWorkspaceUpdates(t *testing.T) { } //nolint:revive // t takes precedence -func createUpdateService(t *testing.T, ctx context.Context, clientID uuid.UUID, updates tailnet.WorkspaceUpdatesProvider) (*tailnettest.FakeCoordinator, proto.DRPCTailnetClient) { +func createUpdateService(t *testing.T, ctx context.Context, clientID uuid.UUID, updates tailnet.WorkspaceUpdatesProvider, auditor tailnet.TunnelAuditor) (*tailnettest.FakeCoordinator, proto.DRPCTailnetClient) { fCoord := tailnettest.NewFakeCoordinator() var coord tailnet.Coordinator = fCoord coordPtr := atomic.Pointer[tailnet.Coordinator]{} @@ -341,7 +395,8 @@ func createUpdateService(t *testing.T, ctx context.Context, clientID uuid.UUID, Name: "client", ID: clientID, Auth: tailnet.ClientUserCoordinateeAuth{ - Auth: &fakeTunnelAuth{}, + Auth: &fakeTunnelAuth{}, + Auditor: auditor, }, }) t.Logf("ServeClient returned; err=%v", err) @@ -360,6 +415,24 @@ func createUpdateService(t *testing.T, ctx context.Context, clientID uuid.UUID, return fCoord, client } +type tunnelAuditDecision struct { + agentID uuid.UUID + authorizationErr error +} + +type recordingTunnelAuditor struct { + decisions []tunnelAuditDecision +} + +func (a *recordingTunnelAuditor) Audit(agentID uuid.UUID, authorizationErr error) { + a.decisions = append(a.decisions, tunnelAuditDecision{ + agentID: agentID, + authorizationErr: authorizationErr, + }) +} + +var _ tailnet.TunnelAuditor = (*recordingTunnelAuditor)(nil) + type fakeTunnelAuth struct{} // AuthorizeTunnel implements tailnet.TunnelAuthorizer. diff --git a/tailnet/tunnel.go b/tailnet/tunnel.go index c575d0b986..23720d7d5c 100644 --- a/tailnet/tunnel.go +++ b/tailnet/tunnel.go @@ -33,40 +33,59 @@ type CoordinateeAuth interface { Authorize(ctx context.Context, req *proto.CoordinateRequest) error } -// SingleTailnetCoordinateeAuth allows all tunnels, since Coderd and wsproxy are allowed to initiate a tunnel to any agent +// TunnelAuditor records AddTunnel authorization decisions. Audit must not +// block because authorization may run while the coordinator mutex is held. +type TunnelAuditor interface { + Audit(agentID uuid.UUID, authorizationErr error) +} + +// SingleTailnetCoordinateeAuth allows all tunnels because coderd and workspace +// proxies may initiate a tunnel to any agent. type SingleTailnetCoordinateeAuth struct{} func (SingleTailnetCoordinateeAuth) Authorize(context.Context, *proto.CoordinateRequest) error { return nil } -// ClientCoordinateeAuth allows connecting to a single, given agent +// ClientCoordinateeAuth allows connecting to a single agent. type ClientCoordinateeAuth struct { AgentID uuid.UUID + Auditor TunnelAuditor } func (c ClientCoordinateeAuth) Authorize(_ context.Context, req *proto.CoordinateRequest) error { - if tun := req.GetAddTunnel(); tun != nil { - uid, err := uuid.FromBytes(tun.Id) + var agentID uuid.UUID + authErr, report := func() (error, bool) { + tun := req.GetAddTunnel() + if tun == nil { + return nil, false + } + var err error + agentID, err = uuid.FromBytes(tun.Id) if err != nil { - return xerrors.Errorf("parse add tunnel id: %w", err) + return xerrors.Errorf("parse add tunnel id: %w", err), false } - - if c.AgentID != uid { - return xerrors.Errorf("invalid agent id, expected %s, got %s", c.AgentID.String(), uid.String()) + if c.AgentID != agentID { + return xerrors.Errorf("invalid agent id, expected %s, got %s", c.AgentID.String(), agentID.String()), true } + return nil, true + }() + if report && c.Auditor != nil { + c.Auditor.Audit(agentID, authErr) + } + if authErr != nil { + return authErr } - return handleClientNodeRequests(req) } -// AgentCoordinateeAuth disallows all tunnels, since agents are not allowed to initiate their own tunnels +// AgentCoordinateeAuth disallows tunnels because agents may not initiate them. type AgentCoordinateeAuth struct { ID uuid.UUID } func (a AgentCoordinateeAuth) Authorize(_ context.Context, req *proto.CoordinateRequest) error { - if tun := req.GetAddTunnel(); tun != nil { + if req.GetAddTunnel() != nil { return xerrors.New("agents cannot open tunnels") } @@ -110,21 +129,33 @@ func (a AgentCoordinateeAuth) authorizeNodePrefixes(prefixes []string) error { } type ClientUserCoordinateeAuth struct { - Auth TunnelAuthorizer + Auth TunnelAuthorizer + Auditor TunnelAuditor } func (a ClientUserCoordinateeAuth) Authorize(ctx context.Context, req *proto.CoordinateRequest) error { - if tun := req.GetAddTunnel(); tun != nil { - uid, err := uuid.FromBytes(tun.Id) - if err != nil { - return xerrors.Errorf("parse add tunnel id: %w", err) + var agentID uuid.UUID + authErr, report := func() (error, bool) { + tun := req.GetAddTunnel() + if tun == nil { + return nil, false } - err = a.Auth.AuthorizeTunnel(ctx, uid) + var err error + agentID, err = uuid.FromBytes(tun.Id) if err != nil { - return xerrors.Errorf("workspace agent not found or you do not have permission") + return xerrors.Errorf("parse add tunnel id: %w", err), false } + if err := a.Auth.AuthorizeTunnel(ctx, agentID); err != nil { + return xerrors.New("workspace agent not found or you do not have permission"), true + } + return nil, true + }() + if report && a.Auditor != nil { + a.Auditor.Audit(agentID, authErr) + } + if authErr != nil { + return authErr } - return handleClientNodeRequests(req) }