mqttjs / mqtt-packet

Parse and generate MQTT packets like a breeze in JS
Other
206 stars 93 forks source link

can't comply with MQTT spec requirements about flag validation #111

Closed jedwards1211 closed 3 years ago

jedwards1211 commented 3 years ago

The Server MUST validate that the reserved flag in the CONNECT Control Packet is set to zero and disconnect the Client if it is not zero [MQTT-3.1.2-3]

If I set that flag to 1, mqtt-packet still parses it just fine, but doesn't emit raw flag bits, so it's impossible for a server implementation to comply with this spec requirement using mqtt-packet.

> var parser = require('mqtt-packet').parser()
> parser.on('packet', console.log)
// the `3` in the following sets the reserved flag to 1
> parser.parse(Buffer.from('100c00044d515454040300000000', 'hex'))
Packet {
  cmd: 'connect',
  retain: false,
  qos: 0,
  dup: false,
  length: 12,
  topic: null,
  payload: null,
  protocolId: 'MQTT',
  protocolVersion: 4,
  clean: true,
  keepalive: 0,
  clientId: ''
}

This goes for many other packet types' flags as well...for example

Bits 3,2,1 and 0 of the fixed header in the PUBREL Control Packet are reserved and MUST be set to 0,0,1 and 0 respectively. The Server MUST treat any other value as malformed and close the Network Connection [MQTT-3.6.1-1].

mqtt-packet should either attach a raw buffer to the packet, or at least some fields that represent the raw flag values.

It might be sufficient to emit an error when any packet's flags are invalid; I think in all cases the server is just supposed to close the connection, but I'm not 100% sure.

mcollina commented 3 years ago

It might be sufficient to emit an error when any packet's flags are invalid; I think in all cases the server is just supposed to close the connection, but I'm not 100% sure.

That would be my preferred approach.