From d58276cab8a8d2ce198d0123ef74ebd1a4a1aaee Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Thu, 11 Jan 2024 17:21:45 +0100 Subject: [PATCH 1/2] serverless/appsec: fix handling of ALB headers & query string When an AWS Application Load Balancer forwards a request to Lambda, it formats headers and query string parameters either as `map[string]string` in the `Headers` and `QueryStringParameters` properties (this is the default behavior), or as `map[string][]string` in `MultiValueHeaders` and `MultiValueQueryStringParameters` (when explicitly enabled by setting the appropriate target group attribute[^1]). We were previously only considering the multi-valued entries, since other event types (such as API Gateway) provide values in both formats all the time. This change uses the multi-valued map if present, and falls back to the single-valued map otherwise. [^1]: https://docs.aws.amazon.com/elasticloadbalancing/latest/application/lambda-functions.html#enable-multi-value-headers --- pkg/serverless/appsec/httpsec/proxy.go | 31 +++++++++++++++-- pkg/serverless/trigger/events.go | 46 ++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/pkg/serverless/appsec/httpsec/proxy.go b/pkg/serverless/appsec/httpsec/proxy.go index a3258921becdd..655ef006cf59b 100644 --- a/pkg/serverless/appsec/httpsec/proxy.go +++ b/pkg/serverless/appsec/httpsec/proxy.go @@ -61,6 +61,8 @@ func (lp *ProxyLifecycleProcessor) OnInvokeStart(startDetails *invocationlifecyc eventType := trigger.GetEventType(lowercaseEventPayload) if eventType == trigger.Unknown { log.Debugf("appsec: proxy-lifecycle: Failed to extract event type") + } else { + log.Debugf("appsec: proxy-lifecycle: Extracted event type: %v", eventType) } var event interface{} @@ -206,14 +208,24 @@ func (lp *ProxyLifecycleProcessor) spanModifier(lastReqId string, chunk *pb.Trac ) case *events.ALBTargetGroupRequest: + var sourceIp string + // ALB is a "trusted" origin, so we extract the source IP from the X-Forwarded-For header. + // ALB also provides headers with fully lower-case names, so we don't have to worry about case. + if xff, found := event.MultiValueHeaders["x-forwarded-for"]; found && len(xff) > 0 { + sourceIp = xff[0] + } else if xff, found := event.Headers["x-forwarded-for"]; found { + sourceIp = xff + } makeContext( &ctx, nil, &event.Path, - event.MultiValueHeaders, - event.MultiValueQueryStringParameters, + // Depending on how the ALB is configured, headers will be either in MultiValueHeaders or Headers (not both). + multiOrSingle(event.MultiValueHeaders, event.Headers), + // Depending on how the ALB is configured, query parameters will be either in MultiValueQueryStringParameters or QueryStringParameters (not both). + multiOrSingle(event.MultiValueQueryStringParameters, event.QueryStringParameters), nil, - "", + sourceIp, &event.Body, event.IsBase64Encoded, ) @@ -258,6 +270,19 @@ func (lp *ProxyLifecycleProcessor) spanModifier(lastReqId string, chunk *pb.Trac } } +// multiOrSingle picks the first non-nil map, and returns the content formatted +// as the multi-map. +func multiOrSingle(multi map[string][]string, single map[string]string) map[string][]string { + if multi == nil && single != nil { + // There is no multi-map, but there is a single-map, so we'll make a multi-map out of that. + multi = make(map[string][]string, len(single)) + for key, value := range single { + multi[key] = []string{value} + } + } + return multi +} + //nolint:revive // TODO(ASM) Fix revive linter type ExecutionContext interface { LastRequestID() string diff --git a/pkg/serverless/trigger/events.go b/pkg/serverless/trigger/events.go index 357cadfaaecdd..95dca9ad76348 100644 --- a/pkg/serverless/trigger/events.go +++ b/pkg/serverless/trigger/events.go @@ -7,6 +7,7 @@ package trigger import ( + "fmt" "strings" jsonEncoder "github.com/json-iterator/go" @@ -294,3 +295,48 @@ func eventRecordsKeyEquals(event map[string]any, key string, val string) bool { } return false } + +func (et AWSEventType) String() string { + switch et { + case Unknown: + return "Unknown" + case APIGatewayEvent: + return "APIGatewayEvent" + case APIGatewayV2Event: + return "APIGatewayV2Event" + case APIGatewayWebsocketEvent: + return "APIGatewayWebsocketEvent" + case APIGatewayLambdaAuthorizerTokenEvent: + return "APIGatewayLambdaAuthorizerTokenEvent" + case APIGatewayLambdaAuthorizerRequestParametersEvent: + return "APIGatewayLambdaAuthorizerRequestParametersEvent" + case ALBEvent: + return "ALBEvent" + case CloudWatchEvent: + return "CloudWatchEvent" + case CloudWatchLogsEvent: + return "CloudWatchLogsEvent" + case CloudFrontRequestEvent: + return "CloudFrontRequestEvent" + case DynamoDBStreamEvent: + return "DynamoDBStreamEvent" + case KinesisStreamEvent: + return "KinesisStreamEvent" + case S3Event: + return "S3Event" + case SNSEvent: + return "SNSEvent" + case SQSEvent: + return "SQSEvent" + case SNSSQSEvent: + return "SNSSQSEvent" + case AppSyncResolverEvent: + return "AppSyncResolverEvent" + case EventBridgeEvent: + return "EventBridgeEvent" + case LambdaFunctionURLEvent: + return "LambdaFunctionURLEvent" + default: + return fmt.Sprintf("EventType(%d)", et) + } +} From 804be2e5d52388c5ad65fb562968e2ef9887d461 Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Fri, 12 Jan 2024 09:53:38 +0100 Subject: [PATCH 2/2] undo XFF change --- pkg/serverless/appsec/httpsec/proxy.go | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/pkg/serverless/appsec/httpsec/proxy.go b/pkg/serverless/appsec/httpsec/proxy.go index 655ef006cf59b..f70abf10ee003 100644 --- a/pkg/serverless/appsec/httpsec/proxy.go +++ b/pkg/serverless/appsec/httpsec/proxy.go @@ -208,14 +208,6 @@ func (lp *ProxyLifecycleProcessor) spanModifier(lastReqId string, chunk *pb.Trac ) case *events.ALBTargetGroupRequest: - var sourceIp string - // ALB is a "trusted" origin, so we extract the source IP from the X-Forwarded-For header. - // ALB also provides headers with fully lower-case names, so we don't have to worry about case. - if xff, found := event.MultiValueHeaders["x-forwarded-for"]; found && len(xff) > 0 { - sourceIp = xff[0] - } else if xff, found := event.Headers["x-forwarded-for"]; found { - sourceIp = xff - } makeContext( &ctx, nil, @@ -225,7 +217,7 @@ func (lp *ProxyLifecycleProcessor) spanModifier(lastReqId string, chunk *pb.Trac // Depending on how the ALB is configured, query parameters will be either in MultiValueQueryStringParameters or QueryStringParameters (not both). multiOrSingle(event.MultiValueQueryStringParameters, event.QueryStringParameters), nil, - sourceIp, + "", &event.Body, event.IsBase64Encoded, )