Obsfucate the LDAP system password in toString and debug.log

community/configuration/src/main/java/org/neo4j/configuration/ConfigOptions.java

@@ -61,7 +61,7 @@ public List<ConfigValue> asConfigValues( @Nonnull Map<String,String> validConfig
                             setting.documentedDefaultValue(),
                         Optional.ofNullable( val.getValue() ),
                             setting.valueDescription(), setting.internal(), setting.dynamic(),
-                            setting.deprecated(), setting.replacement() );
+                            setting.deprecated(), setting.replacement(), setting.isSecret() );
                 } )
                 .collect( Collectors.toList() );
     }

community/configuration/src/main/java/org/neo4j/configuration/ConfigValue.java

@@ -34,21 +34,23 @@ public class ConfigValue
     private final Optional<?> value;
     private final String valueDescription;
     private final boolean internal;
+    private final boolean isSecret;
     private final boolean dynamic;
     private final boolean deprecated;
     private final Optional<String> replacement;
 
     public ConfigValue( @Nonnull String name, @Nonnull Optional<String> description,
             @Nonnull Optional<String> documentedDefaultValue, @Nonnull Optional<?> value,
             @Nonnull String valueDescription, boolean internal, boolean dynamic, boolean deprecated,
-            @Nonnull Optional<String> replacement )
+            @Nonnull Optional<String> replacement, boolean isSecret )
     {
         this.name = name;
         this.description = description;
         this.documentedDefaultValue = documentedDefaultValue;
         this.value = value;
         this.valueDescription = valueDescription;
         this.internal = internal;
+        this.isSecret = isSecret;
         this.dynamic = dynamic;
         this.deprecated = deprecated;
         this.replacement = replacement;
@@ -100,6 +102,11 @@ public boolean internal()
         return internal;
     }
 
+    public boolean isSecret()
+    {
+        return isSecret;
+    }
+
     public boolean dynamic()
     {
         return dynamic;

community/configuration/src/main/java/org/neo4j/configuration/LoadableConfig.java

@@ -76,6 +76,9 @@ default List<ConfigOptions> getConfigOptions()
                     final Internal internalAnnotation = f.getAnnotation( Internal.class );
                     setting.setInternal( internalAnnotation != null );
 
+                    final Secret secretAnnotation = f.getAnnotation( Secret.class );
+                    setting.makeSecret( secretAnnotation != null );
+
                     final Dynamic dynamicAnnotation = f.getAnnotation( Dynamic.class );
                     setting.setDynamic( dynamicAnnotation != null );
                 }

community/configuration/src/main/java/org/neo4j/configuration/Secret.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2002-2018 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.configuration;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Used to define a configuration setting as secret, which means we should not show the value in logs.
+ */
+@Retention( RetentionPolicy.RUNTIME )
+@Target( {ElementType.TYPE, ElementType.FIELD} )
+public @interface Secret
+{
+    String OBSFUCATED = "##########";
+}

community/configuration/src/test/java/org/neo4j/configuration/ConfigValueTest.java

@@ -35,7 +35,7 @@ public class ConfigValueTest
     public void handlesEmptyValue() throws Exception
     {
         ConfigValue value = new ConfigValue( "name", Optional.empty(), Optional.empty(), Optional.empty(),
-                "description", false, false, false, Optional.empty() );
+                "description", false, false, false, Optional.empty(), false );
 
         assertEquals( Optional.empty(), value.value() );
         assertEquals( "null", value.toString() );
@@ -48,8 +48,7 @@ public void handlesEmptyValue() throws Exception
     public void handlesInternal() throws Exception
     {
         ConfigValue value = new ConfigValue( "name", Optional.empty(), Optional.empty(), Optional.empty(),
-                "description", true, false, false,
-                Optional.empty() );
+                "description", true, false, false, Optional.empty(), false );
 
         assertTrue( value.internal() );
     }
@@ -58,7 +57,7 @@ public void handlesInternal() throws Exception
     public void handlesNonEmptyValue() throws Exception
     {
         ConfigValue value = new ConfigValue( "name", Optional.empty(), Optional.empty(), Optional.of( 1 ),
-                "description", false, false, false, Optional.empty() );
+                "description", false, false, false, Optional.empty(), false );
 
         assertEquals( Optional.of( 1 ), value.value() );
         assertEquals( "1", value.toString() );
@@ -71,8 +70,7 @@ public void handlesNonEmptyValue() throws Exception
     public void handlesDeprecationAndReplacement() throws Exception
     {
         ConfigValue value = new ConfigValue( "old_name", Optional.empty(), Optional.empty(), Optional.of( 1 ),
-                "description", false, false, true,
-                Optional.of( "new_name" ) );
+                "description", false, false, true, Optional.of( "new_name" ), false );
 
         assertEquals( Optional.of( 1 ), value.value() );
         assertEquals( "1", value.toString() );
