From 96757f0c58432f982976340c0622b17c3e10d124 Mon Sep 17 00:00:00 2001 From: jean-philippe bempel Date: Thu, 2 Apr 2020 17:54:02 +0200 Subject: [PATCH 1/3] Remove sensitive information from debug log Config.toString() method is dumped when logging in debug the conf. It includes in some case the profile api key when used with env vars. Also proxy password is also dumped. toString method generated by Lombok now excludes both fields --- .../src/main/java/datadog/trace/api/Config.java | 2 +- .../test/groovy/datadog/trace/api/ConfigTest.groovy | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/Config.java b/dd-trace-api/src/main/java/datadog/trace/api/Config.java index e9b6e9c816a..72ed152750a 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/Config.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/Config.java @@ -38,7 +38,7 @@ * system property, but uppercased with '.' -> '_'. */ @Slf4j -@ToString(includeFieldNames = true) +@ToString(includeFieldNames = true, exclude = {"profilingApiKey", "profilingProxyPassword"}) public class Config { /** Config keys below */ private static final String PREFIX = "dd."; diff --git a/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index db37f07a6a1..82e29caab61 100644 --- a/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -406,6 +406,19 @@ class ConfigTest extends DDSpecification { config.profilingApiKey == "test-api-key" } + def "sensitive information removed for toString/debug log"() { + setup: + environmentVariables.set(DD_PROFILING_API_KEY_ENV, "test-secret-api-key") + environmentVariables.set(PROFILING_PROXY_PASSWORD, "test-secret-proxy-password") + + when: + def config = new Config() + + then: + !config.toString().contains("test-secret-api-key") + !config.toString().contains("test-secret-proxy-password") + } + def "sys props override env vars"() { setup: environmentVariables.set(DD_SERVICE_NAME_ENV, "still something else") From 4d15d5ae968f0bf00df95acd7dc7b1f69bd97cf9 Mon Sep 17 00:00:00 2001 From: jean-philippe bempel Date: Thu, 2 Apr 2020 18:26:14 +0200 Subject: [PATCH 2/3] Sensor information instead of excluding the fields --- .../src/main/java/datadog/trace/api/Config.java | 10 +++++++++- .../test/groovy/datadog/trace/api/ConfigTest.groovy | 2 ++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/Config.java b/dd-trace-api/src/main/java/datadog/trace/api/Config.java index 72ed152750a..38fdc20eff7 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/Config.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/Config.java @@ -38,7 +38,7 @@ * system property, but uppercased with '.' -> '_'. */ @Slf4j -@ToString(includeFieldNames = true, exclude = {"profilingApiKey", "profilingProxyPassword"}) +@ToString(includeFieldNames = true) public class Config { /** Config keys below */ private static final String PREFIX = "dd."; @@ -202,6 +202,14 @@ public enum PropagationStyle { /** A tag intended for internal use only, hence not added to the public api DDTags class. */ private static final String INTERNAL_HOST_NAME = "_dd.hostname"; + /** Used for masking sensitive information when doing toString */ + @ToString.Include(name = "profilingApiKey") + private String profilingApiKeyMasker() { return "****"; } + + /** Used for masking sensitive information when doing toString */ + @ToString.Include(name = "profilingProxyPassword") + private String profilingProxyPasswordMasker() { return "****"; } + /** * this is a random UUID that gets generated on JVM start up and is attached to every root span * and every JMX metric that is sent out. diff --git a/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 82e29caab61..59598492885 100644 --- a/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -415,7 +415,9 @@ class ConfigTest extends DDSpecification { def config = new Config() then: + config.toString().contains("profilingApiKey=****"); !config.toString().contains("test-secret-api-key") + config.toString().contains("profilingProxyPassword=****"); !config.toString().contains("test-secret-proxy-password") } From 58ea15e590baca4aa5894cd3033b0c4894dcf759 Mon Sep 17 00:00:00 2001 From: jean-philippe bempel Date: Thu, 2 Apr 2020 19:17:26 +0200 Subject: [PATCH 3/3] if not populated returns null for toString --- dd-trace-api/src/main/java/datadog/trace/api/Config.java | 8 ++++++-- .../src/test/groovy/datadog/trace/api/ConfigTest.groovy | 5 ++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/Config.java b/dd-trace-api/src/main/java/datadog/trace/api/Config.java index 38fdc20eff7..430b01adf02 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/Config.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/Config.java @@ -204,11 +204,15 @@ public enum PropagationStyle { /** Used for masking sensitive information when doing toString */ @ToString.Include(name = "profilingApiKey") - private String profilingApiKeyMasker() { return "****"; } + private String profilingApiKeyMasker() { + return profilingApiKey != null ? "****" : null; + } /** Used for masking sensitive information when doing toString */ @ToString.Include(name = "profilingProxyPassword") - private String profilingProxyPasswordMasker() { return "****"; } + private String profilingProxyPasswordMasker() { + return profilingProxyPassword != null ? "****" : null; + } /** * this is a random UUID that gets generated on JVM start up and is attached to every root span diff --git a/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 59598492885..4eb3ef7ad34 100644 --- a/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -93,6 +93,7 @@ class ConfigTest extends DDSpecification { private static final DD_PROFILING_API_KEY_ENV = "DD_PROFILING_API_KEY" private static final DD_PROFILING_API_KEY_OLD_ENV = "DD_PROFILING_APIKEY" private static final DD_PROFILING_TAGS_ENV = "DD_PROFILING_TAGS" + private static final DD_PROFILING_PROXY_PASSWORD_ENV = "DD_PROFILING_PROXY_PASSWORD" def "verify defaults"() { when: @@ -409,7 +410,7 @@ class ConfigTest extends DDSpecification { def "sensitive information removed for toString/debug log"() { setup: environmentVariables.set(DD_PROFILING_API_KEY_ENV, "test-secret-api-key") - environmentVariables.set(PROFILING_PROXY_PASSWORD, "test-secret-proxy-password") + environmentVariables.set(DD_PROFILING_PROXY_PASSWORD_ENV, "test-secret-proxy-password") when: def config = new Config() @@ -419,6 +420,8 @@ class ConfigTest extends DDSpecification { !config.toString().contains("test-secret-api-key") config.toString().contains("profilingProxyPassword=****"); !config.toString().contains("test-secret-proxy-password") + config.getProfilingApiKey() == "test-secret-api-key" + config.getProfilingProxyPassword() == "test-secret-proxy-password" } def "sys props override env vars"() {