Skip to content

Commit

Permalink
Fix MQTT resubscription on reconnect (#19)
Browse files Browse the repository at this point in the history
MQTT.js contains logic to automatically reconnect to the broker if the
connection is lost, and to automatically resubscribe any subscriptions
if this new connection doesn't resume the previous session. Consuming
clients rely on this behaviour.

Unfortunately, the code in the GSS client implementation, which bypassed
the automatic reconnection in favour of manual reconnection in order to
fetch a new Kerberos authentication packet, was causing the
auto-re-subscription to be bypassed.

Instead, fetch a new authenticator on "close", but allow the
auto-reconnect to still function. There is a race here; it is possible
we will attempt to reconnect before we have fresh credentials. This
cannot be avoided with the current MQTT.js API. In this case the connect
attempt will fail and we will retry; a second authenticator fetch
immediately after the first should succeed immediately, as we have a
service ticket so it's just crypto and no network traffic.
  • Loading branch information
amrc-benmorrow authored Jan 30, 2024
2 parents df59d08 + 8218d7d commit 11e7147
Showing 1 changed file with 27 additions and 7 deletions.
34 changes: 27 additions & 7 deletions lib/service/mqtt.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ function get_verb_from (opts) {
export async function gss_mqtt (url, opts) {
opts ??= {};
const host = new URL(url).hostname;
const reconnect = opts.reconnectPeriod ?? 3000;
const verb = get_verb_from(opts);

/* Has the connection deliberately been closed? */
Expand All @@ -58,15 +57,21 @@ export async function gss_mqtt (url, opts) {
let [ctx, buf] = await gss_init(host);

const mqtt = MQTT.connect(url, {
reconnectPeriod: 3000,
...opts,
protocolVersion: 5,
reconnectPeriod: 0,
properties: {
...opts.properties,
authenticationMethod: "GSSAPI",
authenticationData: buf,
},
});
mqtt.on("packetreceive", pkt => {
if (pkt.cmd != "connack")
return;
verb("MQTT clearing GSS creds");
mqtt.options.properties.authenticationData = "";
});
mqtt.on("connect", ack => {
//verb("Got CONNACK: %o", ack);
const srv_buf = ack.properties.authenticationData;
Expand All @@ -83,15 +88,30 @@ export async function gss_mqtt (url, opts) {
});
});
mqtt.on("close", () => {
verb("MQTT connection closed");
if (ending) return;
timers.setTimeout(reconnect)
.then(() => gss_init(host))

/* We clear the authData when we get a CONNACK (successful or
* not). We don't need to do anything if our AP-REQ hasn't been
* used yet. */
if (mqtt.options.properties.authenticationData != "")
return;

/* XXX This will not necessarily complete before the client
* sends the CONNECT packet. There is nothing we can do about
* this with the existing MQTT.js API. The connect attempt will
* fail (GSS replay) and we will try again.
*
* Previously this code bypassed the auto-reconnect logic and
* explicitly reconnected after fetching the AP-REQ. But this
* also disabled the auto-resubscribe logic, which is not
* helpful.
*/
verb("MQTT fetching new GSS creds");
gss_init(host)
.then(newgss => {
verb("MQTT setting new GSS creds");
[ctx, buf] = newgss;
mqtt.options.properties.authenticationData = buf;
verb("MQTT reconnecting...");
mqtt.reconnect();
});
});
mqtt.on("end", () => {
Expand Down

0 comments on commit 11e7147

Please sign in to comment.