From 67e00f09cdb0c957acd0159fa8d7cba96226a353 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Thu, 21 Sep 2017 17:21:08 +0200 Subject: [PATCH] SONAR-8153 Do not export sensitive settings in System Info --- .../application/config/ClusterSettings.java | 3 +- .../org/sonar/process/ProcessProperties.java | 1 + .../server/authentication/JwtSerializer.java | 5 ++-- .../platform/monitoring/SettingsSection.java | 26 ++++++++++++---- .../monitoring/SettingsSectionTest.java | 30 +++++++++++++++---- 5 files changed, 51 insertions(+), 14 deletions(-) diff --git a/server/sonar-main/src/main/java/org/sonar/application/config/ClusterSettings.java b/server/sonar-main/src/main/java/org/sonar/application/config/ClusterSettings.java index e3cf57f2526..d0cb1b54ad3 100644 --- a/server/sonar-main/src/main/java/org/sonar/application/config/ClusterSettings.java +++ b/server/sonar-main/src/main/java/org/sonar/application/config/ClusterSettings.java @@ -38,6 +38,7 @@ import static java.util.Arrays.stream; import static java.util.Collections.singletonList; import static java.util.stream.Collectors.joining; import static org.apache.commons.lang.StringUtils.isBlank; +import static org.sonar.process.ProcessProperties.AUTH_JWT_SECRET; import static org.sonar.process.ProcessProperties.CLUSTER_ENABLED; import static org.sonar.process.ProcessProperties.CLUSTER_HOSTS; import static org.sonar.process.ProcessProperties.CLUSTER_NODE_HOST; @@ -72,7 +73,7 @@ public class ClusterSettings implements Consumer { switch (nodeType) { case APPLICATION: ensureNotH2(props); - requireValue(props, "sonar.auth.jwtBase64Hs256Secret"); + requireValue(props, AUTH_JWT_SECRET); break; case SEARCH: requireValue(props, SEARCH_HOST); diff --git a/server/sonar-process/src/main/java/org/sonar/process/ProcessProperties.java b/server/sonar-process/src/main/java/org/sonar/process/ProcessProperties.java index 012474d50d7..a8f2f0b3a94 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/ProcessProperties.java +++ b/server/sonar-process/src/main/java/org/sonar/process/ProcessProperties.java @@ -59,6 +59,7 @@ public class ProcessProperties { public static final String WEB_JAVA_OPTS = "sonar.web.javaOpts"; public static final String WEB_JAVA_ADDITIONAL_OPTS = "sonar.web.javaAdditionalOpts"; public static final String WEB_PORT = "sonar.web.port"; + public static final String AUTH_JWT_SECRET = "sonar.auth.jwtBase64Hs256Secret"; public static final String CE_JAVA_OPTS = "sonar.ce.javaOpts"; public static final String CE_JAVA_ADDITIONAL_OPTS = "sonar.ce.javaAdditionalOpts"; diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/JwtSerializer.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/JwtSerializer.java index 477f4f59e89..0f3d55b06fc 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/JwtSerializer.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/JwtSerializer.java @@ -45,6 +45,7 @@ import org.sonar.server.authentication.event.AuthenticationException; import static com.google.common.base.Preconditions.checkNotNull; import static io.jsonwebtoken.impl.crypto.MacProvider.generateKey; import static java.util.Objects.requireNonNull; +import static org.sonar.process.ProcessProperties.AUTH_JWT_SECRET; /** * This class can be used to encode or decode a JWT token @@ -52,8 +53,6 @@ import static java.util.Objects.requireNonNull; @ServerSide public class JwtSerializer implements Startable { - private static final String SECRET_KEY_PROPERTY = "sonar.auth.jwtBase64Hs256Secret"; - private static final SignatureAlgorithm SIGNATURE_ALGORITHM = SignatureAlgorithm.HS256; private final Configuration config; @@ -75,7 +74,7 @@ public class JwtSerializer implements Startable { @Override public void start() { - Optional encodedKey = config.get(SECRET_KEY_PROPERTY); + Optional encodedKey = config.get(AUTH_JWT_SECRET); if (encodedKey.isPresent()) { this.secretKey = decodeSecretKeyProperty(encodedKey.get()); } else { diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/monitoring/SettingsSection.java b/server/sonar-server/src/main/java/org/sonar/server/platform/monitoring/SettingsSection.java index e6e3a473635..961938a46d2 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/monitoring/SettingsSection.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/monitoring/SettingsSection.java @@ -30,12 +30,16 @@ import org.sonar.process.systeminfo.SystemInfoSection; import org.sonar.process.systeminfo.protobuf.ProtobufSystemInfo; import static org.apache.commons.lang.StringUtils.abbreviate; +import static org.apache.commons.lang.StringUtils.containsIgnoreCase; +import static org.apache.commons.lang.StringUtils.endsWithIgnoreCase; +import static org.sonar.process.ProcessProperties.AUTH_JWT_SECRET; import static org.sonar.process.systeminfo.SystemInfoUtils.setAttribute; @ServerSide public class SettingsSection implements SystemInfoSection, Global { - static final int MAX_VALUE_LENGTH = 500; + private static final int MAX_VALUE_LENGTH = 500; + private static final String PASSWORD_VALUE = "xxxxxxxx"; private final Settings settings; public SettingsSection(Settings settings) { @@ -50,11 +54,23 @@ public class SettingsSection implements SystemInfoSection, Global { PropertyDefinitions definitions = settings.getDefinitions(); for (Map.Entry prop : settings.getProperties().entrySet()) { String key = prop.getKey(); - PropertyDefinition def = definitions.get(key); - if (def == null || def.type() != PropertyType.PASSWORD) { - setAttribute(protobuf, key, abbreviate(prop.getValue(), MAX_VALUE_LENGTH)); - } + String value = obfuscateValue(definitions, key, prop.getValue()); + setAttribute(protobuf, key, value); } return protobuf.build(); } + + private static String obfuscateValue(PropertyDefinitions definitions, String key, String value) { + PropertyDefinition def = definitions.get(key); + if (def != null && def.type() == PropertyType.PASSWORD) { + return PASSWORD_VALUE; + } + if (endsWithIgnoreCase(key, ".secured") || + containsIgnoreCase(key, "password") || + containsIgnoreCase(key, "passcode") || + AUTH_JWT_SECRET.equals(key)) { + return PASSWORD_VALUE; + } + return abbreviate(value, MAX_VALUE_LENGTH); + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/platform/monitoring/SettingsSectionTest.java b/server/sonar-server/src/test/java/org/sonar/server/platform/monitoring/SettingsSectionTest.java index b5f5c14dadc..dc6c9c1b16c 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/platform/monitoring/SettingsSectionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/platform/monitoring/SettingsSectionTest.java @@ -56,20 +56,40 @@ public class SettingsSectionTest { ProtobufSystemInfo.Section protobuf = underTest.toProtobuf(); String value = attribute(protobuf, "foo").getStringValue(); - assertThat(value).hasSize(SettingsSection.MAX_VALUE_LENGTH).startsWith("abcde"); + assertThat(value).hasSize(500).startsWith("abcde"); } @Test - public void exclude_password_properties() { - settings.setProperty(PASSWORD_PROPERTY, "abcde"); + public void value_is_obfuscated_if_key_matches_patterns() { + verifyObfuscated(PASSWORD_PROPERTY); + verifyObfuscated("foo.password.something"); + // case insensitive search of "password" term + verifyObfuscated("bar.CheckPassword"); + verifyObfuscated("foo.passcode.something"); + // case insensitive search of "passcode" term + verifyObfuscated("bar.CheckPassCode"); + verifyObfuscated("foo.something.secured"); + verifyObfuscated("bar.something.Secured"); + verifyObfuscated("sonar.auth.jwtBase64Hs256Secret"); + verifyNotObfuscated("securedStuff"); + verifyNotObfuscated("foo"); + } + + private void verifyObfuscated(String key) { + settings.setProperty(key, "foo"); + ProtobufSystemInfo.Section protobuf = underTest.toProtobuf(); + assertThatAttributeIs(protobuf, key, "xxxxxxxx"); + } + + private void verifyNotObfuscated(String key) { + settings.setProperty(key, "foo"); ProtobufSystemInfo.Section protobuf = underTest.toProtobuf(); - assertThat(attribute(protobuf, PASSWORD_PROPERTY)).isNull(); + assertThatAttributeIs(protobuf, key, "foo"); } @Test public void test_monitor_name() { assertThat(underTest.toProtobuf().getName()).isEqualTo("Settings"); - } } -- 2.39.5