Skip to content

Commit 399a58b

Browse files
e-n-0e-n-0
authored
feat(appsec/proxy): enable body processing by default (except GCP SE) (#4069)
Co-authored-by: flavien.darche <[email protected]>
1 parent 99623ba commit 399a58b

File tree

17 files changed

+313
-77
lines changed

17 files changed

+313
-77
lines changed

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

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,22 +22,23 @@ The ASM Service Extension expose some configuration. The configuration can be tw
2222

2323
>**GCP requires that the default configuration for the Service Extension should not change.**
2424
25-
| Environment variable | Default value | Description |
26-
|---|-----------------|---------------------------------------------------------------------------------------------------------------|
27-
| `DD_SERVICE_EXTENSION_HOST` | `0.0.0.0` | Host on where the gRPC and HTTP server should listen to. |
28-
| `DD_SERVICE_EXTENSION_PORT` | `443` | Port used by the gRPC Server.<br>Envoy Google backend’s is only using secure connection to Service Extension. |
29-
| `DD_SERVICE_EXTENSION_HEALTHCHECK_PORT` | `80` | Port used for the HTTP server for the health check. |
25+
| Environment variable | Default value | Description |
26+
|-------------------------------------------|-----------------|---------------------------------------------------------------------------------------------------------------|
27+
| `DD_SERVICE_EXTENSION_HOST` | `0.0.0.0` | Host on where the gRPC and HTTP server should listen to. |
28+
| `DD_SERVICE_EXTENSION_PORT` | `443` | Port used by the gRPC Server.<br>Envoy Google backend’s is only using secure connection to Service Extension. |
29+
| `DD_SERVICE_EXTENSION_HEALTHCHECK_PORT` | `80` | Port used for the HTTP server for the health check. |
3030
| `DD_SERVICE_EXTENSION_OBSERVABILITY_MODE` | `false` | Enable observability mode. This will process a request asynchronously (blocking would be disabled). |
31-
| `DD_SERVICE_EXTENSION_TLS` | `true` | Enable the gRPC TLS layer. Do not modify if you are using GCP. |
32-
| `DD_SERVICE_EXTENSION_TLS_KEY_FILE` | `localhost.key` | Change the default gRPC TLS layer key. Do not modify if you are using GCP. |
33-
| `DD_SERVICE_EXTENSION_TLS_CERT_FILE` | `localhost.crt` | Change the default gRPC TLS layer cert. Do not modify if you are using GCP. |
31+
| `DD_APPSEC_BODY_PARSING_SIZE_LIMIT` | `10485760` | Maximum size of the bodies to be processed in bytes. If set to 0, the bodies are not processed. |
32+
| `DD_SERVICE_EXTENSION_TLS` | `true` | Enable the gRPC TLS layer. Do not modify if you are using GCP. |
33+
| `DD_SERVICE_EXTENSION_TLS_KEY_FILE` | `localhost.key` | Change the default gRPC TLS layer key. Do not modify if you are using GCP. |
34+
| `DD_SERVICE_EXTENSION_TLS_CERT_FILE` | `localhost.crt` | Change the default gRPC TLS layer cert. Do not modify if you are using GCP. |
3435

3536
> The Service Extension need to be connected to a deployed [Datadog agent](https://docs.datadoghq.com/agent).
3637
37-
| Environment variable | Default value | Description |
38-
|---|---|---|
39-
| `DD_AGENT_HOST` | `N/A` | Host of a running Datadog Agent. |
40-
| `DD_TRACE_AGENT_PORT` | `8126` | Port of a running Datadog Agent. |
38+
| Environment variable | Default value | Description |
39+
|-----------------------|---------------|----------------------------------|
40+
| `DD_AGENT_HOST` | `N/A` | Host of a running Datadog Agent. |
41+
| `DD_TRACE_AGENT_PORT` | `8126` | Port of a running Datadog Agent. |
4142

4243
### SSL Configuration
4344

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,21 @@ func intEnv(key string, def int) int {
2727
return v
2828
}
2929

30+
// intEnvNil returns the parsed int value of an environment variable if it exists, or
31+
// return nil if unset or failed to parse.
32+
func intEnvNil(key string) *int {
33+
vv, ok := env.Lookup(key)
34+
if !ok {
35+
return nil
36+
}
37+
v, err := strconv.Atoi(vv)
38+
if err != nil {
39+
log.Warn("Non-integer value for env var %s. Parse failed with error: %v", key, err)
40+
return nil
41+
}
42+
return &v
43+
}
44+
3045
// IpEnv returns the valid IP value of an environment variable, or def otherwise.
3146
func ipEnv(key string, def net.IP) net.IP {
3247
vv, ok := env.Lookup(key)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ type serviceExtensionConfig struct {
4545
extensionHost string
4646
healthcheckPort string
4747
observabilityMode bool
48-
bodyParsingSizeLimit int
48+
bodyParsingSizeLimit *int
4949
tls *tlsConfig
5050
}
5151

@@ -92,7 +92,7 @@ func loadConfig() serviceExtensionConfig {
9292
healthcheckPortInt := intEnv("DD_SERVICE_EXTENSION_HEALTHCHECK_PORT", 80)
9393
extensionHostStr := ipEnv("DD_SERVICE_EXTENSION_HOST", net.IP{0, 0, 0, 0}).String()
9494
observabilityMode := boolEnv("DD_SERVICE_EXTENSION_OBSERVABILITY_MODE", false)
95-
bodyParsingSizeLimit := intEnv("DD_APPSEC_BODY_PARSING_SIZE_LIMIT", 0)
95+
bodyParsingSizeLimit := intEnvNil("DD_APPSEC_BODY_PARSING_SIZE_LIMIT")
9696
enableTLS := boolEnv("DD_SERVICE_EXTENSION_TLS", true)
9797
keyFile := stringEnv("DD_SERVICE_EXTENSION_TLS_KEY_FILE", "localhost.key")
9898
certFile := stringEnv("DD_SERVICE_EXTENSION_TLS_CERT_FILE", "localhost.crt")

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

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func TestLoadConfig_VariousCases(t *testing.T) {
7878
healthcheckPort string
7979
extensionHost string
8080
observabilityMode bool
81-
bodyParsingSizeLimit int
81+
bodyParsingSizeLimit *int
8282
tlsEnabled bool
8383
tlsCertFile string
8484
tlsKeyFile string
@@ -92,7 +92,7 @@ func TestLoadConfig_VariousCases(t *testing.T) {
9292
{
9393
name: "defaults",
9494
env: nil,
95-
want: want{"443", "80", "0.0.0.0", false, 0, true, "localhost.key", "localhost.crt"},
95+
want: want{"443", "80", "0.0.0.0", false, nil, true, "localhost.crt", "localhost.key"},
9696
},
9797
{
9898
name: "valid overrides",
@@ -103,7 +103,7 @@ func TestLoadConfig_VariousCases(t *testing.T) {
103103
"DD_SERVICE_EXTENSION_OBSERVABILITY_MODE": "true",
104104
"DD_APPSEC_BODY_PARSING_SIZE_LIMIT": "100000000",
105105
},
106-
want: want{"1234", "4321", "127.0.0.1", true, 100000000, true, "localhost.key", "localhost.crt"},
106+
want: want{"1234", "4321", "127.0.0.1", true, intPtr(100000000), true, "localhost.crt", "localhost.key"},
107107
},
108108
{
109109
name: "bad values fall back",
@@ -114,22 +114,22 @@ func TestLoadConfig_VariousCases(t *testing.T) {
114114
"DD_APPSEC_BODY_PARSING_SIZE_LIMIT": "notanint",
115115
"DD_SERVICE_EXTENSION_HOST": "notanip",
116116
},
117-
want: want{"443", "80", "0.0.0.0", false, 0, true, "localhost.key", "localhost.crt"},
117+
want: want{"443", "80", "0.0.0.0", false, nil, true, "localhost.crt", "localhost.key"},
118118
},
119119
{
120120
name: "no-tls",
121121
env: map[string]string{
122122
"DD_SERVICE_EXTENSION_TLS": "false",
123123
},
124-
want: want{"443", "80", "0.0.0.0", false, 0, false, "localhost.key", "localhost.crt"},
124+
want: want{"443", "80", "0.0.0.0", false, nil, false, "localhost.key", "localhost.crt"},
125125
},
126126
{
127127
name: "custom-tls",
128128
env: map[string]string{
129129
"DD_SERVICE_EXTENSION_TLS_KEY_FILE": "/tls/tls.key",
130130
"DD_SERVICE_EXTENSION_TLS_CERT_FILE": "/tls/tls.crt",
131131
},
132-
want: want{"443", "80", "0.0.0.0", false, 0, true, "/tls/tls.key", "/tls/tls.crt"},
132+
want: want{"443", "80", "0.0.0.0", false, nil, true, "/tls/tls.crt", "/tls/tls.key"},
133133
},
134134
}
135135

@@ -156,10 +156,19 @@ func TestLoadConfig_VariousCases(t *testing.T) {
156156
assert.Equal(t, tc.want.observabilityMode, cfg.observabilityMode, "observabilityMode")
157157
assert.Equal(t, tc.want.bodyParsingSizeLimit, cfg.bodyParsingSizeLimit, "bodyParsingSizeLimit")
158158

159+
assert.Equal(t, tc.want.tlsEnabled, cfg.tls != nil, "tlsEnabled")
160+
if cfg.tls != nil {
161+
assert.Equal(t, tc.want.tlsCertFile, cfg.tls.certFile, "tlsCertFile")
162+
assert.Equal(t, tc.want.tlsKeyFile, cfg.tls.keyFile, "tlsKeyFile")
163+
}
159164
})
160165
}
161166
}
162167

168+
func intPtr(v int) *int {
169+
return &v
170+
}
171+
163172
// Helpers
164173
func unsetEnv(keys ...string) {
165174
for _, k := range keys {

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

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"context"
1010
"errors"
1111
"io"
12-
"sync/atomic"
1312

1413
"github.com/DataDog/dd-trace-go/v2/instrumentation"
1514
"github.com/DataDog/dd-trace-go/v2/instrumentation/appsec/proxy"
@@ -41,20 +40,26 @@ type AppsecEnvoyConfig struct {
4140
Integration Integration
4241
BlockingUnavailable bool
4342
Context context.Context
44-
BodyParsingSizeLimit int
43+
BodyParsingSizeLimit *int
4544
}
4645

4746
// appsecEnvoyExternalProcessorServer is a server that implements the Envoy ExternalProcessorServer interface.
4847
type appsecEnvoyExternalProcessorServer struct {
4948
envoyextproc.ExternalProcessorServer
5049
config AppsecEnvoyConfig
51-
requestCounter atomic.Uint32
5250
messageProcessor proxy.Processor
5351
}
5452

5553
// AppsecEnvoyExternalProcessorServer creates a new external processor server with AAP enabled
5654
func AppsecEnvoyExternalProcessorServer(userImplementation envoyextproc.ExternalProcessorServer, config AppsecEnvoyConfig) envoyextproc.ExternalProcessorServer {
57-
processor := &appsecEnvoyExternalProcessorServer{
55+
switch config.Integration {
56+
case GCPServiceExtensionIntegration, EnvoyIntegration, IstioIntegration, EnvoyGatewayIntegration:
57+
default:
58+
instr.Logger().Error("external_processing: invalid proxy integration type %d. Defaulting to GCPServiceExtensionIntegration", config.Integration)
59+
config.Integration = GCPServiceExtensionIntegration
60+
}
61+
62+
return &appsecEnvoyExternalProcessorServer{
5863
ExternalProcessorServer: userImplementation,
5964
config: config,
6065
messageProcessor: proxy.NewProcessor(proxy.ProcessorConfig{
@@ -66,15 +71,6 @@ func AppsecEnvoyExternalProcessorServer(userImplementation envoyextproc.External
6671
BlockMessageFunc: blockActionFunc,
6772
}, instr),
6873
}
69-
70-
switch config.Integration {
71-
case GCPServiceExtensionIntegration, EnvoyIntegration, IstioIntegration, EnvoyGatewayIntegration:
72-
default:
73-
instr.Logger().Error("external_processing: invalid proxy integration type %d. Defaulting to GCPServiceExtensionIntegration", config.Integration)
74-
config.Integration = GCPServiceExtensionIntegration
75-
}
76-
77-
return processor
7874
}
7975

8076
type processServerKeyType struct{}

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

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,35 +87,49 @@ func (i Integration) String() string {
8787
}
8888
}
8989

90+
func (m messageRequestHeaders) BodyParsingSizeLimit(ctx context.Context) int {
91+
switch m.component(ctx) {
92+
case componentNameGCPServiceExtension:
93+
return 0
94+
default:
95+
return proxy.DefaultBodyParsingSizeLimit
96+
}
97+
}
98+
9099
func (m messageRequestHeaders) SpanOptions(ctx context.Context) []tracer.StartSpanOption {
100+
return []tracer.StartSpanOption{tracer.Tag(ext.Component, m.component(ctx))}
101+
}
102+
103+
func (m messageRequestHeaders) component(ctx context.Context) string {
91104
// As the integration (callout container) is run by default with the GCP Service Extension value,
92105
// we can consider that if this flag is false, it means that it is running in a custom integration.
93106
if m.integration != GCPServiceExtensionIntegration {
94-
return []tracer.StartSpanOption{tracer.Tag(ext.Component, m.integration.String())}
107+
return m.integration.String()
95108
}
96109

97110
// In newer version of the documentation, customers are instructed to inject the
98111
// Datadog integration header in their Envoy configuration to identify the integration.
99112
if md, ok := metadata.FromIncomingContext(ctx); ok {
100113
valuesEnvoy := md.Get(datadogEnvoyIntegrationHeader)
101114
if len(valuesEnvoy) > 0 && valuesEnvoy[0] == "1" {
102-
return []tracer.StartSpanOption{tracer.Tag(ext.Component, componentNameEnvoy)}
115+
return componentNameEnvoy
103116
}
104117

105118
valuesIstio := md.Get(datadogIntegrationHeader)
106119
if len(valuesIstio) > 0 && valuesIstio[0] == "1" {
107-
return []tracer.StartSpanOption{tracer.Tag(ext.Component, componentNameIstio)}
120+
return componentNameIstio
108121
}
109122

110123
// We don't have the ability to add custom headers in envoy gateway EnvoyExtensionPolicy CRD.
111124
// So we fall back to detecting if we are running in k8s or not.
112125
// If we are running in k8s, we assume it is Envoy Gateway, otherwise GCP Service Extension.
113126
if isK8s() {
114-
return []tracer.StartSpanOption{tracer.Tag(ext.Component, componentNameEnvoyGateway)}
127+
return componentNameEnvoyGateway
115128
}
116129
}
117130

118-
return []tracer.StartSpanOption{tracer.Tag(ext.Component, componentNameGCPServiceExtension)}
131+
return componentNameGCPServiceExtension
132+
119133
}
120134

121135
type responseHeadersEnvoy struct {

0 commit comments

Comments
 (0)