moquette-io / moquette

Java MQTT lightweight broker
http://moquette-io.github.io/moquette/
Apache License 2.0
2.27k stars 814 forks source link

if auth is enabled or not, it doesn't seem to be taken into account, but a wrong password is #729

Closed dentex closed 1 year ago

dentex commented 1 year ago

Expected behavior

If I enable authentication in the server config, I would expect a client to be unable to connect, unless it uses username and password.

Actual behavior

If I enable authentication in the server config, it correctly detects connection attempts with wrong username or password, refusing to connect, but if I simply disable auth in the client, it can connect anyway to the server, regardless of its auth settings.

Steps to reproduce

I'm configuring a broker server like this (platform: Android):

Server server = BrokerInstance.getBrokerInstance(); //returns a server instance via `new Server()`
final Properties props = new Properties();

props.setProperty(BrokerConstants.HOST_PROPERTY_NAME, "192.168.1.10");
props.setProperty(BrokerConstants.PORT_PROPERTY_NAME, "1883");
props.setProperty(BrokerConstants.WEB_SOCKET_PORT_PROPERTY_NAME, "8080");

props.setProperty(BrokerConstants.NEED_CLIENT_AUTH, "true");

String username = App.settings.getString(App.getCtx().getString(R.string.mqtt_username), App.default_username);
String password = App.settings.getString(App.getCtx().getString(R.string.mqtt_password), App.default_password);
String sha256hex = sha256Hex(password);
String filename = "password.conf";
File file = new File(App.getCtx().getFilesDir(), filename);

String fileContents = username + ":" + sha256hex;
try (FileOutputStream fos = App.getCtx().openFileOutput(filename, Context.MODE_PRIVATE)) {
    fos.write(fileContents.getBytes());
    props.setProperty(BrokerConstants.PASSWORD_FILE_PROPERTY_NAME, file.getAbsolutePath());
} catch (IOException e) {
    //pass
}

MemoryConfig memoryConfig = new MemoryConfig(props);
server.startServer(memoryConfig);

Let me know if I need to write down also the client configuration (org.eclipse.paho.client.mqttv3).

Minimal yet complete reproducer code (or URL to code) or complete log file

The logs are from my app; I'm not sure what kind of logs are requested.

Here I'm publishing a message on the server's app build (it shares the connection properties with the server, they cannot be different) Maybe an unusual use case, but I have a "publisher" build of the app that enables the server and a 'publishing only' client and a "subscriber" build that is receiving messages only.

---------------------------- PROCESS STARTED (6022) for package dentex.mqtt.publisher ----------------------------
17:47:53.018 BrokerServiceConnection dentex.mqtt.publisher    D  Starting MQTT broker service
17:47:53.043 BrokerService           dentex.mqtt.publisher    D  Starting broker notification service
17:47:53.061 Broker                  dentex.mqtt.publisher    D  Using authentication
17:47:53.069 Broker                  dentex.mqtt.publisher    D  -- listing properties --
                                                                 port=1883
                                                                 websocket_port=8080
                                                                 host=10.0.2.16
                                                                 password_file=/data/user/0/dentex.mqtt.publisher/fi...
                                                                 need_client_auth=true
17:47:54.216 Broker                  dentex.mqtt.publisher    D  MQTT broker service started
17:47:54.216 BrokerService           dentex.mqtt.publisher    D  Thread is running
//publishing a message
17:50:16.910 MessageInterceptor      dentex.mqtt.publisher    I   -> published messages: 1
17:50:17.305 Client                  dentex.mqtt.publisher    I  message 'Epoch is 1676739016850' published

Now the "subscriber" build of the app, first connecting without authentication

---------------------------- PROCESS STARTED (6777) for package dentex.mqtt.subscriber ----------------------------
17:54:57.109 ClientServiceConnection dentex.mqtt.subscriber   D  Starting MQTT client service
17:54:57.127 ClientService           dentex.mqtt.subscriber   D  Starting client notification service
17:54:57.128 Client                  dentex.mqtt.subscriber   D  Request for a new MQTT client received
17:54:57.130 Client                  dentex.mqtt.subscriber   D  Server URI: tcp://10.0.2.16:1883
17:54:57.133 Client                  dentex.mqtt.subscriber   D  MQTT client service started
17:54:57.138 ClientService           dentex.mqtt.subscriber   D  Client thread is running
17:54:57.664 Client                  dentex.mqtt.subscriber   V  'test_subscriber' connected to broker
17:54:57.670 Client                  dentex.mqtt.subscriber   D  Subscribing to topic 'DEV'
17:54:58.352 Client                  dentex.mqtt.subscriber   I  message '{"epoch":"Epoch is 1676739016850","number":1}' received

here, I'm enabling auth preferences, but a wrong password is used

17:55:08.203 SettingsFragment        dentex.mqtt.subscriber   V  onPreferenceChange
17:55:11.154 SettingsFragment        dentex.mqtt.subscriber   V  Reconnect MQTT Client preference clicked
17:55:15.757 Client                  dentex.mqtt.subscriber   D  Disconnecting client
17:55:15.769 Client                  dentex.mqtt.subscriber   D  Reconnecting client
17:55:15.770 Client                  dentex.mqtt.subscriber   D  Request for a new MQTT client received
17:55:15.786 Client                  dentex.mqtt.subscriber   D  Server URI: tcp://10.0.2.16:1883
17:55:15.787 Client                  dentex.mqtt.subscriber   V  Client connecting using authentication
17:55:16.481 Client                  dentex.mqtt.subscriber   W  Failed to connect

...and a correct password:

