Will publishing is not working as expected when Will Delay Interval is longer than Session Expiry Interval

[wlisac]

Overview

The Will Message is not published when the Will Delay Interval is longer than the Session Expiry Interval.

Steps to Reproduce

• Start the mosquitto broker

  $ docker run -it -p 1883:1883 eclipse-mosquitto
  

• Add a subscriber to a will-topic topic

  $ mosquitto_sub -h localhost -p 1883 -t 'will-topic' -d
  

• Add a subscriber with a LWT and Will Delay Interval that is longer than the Session Expiry Interval

  $ mosquitto_sub -h localhost -p 1883 -t 'test' -d --will-payload 'goodbye' --will-topic 'will-topic' -V 5 -D WILL will-delay-interval 30 -D CONNECT session-expiry-interval 5
  

• Terminate the connection of the subscriber with the LWT using ⌃c to trigger a disconnect

Expected Results

The Will Message should be published when the session expires (after 5 seconds).

Relevant sections from the MQTT 5.0 spec:

The Will Message MUST be published after the Network Connection is subsequently closed and either the Will Delay Interval has elapsed or the Session ends, unless the Will Message has been deleted by the Server on receipt of a DISCONNECT packet with Reason Code 0x00 (Normal disconnection) or a new Network Connection for the ClientID is opened before the Will Delay Interval has elapsed [MQTT-3.1.2-8].

The Server delays publishing the Client’s Will Message until the Will Delay Interval has passed or the Session ends, whichever happens first.

Non-normative comment
The Client can arrange for the Will Message to notify that Session Expiry has occurred by setting the Will Delay Interval to be longer than the Session Expiry Interval and sending DISCONNECT with Reason Code 0x04 (Disconnect with Will Message).

Observed Results

The Will Message is not published when either the Will Delay Interval has elapsed or the session ends when the Will Delay Interval is longer than the Session Expiry Interval.

I've also sometimes seen the broker exit with a status of 139 after the Will Delay Interval (30 seconds in this example).

Additional Notes

The Will Publish works as expected when the Will Delay Interval is shorter than the Session Expiry Interval.

[ralight]

Thanks for the good description, I'm looking at this now.

[ralight]

I believe this is fixed in the fixes branch, are you able to confirm it works for you?

[wlisac]

@ralight thanks for the quick response. I just tested the fix and it works as expected 👍

[wlisac]

@ralight I was running an automated test suite for an MQTT client that I'm writing for SwiftNIO and I noticed a race / crash still in play with LWT, Will Delay, and Session Expiry after the fix.

It would be a little difficult for me to share that test suite since the project is still WIP (although I'm open to it if needed, since it's reproducing the bug consistently), but I was able to quickly put together a simple node.js script that often reproduces the same issue.

I've attached the an archive of the node.js app that can reproduce the race / crash.

$ node test.js

Let me know what you think / if it makes sense and I'm happy to provide more info to help close this out.

Thanks again,
Will

mqtt-js-test.zip

[ralight]

I think the new commit sorts that out.

[wlisac]

@ralight tests pass – thanks 🙌

[ralight]

Thanks for confirming.

[carnil]

@ralight: Can you confirm if this is CVE-2019-11778?