@@ -85,8 +83,7 @@ public void handlesDeprecationAndReplacement() throws Exception
     public void handlesValueDescription() throws Exception
     {
         ConfigValue value = new ConfigValue( "old_name", Optional.empty(), Optional.empty(), Optional.of( 1 ),
-                "a simple integer", false, false, true,
-                Optional.of( "new_name" ) );
+                "a simple integer", false, false, true, Optional.of( "new_name" ), false );
 
         assertEquals( Optional.of( 1 ), value.value() );
         assertEquals( "1", value.toString() );

community/graphdb-api/src/main/java/org/neo4j/graphdb/config/BaseSetting.java

@@ -31,6 +31,7 @@ public abstract class BaseSetting<T> implements Setting<T>
     private boolean deprecated;
     private String replacement;
     private boolean internal;
+    private boolean isSecret;
     private boolean dynamic;
     private String documentedDefaultValue;
     private String description;
@@ -68,6 +69,17 @@ public void setInternal( final boolean val )
         this.internal = val;
     }
 
+    @Override
+    public boolean isSecret()
+    {
+        return this.isSecret;
+    }
+
+    public void makeSecret( final boolean val )
+    {
+        this.isSecret = val;
+    }
+
     @Override
     public Optional<String> documentedDefaultValue()
     {

community/graphdb-api/src/main/java/org/neo4j/graphdb/config/SettingGroup.java

@@ -60,6 +60,11 @@ public interface SettingGroup<T> extends SettingValidator
      */
     boolean internal();
 
+    /**
+     * @return {@code true} if secret setting (should be hidden), false otherwise.
+     */
+    boolean isSecret();
+
     /**
      * @return the documented default value if it needs special documentation, empty if default value is good as is.
      */

community/kernel/src/main/java/org/neo4j/kernel/configuration/Config.java

@@ -26,6 +26,7 @@
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -44,10 +45,12 @@
 import org.neo4j.configuration.ConfigOptions;
 import org.neo4j.configuration.ConfigValue;
 import org.neo4j.configuration.LoadableConfig;
+import org.neo4j.configuration.Secret;
 import org.neo4j.graphdb.config.BaseSetting;
 import org.neo4j.graphdb.config.Configuration;
 import org.neo4j.graphdb.config.InvalidSettingException;
 import org.neo4j.graphdb.config.Setting;
+import org.neo4j.graphdb.config.SettingGroup;
 import org.neo4j.graphdb.config.SettingValidator;
 import org.neo4j.graphdb.factory.GraphDatabaseSettings;
 import org.neo4j.helpers.collection.MapUtil;
@@ -82,6 +85,7 @@ public class Config implements DiagnosticsProvider, Configuration
     private final List<ConfigOptions> configOptions;
 
     private final Map<String,String> params = new CopyOnWriteHashMap<>(); // Read heavy workload
+    private final Set<String> secrets = new HashSet<>();
     private final Map<String, Collection<BiConsumer<String,String>>> updateListeners = new ConcurrentHashMap<>();
     private final ConfigurationMigrator migrator;
     private final List<ConfigurationValidator> validators = new ArrayList<>();
@@ -390,6 +394,14 @@ private Config( File configFile,
                 .map( BaseSetting.class::cast )
                 .forEach( setting -> settingsMap.put( setting.name(), setting ) );
 
+        // Find secret settings
+        configOptions.stream()
+                .map( ConfigOptions::settingGroup )
+                .filter( SettingGroup::isSecret )
+                .filter( BaseSetting.class::isInstance )
+                .map( BaseSetting.class::cast )
+                .forEach( setting -> secrets.add( setting.name() ) );
+
         validators.addAll( additionalValidators );
         migrator = new AnnotationBasedConfigurationMigrator( settingsClasses );
         this.overriddenDefaults.putAll( overriddenDefaults );
@@ -741,11 +753,16 @@ public void dump( DiagnosticsPhase phase, Logger logger )
             logger.log( "Neo4j Kernel properties:" );
             for ( Map.Entry<String,String> param : params.entrySet() )
             {
-                logger.log( "%s=%s", param.getKey(), param.getValue() );
+                logger.log( "%s=%s", param.getKey(), obsfucateIfSecret( param ) );
             }
         }
     }
 