18:03:02.267 SettingsFragment        dentex.mqtt.subscriber   V  onPreferenceChange
18:03:07.998 SettingsFragment        dentex.mqtt.subscriber   V  Reconnect MQTT Client preference clicked
18:03:09.838 Client                  dentex.mqtt.subscriber   D  Disconnecting client
18:03:09.839 Client                  dentex.mqtt.subscriber   D  Reconnecting client
18:03:09.840 Client                  dentex.mqtt.subscriber   D  Request for a new MQTT client received
18:03:09.841 Client                  dentex.mqtt.subscriber   D  Server URI: tcp://10.0.2.16:1883
18:03:09.841 Client                  dentex.mqtt.subscriber   V  Client connecting using authentication
18:03:10.207 Client                  dentex.mqtt.subscriber   V  'test_subscriber' connected to broker
18:03:10.210 Client                  dentex.mqtt.subscriber   D  Subscribing to topic 'DEV'
18:03:10.914 Client                  dentex.mqtt.subscriber   I  message '{"epoch":"Epoch is 1676739016850","number":1}' received

Moquette MQTT version

0.16

JVM version (e.g. java -version)

11.0.15

OS version (e.g. uname -a)

Android, the version actually doesn't matter, realdevice, or emulator. This is the emulator: Linux localhost 4.14.175-g6f3fc9538452 #1 SMP PREEMPT Wed Apr 8 17:38:09 UTC 2020 i686

hylkevds commented 1 year ago

I had a quick look at the code while on the train. The NEED_CLIENT_AUTH property is only evaluated for SSL connections. While at first it may seem sensible to not allow passwords to be sent over non-encrypted connections, this fails to take into account that Moquette may be behind an SSL-terminating proxy. Thus even though Moquette may not know it, the connection may still be secured.

This problem also does not happen when explicitly setting an Authenticator and AuthorizorPolicy using the four-parameter Server.start() method.

dentex commented 1 year ago

Hello. Thanks for reaching out. Are you saying that this issue can be avoided by setting an Authenticator and AuthorizerPolicy using an alternate Server.start() method? Would this be a way to explicitly set authentication and authorization policies for the connection, rather than relying on the NEED_CLIENT_AUTH property?

hylkevds commented 1 year ago

Yes, the alternative start method seems to be a good workaround for this problem for now.

Using the alternative start method you can pass your own implementations of the Authenticator and AuthorizorPolicy. This means you can use any source for your user database, like a relational database, or ldap server. The NEED_CLIENT_AUTH does not need to be set for this to work.

dentex commented 1 year ago

I think I may have implemented the 3 additional parameters for the Server.start() method.

public static class MySslContextCreator implements ISslContextCreator {
    @Override
    public SslContext initSSLContext() {
        SelfSignedCertificate cert;
        try {
            cert = new SelfSignedCertificate();
        } catch (CertificateException e) {
            throw new RuntimeException(e);
        }

        try {
            Log.d(DEBUG_TAG, "Using certificate: " + cert.cert().getSubjectX500Principal().getName());
            return SslContextBuilder.forServer(cert.key(), cert.cert())
                    .sslProvider(SslProvider.JDK)
                    .build();
        } catch (SSLException e) {
            throw new RuntimeException(e);
        }
    }
}

public static class MyAuthenticator implements IAuthenticator {
    @Override
    public boolean checkValid(String clientId, String username, byte[] password) {
        String passwordString = new String(password, StandardCharsets.UTF_8);
        String un = App.settings.getString(App.getCtx().getString(R.string.mqtt_username), App.default_username);
        String pw = App.settings.getString(App.getCtx().getString(R.string.mqtt_password), App.default_password);
        boolean isValid = un.equals(username) && pw.equals(passwordString);
        Log.v(DEBUG_TAG, "checkValid: "  + clientId + " " + username + " -> " + isValid);
        return isValid;
    }
}

public static class MyAuthorizatorPolicy implements IAuthorizatorPolicy {
    @Override
    public boolean canWrite(Topic topic, String user, String client) {
        return true;
    }

    @Override
    public boolean canRead(Topic topic, String user, String client) {
        return true;
    }
}

doing

server.startServer(memoryConfig, interceptors, new MySslContextCreator(), new MyAuthenticator(), new MyAuthorizatorPolicy());

I noticed that the behavior is the same as before. If, when configuring the client, I don't add username and password, I can connect anyway. Basically, the logic in MyAuthenticator is skipped. The logcat whould print checkValid: CLIENT_ID USERNAME -> false... but this happens only if I choose wrong credentials (or checkValid -> true for correct credentials).

@hylkevds can you provide any insight? Thanks.

dentex commented 1 year ago

I think I may have found how to circumvent the issue: use ALLOW_ANONYMOUS_PROPERTY_NAME.

boolean isAuthEnabled = App.settings.getBoolean(...) //get the boolean from the app's settings.
props.setProperty(BrokerConstants.ALLOW_ANONYMOUS_PROPERTY_NAME, String.valueOf(!isAuthEnabled));

...and although settings also props.setProperty(BrokerConstants.NEED_CLIENT_AUTH, String.valueOf(isAuthEnabled)); would make sense, it's irrelevant.

andsel commented 1 year ago

need_client_auth property is used to authenticate the client's TLS certificate. This means that if the client's certificate could be authenticated (or validated) against the CA authorities or the certificates present in the broker's trust store, it's permitted to open a connect to the server. This doesn't mean that it's authenticated in terms of MQTT protocol user/password credential pair.

That's credentials are instead validated against the authenticator provided, which could reject also if the TLS authentication was successful.

dentex commented 1 year ago

Got it. Thanks.

andsel commented 1 year ago

I close it as question. If you want to contribute describing better this in the doc, feel free to create a PR to https://github.com/moquette-io/moquette/blob/gh-pages