Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dependent on simple authentication and internal config, the client password is now cleared or kept. #543

Merged
merged 2 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/main/java/com/hivemq/bootstrap/ClientConnection.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
21 changes: 21 additions & 0 deletions src/main/java/com/hivemq/bootstrap/UndefinedClientConnection.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -81,6 +82,7 @@ public class UndefinedClientConnection implements ClientConnectionContext {
@Nullable ByteBuffer authData;
@Nullable Mqtt5UserProperties authUserProperties;
@Nullable ScheduledFuture<?> authFuture;
@Nullable Boolean clearPasswordAfterAuth;

@Nullable Long maxPacketSizeSend;

Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/com/hivemq/extensions/auth/ConnectAuthOutput.java
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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();
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
17 changes: 17 additions & 0 deletions src/main/java/com/hivemq/mqtt/handler/connect/ConnectHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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")
Expand Down