NRCHKB / node-red-contrib-homekit-bridged

Node-RED Contribution - HomeKit Bridged : Node-RED nodes to simulate Apple HomeKit devices.
https://nrchkb.github.io
MIT License
412 stars 52 forks source link

[Bug]: Homekit PIN saved to flows.json #507

Closed NickBorgers closed 1 year ago

NickBorgers commented 1 year ago

NRCHKB Plugin Version

1.4.3

Node JS Version

not relevant

NPM Version

not relevant

Node-RED Version

2.2.3

Operating System

Docker

What happened?

This node stores the token in a way that causes it to be written to flows.json instead of flows_creds.json. This is definitely exposing peoples' credentials because I'm using the Projects feature to back up my flows to GitHub.

How to reproduce?

Create one of your nodes and then look in flows.json

Expected behavior:

Anything sensitive should be saved using the Node Red provided functionality for credentials.

Additional comments?

I honestly don't have an assessment of how bad this is at the moment. I can see your guidance on bad security codes, so it seems like the confidentiality of this PIN may be important.

The Discord invite link here is expired, so I tried DMing the indicated Discord user but I'd basically be random spam.

Relevant log output

No response

X-Link to this issue I raised for the same problem on a different node's project X-Link 2

Shaquu commented 1 year ago

@NickBorgers thanks for the report!

First of all, here is the new link for discord https://discord.gg/uvYac5u

Regarding the issue, is it related to the pin code? If yes, I believe the code despite being called security code from time to time is partially public. On official homekit accessories, you will find it on the sticker on the device itself. In the end, only one client can pair at once.

Shaquu commented 1 year ago

Just wanted to clarify, I am open to improving security on our node ;) Just let's discuss the steps first so we can proceed!

NickBorgers commented 1 year ago

I will confess ignorance on the security implications of the code. One distinction to make RE them being printed on accessories is that viewing the code would require physical access inside my home vs the Projects feature encourages me to make the PIN code public on the Internet.

If only one client can pair at once, this is probably not a big deal.

If the PIN code is not security relevant, why do you suggest that certain weak codes are problematic? I actually took that documentation as "signal" that the value should not be so public.

I'll dig into this in greater detail as I'd like to understand Homekit's security model better. I'm not raising this as a serious issue at this point, but also not clear enough to feel entirely comfortable.

Shaquu commented 1 year ago

It is basen on HomeKit documentation.

https://github.com/NRCHKB/node-red-contrib-homekit-bridged/blob/master/docs/HAP-Specification-Non-Commercial-Version.pdf

NickBorgers commented 1 year ago

I'm going to close this issue, as I'm personally dropping the concern. I think your explanation that only one device can be paired with the accessory (the node red module) is reasonable.

One observation from the Apple specification is that they require you to use a CSPRNG to generate the PIN, which would suggest they view it as an important credential. However, the misuse case they're guarding against may be as simple as not making it too easy for someone to "damage" a shipment of Homekit devices by guessing all of their PINs and pairing with them before an end-user opens the box.

It won't make any difference for me personally because I'm working on automatically publishing screenshots of my flows (just stuck on a weird Actions problem) like this: State Tracking

Shaquu commented 1 year ago

Thanks for the input @NickBorgers I hope to see you around :)