USM: enable Go TLS monitoring by default (#31064)

DataDog · Nov 28, 2024 · a16fd69 · a16fd69
1 parent 83246bd
commit a16fd69
Show file tree

Hide file tree

Showing 9 changed files with 40 additions and 20 deletions.
diff --git a/cmd/system-probe/config/adjust_usm.go b/cmd/system-probe/config/adjust_usm.go
@@ -30,6 +30,7 @@ func adjustUSM(cfg model.Config) {
 	deprecateBool(cfg, netNS("enable_http_monitoring"), smNS("enable_http_monitoring"))
 	deprecateBool(cfg, netNS("enable_https_monitoring"), smNS("tls", "native", "enabled"))
 	deprecateBool(cfg, smNS("enable_go_tls_support"), smNS("tls", "go", "enabled"))
+	applyDefault(cfg, smNS("tls", "go", "enabled"), true)
 	deprecateGeneric(cfg, netNS("http_replace_rules"), smNS("http_replace_rules"))
 	deprecateInt64(cfg, netNS("max_tracked_http_connections"), smNS("max_tracked_http_connections"))
 	applyDefault(cfg, smNS("max_tracked_http_connections"), 1024)

diff --git a/comp/metadata/inventoryagent/inventoryagentimpl/inventoryagent_test.go b/comp/metadata/inventoryagent/inventoryagentimpl/inventoryagent_test.go
@@ -491,7 +491,7 @@ func TestFetchSystemProbeAgent(t *testing.T) {
 	assert.False(t, ia.data["feature_usm_redis_enabled"].(bool))
 	assert.False(t, ia.data["feature_usm_http2_enabled"].(bool))
 	assert.True(t, ia.data["feature_usm_istio_enabled"].(bool))
-	assert.False(t, ia.data["feature_usm_go_tls_enabled"].(bool))
+	assert.True(t, ia.data["feature_usm_go_tls_enabled"].(bool))
 	assert.False(t, ia.data["feature_discovery_enabled"].(bool))
 	assert.False(t, ia.data["feature_tcp_queue_length_enabled"].(bool))
 	assert.False(t, ia.data["feature_oom_kill_enabled"].(bool))

diff --git a/pkg/network/config/config_test.go b/pkg/network/config/config_test.go
@@ -1464,34 +1464,34 @@ func TestUSMTLSNativeEnabled(t *testing.T) {
 func TestUSMTLSGoEnabled(t *testing.T) {
 	t.Run("via deprecated YAML", func(t *testing.T) {
 		mockSystemProbe := mock.NewSystemProbe(t)
-		mockSystemProbe.SetWithoutSource("service_monitoring_config.enable_go_tls_support", true)
+		mockSystemProbe.SetWithoutSource("service_monitoring_config.enable_go_tls_support", false)
 		cfg := New()
 
-		require.True(t, cfg.EnableGoTLSSupport)
+		require.False(t, cfg.EnableGoTLSSupport)
 	})
 
 	t.Run("via deprecated ENV variable", func(t *testing.T) {
 		mock.NewSystemProbe(t)
-		t.Setenv("DD_SERVICE_MONITORING_CONFIG_ENABLE_GO_TLS_SUPPORT", "true")
+		t.Setenv("DD_SERVICE_MONITORING_CONFIG_ENABLE_GO_TLS_SUPPORT", "false")
 		cfg := New()
 
-		require.True(t, cfg.EnableGoTLSSupport)
+		require.False(t, cfg.EnableGoTLSSupport)
 	})
 
 	t.Run("via YAML", func(t *testing.T) {
 		mockSystemProbe := mock.NewSystemProbe(t)
-		mockSystemProbe.SetWithoutSource("service_monitoring_config.tls.go.enabled", true)
+		mockSystemProbe.SetWithoutSource("service_monitoring_config.tls.go.enabled", false)
 		cfg := New()
 
-		require.True(t, cfg.EnableGoTLSSupport)
+		require.False(t, cfg.EnableGoTLSSupport)
 	})
 
 	t.Run("via ENV variable", func(t *testing.T) {
 		mock.NewSystemProbe(t)
-		t.Setenv("DD_SERVICE_MONITORING_CONFIG_TLS_GO_ENABLED", "true")
+		t.Setenv("DD_SERVICE_MONITORING_CONFIG_TLS_GO_ENABLED", "false")
 		cfg := New()
 
-		require.True(t, cfg.EnableGoTLSSupport)
+		require.False(t, cfg.EnableGoTLSSupport)
 	})
 
 	t.Run("Deprecated is enabled, new is disabled", func(t *testing.T) {
@@ -1512,22 +1512,29 @@ func TestUSMTLSGoEnabled(t *testing.T) {
 		require.True(t, cfg.EnableGoTLSSupport)
 	})
 
-	t.Run("Both enabled", func(t *testing.T) {
+	t.Run("Both disabled", func(t *testing.T) {
 		mock.NewSystemProbe(t)
-		t.Setenv("DD_SERVICE_MONITORING_CONFIG_ENABLE_GO_TLS_SUPPORT", "true")
-		t.Setenv("DD_SERVICE_MONITORING_CONFIG_TLS_GO_ENABLED", "true")
+		t.Setenv("DD_SERVICE_MONITORING_CONFIG_ENABLE_GO_TLS_SUPPORT", "false")
+		t.Setenv("DD_SERVICE_MONITORING_CONFIG_TLS_GO_ENABLED", "false")
 		cfg := New()
 
-		require.True(t, cfg.EnableGoTLSSupport)
+		require.False(t, cfg.EnableGoTLSSupport)
 	})
 
-	t.Run("Not enabled", func(t *testing.T) {
-		mock.NewSystemProbe(t)
+	t.Run("Deprecated is disabled takes precedence over default", func(t *testing.T) {
+		mockSystemProbe := mock.NewSystemProbe(t)
+		mockSystemProbe.SetWithoutSource("service_monitoring_config.enable_go_tls_support", false)
 		cfg := New()
 
-		// Default value.
 		require.False(t, cfg.EnableGoTLSSupport)
 	})
+
+	t.Run("Enabled by default", func(t *testing.T) {
+		mock.NewSystemProbe(t)
+		cfg := New()
+
+		require.True(t, cfg.EnableGoTLSSupport)
+	})
 }
 
 func TestUSMTLSGoExcludeSelf(t *testing.T) {

diff --git a/pkg/network/usm/kafka_monitor_test.go b/pkg/network/usm/kafka_monitor_test.go
@@ -1622,10 +1622,8 @@ func getDefaultTestConfiguration(tls bool) *config.Config {
 	cfg := config.New()
 	cfg.EnableKafkaMonitoring = true
 	cfg.MaxTrackedConnections = 1000
-	if tls {
-		cfg.EnableGoTLSSupport = true
-		cfg.GoTLSExcludeSelf = true
-	}
+	cfg.EnableGoTLSSupport = tls
+	cfg.GoTLSExcludeSelf = tls
 	return cfg
 }
 
@@ -1727,6 +1725,7 @@ func TestLoadKafkaBinary(t *testing.T) {
 func loadKafkaBinary(t *testing.T, debug bool) {
 	cfg := config.New()
 	// We don't have a way of enabling kafka without http at the moment
+	cfg.EnableGoTLSSupport = false
 	cfg.EnableKafkaMonitoring = true
 	cfg.MaxTrackedConnections = 1000
 	cfg.BPFDebug = debug

diff --git a/pkg/network/usm/monitor_test.go b/pkg/network/usm/monitor_test.go
@@ -81,6 +81,7 @@ func TestMonitorProtocolFail(t *testing.T) {
 			patchProtocolMock(t, tt.spec)
 
 			cfg := config.New()
+			cfg.EnableGoTLSSupport = false
 			cfg.EnableHTTPMonitoring = true
 			cfg.EnableIstioMonitoring = false
 

diff --git a/pkg/network/usm/monitor_tls_test.go b/pkg/network/usm/monitor_tls_test.go
@@ -69,6 +69,7 @@ func (s *tlsSuite) TestHTTPSViaLibraryIntegration() {
 	t := s.T()
 
 	cfg := config.New()
+	cfg.EnableGoTLSSupport = false
 	cfg.EnableHTTPMonitoring = true
 	cfg.EnableNativeTLSMonitoring = true
 	/* enable protocol classification : TLS */
@@ -284,6 +285,7 @@ func (s *tlsSuite) TestOpenSSLVersions() {
 	t := s.T()
 
 	cfg := config.New()
+	cfg.EnableGoTLSSupport = false
 	cfg.EnableNativeTLSMonitoring = true
 	cfg.EnableHTTPMonitoring = true
 	usmMonitor := setupUSMTLSMonitor(t, cfg)
@@ -343,6 +345,7 @@ func (s *tlsSuite) TestOpenSSLVersionsSlowStart() {
 	t := s.T()
 
 	cfg := config.New()
+	cfg.EnableGoTLSSupport = false
 	cfg.EnableNativeTLSMonitoring = true
 	cfg.EnableHTTPMonitoring = true
 
@@ -902,6 +905,7 @@ func (s *tlsSuite) TestNodeJSTLS() {
 	require.NoError(t, err)
 
 	cfg := config.New()
+	cfg.EnableGoTLSSupport = false
 	cfg.EnableHTTPMonitoring = true
 	cfg.EnableNodeJSMonitoring = true
 

diff --git a/pkg/network/usm/tests/tracer_usm_linux_test.go b/pkg/network/usm/tests/tracer_usm_linux_test.go
@@ -299,6 +299,7 @@ func (s *USMSuite) TestIgnoreTLSClassificationIfApplicationProtocolWasDetected()
 	t := s.T()
 	cfg := tracertestutil.Config()
 	cfg.ServiceMonitoringEnabled = true
+	cfg.EnableGoTLSSupport = false
 	// USM cannot be enabled without a protocol.
 	cfg.EnableHTTPMonitoring = true
 	cfg.ProtocolClassificationEnabled = true

diff --git a/pkg/network/usm/usm_http2_monitor_test.go b/pkg/network/usm/usm_http2_monitor_test.go
@@ -1511,6 +1511,7 @@ func (s *usmHTTP2Suite) TestRawHuffmanEncoding() {
 func TestHTTP2InFlightMapCleaner(t *testing.T) {
 	skipIfKernelNotSupported(t)
 	cfg := config.New()
+	cfg.EnableGoTLSSupport = false
 	cfg.EnableIstioMonitoring = false
 	cfg.EnableHTTP2Monitoring = true
 	cfg.HTTP2DynamicTableMapCleanerInterval = 5 * time.Second

diff --git a/releasenotes/notes/enable-gotls-by-default-f9c4fe517d075bcc.yaml b/releasenotes/notes/enable-gotls-by-default-f9c4fe517d075bcc.yaml
@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    USM now monitors TLS traffic encrypted with Go TLS by default.
+    To disable this feature, set the `service_monitoring_config.tls.go.enabled`
+    configuration option to false.