Behavior with malformed packets

[scompo]

Hello, I was facing reconnection issues with some clients.

After analyzing the TCP traffic with tcpdump I've noticed that a client sent/or I received a malformed PINGREQ packet, it was supposed to be 0xc0 followed by 0x00 instead I'm getting 0x0a followed by 0xc0.

Right now the client doesn't get a response from it's (broken) PINGREQ and resets itself as per specification.

Is this the expected broker behavior in this case?

I think the packet it's ignored (no log mentions), should the client be disconnected or notified?

EDIT:

I'm using version 2.0.12 from snap

I looked a bit at the code and in src/read_handle.c this case should fall into the default case branch, being a 3.1.x spec client, no action is performed.

Is this correct? Should the client be disconnected?

[ralight]

I'm not quite clear what is happening from your description I'm afraid. If the client sends a malformed PINGREQ, I would expect it to be disconnected. If you can reproduce this or still have a pcap file, I'd suggest taking a look in wireshark - the MQTT dissectors are very good and will flag malformed packets so it is much easier to see what is going on.

[scompo]

From the capture I'm receiving the malformed packet on the server and no activity from the broker, just an acknowledgement of the tcp packet sent.

The broker just does nothing and leaves the connection opened, then the client receives no response and closes the connection.

From what I can see in the code a disconnect is issued only if the protocol il version 5, is this the expect behavior from the broker as specified from the protocol?

[ckrey]

I successfully reproduced the problem:
Your are actually not sending a malformed PINGREQ packet (c000), but a reserved command (0A) with a remaining length of c0, which is an illegal length specification. The broker expects more bytes to read, but does not receive any. This is why it is waiting.

It is obviously a bug in the client to send a reserved command, but the reserved command should be rejected by the broker before waiting for a remaining length to be transmitted.

[ralight]

Thank you, this is now fixed for 2.0.21.

[scompo]

Yeah, thank you, it was a 3rd party issue with their code and we ended up telling them to fix it. Thank you for fixing the potential problem on your part too!