Potential security issue with the alwaysOpenSwitch

[nihalgonsalves]

While working on #42 I realised that the Nello Webhook isn't really authenticated or verified, and nor do they provide any way of doing so.

As webhoook actions are normally just notifications, the most an attacker could do is send fake notifications to Homebridge and update the state wrongly.

However, with the alwaysOpenSwitch enabled, an attacker could theoretically open the door by sending a deny action to the webhoook endpoint.

Perhaps @ermalguni you could provide some insight into whether authentication is planned?

---

✅ Resolved by #55, #56, AlexanderBabel/nello-backend#6

[ermalguni]

Hi @nihalgonsalves, we have kept authentication part as TBD because we could not fully anticipate how the webhooks would be used. To account for that, the webhooks are supposed to provide only info data and nothing more. And as you also point out all a possible attacker can do is to provide false events, provided that he already go hold of your webhook URL.

Any changes to the webhooks would need to not break any current integration. That being the case, we are leaning more into signing the webhooks via a pre shared key and storing the HMAC in the header.

[nihalgonsalves]

@ermalguni Thanks for the response!

Yes, including a new header and recommending that new integrations verify it would ensure backwards compatibility. Slack does a similar thing for their webhooks.

@lukasroegner I believe that it would make sense to deprecate the alwaysOpenSwitch switch or warn users about the security implications.

Especially because 99% of the users of this library use the webhook relay service that is a single point of failure which can compromise everybody if breached.

[lukasroegner]

@ermalguni Absolutely makes sense, that would not break existing integrations.

@nihalgonsalves Regarding the alwaysOpenSwitch, we should discuss with @AlexanderBabel. He implemented the switch and I don't want to remove it without talking to him. Maybe we should explicitly warn users.

[AlexanderBabel]

I agree on a warning mechanism. I also think we should rename the variable in the json file to something like dangerouslyEnableAlwaysOpenSwitch.

@ermalguni how long will it take to implement the signature method? As soon as you're finished I would integrate a signature check in the relay service. Please keep me updated about this!

[ermalguni]

@AlexanderBabel we are trying to squeeze it it either the next week or the week after that. Should not take more than a couple of days. Do you need anything specific from our side to ease your dev time?

[lukasroegner]

@ermalguni Any news on this topic?

[ermalguni]

@lukasroegner we have implemented it and it is released already. The remaining thing is to update the docs.

[nihalgonsalves]

Not really relevant anymore (see #48)

[nihalgonsalves]

Resolved partially by: #54 (i.e. f054b02)

• Renamed to dangerouslyEnableAlwaysOpenSwitch
• Local webhook server uses a unique path too
• Both the local webhook path and the socket backend is reconnected periodically (when locations are updated, default 1 h) to generate new URLs

[nihalgonsalves]

@ermalguni I know that #48 means that this is unlikely, but since it was already released a long time ago, could you possibly tell us about how to enable authentication?

[ermalguni]

@nihalgonsalves as far as I can remember all you need to do to enable it is to provide the attribute key - while creating/updating the webhook - with the string of the encryption key. Every request that has the key set will return with a HMAC header.

The request should look sth like this:

{
  "url": "http://example.com/path-to-webhook",
  "key": "<YOUR KEY HERE>",
  "actions": [
    "sp_activate",
    "sp_deactivate"
  ]
}

[nihalgonsalves]

Thanks @ermalguni !

I used this payload:

{
  "url": "<url>",
  "key": "<key>",
  "actions": [
    "sp_activate",
    "swipe",
    "geo",
    "tw",
    "deny"
  ]
}

and now receive a x-nello-hook-hmac header. I'm not really sure where to go from here since it doesn't contain timestamps/etc. I tried this but it doesn't match:

const key = '<key>';
const hmac = crypto.createHmac('sha256', key);

// hmac.update('PUT');
// hmac.update('/');

const contentHash = crypto.createHash('md5');
contentHash.update(rawBody);

hmac.update(contentHash.digest('hex'));

console.log(hmac.digest('hex'));

[ermalguni]

@nihalgonsalves If I remember correctly the HMAC contains the sha256 of the payload encoded in utf8. In this way at least you know that the response came from the nello backend and not from some malicious user(unless they got hold of your key). So on your side, you have to stringify the JSON that you receive and generate the digest with sha256 and compare your digest with the digest in the header.

I do not think that we added the timestamp, but I am not sure.

[nihalgonsalves]

Thanks a lot @ermalguni, it works now! I'm curious, what are the sp_activate and sp_deactivate webhook event types?