Apollon77 / hap-controller-node

Node.js library to implement a HAP (HomeKit) controller
Mozilla Public License 2.0
54 stars 16 forks source link

Optimize used PairSetup method #33

Closed Apollon77 closed 3 years ago

Apollon77 commented 3 years ago

Hi,

this is a followup on #31.

I did a bit more research on the two Pair mechanisms and I see the following picture:

There are kind of two types of Homekit compartible devices:

This would bring me API wise to the following:

If the user can provide a pin BEFORE the pair process was started at all it should be a insecure one. This means that as soon as PIN is provided directly it should be PairSetup to be used

This would mean that we could change the used default to use the other method. I know this would be "breaking" according to my last PR, but maybe better. Alternatively I could do a PR for the Readme on which method to use when...

What do you think?

mrstegeman commented 3 years ago

I'm not sure this is entirely accurate. When building the library, I only had devices to test with that had static PINs, and I was using PairSetupWithAuth for all of them with no issues.

Apollon77 commented 3 years ago

Grmpf :-) Then reality is even more complex that I thought ... I will look more into the HomeAssistant(python) code, maybe they get more infos. And I also opened a thread in the Apple Developer Forum ... maybe someone has more infos there.

DO we want to leae open here to collect details?

mrstegeman commented 3 years ago

Sure, feel free to leave it open.

Apollon77 commented 3 years ago

I think I found it :-))

The discovery respnse contains some data ... one is "ff" which are feature flags ... and with them you can detect the method:

Can you check your deice what discovery returns?

My Hue and also one Meross devcie both return "ff=2" which would match to SUPPORTS_SOFTWARE_AUTHENTICATION which means PairSetup :-) So if your device returns 1 here we have it ...

mrstegeman commented 3 years ago

Interesting! Those could be filled in here: https://github.com/mrstegeman/hap-controller-node/blob/main/src/protocol/pairing-protocol.ts#L41-L45

Unfortunately, I don't have the time to do that. This bit doesn't make much sense to me:

        elif self.info["ff"] & FeatureFlags.SUPPORTS_SOFTWARE_AUTHENTICATION:
            with_auth = False
Apollon77 commented 3 years ago

I will do a PR for that in the next days. In my document of hap specs it is a different table and table only says "0x04-0x80 3-8 Reserved." :-) No info on the two bits

Apollon77 commented 3 years ago

@mrstegeman I thought a bit and yes I now know how to basically autodetect the pairing method. But that need more info that in in the service object. For IP it works that way. For BLE it would work if we read some data on discovery (or later before pair-m1) from device characteristic "public.hap.characteristic.pairing.features").

So best idea in my eyes would be to break the HTTL/GattClient constructor and change to handing over the service object instead of the single parameters ... what do you think? Too breaking or ok?

Apollon77 commented 3 years ago

Ok, a friend of mine provided me a way to define an "overloaded constructor" that could support both ways ... but "nice code" is something different ;-))

see https://www.typescriptlang.org/play?#code/C4TwDgpgBAYg9nAggJwOYGcoF4BQV9QA+UA2gIZoCMAXFOsMgJYB2qANFBagEy0BGCADYQyzDlwDMtZgFcAtnwjIOgsvRSoA-LQAUASmwA+KADc4jACYBdPAWLk0AIWnzFyqKvVptUfUdPm1jg4AMaemPBwUADetvghcMz0yDIhwHDIOgB0OVzotJEa6AaxBGVQjABmvnlZwqzAABbYWFhQlETEtfWoTS1t3CVx5WUJScCkXI4qasAaVticaOgA3MNlAL5QEILo0FU1y3UQDc2tbRKdSxjHp-1QACxDIyNj9JNU4mjcX6gSM15UAs2nk1i8oBthpCNkA

Apollon77 commented 3 years ago

Ok, nevermid ... also that way is mixing too many thing ... I have a better idea

Apollon77 commented 3 years ago

see https://github.com/mrstegeman/hap-controller-node/pull/36

Apollon77 commented 3 years ago

It is merged but tests pending

Apollon77 commented 3 years ago

@mrstegeman I re-read the specs and did now understood the PairTypeFFlags and will also adjust the library accordingly

Apollon77 commented 3 years ago

Ok, I will push a new version to GitHub soon. I added support for transient pairSetup calls too