Store passwords in arrays and clear after use

Do not store passwords in memory as String objects, since the
contents of the internal array will survive until garbage collected.

- Change credentials fields of AuthToken map to UTF8 encoded byte[]
- Clear credentials immediately after use by overwriting with zeroes
- Change UserManager interface methods to take password as byte[]
- Convert to char[] in plugin auth token

community/bolt/src/main/java/org/neo4j/bolt/security/auth/BasicAuthentication.java

@@ -91,16 +91,18 @@ private AuthenticationResult update( Map<String,Object> authToken )
     {
         try
         {
+            // We need to copy the new password here since it will be cleared by login()
+            byte[] newPassword = AuthToken.safeCastCredentials( NEW_CREDENTIALS, authToken ).clone();
+
             LoginContext loginContext = authManager.login( authToken );
 
             switch ( loginContext.subject().getAuthenticationResult() )
             {
             case SUCCESS:
             case PASSWORD_CHANGE_REQUIRED:
-                String newPassword = AuthToken.safeCast( NEW_CREDENTIALS, authToken );
                 String username = AuthToken.safeCast( PRINCIPAL, authToken );
                 userManagerSupplier.getUserManager( loginContext.subject(), false )
-                        .setUserPassword( username, newPassword, false );
+                        .setUserPassword( username, newPassword, false ); // NOTE: This will overwrite newPassword with zeroes
                 loginContext.subject().setPasswordChangeNoLongerRequired();
                 break;
             default:

community/bolt/src/test/java/org/neo4j/bolt/security/auth/BasicAuthenticationTest.java

@@ -36,6 +36,7 @@
 import org.neo4j.server.security.auth.BasicAuthManager;
 import org.neo4j.server.security.auth.InMemoryUserRepository;
 import org.neo4j.server.security.auth.UserRepository;
+import org.neo4j.string.UTF8;
 import org.neo4j.time.Clocks;
 
 import static java.util.Collections.singletonList;
@@ -57,7 +58,7 @@ public void shouldNotDoAnythingOnSuccess() throws Exception
     {
         // When
         AuthenticationResult result =
-                authentication.authenticate( map( "scheme", "basic", "principal", "mike", "credentials", "secret2" ) );
+                authentication.authenticate( map( "scheme", "basic", "principal", "mike", "credentials", UTF8.encode( "secret2" ) ) );
 
         // Then
         assertThat( result.getLoginContext().subject().username(), equalTo( "mike" ) );
@@ -72,15 +73,15 @@ public void shouldThrowAndLogOnFailure() throws Exception
         exception.expectMessage( "The client is unauthorized due to authentication failure." );
 
         // When
-        authentication.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", "banana" ) );
+        authentication.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", UTF8.encode( "banana" ) ) );
     }
 
     @Test
     public void shouldIndicateThatCredentialsExpired() throws Exception
     {
         // When
         AuthenticationResult result =
-                authentication.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", "secret" ) );
+                authentication.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", UTF8.encode( "secret" ) ) );
 
         // Then
         assertTrue( result.credentialsExpired() );
@@ -97,7 +98,7 @@ public void shouldFailWhenTooManyAttempts() throws Exception
         {
             try
             {
-                auth.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", "gelato" ) );
+                auth.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", UTF8.encode( "gelato" ) ) );
             }
             catch ( AuthenticationException e )
             {
@@ -111,26 +112,27 @@ public void shouldFailWhenTooManyAttempts() throws Exception
         exception.expectMessage( "The client has provided incorrect authentication details too many times in a row." );
 
         //When
-        auth.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", "gelato" ) );
+        auth.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", UTF8.encode( "gelato" ) ) );
     }
 
     @Test
     public void shouldBeAbleToUpdateCredentials() throws Exception
     {
         // When
         authentication.authenticate(
-                map( "scheme", "basic", "principal", "mike", "credentials", "secret2", "new_credentials", "secret" ) );
+                map( "scheme", "basic", "principal", "mike", "credentials", UTF8.encode( "secret2" ),
+                        "new_credentials", UTF8.encode( "secret" ) ) );
 
         // Then
-        authentication.authenticate( map( "scheme", "basic", "principal", "mike", "credentials", "secret" ) );
+        authentication.authenticate( map( "scheme", "basic", "principal", "mike", "credentials", UTF8.encode( "secret" ) ) );
     }
 
     @Test
     public void shouldBeAbleToUpdateExpiredCredentials() throws Exception
     {
         // When
         AuthenticationResult result = authentication.authenticate(
-                map( "scheme", "basic", "principal", "bob", "credentials", "secret", "new_credentials", "secret2" ) );
+                map( "scheme", "basic", "principal", "bob", "credentials", UTF8.encode( "secret" ), "new_credentials", UTF8.encode( "secret2" ) ) );
 
         // Then
         assertThat(result.credentialsExpired(), equalTo( false ));
@@ -145,8 +147,8 @@ public void shouldNotBeAbleToUpdateCredentialsIfOldCredentialsAreInvalid() throw
         exception.expectMessage( "The client is unauthorized due to authentication failure." );
 
         // When
-        authentication.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", "gelato",
-                "new_credentials", "secret2" ) );
+        authentication.authenticate( map( "scheme", "basic", "principal", "bob", "credentials", UTF8.encode( "gelato" ),
+                "new_credentials", UTF8.encode( "secret2" ) ) );
     }
 
     @Test
@@ -157,7 +159,7 @@ public void shouldThrowWithNoScheme() throws Exception
         exception.expect( hasStatus( Status.Security.Unauthorized ) );
 
         // When
-        authentication.authenticate( map( "principal", "bob", "credentials", "secret" ) );
+        authentication.authenticate( map( "principal", "bob", "credentials", UTF8.encode( "secret" ) ) );
     }
 
     @Test
