wolfSSL / wolfMQTT

wolfMQTT is a small, fast, portable MQTT client implementation, including support for TLS 1.3.
https://www.wolfssl.com
GNU General Public License v2.0
526 stars 156 forks source link

Timeout ignored when TLS is active #141

Closed mh-ba-dg closed 3 years ago

mh-ba-dg commented 4 years ago

Hi there,

MQTT version: 1.4

When TLS is active, the client-timeout is ignored. This can lead to infinite loops if the server is not sending the ACK for QoS1. (What happened, therefore this bug report)

static int MqttSocket_ReadDo(MqttClient *client, byte* buf, int buf_len,
    int timeout_ms)
{
    int rc;

#ifdef ENABLE_MQTT_TLS
    if (client->flags & MQTT_CLIENT_FLAG_IS_TLS) {
        client->tls.timeout_ms = timeout_ms; // <- it is set here, but never used!
        rc = wolfSSL_read(client->tls.ssl, (char*)buf, buf_len);
        if (rc < 0) {
        #ifdef WOLFMQTT_DEBUG_SOCKET
            int error = wolfSSL_get_error(client->tls.ssl, 0);
            if (error != WOLFSSL_ERROR_WANT_READ) {
                PRINTF("MqttSocket_Read: SSL Error=%d", error);
            }
        #endif

Environment: Microchip Harmony v2.6 Defines:

define WOLFSSL_BASE64_ENCODE

define WOLFMQTT_CUSTOM_MALLOC

define WOLFMQTT_MALLOC(s) pvPortMalloc((s))

define WOLFMQTT_FREE(p) vPortFree((p))

define ENABLE_MQTT_TLS

define WOLFMQTT_NONBLOCK

define MICROCHIP_MPLAB_HARMONY

define MICROCHIP_TCPIP

define MICROCHIP_PIC32

define MICROCHIP_PIC32_RNG

embhorn commented 4 years ago

Hello @Daniel-Gruber

Thanks for contacting wolfSSL. The wolfMQTT client sets IO callbacks in wolfSSL, that in turn use the read and write functions defined in mqttnet.c. In this way, the timeout values are used (as long as WOLFMQTT_NO_TIMEOUT is not defined).

Thanks, Eric Blankenhorn wolfSSL Support

mh-ba-dg commented 4 years ago

Ok, but that means that only actual transmissions are guarded by the timeout. The case that mqtt is waiting for an answer of the server, is not guarded by the timeout. And therefore, when the server is never sending an answer, one will receive CONTINUE forever.

Or am I missing something? If the timeout is not meant for that case, a hint somewhere in the documentary would be nice.

But I would prefer if the timeout is guarding the whole publishing process and not just the transmission part :-)

embhorn commented 3 years ago

HI @Daniel-Gruber

I am circling back through older issues... There is a client timeout, such that the client should send a ping request packet to keep the connection alive. If the broker does not respond with a ping response before the command timeout, the connection should be closed.