+    private String obsfucateIfSecret( Map.Entry<String,String> param )
+    {
+        return secrets.contains( param.getKey() ) ? Secret.OBSFUCATED : param.getValue();
+    }
+
     /**
      * Migrates and validates all string values in the provided <code>settings</code> map.
      *
@@ -942,7 +959,7 @@ public String toString()
     {
         return params.entrySet().stream()
                 .sorted( Comparator.comparing( Map.Entry::getKey ) )
-                .map( entry -> entry.getKey() + "=" + entry.getValue() )
+                .map( entry -> entry.getKey() + "=" + obsfucateIfSecret( entry ) )
                 .collect( Collectors.joining( ", ") );
     }
 }

community/kernel/src/main/java/org/neo4j/kernel/configuration/ConnectorValidator.java

@@ -228,6 +228,12 @@ public boolean internal()
         return false;
     }
 
+    @Override
+    public boolean isSecret()
+    {
+        return false;
+    }
+
     @Override
     public Optional<String> documentedDefaultValue()
     {

community/kernel/src/main/java/org/neo4j/kernel/configuration/Settings.java

@@ -1227,6 +1227,12 @@ public boolean internal()
                 return newSetting.internal();
             }
 
+            @Override
+            public boolean isSecret()
+            {
+                return newSetting.isSecret();
+            }
+
             @Override
             public Optional<String> documentedDefaultValue()
             {

community/kernel/src/main/java/org/neo4j/kernel/configuration/ssl/SslPolicyConfigValidator.java

@@ -157,6 +157,12 @@ public boolean internal()
         return false;
     }
 
+    @Override
+    public boolean isSecret()
+    {
+        return false;
+    }
+
     @Override
     public Optional<String> documentedDefaultValue()
     {

community/kernel/src/test/java/org/neo4j/kernel/configuration/ConfigTest.java

@@ -39,6 +39,7 @@
 import org.neo4j.configuration.Internal;
 import org.neo4j.configuration.LoadableConfig;
 import org.neo4j.configuration.ReplacedBy;
+import org.neo4j.configuration.Secret;
 import org.neo4j.graphdb.config.InvalidSettingException;
 import org.neo4j.graphdb.config.Setting;
 import org.neo4j.graphdb.factory.GraphDatabaseSettings;
@@ -101,6 +102,9 @@ public static class MySettingsWithDefaults implements LoadableConfig
 
         @Deprecated
         public static final Setting<String> oldSetting = setting( "some_setting", STRING, "Has no replacement" );
+
+        @Secret
+        public static final Setting<String> password = setting( "password", STRING, "This text should not appear in logs or toString" );
     }
 
     private static class HelloHasToBeNeo4jConfigurationValidator implements ConfigurationValidator
@@ -249,16 +253,35 @@ public void shouldSetInternalParameter()
     {
         // Given
         Config config = Config.builder()
-                              .withSetting( MySettingsWithDefaults.secretSetting, "false" )
-                              .withSetting( MySettingsWithDefaults.hello, "ABC" )
-                              .withConfigClasses( Arrays.asList( mySettingsWithDefaults, myMigratingSettings ) )
-                              .build();
+                .withSetting( MySettingsWithDefaults.secretSetting, "false" )
+                .withSetting( MySettingsWithDefaults.hello, "ABC" )
+                .withConfigClasses( Arrays.asList( mySettingsWithDefaults, myMigratingSettings ) )
+                .build();
 
         // Then
         assertTrue( config.getConfigValues().get( MySettingsWithDefaults.secretSetting.name() ).internal() );
         assertFalse( config.getConfigValues().get( MySettingsWithDefaults.hello.name() ).internal() );
     }
 
+    @Test
+    public void shouldSetSecretParameter()
+    {
+        // Given
+        Config config = Config.builder()
+                .withSetting( MySettingsWithDefaults.password, "this should not be visible" )
+                .withSetting( MySettingsWithDefaults.hello, "ABC" )
+                .withConfigClasses( Arrays.asList( mySettingsWithDefaults, myMigratingSettings ) )
+                .build();
+
+        // Then
+        assertTrue( config.getConfigValues().get( MySettingsWithDefaults.password.name() ).isSecret() );
+        assertFalse( config.getConfigValues().get( MySettingsWithDefaults.hello.name() ).isSecret() );
+        String configText = config.toString();
+        assertTrue( configText.contains( Secret.OBSFUCATED ) );
+        assertFalse( configText.contains( "this should not be visible" ) );
+        assertFalse( configText.contains( config.get( MySettingsWithDefaults.password ) ) );
+    }
+
     @Test
     public void shouldSetDocumentedDefaultValue()
             throws Exception

enterprise/security/src/main/java/org/neo4j/server/security/enterprise/configuration/SecuritySettings.java

@@ -27,6 +27,7 @@
 import org.neo4j.configuration.Description;
 import org.neo4j.configuration.Internal;
 import org.neo4j.configuration.LoadableConfig;
+import org.neo4j.configuration.Secret;
 import org.neo4j.graphdb.config.Setting;
 import org.neo4j.graphdb.factory.GraphDatabaseSettings;
 import org.neo4j.logging.Level;
@@ -225,6 +226,7 @@ public class SecuritySettings implements LoadableConfig
     public static final Setting<String> ldap_authorization_system_username =
             setting( "dbms.security.ldap.authorization.system_username", STRING, NO_DEFAULT );
 
+    @Secret
     @Description(
             "An LDAP system account password to use for authorization searches when " +
             "`dbms.security.ldap.authorization.use_system_account` is `true`." )