Merge pull request #543 from hivemq/improvement/clear-auth-password-2…

…6231/latest dependent on simple authentication and internal config, the client password is now cleared or kept.
hivemq · Oct 28, 2024 · ac6ba24 · ac6ba24
2 parents 102b73e + dcfaaef
commit ac6ba24
Show file tree

Hide file tree

Showing 9 changed files with 136 additions and 0 deletions.
diff --git a/src/main/java/com/hivemq/bootstrap/ClientConnection.java b/src/main/java/com/hivemq/bootstrap/ClientConnection.java
@@ -40,6 +40,7 @@
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.nio.ByteBuffer;
+import java.util.Arrays;
 import java.util.Optional;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -93,6 +94,7 @@ public class ClientConnection implements ClientConnectionContext {
     private @Nullable ByteBuffer authData;
     private @Nullable Mqtt5UserProperties authUserProperties;
     private @Nullable ScheduledFuture<?> authFuture;
+    private @Nullable Boolean clearPasswordAfterAuth;
 
     private @Nullable ClientContextImpl extensionClientContext;
     private @Nullable ClientEventListeners extensionClientEventListeners;
@@ -770,4 +772,23 @@ public int getMaxInflightWindow(final int defaultMaxInflightWindow) {
 
         return Optional.empty();
     }
+
+    @Override
+    public void setClearPasswordAfterAuth(final @Nullable Boolean clearPasswordAfterAuth) {
+        this.clearPasswordAfterAuth = clearPasswordAfterAuth;
+    }
+
+    @Override
+    public @NotNull Optional<Boolean> isClearPasswordAfterAuth() {
+        return Optional.ofNullable(clearPasswordAfterAuth);
+    }
+
+    @Override
+    public void clearPassword(){
+        if(authPassword == null) {
+            return;
+        }
+        Arrays.fill(authPassword, (byte) 0);
+        authPassword = null;
+    }
 }
diff --git a/src/main/java/com/hivemq/bootstrap/ClientConnectionContext.java b/src/main/java/com/hivemq/bootstrap/ClientConnectionContext.java
@@ -219,4 +219,10 @@ public interface ClientConnectionContext {
     @NotNull Optional<String> getChannelIP();
 
     @NotNull Optional<InetAddress> getChannelAddress();
+
+    void setClearPasswordAfterAuth(@Nullable Boolean clearPasswordAfterAuth);
+
+    @NotNull Optional<Boolean> isClearPasswordAfterAuth();
+
+    void clearPassword();
 }
diff --git a/src/main/java/com/hivemq/bootstrap/UndefinedClientConnection.java b/src/main/java/com/hivemq/bootstrap/UndefinedClientConnection.java
@@ -38,6 +38,7 @@
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.nio.ByteBuffer;
+import java.util.Arrays;
 import java.util.Optional;
 import java.util.concurrent.ScheduledFuture;
 
@@ -81,6 +82,7 @@ public class UndefinedClientConnection implements ClientConnectionContext {
     @Nullable ByteBuffer authData;
     @Nullable Mqtt5UserProperties authUserProperties;
     @Nullable ScheduledFuture<?> authFuture;
+    @Nullable Boolean clearPasswordAfterAuth;
 
     @Nullable Long maxPacketSizeSend;
 
@@ -506,4 +508,23 @@ public void setExtensionConnectionInformation(final @NotNull ConnectionInformati
 
         return Optional.empty();
     }
+
+    @Override
+    public void setClearPasswordAfterAuth(final @Nullable Boolean clearPasswordAfterAuth) {
+        this.clearPasswordAfterAuth = clearPasswordAfterAuth;
+    }
+
+    @Override
+    public @NotNull Optional<Boolean> isClearPasswordAfterAuth() {
+        return Optional.ofNullable(clearPasswordAfterAuth);
+    }
+
+    @Override
+    public void clearPassword(){
+        if(authPassword == null) {
+            return;
+        }
+        Arrays.fill(authPassword, (byte) 0);
+        authPassword = null;
+    }
 }
