From dae68d772b7f13212a42ac9bfd11f433c9eeb87b Mon Sep 17 00:00:00 2001 From: Aditya Sharma <51014728+aditya2548@users.noreply.github.com> Date: Thu, 17 Apr 2025 18:16:25 +0000 Subject: [PATCH] Add parsing logic for User-Agent headers (#5645) * feat(ts_telemetry): add parsing logic for user agent headers * feat(ts_telemetry): update gRPC observability for user-agent headers * feat(ts_telemetry): add observation tags * feat(ts_telemetry): Resolved comments * feat(ts_telemetry): Removed loop variable capture * feat(ts_telemetry): Updated userAgentKey to be inline * feat(ts_telemetry): Add support for userAgent as per RFC 9110 * feat(ts_telemetry): Resolved comments * Fix newline * feat(ts_telemetry): Moved user-agent parsing to interceptor * feat(ts_telemetry): Added user-agent parsing complex test case * Update userAgentsKey comment Co-authored-by: Johan Brandhorst-Satzkorn * feat(ts_telemetry): Updated changelog * feat(ts_telemetry): Resolved comments --------- Co-authored-by: Johan Brandhorst-Satzkorn --- internal/daemon/controller/gateway.go | 11 ++ internal/daemon/controller/gateway_test.go | 22 ++++ internal/daemon/controller/interceptor.go | 70 +++++++++++- .../daemon/controller/interceptor_test.go | 108 ++++++++++++++++++ internal/event/event.go | 8 ++ internal/event/event_observation.go | 3 + internal/event/event_observation_test.go | 44 +++++++ .../api/services/authtokens_service.pb.go | 2 +- .../servers/services/session_service.pb.go | 2 +- .../resources/authtokens/v1/authtoken.proto | 2 +- .../api/services/v1/authtokens_service.proto | 2 +- .../servers/services/v1/session_service.proto | 2 +- .../api/resources/authtokens/authtoken.pb.go | 2 +- 13 files changed, 271 insertions(+), 7 deletions(-) diff --git a/internal/daemon/controller/gateway.go b/internal/daemon/controller/gateway.go index 96b1e758f8..eaf2ec3b63 100644 --- a/internal/daemon/controller/gateway.go +++ b/internal/daemon/controller/gateway.go @@ -66,6 +66,7 @@ func (noDelimiterStreamingMarshaler) Delimiter() []byte { func newGrpcGatewayMux() *runtime.ServeMux { return runtime.NewServeMux( runtime.WithMetadata(correlationIdAnnotator), + runtime.WithMetadata(userAgentHeadersAnnotator), runtime.WithMarshalerOption(runtime.MIMEWildcard, &noDelimiterStreamingMarshaler{ &runtime.HTTPBodyMarshaler{ Marshaler: handlers.JSONMarshaler(), @@ -100,6 +101,16 @@ func correlationIdAnnotator(_ context.Context, req *http.Request) metadata.MD { }) } +func userAgentHeadersAnnotator(_ context.Context, req *http.Request) metadata.MD { + userAgent := req.Header.Get("User-Agent") + if userAgent == "" { + return metadata.MD{} + } + return metadata.New(map[string]string{ + userAgentsKey: userAgent, + }) +} + // newGrpcServerListener will create an in-memory listener for the gRPC server. func newGrpcServerListener() grpcServerListener { buffer := globals.DefaultMaxRequestSize // seems like a reasonable size for the ring buffer, but then happily change the size if more info becomes available diff --git a/internal/daemon/controller/gateway_test.go b/internal/daemon/controller/gateway_test.go index 0566a4d82d..c5293151f5 100644 --- a/internal/daemon/controller/gateway_test.go +++ b/internal/daemon/controller/gateway_test.go @@ -95,6 +95,28 @@ func Test_correlationIdAnnotator(t *testing.T) { assert.Equal(t, corId, corIds[0]) } +func Test_clientAgentHeadersAnnotator(t *testing.T) { + t.Parallel() + t.Run("returns metadata with user-agent", func(t *testing.T) { + t.Parallel() + req := &http.Request{ + Header: map[string][]string{ + "User-Agent": {"Boundary-client-agent/0.1.4"}, + }, + } + md := userAgentHeadersAnnotator(context.Background(), req) + require.NotNil(t, md) + assert.Equal(t, []string{"Boundary-client-agent/0.1.4"}, md.Get("userAgents")) + }) + + t.Run("returns empty metadata if no user-agent header", func(t *testing.T) { + t.Parallel() + req := &http.Request{Header: map[string][]string{}} + md := userAgentHeadersAnnotator(context.Background(), req) + assert.Empty(t, md) + }) +} + func Test_WithDisablePathLengthFallback(t *testing.T) { ctx := context.Background() reqPath := "/v1/example" diff --git a/internal/daemon/controller/interceptor.go b/internal/daemon/controller/interceptor.go index 5a62ab1290..25e0dd8c47 100644 --- a/internal/daemon/controller/interceptor.go +++ b/internal/daemon/controller/interceptor.go @@ -9,7 +9,9 @@ import ( "fmt" "net/http" "reflect" + "regexp" "runtime/debug" + "strings" "time" grpc_recovery "github.com/grpc-ecosystem/go-grpc-middleware/recovery" @@ -27,6 +29,7 @@ import ( "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/requests" "github.com/hashicorp/go-uuid" + "github.com/hashicorp/go-version" "github.com/mr-tron/base58" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -45,8 +48,16 @@ const ( // apiErrHeader defines an http header for encoded api errors from the // grpc server. apiErrHeader = "x-api-err" + + // boundaryClientAgentProduct defines the product name used to identify the + // Boundary client agent in user-agent parsing and validation logic. + boundaryClientAgentProduct = "Boundary-client-agent" ) +// Regular expression to parse user-agent product, version, and comments +// Follows the structure defined in RFC 9110: https://datatracker.ietf.org/doc/html/rfc9110#name-user-agent +var userAgentRegex = regexp.MustCompile(`(?P[^\s/()]+)/(?P[^\s()]+)(?: \((?P[^)]+)\))?`) + // customContextServerStream wraps the grpc.ServerStream interface and lets us // set a custom context type customContextServerStream struct { @@ -448,14 +459,24 @@ func eventsRequestInterceptor( _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error, ) { + var userAgents []*event.UserAgent + if md, ok := metadata.FromIncomingContext(interceptorCtx); ok { + if values := md.Get(userAgentsKey); len(values) > 0 { + userAgents = parseUserAgents(values[0]) + } + } if msg, ok := req.(proto.Message); ok { // Clone the request before writing it to the audit log, // in case downstream interceptors modify it. clonedMsg := proto.Clone(msg) + request := &event.Request{ + Details: clonedMsg, + UserAgents: userAgents, + } if err := event.WriteAudit(interceptorCtx, op, event.WithRequest(&event.Request{Details: clonedMsg})); err != nil { return req, status.Errorf(codes.Internal, "unable to write request msg audit: %s", err) } - if err := event.WriteObservation(interceptorCtx, op, event.WithRequest(&event.Request{Details: clonedMsg})); err != nil { + if err := event.WriteObservation(interceptorCtx, op, event.WithRequest(request)); err != nil { return req, status.Errorf(codes.Internal, "unable to write request msg observation: %s", err) } } @@ -464,6 +485,53 @@ func eventsRequestInterceptor( } } +// parseUserAgents extracts structured UserAgent data from a raw User-Agent header string. +// Version validation is applied only to Boundary-client-agent entries, which are excluded +// if the version starts with 'v' or is not a valid semantic version. +// Comments are split and normalized into a slice of strings. +func parseUserAgents(rawUserAgent string) []*event.UserAgent { + var userAgents []*event.UserAgent + matches := userAgentRegex.FindAllStringSubmatch(rawUserAgent, -1) + + for _, match := range matches { + product := strings.TrimSpace(match[1]) + agentVersion := strings.TrimSpace(match[2]) + + // Only apply version validation for Boundary-client-agent + if product == boundaryClientAgentProduct { + if strings.HasPrefix(agentVersion, "v") { + // Invalid version format (starting with 'v') + continue + } + if _, err := version.NewSemver(agentVersion); err != nil { + // Invalid version + continue + } + } + + agentData := &event.UserAgent{ + Product: product, + ProductVersion: agentVersion, + } + + if len(match) > 3 && match[3] != "" { + // Clean up and split comments + commentsRaw := strings.Split(match[3], ";") + var comments []string + for _, c := range commentsRaw { + if trimmed := strings.TrimSpace(c); trimmed != "" { + comments = append(comments, trimmed) + } + } + if len(comments) > 0 { + agentData.Comments = comments + } + } + userAgents = append(userAgents, agentData) + } + return userAgents +} + func eventsResponseInterceptor( _ context.Context, ) grpc.UnaryServerInterceptor { diff --git a/internal/daemon/controller/interceptor_test.go b/internal/daemon/controller/interceptor_test.go index 949e8f42bc..fe8544c80a 100644 --- a/internal/daemon/controller/interceptor_test.go +++ b/internal/daemon/controller/interceptor_test.go @@ -879,6 +879,114 @@ func Test_statusCodeInterceptor(t *testing.T) { } } +func Test_parseUserAgents(t *testing.T) { + t.Parallel() + tests := []struct { + name string + rawUserAgent string + expected []*event.UserAgent + }{ + { + name: "valid single user-agent", + rawUserAgent: "Boundary-client-agent/0.1.4", + expected: []*event.UserAgent{ + { + Product: "Boundary-client-agent", + ProductVersion: "0.1.4", + }, + }, + }, + { + name: "multiple valid agents with comments", + rawUserAgent: "Boundary-client-agent/0.1.4 (foo; bar); AnotherApp/2.0.0 (baz )", + expected: []*event.UserAgent{ + { + Product: "Boundary-client-agent", + ProductVersion: "0.1.4", + Comments: []string{"foo", "bar"}, + }, + { + Product: "AnotherApp", + ProductVersion: "2.0.0", + Comments: []string{"baz"}, + }, + }, + }, + { + name: "complex but valid user agents", + rawUserAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.88 Safari/537.36 surveyon/2.9.5 (iPhone; CPU iPhone OS 12_5_7 like Mac OS X)", + expected: []*event.UserAgent{ + { + Product: "Mozilla", + ProductVersion: "5.0", + Comments: []string{"Macintosh", "Intel Mac OS X 10_15_7"}, + }, + { + Product: "AppleWebKit", + ProductVersion: "537.36", + Comments: []string{"KHTML, like Gecko"}, + }, + { + Product: "Chrome", + ProductVersion: "87.0.4280.88", + }, + { + Product: "Safari", + ProductVersion: "537.36", + }, + { + Product: "surveyon", + ProductVersion: "2.9.5", + Comments: []string{"iPhone", "CPU iPhone OS 12_5_7 like Mac OS X"}, + }, + }, + }, + { + name: "invalid client-agent version format (starts with 'v')", + rawUserAgent: "Boundary-client-agent/v0.1.4", + expected: nil, + }, + { + name: "invalid client-agent version format (non-semver)", + rawUserAgent: "Boundary-client-agent/0.1.x", + expected: nil, + }, + { + name: "empty user-agent", + rawUserAgent: "", + expected: nil, + }, + { + name: "valid non client-agent user-agent", + rawUserAgent: "SomeOtherApp/v1.2.3", + expected: []*event.UserAgent{ + { + Product: "SomeOtherApp", + ProductVersion: "v1.2.3", + }, + }, + }, + { + name: "mixed valid and invalid agents", + rawUserAgent: "Boundary-client-agent/0.1.4 NoVersionApp SomeOtherApp/", + expected: []*event.UserAgent{ + { + Product: "Boundary-client-agent", + ProductVersion: "0.1.4", + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + result := parseUserAgents(tt.rawUserAgent) + assert.ElementsMatch(t, tt.expected, result) + }) + } +} + func Test_workerRequestInfoInterceptor(t *testing.T) { factoryCtx := context.Background() requestCtx := context.Background() diff --git a/internal/event/event.go b/internal/event/event.go index 0e517e7ded..e1401c8cc5 100644 --- a/internal/event/event.go +++ b/internal/event/event.go @@ -52,6 +52,7 @@ type Request struct { Endpoint string `json:"endpoint,omitempty" class:"public"` // std audit field Details proto.Message `json:"details,omitempty"` // boundary field DetailsUpstreamMessage *UpstreamMessage `json:"details_upstream_message,omitempty"` // boundary field + UserAgents []*UserAgent `json:"user_agents,omitempty"` // boundary field } type Response struct { @@ -64,3 +65,10 @@ type UpstreamMessage struct { Type string `json:"type,omitempty" class:"public"` // boundary field Message proto.Message `json:"message,omitempty"` // boundary field } + +// UserAgent defines the fields parsed from a request's User-Agent header. +type UserAgent struct { + Product string `json:"product,omitempty"` // product identifier + ProductVersion string `json:"product_version,omitempty"` // version number of the product + Comments []string `json:"comments,omitempty"` // comments about the product +} diff --git a/internal/event/event_observation.go b/internal/event/event_observation.go index 6e8dcd0b62..c8aae810a2 100644 --- a/internal/event/event_observation.go +++ b/internal/event/event_observation.go @@ -137,6 +137,9 @@ func (o *observation) ComposeFrom(events []*eventlogger.Event) (eventlogger.Even if g.Request.DetailsUpstreamMessage != nil { msgReq.DetailsUpstreamMessage = g.Request.DetailsUpstreamMessage } + if g.Request.UserAgents != nil { + msgReq.UserAgents = g.Request.UserAgents + } payload[RequestField] = msgReq } if g.Response != nil { diff --git a/internal/event/event_observation_test.go b/internal/event/event_observation_test.go index 3adff52cf0..9394fbfc87 100644 --- a/internal/event/event_observation_test.go +++ b/internal/event/event_observation_test.go @@ -503,6 +503,50 @@ func Test_composeFromTelemetryFiltering(t *testing.T) { }, }, }, + { + name: "with-request-client-headers-with-telemetry", + fromOp: Op("with-request-client-headers-with-telemetry"), + opts: []Option{ + WithId("with-request-client-headers-with-telemetry"), + WithRequestInfo(TestRequestInfo(t)), + WithFlush(), + WithRequest(&Request{ + Operation: "op", + Endpoint: "/auth-tokens/", + Details: testWorkerStatus(t), + UserAgents: []*UserAgent{{ + Product: "Boundary-client-agent", + ProductVersion: "0.1.4", + }}, + }), + WithTelemetry(), + }, + wantObservation: &observation{ + ID: "with-request-client-headers-with-telemetry", + Flush: true, + Version: errorVersion, + Op: Op("with-request-client-headers-with-telemetry"), + RequestInfo: TestRequestInfo(t), + Request: &Request{ + Operation: "op", + Endpoint: "/auth-tokens/", + Details: testWorkerStatus(t), + UserAgents: []*UserAgent{{ + Product: "Boundary-client-agent", + ProductVersion: "0.1.4", + }}, + }, + }, + wantFilteredRequest: &Request{ + Operation: "op", + Endpoint: "/auth-tokens/", + Details: testWorkerStatusObservable(t), + UserAgents: []*UserAgent{{ + Product: "Boundary-client-agent", + ProductVersion: "0.1.4", + }}, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/gen/controller/api/services/authtokens_service.pb.go b/internal/gen/controller/api/services/authtokens_service.pb.go index 9e0601bcea..d82139d180 100644 --- a/internal/gen/controller/api/services/authtokens_service.pb.go +++ b/internal/gen/controller/api/services/authtokens_service.pb.go @@ -29,7 +29,7 @@ const ( type GetAuthTokenRequest struct { state protoimpl.MessageState `protogen:"open.v1"` - Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty" class:"public"` // @gotags: `class:"public"` + Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty" class:"public" eventstream:"observation"` // @gotags: `class:"public" eventstream:"observation"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } diff --git a/internal/gen/controller/servers/services/session_service.pb.go b/internal/gen/controller/servers/services/session_service.pb.go index 01852e60ff..dbef1c8f3e 100644 --- a/internal/gen/controller/servers/services/session_service.pb.go +++ b/internal/gen/controller/servers/services/session_service.pb.go @@ -715,7 +715,7 @@ type CloseConnectionRequestData struct { ConnectionId string `protobuf:"bytes,10,opt,name=connection_id,json=connectionId,proto3" json:"connection_id,omitempty" class:"public" eventstream:"observation"` // @gotags: `class:"public" eventstream:"observation"` BytesUp int64 `protobuf:"varint,20,opt,name=bytes_up,json=bytesUp,proto3" json:"bytes_up,omitempty" class:"public"` // @gotags: `class:"public"` BytesDown int64 `protobuf:"varint,30,opt,name=bytes_down,json=bytesDown,proto3" json:"bytes_down,omitempty" class:"public"` // @gotags: `class:"public"` - Reason string `protobuf:"bytes,40,opt,name=reason,proto3" json:"reason,omitempty" class:"public"` // @gotags: `class:"public"` + Reason string `protobuf:"bytes,40,opt,name=reason,proto3" json:"reason,omitempty" class:"public" eventstream:"observation"` // @gotags: `class:"public" eventstream:"observation"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } diff --git a/internal/proto/controller/api/resources/authtokens/v1/authtoken.proto b/internal/proto/controller/api/resources/authtokens/v1/authtoken.proto index 2d4a1bf5c2..5014f68800 100644 --- a/internal/proto/controller/api/resources/authtokens/v1/authtoken.proto +++ b/internal/proto/controller/api/resources/authtokens/v1/authtoken.proto @@ -13,7 +13,7 @@ option go_package = "github.com/hashicorp/boundary/sdk/pbs/controller/api/resour // AuthToken contains all fields related to an Auth Token resource message AuthToken { // Output only. The ID of the Auth Token. - string id = 10; // @gotags: `class:"public"` + string id = 10; // @gotags: `class:"public" eventstream:"observation"` // The Scope in which this Auth Token was generated. string scope_id = 20 [json_name = "scope_id"]; // @gotags: `class:"public"` diff --git a/internal/proto/controller/api/services/v1/authtokens_service.proto b/internal/proto/controller/api/services/v1/authtokens_service.proto index c617e847ed..0793d5177e 100644 --- a/internal/proto/controller/api/services/v1/authtokens_service.proto +++ b/internal/proto/controller/api/services/v1/authtokens_service.proto @@ -49,7 +49,7 @@ service AuthTokenService { } message GetAuthTokenRequest { - string id = 1; // @gotags: `class:"public"` + string id = 1; // @gotags: `class:"public" eventstream:"observation"` } message GetAuthTokenResponse { diff --git a/internal/proto/controller/servers/services/v1/session_service.proto b/internal/proto/controller/servers/services/v1/session_service.proto index 1ff302a31d..fc2383005d 100644 --- a/internal/proto/controller/servers/services/v1/session_service.proto +++ b/internal/proto/controller/servers/services/v1/session_service.proto @@ -126,7 +126,7 @@ message CloseConnectionRequestData { string connection_id = 10; // @gotags: `class:"public" eventstream:"observation"` int64 bytes_up = 20; // @gotags: `class:"public"` int64 bytes_down = 30; // @gotags: `class:"public"` - string reason = 40; // @gotags: `class:"public"` + string reason = 40; // @gotags: `class:"public" eventstream:"observation"` } message CloseConnectionRequest { diff --git a/sdk/pbs/controller/api/resources/authtokens/authtoken.pb.go b/sdk/pbs/controller/api/resources/authtokens/authtoken.pb.go index fd30b139cf..21e62d110b 100644 --- a/sdk/pbs/controller/api/resources/authtokens/authtoken.pb.go +++ b/sdk/pbs/controller/api/resources/authtokens/authtoken.pb.go @@ -30,7 +30,7 @@ const ( type AuthToken struct { state protoimpl.MessageState `protogen:"open.v1"` // Output only. The ID of the Auth Token. - Id string `protobuf:"bytes,10,opt,name=id,proto3" json:"id,omitempty" class:"public"` // @gotags: `class:"public"` + Id string `protobuf:"bytes,10,opt,name=id,proto3" json:"id,omitempty" class:"public" eventstream:"observation"` // @gotags: `class:"public" eventstream:"observation"` // The Scope in which this Auth Token was generated. ScopeId string `protobuf:"bytes,20,opt,name=scope_id,proto3" json:"scope_id,omitempty" class:"public"` // @gotags: `class:"public"` // Output only. Scope information for this resource.