@@ -182,7 +184,7 @@ public void shouldFailOnMalformedToken() throws Exception
 
         // When
         authentication
-                .authenticate( map( "scheme", "basic", "principal", singletonList( "bob" ), "credentials", "secret" ) );
+                .authenticate( map( "scheme", "basic", "principal", singletonList( "bob" ), "credentials", UTF8.encode( "secret" ) ) );
     }
 
     @Before
@@ -200,8 +202,8 @@ private static Authentication createAuthentication( int maxFailedAttempts ) thro
 
         BasicAuthManager manager = new BasicAuthManager( users, policy, Clocks.systemClock(), users, config );
         Authentication authentication = new BasicAuthentication( manager, manager );
-        manager.newUser( "bob", "secret", true );
-        manager.newUser( "mike", "secret2", false );
+        manager.newUser( "bob", UTF8.encode( "secret" ), true );
+        manager.newUser( "mike", UTF8.encode( "secret2" ), false );
 
         return authentication;
     }

community/community-it/community-it/src/test/java/org/neo4j/server/security/auth/AuthProceduresIT.java

@@ -46,6 +46,7 @@
 import org.neo4j.kernel.api.exceptions.InvalidArgumentsException;
 import org.neo4j.kernel.api.security.exception.InvalidAuthTokenException;
 import org.neo4j.kernel.internal.GraphDatabaseAPI;
+import org.neo4j.string.UTF8;
 import org.neo4j.test.TestGraphDatabaseBuilder;
 import org.neo4j.test.TestGraphDatabaseFactory;
 