diff --git a/src/main/java/com/hivemq/configuration/service/InternalConfigurations.java b/src/main/java/com/hivemq/configuration/service/InternalConfigurations.java
@@ -450,6 +450,7 @@ public class InternalConfigurations {
     public static final int NETTY_COUNT_OF_CONNECTIONS_IN_SHUTDOWN_PARTITION = 100;
 
     public static final double MQTT_CONNECTION_KEEP_ALIVE_FACTOR = 1.5;
+    public static final boolean MQTT_CONNECTION_AUTH_CLEAR_PASSWORD = true;
 
     public static final long DISCONNECT_KEEP_ALIVE_BATCH = 100;
 

diff --git a/src/main/java/com/hivemq/extensions/auth/ConnectAuthContext.java b/src/main/java/com/hivemq/extensions/auth/ConnectAuthContext.java
@@ -63,6 +63,7 @@ public ConnectAuthContext(
     void succeedAuthentication(final @NotNull ConnectAuthOutput output) {
         super.succeedAuthentication(output);
         final ClientConnectionContext clientConnectionContext = ClientConnectionContext.of(ctx.channel());
+        clientConnectionContext.setClearPasswordAfterAuth(output.isClearPasswordAfterAuth());
         clientConnectionContext.setAuthData(output.getAuthenticationData());
         clientConnectionContext.setAuthUserProperties(Mqtt5UserProperties.of(output.getOutboundUserProperties()
                 .asInternalList()));

diff --git a/src/main/java/com/hivemq/extensions/auth/ConnectAuthOutput.java b/src/main/java/com/hivemq/extensions/auth/ConnectAuthOutput.java
@@ -42,6 +42,7 @@ public class ConnectAuthOutput extends AuthOutput<EnhancedAuthOutput> implements
 
     private @NotNull Mqtt5ConnAckReasonCode reasonCode = Mqtt5ConnAckReasonCode.NOT_AUTHORIZED;
     private @NotNull Mqtt5ConnAckReasonCode timeoutReasonCode = Mqtt5ConnAckReasonCode.NOT_AUTHORIZED;
+    private @Nullable Boolean clearPasswordAfterAuth;
     private final boolean supportsEnhancedAuth;
 
     public ConnectAuthOutput(
@@ -68,6 +69,11 @@ private void setDefaultReasonStrings() {
         timeoutReasonString = ReasonStrings.AUTH_FAILED_EXTENSION_TIMEOUT;
     }
 
+    public void authenticateSuccessfully(final boolean clearPasswordAfterAuth) {
+        this.clearPasswordAfterAuth = clearPasswordAfterAuth;
+        super.authenticateSuccessfully();
+    }
+
     @Override
     public void authenticateSuccessfully(final @NotNull ByteBuffer authenticationData) {
         checkEnhancedAuthSupport();
@@ -193,6 +199,10 @@ void failByThrowable(final @NotNull Throwable throwable) {
         return reasonCode;
     }
 
+    public @Nullable Boolean isClearPasswordAfterAuth() {
+        return clearPasswordAfterAuth;
+    }
+
     private static @NotNull Mqtt5ConnAckReasonCode checkReasonCode(final @NotNull ConnackReasonCode reasonCode) {
         checkNotNull(reasonCode, "CONNACK reason code must never be null");
         checkArgument(reasonCode != ConnackReasonCode.SUCCESS,

diff --git a/src/main/java/com/hivemq/extensions/auth/ConnectSimpleAuthOutput.java b/src/main/java/com/hivemq/extensions/auth/ConnectSimpleAuthOutput.java
@@ -43,6 +43,11 @@ public void authenticateSuccessfully() {
         delegate.authenticateSuccessfully();
     }
 
+    @Override
+    public void authenticateSuccessfully(final boolean clearPasswordAfterAuth) {
+        delegate.authenticateSuccessfully(clearPasswordAfterAuth);
+    }
+
     @Override
     public void failAuthentication() {
         delegate.failAuthentication();

diff --git a/src/main/java/com/hivemq/mqtt/handler/connect/ConnectHandler.java b/src/main/java/com/hivemq/mqtt/handler/connect/ConnectHandler.java
@@ -79,6 +79,7 @@
 import javax.inject.Singleton;
 import java.nio.ByteBuffer;
 import java.util.NoSuchElementException;
+import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 
 import static com.hivemq.bootstrap.netty.ChannelHandlerNames.AUTH_IN_PROGRESS_MESSAGE_HANDLER;
@@ -88,6 +89,7 @@
 import static com.hivemq.bootstrap.netty.ChannelHandlerNames.MQTT_MESSAGE_BARRIER;
 import static com.hivemq.bootstrap.netty.ChannelHandlerNames.MQTT_PUBLISH_FLOW_HANDLER;
 import static com.hivemq.configuration.service.InternalConfigurations.AUTH_DENY_UNAUTHENTICATED_CONNECTIONS;
+import static com.hivemq.configuration.service.InternalConfigurations.MQTT_CONNECTION_AUTH_CLEAR_PASSWORD;
 import static com.hivemq.mqtt.message.connack.CONNACK.KEEP_ALIVE_NOT_SET;
 import static com.hivemq.mqtt.message.connack.Mqtt5CONNACK.DEFAULT_MAXIMUM_PACKET_SIZE_NO_LIMIT;
 
@@ -213,6 +215,7 @@ public void connectSuccessfulUndecided(
             final @NotNull CONNECT connect,
             final @Nullable ModifiableClientSettingsImpl clientSettings) {
 
+        clearPasswordIfWanted(clientConnectionContext);
         if (AUTH_DENY_UNAUTHENTICATED_CONNECTIONS.get()) {
             mqttConnacker.connackError(clientConnectionContext.getChannel(),
                     PluginAuthenticatorServiceImpl.AUTH_FAILED_LOG,
@@ -235,11 +238,25 @@ public void connectSuccessfulAuthenticated(
             final @NotNull CONNECT connect,
             final @Nullable ModifiableClientSettingsImpl clientSettings) {
 
+        clearPasswordIfWanted(clientConnectionContext);
         clientConnectionContext.proposeClientState(ClientState.AUTHENTICATED);
         cleanChannelAttributesAfterAuth(clientConnectionContext);
         connectAuthenticated(ctx, clientConnectionContext, connect, clientSettings);
     }
 
+    private void clearPasswordIfWanted(final @NotNull ClientConnectionContext clientConnectionContext) {
+        final Optional<Boolean> clearPasswordAfterAuthOptional = clientConnectionContext.isClearPasswordAfterAuth();
+        if(clearPasswordAfterAuthOptional.isPresent()) {
+            if(clearPasswordAfterAuthOptional.get()) {
+                clientConnectionContext.clearPassword();
+            }
+        } else {
+            if(MQTT_CONNECTION_AUTH_CLEAR_PASSWORD) {
+                clientConnectionContext.clearPassword();
+            }
+        }
+    }
+
     private static void cleanChannelAttributesAfterAuth(final @NotNull ClientConnectionContext clientConnectionContext) {
         final ChannelPipeline pipeline = clientConnectionContext.getChannel().pipeline();
         if (pipeline.context(AUTH_IN_PROGRESS_MESSAGE_HANDLER) != null) {

diff --git a/src/test/java/com/hivemq/mqtt/handler/connect/ConnectHandlerTest.java b/src/test/java/com/hivemq/mqtt/handler/connect/ConnectHandlerTest.java
@@ -102,12 +102,14 @@
 import util.TestMqttDecoder;
 
 import javax.inject.Provider;
+import java.nio.charset.StandardCharsets;
 import java.util.Map;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 
+import static com.hivemq.configuration.service.InternalConfigurations.MQTT_CONNECTION_AUTH_CLEAR_PASSWORD;
 import static com.hivemq.extension.sdk.api.auth.parameter.OverloadProtectionThrottlingLevel.NONE;
 import static com.hivemq.mqtt.message.connack.CONNACK.KEEP_ALIVE_NOT_SET;
 import static com.hivemq.mqtt.message.connect.Mqtt5CONNECT.SESSION_EXPIRY_MAX;
@@ -1497,6 +1499,58 @@ public void test_connectUnauthenticated_connectMessageCleared() {
         assertNull(clientConnection.getAuthConnect());
     }
 
+    @Test(timeout = 5000)
+    public void test_contextWantsPasswordErasure_passwordCleared() {
+        createHandler();
+        final CONNECT connect =
+                new CONNECT.Mqtt5Builder().withClientIdentifier("client").build();
+
+        clientConnectionContext.setClearPasswordAfterAuth(true);
+        clientConnectionContext.setAuthPassword("password".getBytes(StandardCharsets.UTF_8));
+
+        final ModifiableClientSettingsImpl clientSettings = new ModifiableClientSettingsImpl(65535, null);
+        handler.connectSuccessfulAuthenticated(ctx, clientConnectionContext, connect, clientSettings);
+
+        final ClientConnection clientConnection = ClientConnection.of(channel);
+        assertNull(clientConnection.getAuthPassword());
+    }
+
+    @Test(timeout = 5000)
+    public void test_contextDoesNotWantPasswordErasure_passwordKept() {
+        createHandler();
+        final CONNECT connect =
+                new CONNECT.Mqtt5Builder().withClientIdentifier("client").build();
+
+        clientConnectionContext.setClearPasswordAfterAuth(false);
+        clientConnectionContext.setAuthPassword("password".getBytes(StandardCharsets.UTF_8));
+
+        final ModifiableClientSettingsImpl clientSettings = new ModifiableClientSettingsImpl(65535, null);
+        handler.connectSuccessfulAuthenticated(ctx, clientConnectionContext, connect, clientSettings);
+
+        final ClientConnection clientConnection = ClientConnection.of(channel);
+        assertNotNull(clientConnection.getAuthPassword());
+    }
+
+    @Test(timeout = 5000)
+    public void test_contextDoesNotDecidePasswordErasure_passwordKeptOrCleared() {
+        createHandler();
+        final CONNECT connect =
+                new CONNECT.Mqtt5Builder().withClientIdentifier("client").build();
+
+        clientConnectionContext.setClearPasswordAfterAuth(null);
+        clientConnectionContext.setAuthPassword("password".getBytes(StandardCharsets.UTF_8));
+
+        final ModifiableClientSettingsImpl clientSettings = new ModifiableClientSettingsImpl(65535, null);
+        handler.connectSuccessfulAuthenticated(ctx, clientConnectionContext, connect, clientSettings);
+
+        final ClientConnection clientConnection = ClientConnection.of(channel);
+        if(MQTT_CONNECTION_AUTH_CLEAR_PASSWORD) {
+            assertNull(clientConnection.getAuthPassword());
+        } else {
+            assertNotNull(clientConnection.getAuthPassword());
+        }
+    }
+
     @Test
     public void test_start_connection_persistent() throws Exception {
         final CONNECT connect = new CONNECT.Mqtt3Builder().withClientIdentifier("client")