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

Anonymous MQTT not working #355

Closed
J4nsen opened this issue Jan 5, 2024 · 9 comments
Closed

Anonymous MQTT not working #355

J4nsen opened this issue Jan 5, 2024 · 9 comments

Comments

@J4nsen
Copy link

J4nsen commented Jan 5, 2024

Hi, my mqtt-server (mosquitto) only allows anonymous connections or ones for existing users.

However, ESPWortuhr always seems to set the user and password to 29 empty spaces, when the fields in the web ui are blank.
image

It would be nice, if a connection without username and password would be made, when the fields are empty, i.e., setting username and password to NULL on the connect-function.

Thanks for the nice project! :)

@dbambus
Copy link
Collaborator

dbambus commented Jan 5, 2024

Hey @J4nsen,

correct me if I m wrong, but the 20 fields in Hex are supposed to be NULLs not blank space ( 40). In the payload handler of the Website include/WebPageAdapter.h the code is checking in the Payload for spaces in reverse order.

    for (int8_t counter = PAYLOAD_LENGTH - 2; counter > -1; counter--) {
        if (!isSpace(text[counter])) {
            index = counter + 1;
            break;
        }
    }

So the username and the password should be empty.

What could be done, adding a check for empty mqtt.user/mqtt.password in the include/mqtt.h and hand the function parameters over as NULL, as described in the doc of the MQTT client.

boolean PubSubClient::connect(const char *id) {
    return connect(id,NULL,NULL,0,0,0,0,1);
}

boolean PubSubClient::connect(const char *id, const char *user, const char *pass) {
    return connect(id,user,pass,0,0,0,0,1);
}

Could you try that ?

Cheers
David

@dbambus
Copy link
Collaborator

dbambus commented Jan 5, 2024

And, what build are you using ? Are you on the most recent git state ?

@J4nsen
Copy link
Author

J4nsen commented Jan 5, 2024

Hi @dbambus, thanks for the fast reply.

correct me if I m wrong, but the 20 fields in Hex are supposed to be NULLs not blank space ( 40). In the payload handler of the Website include/WebPageAdapter.h the code is checking in the Payload for spaces in reverse order.

Wireshark is showing hex values, 20 is a space, 40 would be an @-char.

    for (int8_t counter = PAYLOAD_LENGTH - 2; counter > -1; counter--) {
        if (!isSpace(text[counter])) {
            index = counter + 1;
            break;
        }
    }

So the username and the password should be empty.

I dont completely understand the code (yet). What I see is that when opening the web-ui there are whitespaces in the username and password field. Even after removing them and saving the settings they reappear after reloading the ui.

What could be done, adding a check for empty mqtt.user/mqtt.password in the include/mqtt.h and hand the function parameters over as NULL, as described in the doc of the MQTT client.

boolean PubSubClient::connect(const char *id) {
    return connect(id,NULL,NULL,0,0,0,0,1);
}

boolean PubSubClient::connect(const char *id, const char *user, const char *pass) {
    return connect(id,user,pass,0,0,0,0,1);
}

Could you try that ?

I've applied the following patch. Wordclock is now able to connect to the broker :)

diff --git a/include/mqtt.hpp b/include/mqtt.hpp
index d5ac0b2..a556552 100644
--- a/include/mqtt.hpp
+++ b/include/mqtt.hpp
@@ -16,14 +16,14 @@ PubSubClient mqttClient(client);
 void Mqtt::init() {
     mqttClient.setServer(G.mqtt.serverAdress, G.mqtt.port);
     mqttClient.setCallback(callback);
-    mqttClient.connect(G.mqtt.clientId, G.mqtt.user, G.mqtt.password);
+    mqttClient.connect(G.mqtt.clientId);
     mqttClient.subscribe((std::string(G.mqtt.topic) + "/cmd").c_str());
 }
 
 //------------------------------------------------------------------------------
 
 void Mqtt::reInit() {
-    mqttClient.connect(G.mqtt.clientId, G.mqtt.user, G.mqtt.password);
+    mqttClient.connect(G.mqtt.clientId);
     reconnect();
 }

However, the connection seems a bit unstable. I see error messages like these in my broker log:

1704464320: Client Wordclock has exceeded timeout, disconnecting.
[...]
1704464588: New client connected from 192.168.123.123:51867 as Wordclock (p2, c1, k15).
1704464606: Client Wordclock disconnected due to malformed packet.

And, what build are you using ? Are you on the most recent git state ?

I'm on the latest git commit. I could write a PR which checks for an whitespace-only username/password and then uses the connect function without credentails. However, I think there is another problem, e.g., why am I unable to clear the whitespaces in the webui?

@dbambus
Copy link
Collaborator

dbambus commented Jan 5, 2024

Hey,

I will elaborate the whitespaces once I finished work for today and also I ll try Wireshark and take a look at the mosquitto logs for it. Never had that issue before. Maybe try another QoS Level ? See PubSubClient.cpp for more Doc.

Just a question, do you have your mosquitto running in a Docker ? Maybe take could also be an indicator: https://stackoverflow.com/questions/75534407/mqtt-malformed-packet

@J4nsen
Copy link
Author

J4nsen commented Jan 5, 2024

Hi @dbambus, no hurry :)

Just a question, do you have your mosquitto running in a Docker ? Maybe take could also be an indicator: https://stackoverflow.com/questions/75534407/mqtt-malformed-packet

mosquitto is running directly on the host. The reason why some clients cannot connect is that im running the dynamic-security plugin with allow_anonymous = true, which basically rejects mqtt-clients with a username, if they are not listed in the local ACLs.

This problem isn't new to me. For example, for Tasmota clients I have to disable/clear the username/password with "MqttUser 0" and "MqttPassword 0" or else it will use the firmware default.

@dbambus
Copy link
Collaborator

dbambus commented Apr 21, 2024

Hey @J4nsen,

i uploaded an update to the software to handle anonymous logins to the Matt broker, with #396. Hereby I close this issue to keep an overview. Really thank you for this issue :-), maybe you ll find some more improvements for the software.
David

@dbambus dbambus closed this as completed Apr 21, 2024
@J4nsen
Copy link
Author

J4nsen commented Apr 22, 2024

Hey @dbambus, awesome! :)

I had a quick look at the code and saw that the reInit-function could potentially be called with user and password and thus break the reconnection attempt for anonymous access?

@dbambus
Copy link
Collaborator

dbambus commented Apr 22, 2024

Hey,

I ve rewritten the part in the latest unmerged pull request. But it is not yet tested ;-) maybe you can do that ?

Cheers
David

@dbambus
Copy link
Collaborator

dbambus commented Apr 22, 2024

Fixed with #397

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants