Closing the AUTH exchange. about esp-emqtt5 HOT 13 CLOSED

I think you're right. In the standard, there isn't any limitation to the use of the AUTH packet.

It doesn't have to be used inside a CONNECT/CONNACK exchange, it can be used at any time (for example, if you want to renegotiate the session keys). In that case, you'll have a Client => AUTH => Server followed by a Server => AUTH => Client (see section 4.12 and 4.13 of the standard)

I agree that the most used case is what you've described. In that case, the Client => AUTH => Server is alone and the server is answering with Server => CONNACK => Client. The sequence in the code is thus:

1. Client code calls connectTo
2. Inside the method, the server answers with AUTH, the code calls authReceived callback
3. Inside the callback you're calling auth method
4. The method expects a AUTH packet back from the server, which is wrong in that specific case, since a CONNACK is received, leading to ProtocolError
5. The authReceived callback returns
6. The connectTo method finishes since it doesn't loop to retry receiving another packet, it doesn't parse the CONNACK packet

I think I need to modify the code a bit to support this scheme correctly. I'll change the code and let you know.

from esp-emqtt5.

Ok, please pull the authentication branch from this repository and try again (and report). This is an API breaking change, authReceived changed signature. You must return true if the authentication should continue from your side.

This branch changes the logic of the connectTo method. In case you send an authentication properties and the server replies with AUTH, it'll enter an endless loop of packet parsing, dealing with either CONNACK (it'll exit the loop) or AUTH packets.
If any AUTH packet doesn't fit your need, you can return false in authReceived and it'll close the connection. If you call auth method from your authReceived callback (from a connectTo call), then it'll return Success if it receives a AUTH or CONNACK packet. You'll then return true from your callback and the connectTo method will parse the CONNACK packet and finish successfully.

Please notice that if you had a AUTH within AUTH challenge (like for Kerberos), you'll get such callstack:

1. MQTTClient::connectTo
2. your::authReceived callback
3. MQTTClient::auth
4. your::authReceived callback again for the second AUTH packet challenge
5. MQTTClient::auth
6. CONNACK receiving

This is a small price to pay on the stack depth to avoid rewriting the packet processing logic.
Said differently, the deeper the AUTH challenge, the deeper the stack recursion since each packet add 2 layers in the callstack.

Please test and report if it works for you so I could merge in the code base.

from esp-emqtt5.

Hi, I tried to compile my project with the authentication branch an got a lot of errors. I attach the log file. The same happened when I tried to compile the test example.
esp-emqtt5.txt

Thanks.

from esp-emqtt5.

Ok, please pull the same branch again and try again. The patch I made wasn't applied correctly at the right places. Thanks!

from esp-emqtt5.

Test example works fine now, I'll test with my project and report back.

Cheers.

from esp-emqtt5.

Now this is embarrassing, but I don get how to call the MQTTClient::auth method from inside the authReceived callback. If I do client.auth(...) I get an error: 'client' was not declared in this scope Can you help me?

Thanks.

from esp-emqtt5.

That's hard to say without seeing your code.
In this library if you have declared a class for the callback, this class should have a MQTTClient client member where you're manipulating the client (if you need so).

Typically, you can do like this C++ way:

struct MQTTCallbacks : public Network::Client::MessageReceived
{
    void messageReceived(const Network::Client::MQTTv5::DynamicStringView & topic, const Network::Client::MQTTv5::DynamicBinDataView & payload,
                         const uint16 packetIdentifier, const Network::Client::MQTTv5::PropertiesView & properties)
    {
        ESP_LOGI(LOGNAME, "Msg received: (%04X)", packetIdentifier);
        ESP_LOGI(LOGNAME, "  Topic: %.*s", topic.length, topic.data);
        ESP_LOGI(LOGNAME, "  Payload: %.*s", payload.length, payload.data);
    }

    MQTTClient client;
 

    bool authReceived(...)
    {
        return client.auth(...) == ErrorType::Success;
    }

    MQTTCallback(const char* clientID): client(clientID, this) {}
};

// in your main
MQTTCallback cb;
cb.client.connectTo(...);

or you can, like the test, use it like a global variable (copy and paste of the test code, adjusted), C way:

struct MessageReceiver : public Network::Client::MessageReceived
{
    void messageReceived(const Network::Client::MQTTv5::DynamicStringView & topic, const Network::Client::MQTTv5::DynamicBinDataView & payload,
                         const uint16 packetIdentifier, const Network::Client::MQTTv5::PropertiesView & properties)
    {
        ESP_LOGI(LOGNAME, "Msg received: (%04X)", packetIdentifier);
        ESP_LOGI(LOGNAME, "  Topic: %.*s", topic.length, topic.data);
        ESP_LOGI(LOGNAME, "  Payload: %.*s", payload.length, payload.data);
    }

    // Forward declare the authentication method (you can't use client here since it's not declared yet)
    bool auth(...);
};

MessageReceiver receiver;
Network::Client::MQTTv5 client("eMQTT5", &receiver);

// Implement the method. You can use the global client here since it's now declared
bool MessageReceiver::auth(...) 
{
    return client.auth(...) == Success;
}

from esp-emqtt5.

I'm basically leveraging your test example code, so I implemented the method the C way:

struct MessageReceiver : public Network::Client::MessageReceived
{
    void messageReceived(const Network::Client::MQTTv5::DynamicStringView &topic, const Network::Client::MQTTv5::DynamicBinDataView &payload,
                         const uint16 packetIdentifier, const Network::Client::MQTTv5::PropertiesView &properties)
    {
        ESP_LOGI(LOGNAME, "Msg received: (%04X)", packetIdentifier);
        ESP_LOGI(LOGNAME, "  Topic: %.*s", topic.length, topic.data);
        ESP_LOGI(LOGNAME, "  Payload: %.*s", payload.length, payload.data);
    }

    bool authReceived(const Network::Client::MQTTv5::ReasonCodes reasonCode, const Network::Client::MQTTv5::DynamicStringView &authMethod,
                      const Network::Client::MQTTv5::DynamicBinDataView &authData, const Network::Client::MQTTv5::PropertiesView &properties);
};

and then:

MessageReceiver receiver;
Network::Client::MQTTv5 client("eMQTT5", &receiver);

bool MessageReceiver::authReceived(const Network::Client::MQTTv5::ReasonCodes reasonCode, const Network::Client::MQTTv5::DynamicStringView &authMethod,
                                   const Network::Client::MQTTv5::DynamicBinDataView &authData, const Network::Client::MQTTv5::PropertiesView &properties)
{
    ESP_LOGI(LOGNAME, "Auth packet received");
    ESP_LOGI(LOGNAME, "  AuthMethod: %.*s", authMethod.length, authMethod.data);
    ESP_LOGI(LOGNAME, "  AuthData: %.*s", authData.length, authData.data);
    ESP_LOGI(LOGNAME, "  Reason Code: %d", (int)reasonCode);

    if (Network::Client::MQTTv5::ErrorType ret = client.auth(Protocol::MQTT::V5::ContinueAuthentication, authMethod, authData))
    {
        ESP_LOGE(LOGNAME, "Failed auth to %s with error: %d", MQTT_HOST, (int)ret);
        return false;
    }
    return true;
}

Currently I'm not making any additional checks on the authReceived callback since I'm only interested in the CONNECT -> AUTH -> AUTH -> CONNACK exchange.

Now, I'm getting a weird error, the MQTTClient::auth method fails with an error code -4. Doing a little debugging, for some reason it fails this check:

if (reasonCode != Protocol::MQTT::V5::Success || reasonCode != Protocol::MQTT::V5::ContinueAuthentication || reasonCode != Protocol::MQTT::V5::ReAuthenticate)
            return ErrorType::BadParameter;

I changed the check to this:

if (reasonCode != Protocol::MQTT::V5::ContinueAuthentication)
            return ErrorType::BadParameter;

to keep with the testing, but now the method gets stuck without sendin any error code, and doesn't even send the AUTH packet.

Doing a little more debugging it seems it gets stuck at this instruction:

ScopedLock scope(impl->lock);

Do you think you can take a look?

Thanks.

from esp-emqtt5.

It's very hard to debug this without having access to your broker and the expected authentication scheme. Can you provide some code reference, or, at least, some packet dump for all the expected exchanges?

As for the scope lock, I'll have a look, it's possible it's a re-entrancy issue, the mutex implementation on ESP32 might not be re-entrant.

from esp-emqtt5.

I think I can grant you access to our test broker no problem, but I have to ask my teacher first. I’ll send you the details as soon as he responds. Thank you.

from esp-emqtt5.

I think I've fixed all reentrancy issues and the bad checks in AUTH. I'm successfully connected to your broker, passed authentication scheme you've implemented and published a "Is working" to /test_method/auth topic.

Please pull the authentication branch again to get the latest code. You can also find example code in eMQTT5's test client here.

You'll need to adapt to fit your scheme, obviously (but it's pretty straightforward).

from esp-emqtt5.

It works flawlessly now, thank you very much. Would you mind if I include your name in my thesis acknowledgements? It's the least I can do.

Cheers!

from esp-emqtt5.

Hi, that's a great news. You can cite me, but I don't want to tell my name publicly on this website. Would you mind sending me an email (see my blog's footer for the gmail account name), so I can directly answer to you privately?

Thanks!

from esp-emqtt5.