@@ -109,7 +110,7 @@ public void shouldNotChangeOwnPasswordIfNewPasswordInvalid()
     public void newUserShouldBeAbleToChangePassword() throws Throwable
     {
         // Given
-        authManager.newUser( "andres", "banana", true );
+        authManager.newUser( "andres", UTF8.encode( "banana" ), true );
 
         // Then
         assertEmpty( login("andres", "banana"), "CALL dbms.changePassword('abc')" );
@@ -119,7 +120,7 @@ public void newUserShouldBeAbleToChangePassword() throws Throwable
     public void newUserShouldNotBeAbleToCallOtherProcedures() throws Throwable
     {
         // Given
-        authManager.newUser( "andres", "banana", true );
+        authManager.newUser( "andres", UTF8.encode( "banana" ), true );
         LoginContext user = login("andres", "banana");
 
         // Then
@@ -200,7 +201,7 @@ public void shouldNotCreateExistingUser()
     @Test
     public void shouldDeleteUser() throws Exception
     {
-        authManager.newUser( "andres", "123", false );
+        authManager.newUser( "andres", UTF8.encode( "123" ), false );
         assertEmpty( admin, "CALL dbms.security.deleteUser('andres')" );
         try
         {
@@ -228,15 +229,15 @@ public void shouldNotDeleteNonExistentUser()
     @Test
     public void shouldListUsers() throws Exception
     {
-        authManager.newUser( "andres", "123", false );
+        authManager.newUser( "andres", UTF8.encode( "123" ), false );
         assertSuccess( admin, "CALL dbms.security.listUsers() YIELD username",
                 r -> assertKeyIs( r, "username", "neo4j", "andres" ) );
     }
 
     @Test
     public void shouldReturnUsersWithFlags() throws Exception
     {
-        authManager.newUser( "andres", "123", false );
+        authManager.newUser( "andres", UTF8.encode( "123" ), false );
         Map<String,Object> expected = map(
                 "neo4j", listOf( PWD_CHANGE ),
                 "andres", listOf()
@@ -251,7 +252,7 @@ public void shouldShowCurrentUser() throws Exception
         assertSuccess( admin, "CALL dbms.showCurrentUser()",
                 r -> assertKeyIsMap( r, "username", "flags", map( "neo4j", listOf( PWD_CHANGE ) ) ) );
 
-        authManager.newUser( "andres", "123", false );
+        authManager.newUser( "andres", UTF8.encode( "123" ), false );
         LoginContext andres = login( "andres", "123" );
         assertSuccess( andres, "CALL dbms.showCurrentUser()",
                 r -> assertKeyIsMap( r, "username", "flags", map( "andres", listOf() ) ) );

community/kernel/src/main/java/org/neo4j/kernel/api/security/AuthManager.java

@@ -32,6 +32,9 @@ public interface AuthManager extends Lifecycle
 {
     /**
      * Log in using the provided authentication token
+     *
+     * NOTE: The authToken will be cleared of any credentials
+     *
      * @param authToken The authentication token to login with. Typically contains principals and credentials.
      * @return An AuthSubject representing the newly logged-in user
      * @throws InvalidAuthTokenException if the authentication token is malformed
@@ -66,6 +69,7 @@ public void shutdown()
         @Override
         public LoginContext login( Map<String,Object> authToken )
         {
+            AuthToken.clearCredentials( authToken );
             return LoginContext.AUTH_DISABLED;
         }
     };

community/kernel/src/main/java/org/neo4j/kernel/api/security/AuthToken.java

@@ -21,10 +21,12 @@
 
 import org.apache.commons.lang3.StringUtils;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.Map;
 
 import org.neo4j.kernel.api.security.exception.InvalidAuthTokenException;
+import org.neo4j.string.UTF8;
 
 import static java.lang.String.format;
 import static org.neo4j.helpers.collection.MapUtil.map;
@@ -55,6 +57,21 @@ else if ( !(value instanceof String) )
         return (String) value;
     }
 
+    static byte[] safeCastCredentials( String key, Map<String,Object> authToken ) throws InvalidAuthTokenException
+    {
+        Object value = authToken.get( key );
+        if ( value == null )
+        {
+            throw invalidToken( "missing key `" + key + "`" );
+        }
+        else if ( !(value instanceof byte[]) )
+        {
+            throw invalidToken( "the value associated with the key `" + key + "` must be a byte[] but was: "
+                    + value.getClass().getSimpleName() );
+        }
+        return (byte[]) value;
+    }
+
     static Map<String,Object> safeCastMap( String key, Map<String,Object> authToken )
             throws InvalidAuthTokenException
     {
@@ -75,6 +92,21 @@ else if ( value instanceof Map )
         }
     }
 
+    static void clearCredentials( Map<String,Object> authToken )
+    {
+        Object credentials = authToken.get( CREDENTIALS );
+        if ( credentials != null && credentials instanceof byte[] )
+        {
+            Arrays.fill( (byte[]) credentials, (byte) 0 );
+        }
+
+        Object newCredentials = authToken.get( NEW_CREDENTIALS );
+        if ( newCredentials != null && newCredentials instanceof byte[] )
+        {
+            Arrays.fill( (byte[]) newCredentials, (byte) 0 );
+        }
+    }
+
     static InvalidAuthTokenException invalidToken( String explanation )
     {
         if ( StringUtils.isNotEmpty( explanation ) && !explanation.matches( "^[,.:;].*" ) )
@@ -84,28 +116,54 @@ static InvalidAuthTokenException invalidToken( String explanation )
         return new InvalidAuthTokenException( format( "Unsupported authentication token%s", explanation ) );
     }
 
-    static Map<String,Object> newBasicAuthToken( String username, String password )
+    static Map<String,Object> newBasicAuthToken( String username, byte[] password )
     {
         return map( AuthToken.SCHEME_KEY, BASIC_SCHEME, AuthToken.PRINCIPAL, username, AuthToken.CREDENTIALS,
                 password );
     }
 
-    static Map<String,Object> newBasicAuthToken( String username, String password, String realm )
+    static Map<String,Object> newBasicAuthToken( String username, byte[] password, String realm )
     {
         return map( AuthToken.SCHEME_KEY, BASIC_SCHEME, AuthToken.PRINCIPAL, username, AuthToken.CREDENTIALS, password,
                 AuthToken.REALM_KEY, realm );
     }
 
-    static Map<String,Object> newCustomAuthToken( String principle, String credentials, String realm, String scheme )
+    static Map<String,Object> newCustomAuthToken( String principle, byte[] credentials, String realm, String scheme )
     {
         return map( AuthToken.SCHEME_KEY, scheme, AuthToken.PRINCIPAL, principle, AuthToken.CREDENTIALS, credentials,
                 AuthToken.REALM_KEY, realm );
     }
 
-    static Map<String,Object> newCustomAuthToken( String principle, String credentials, String realm, String scheme,
+    static Map<String,Object> newCustomAuthToken( String principle, byte[] credentials, String realm, String scheme,
             Map<String,Object> parameters )
     {
         return map( AuthToken.SCHEME_KEY, scheme, AuthToken.PRINCIPAL, principle, AuthToken.CREDENTIALS, credentials,
                 AuthToken.REALM_KEY, realm, AuthToken.PARAMETERS, parameters );
     }
+
+    // For testing purposes only
+    static Map<String,Object> newBasicAuthToken( String username, String password )
+    {
+        return newBasicAuthToken( username, UTF8.encode( password ) );
+    }
+
+    // For testing purposes only
+    static Map<String,Object> newBasicAuthToken( String username, String password, String realm )
+    {
+        return newBasicAuthToken( username, UTF8.encode( password ), realm );
+    }
+
+    // For testing purposes only
+    static Map<String,Object> newCustomAuthToken( String principle, String credentials, String realm, String scheme )
+    {
+        return newCustomAuthToken( principle, UTF8.encode( credentials ), realm, scheme );
+    }
+
+    // For testing purposes only
+    static Map<String,Object> newCustomAuthToken( String principle, String credentials, String realm, String scheme,
+            Map<String,Object> parameters )
+    {
+        return newCustomAuthToken( principle, UTF8.encode( credentials ), realm, scheme, parameters );
+    }
+
 }

community/kernel/src/main/java/org/neo4j/kernel/api/security/PasswordPolicy.java

@@ -24,5 +24,5 @@
 public interface PasswordPolicy
 {
     // TODO: We may want to reintroduce AuthSubject here to be able to check against repeating last used passwords etc.
-    void validatePassword( String password ) throws InvalidArgumentsException;
+    void validatePassword( byte[] password ) throws InvalidArgumentsException;
 }

community/kernel/src/main/java/org/neo4j/kernel/api/security/UserManager.java

@@ -20,6 +20,7 @@
 package org.neo4j.kernel.api.security;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.Set;
 
 import org.neo4j.kernel.api.exceptions.InvalidArgumentsException;
@@ -30,7 +31,10 @@ public interface UserManager
     String INITIAL_USER_NAME = "neo4j";
     String INITIAL_PASSWORD = "neo4j";
 
-    User newUser( String username, String initialPassword, boolean requirePasswordChange )
+    /**
+     * NOTE: The initialPassword byte array will be cleared (overwritten with zeroes)
+     */
+    User newUser( String username, byte[] initialPassword, boolean requirePasswordChange )
             throws IOException, InvalidArgumentsException;
 
     boolean deleteUser( String username ) throws IOException, InvalidArgumentsException;
@@ -39,16 +43,20 @@ User newUser( String username, String initialPassword, boolean requirePasswordCh
 
     User silentlyGetUser( String username );
 
-    void setUserPassword( String username, String password, boolean requirePasswordChange )
+    /**
+     * NOTE: The password byte array will be cleared (overwritten with zeroes)
+     */
+    void setUserPassword( String username, byte[] password, boolean requirePasswordChange )
             throws IOException, InvalidArgumentsException;
 
     Set<String> getAllUsernames();
 
     UserManager NO_AUTH = new UserManager()
     {
         @Override
-        public User newUser( String username, String initialPassword, boolean requirePasswordChange )
+        public User newUser( String username, byte[] initialPassword, boolean requirePasswordChange )
         {
+            Arrays.fill( initialPassword, (byte) 0 );
             return null;
         }
 
@@ -71,8 +79,9 @@ public User silentlyGetUser( String username )
         }
 
         @Override
-        public void setUserPassword( String username, String password, boolean requirePasswordChange )
+        public void setUserPassword( String username, byte[] password, boolean requirePasswordChange )
         {
+            Arrays.fill( password, (byte) 0 );
         }
 
         @Override