feat(appsec/proxy): enable body processing by default (except GCP SE)

contrib/envoyproxy/go-control-plane/cmd/serviceextensions/README.md

@@ -22,22 +22,23 @@ The ASM Service Extension expose some configuration. The configuration can be tw
 
 >**GCP requires that the default configuration for the Service Extension should not change.**
 
-| Environment variable | Default value   | Description                                                                                                   |
-|---|-----------------|---------------------------------------------------------------------------------------------------------------|
-| `DD_SERVICE_EXTENSION_HOST` | `0.0.0.0`       | Host on where the gRPC and HTTP server should listen to.                                                      |
-| `DD_SERVICE_EXTENSION_PORT` | `443`           | Port used by the gRPC Server.<br>Envoy Google backend’s is only using secure connection to Service Extension. |
-| `DD_SERVICE_EXTENSION_HEALTHCHECK_PORT` | `80`            | Port used for the HTTP server for the health check.                                                           |
+| Environment variable                      | Default value   | Description                                                                                                   |
+|-------------------------------------------|-----------------|---------------------------------------------------------------------------------------------------------------|
+| `DD_SERVICE_EXTENSION_HOST`               | `0.0.0.0`       | Host on where the gRPC and HTTP server should listen to.                                                      |
+| `DD_SERVICE_EXTENSION_PORT`               | `443`           | Port used by the gRPC Server.<br>Envoy Google backend’s is only using secure connection to Service Extension. |
+| `DD_SERVICE_EXTENSION_HEALTHCHECK_PORT`   | `80`            | Port used for the HTTP server for the health check.                                                           |
 | `DD_SERVICE_EXTENSION_OBSERVABILITY_MODE` | `false`         | Enable observability mode. This will process a request asynchronously (blocking would be disabled).           |
-| `DD_SERVICE_EXTENSION_TLS` | `true`          | Enable the gRPC TLS layer. Do not modify if you are using GCP.                                                |
-| `DD_SERVICE_EXTENSION_TLS_KEY_FILE` | `localhost.key` | Change the default gRPC TLS layer key. Do not modify if you are using GCP.                                    |
-| `DD_SERVICE_EXTENSION_TLS_CERT_FILE` | `localhost.crt` | Change the default gRPC TLS layer cert. Do not modify if you are using GCP.                                   |
+| `DD_APPSEC_BODY_PARSING_SIZE_LIMIT`       | `10000000`      | Maximum size of the bodies to be processed in bytes. If set to 0, the bodies are not processed.               |
+| `DD_SERVICE_EXTENSION_TLS`                | `true`          | Enable the gRPC TLS layer. Do not modify if you are using GCP.                                                |
+| `DD_SERVICE_EXTENSION_TLS_KEY_FILE`       | `localhost.key` | Change the default gRPC TLS layer key. Do not modify if you are using GCP.                                    |
+| `DD_SERVICE_EXTENSION_TLS_CERT_FILE`      | `localhost.crt` | Change the default gRPC TLS layer cert. Do not modify if you are using GCP.                                   |
 
 > The Service Extension need to be connected to a deployed [Datadog agent](https://docs.datadoghq.com/agent).
 
-| Environment variable | Default value | Description |
-|---|---|---|
-| `DD_AGENT_HOST` | `N/A` | Host of a running Datadog Agent. |
-| `DD_TRACE_AGENT_PORT` | `8126` | Port of a running Datadog Agent. |
+| Environment variable  | Default value | Description                      |
+|-----------------------|---------------|----------------------------------|
+| `DD_AGENT_HOST`       | `N/A`         | Host of a running Datadog Agent. |
+| `DD_TRACE_AGENT_PORT` | `8126`        | Port of a running Datadog Agent. |
 
 ### SSL Configuration
 

contrib/envoyproxy/go-control-plane/cmd/serviceextensions/env.go

@@ -27,6 +27,21 @@ func intEnv(key string, def int) int {
 	return v
 }
 
+// intEnvNil returns the parsed int value of an environment variable if it exists, or
+// return nil if unset or failed to parse.
+func intEnvNil(key string) *int {
+	vv, ok := env.Lookup(key)
+	if !ok {
+		return nil
+	}
+	v, err := strconv.Atoi(vv)
+	if err != nil {
+		log.Warn("Non-integer value for env var %s. Parse failed with error: %v", key, err)
+		return nil
+	}
+	return &v
+}
+
 // IpEnv returns the valid IP value of an environment variable, or def otherwise.
 func ipEnv(key string, def net.IP) net.IP {
 	vv, ok := env.Lookup(key)

contrib/envoyproxy/go-control-plane/cmd/serviceextensions/main.go

@@ -45,7 +45,7 @@ type serviceExtensionConfig struct {
 	extensionHost        string
 	healthcheckPort      string
 	observabilityMode    bool
-	bodyParsingSizeLimit int
+	bodyParsingSizeLimit *int
 	tls                  *tlsConfig
 }
 
@@ -92,7 +92,7 @@ func loadConfig() serviceExtensionConfig {
 	healthcheckPortInt := intEnv("DD_SERVICE_EXTENSION_HEALTHCHECK_PORT", 80)
 	extensionHostStr := ipEnv("DD_SERVICE_EXTENSION_HOST", net.IP{0, 0, 0, 0}).String()
 	observabilityMode := boolEnv("DD_SERVICE_EXTENSION_OBSERVABILITY_MODE", false)
-	bodyParsingSizeLimit := intEnv("DD_APPSEC_BODY_PARSING_SIZE_LIMIT", 0)
+	bodyParsingSizeLimit := intEnvNil("DD_APPSEC_BODY_PARSING_SIZE_LIMIT")
 	enableTLS := boolEnv("DD_SERVICE_EXTENSION_TLS", true)
 	keyFile := stringEnv("DD_SERVICE_EXTENSION_TLS_KEY_FILE", "localhost.key")
 	certFile := stringEnv("DD_SERVICE_EXTENSION_TLS_CERT_FILE", "localhost.crt")

contrib/envoyproxy/go-control-plane/cmd/serviceextensions/main_test.go

@@ -78,7 +78,7 @@ func TestLoadConfig_VariousCases(t *testing.T) {
 		healthcheckPort      string
 		extensionHost        string
 		observabilityMode    bool
-		bodyParsingSizeLimit int
+		bodyParsingSizeLimit *int
 		tlsEnabled           bool
 		tlsCertFile          string
 		tlsKeyFile           string
@@ -92,7 +92,7 @@ func TestLoadConfig_VariousCases(t *testing.T) {
 		{
 			name: "defaults",
 			env:  nil,
-			want: want{"443", "80", "0.0.0.0", false, 0, true, "localhost.key", "localhost.crt"},
+			want: want{"443", "80", "0.0.0.0", false, nil, true, "localhost.crt", "localhost.key"},
 		},
 		{
 			name: "valid overrides",
@@ -103,7 +103,7 @@ func TestLoadConfig_VariousCases(t *testing.T) {
 				"DD_SERVICE_EXTENSION_OBSERVABILITY_MODE": "true",
 				"DD_APPSEC_BODY_PARSING_SIZE_LIMIT":       "100000000",
 			},
-			want: want{"1234", "4321", "127.0.0.1", true, 100000000, true, "localhost.key", "localhost.crt"},
+			want: want{"1234", "4321", "127.0.0.1", true, intPtr(100000000), true, "localhost.crt", "localhost.key"},
 		},
 		{
 			name: "bad values fall back",
@@ -114,22 +114,22 @@ func TestLoadConfig_VariousCases(t *testing.T) {
 				"DD_APPSEC_BODY_PARSING_SIZE_LIMIT":       "notanint",
 				"DD_SERVICE_EXTENSION_HOST":               "notanip",
 			},
-			want: want{"443", "80", "0.0.0.0", false, 0, true, "localhost.key", "localhost.crt"},
+			want: want{"443", "80", "0.0.0.0", false, nil, true, "localhost.crt", "localhost.key"},
 		},
 		{
 			name: "no-tls",
 			env: map[string]string{
 				"DD_SERVICE_EXTENSION_TLS": "false",
 			},
-			want: want{"443", "80", "0.0.0.0", false, 0, false, "localhost.key", "localhost.crt"},
+			want: want{"443", "80", "0.0.0.0", false, nil, false, "localhost.key", "localhost.crt"},
 		},
 		{
 			name: "custom-tls",
 			env: map[string]string{
 				"DD_SERVICE_EXTENSION_TLS_KEY_FILE":  "/tls/tls.key",
 				"DD_SERVICE_EXTENSION_TLS_CERT_FILE": "/tls/tls.crt",
 			},
-			want: want{"443", "80", "0.0.0.0", false, 0, true, "/tls/tls.key", "/tls/tls.crt"},
+			want: want{"443", "80", "0.0.0.0", false, nil, true, "/tls/tls.crt", "/tls/tls.key"},
 		},
 	}
 
@@ -156,10 +156,19 @@ func TestLoadConfig_VariousCases(t *testing.T) {
 			assert.Equal(t, tc.want.observabilityMode, cfg.observabilityMode, "observabilityMode")
 			assert.Equal(t, tc.want.bodyParsingSizeLimit, cfg.bodyParsingSizeLimit, "bodyParsingSizeLimit")
 
+			assert.Equal(t, tc.want.tlsEnabled, cfg.tls != nil, "tlsEnabled")
+			if cfg.tls != nil {
+				assert.Equal(t, tc.want.tlsCertFile, cfg.tls.certFile, "tlsCertFile")
+				assert.Equal(t, tc.want.tlsKeyFile, cfg.tls.keyFile, "tlsKeyFile")
+			}
 		})
 	}
 }
 
+func intPtr(v int) *int {
+	return &v
+}
+
 // Helpers
 func unsetEnv(keys ...string) {
 	for _, k := range keys {

contrib/envoyproxy/go-control-plane/envoy.go

@@ -9,7 +9,6 @@ import (
 	"context"
 	"errors"
 	"io"
-	"sync/atomic"
 
 	"github.com/DataDog/dd-trace-go/v2/instrumentation"
 	"github.com/DataDog/dd-trace-go/v2/instrumentation/appsec/proxy"
@@ -41,20 +40,32 @@ type AppsecEnvoyConfig struct {
 	Integration          Integration
 	BlockingUnavailable  bool
 	Context              context.Context
-	BodyParsingSizeLimit int
+	BodyParsingSizeLimit *int
 }
 
 // appsecEnvoyExternalProcessorServer is a server that implements the Envoy ExternalProcessorServer interface.
 type appsecEnvoyExternalProcessorServer struct {
 	envoyextproc.ExternalProcessorServer
 	config           AppsecEnvoyConfig
-	requestCounter   atomic.Uint32
 	messageProcessor proxy.Processor
 }
 
 // AppsecEnvoyExternalProcessorServer creates a new external processor server with AAP enabled
 func AppsecEnvoyExternalProcessorServer(userImplementation envoyextproc.ExternalProcessorServer, config AppsecEnvoyConfig) envoyextproc.ExternalProcessorServer {
-	processor := &appsecEnvoyExternalProcessorServer{
+	switch config.Integration {
+	case GCPServiceExtensionIntegration:
+	case EnvoyIntegration, IstioIntegration, EnvoyGatewayIntegration:
+		// Set default body parsing size limit if not specified for non-default integrations
+		if config.BodyParsingSizeLimit == nil {
+			defaultBody := proxy.DefaultBodyParsingSizeLimit
+			config.BodyParsingSizeLimit = &defaultBody
+		}
+	default:
+		instr.Logger().Error("external_processing: invalid proxy integration type %d. Defaulting to GCPServiceExtensionIntegration", config.Integration)
+		config.Integration = GCPServiceExtensionIntegration
+	}
+
+	return &appsecEnvoyExternalProcessorServer{
 		ExternalProcessorServer: userImplementation,
 		config:                  config,
 		messageProcessor: proxy.NewProcessor(proxy.ProcessorConfig{
@@ -66,15 +77,6 @@ func AppsecEnvoyExternalProcessorServer(userImplementation envoyextproc.External
 			BlockMessageFunc:     blockActionFunc,
 		}, instr),
 	}
-
-	switch config.Integration {
-	case GCPServiceExtensionIntegration, EnvoyIntegration, IstioIntegration, EnvoyGatewayIntegration:
-	default:
-		instr.Logger().Error("external_processing: invalid proxy integration type %d. Defaulting to GCPServiceExtensionIntegration", config.Integration)
-		config.Integration = GCPServiceExtensionIntegration
-	}
-
-	return processor
 }
 
 type processServerKeyType struct{}

contrib/envoyproxy/go-control-plane/envoy_messages.go

@@ -87,35 +87,49 @@ func (i Integration) String() string {
 	}
 }
 
+func (m messageRequestHeaders) BodyParsingSizeLimit(ctx context.Context) int {
+	switch m.component(ctx) {
+	case componentNameGCPServiceExtension:
+		return 0
+	default:
+		return proxy.DefaultBodyParsingSizeLimit
+	}
+}
+
 func (m messageRequestHeaders) SpanOptions(ctx context.Context) []tracer.StartSpanOption {
+	return []tracer.StartSpanOption{tracer.Tag(ext.Component, m.component(ctx))}
+}
+
+func (m messageRequestHeaders) component(ctx context.Context) string {
 	// As the integration (callout container) is run by default with the GCP Service Extension value,
 	// we can consider that if this flag is false, it means that it is running in a custom integration.
 	if m.integration != GCPServiceExtensionIntegration {
-		return []tracer.StartSpanOption{tracer.Tag(ext.Component, m.integration.String())}
+		return m.integration.String()
 	}
 
 	// In newer version of the documentation, customers are instructed to inject the
 	// Datadog integration header in their Envoy configuration to identify the integration.
 	if md, ok := metadata.FromIncomingContext(ctx); ok {
 		valuesEnvoy := md.Get(datadogEnvoyIntegrationHeader)
 		if len(valuesEnvoy) > 0 && valuesEnvoy[0] == "1" {
-			return []tracer.StartSpanOption{tracer.Tag(ext.Component, componentNameEnvoy)}
+			return componentNameEnvoy
 		}
 
 		valuesIstio := md.Get(datadogIntegrationHeader)
 		if len(valuesIstio) > 0 && valuesIstio[0] == "1" {
-			return []tracer.StartSpanOption{tracer.Tag(ext.Component, componentNameIstio)}
+			return componentNameIstio
 		}
 
 		// We don't have the ability to add custom headers in envoy gateway EnvoyExtensionPolicy CRD.
 		// So we fall back to detecting if we are running in k8s or not.
 		// If we are running in k8s, we assume it is Envoy Gateway, otherwise GCP Service Extension.
 		if isK8s() {
-			return []tracer.StartSpanOption{tracer.Tag(ext.Component, componentNameEnvoyGateway)}
+			return componentNameEnvoyGateway
 		}
 	}
 
-	return []tracer.StartSpanOption{tracer.Tag(ext.Component, componentNameGCPServiceExtension)}
+	return componentNameGCPServiceExtension
+
 }
 
 type responseHeadersEnvoy struct {