Adding support for the new encryption protocol (updated version) #267
Conversation
This adds support for the new TP-Link discovery and encryption protocols. It is currently incomplete - only devices without username and password are current supported, and single device discovery is not implemented. Discovery should find both old and new devices. When accessing a device by IP the --klap option can be specified on the command line to active the new connection protocol.
Restore the incorrectly commented out old discovery mechanism, so both old and new devices are found.
Add the --user and --password command line options which can be used to provide a username and password to authenticate to the smart device using the KLAP protocol. Check the 'owner' field in the discovery packet, and only attempt to authenticate to devices for which we have a password.
Add a new class which holds authentication information for the KLAP protocol
Making an emeter request against an HS100 plug crashes it. So don't.
|
👋 What's needed to get this reviewed/merged? Anything I can do to help test? Also, is there any documentation on how to monkeypatch hassio, i'm happy to get this branch running on my install. |
There was a problem hiding this comment.
Thanks for reviving the PR and working on this! I added some comments inline, as a general comment we are going to need unittests for the implementation.
API-wise it might be better if the KAP would not inherit from the TPLinkSmartHomeProtocol, but that the given protocol (be it the xor one or klap) would be used as necessary by _encrypt and _decrypt. This would simplify the protocol initialization to always be TPLinkSmartHomeProtocol(host, authentication=None) which opens it for future encryption protocols, if the need ever comes up.
What do you think, does that make sense?
| if __name__ == "__main__": | ||
| logging.basicConfig(level=logging.INFO) | ||
| loop = asyncio.get_event_loop() | ||
|
|
||
| async def _on_device(dev): | ||
| await dev.update() | ||
| _LOGGER.info("Got device: %s", dev) | ||
|
|
||
| devices = loop.run_until_complete(Discover.discover(on_discovered=_on_device)) | ||
| for ip, dev in devices.items(): | ||
| print(f"[{ip}] {dev}") |
There was a problem hiding this comment.
| if __name__ == "__main__": | |
| logging.basicConfig(level=logging.INFO) | |
| loop = asyncio.get_event_loop() | |
| async def _on_device(dev): | |
| await dev.update() | |
| _LOGGER.info("Got device: %s", dev) | |
| devices = loop.run_until_complete(Discover.discover(on_discovered=_on_device)) | |
| for ip, dev in devices.items(): | |
| print(f"[{ip}] {dev}") |
Let's remove this. If needed, we should improve the kasa discover instead of this.
|
|
||
| Encryption/Decryption methods based on the works of | ||
| Lubomir Stroetmann and Tobias Esser | ||
|
|
||
| https://www.softscheck.com/en/reverse-engineering-tp-link-hs110/ | ||
| https://github.com/softScheck/tplink-smartplug/ | ||
|
|
||
| which are licensed under the Apache License, Version 2.0 | ||
| http://www.apache.org/licenses/LICENSE-2.0 |
There was a problem hiding this comment.
I don't think this file is anymore based on the work done in those sources :-)
| local_hash.hex(), | ||
| server_hash.hex(), | ||
| ) | ||
| raise SmartDeviceException("Server response doesn't match our challenge") |
There was a problem hiding this comment.
| raise SmartDeviceException("Server response doesn't match our challenge") | |
| raise SmartDeviceException("Device response doesn't match our challenge") |
| server_hash.hex(), | ||
| ) | ||
| raise SmartDeviceException("Server response doesn't match our challenge") | ||
| else: |
There was a problem hiding this comment.
Unnecessary else, either output that to debug log without it, or remove if it's not useful to have around.
| elif bulb: | ||
| dev = SmartBulb(host) | ||
| elif plug: | ||
| dev = SmartPlug(host) | ||
| dev = SmartPlug(host, authentication) |
There was a problem hiding this comment.
The authentication should probably be passed to all of these? Please rebase and skip the authentication for the deprecated flags (see https://github.com/python-kasa/python-kasa/blob/master/kasa/cli.py#L99).
| @@ -205,13 +205,13 @@ async def erase_emeter_stats(self): | |||
| @requires_update | |||
| def emeter_this_month(self) -> Optional[float]: | |||
| """Return this month's energy consumption in kWh.""" | |||
| return sum([plug.emeter_this_month for plug in self.children]) | |||
| return sum(plug.emeter_this_month for plug in self.children) | |||
There was a problem hiding this comment.
This are unrelated changes, but they should disappear when you rebase (the master has this already), so please rebase.
| if resp.status == 403: | ||
| self.handshake_done = False | ||
|
|
There was a problem hiding this comment.
Should this raise an exception, or is it expected to fall through to the decryption procedures?
| def _get_new_device_class(info: dict) -> Type[SmartDevice]: | ||
| """Find SmartDevice subclass given new discovery payload.""" | ||
| if "result" not in info: | ||
| raise SmartDeviceException("No 'result' in discovery response") | ||
|
|
||
| if "device_type" not in info["result"]: | ||
| raise SmartDeviceException("No 'device_type' in discovery result") | ||
|
|
||
| dtype = info["result"]["device_type"] | ||
|
|
||
| if dtype == "IOT.SMARTPLUGSWITCH": | ||
| return SmartPlug | ||
|
|
||
| raise SmartDeviceException("Unknown device type: %s", dtype) |
There was a problem hiding this comment.
I don't like the fact that there are two separate paths for the discovery, we should find a way to somehow generalize the code to handle model detection for both discovery protocols.
| if device_class is not None: | ||
| if authentication is None: | ||
| dev = device_class(host) | ||
| else: | ||
| dev = device_class(host, authentication) | ||
| await dev.update() | ||
| return dev |
There was a problem hiding this comment.
| if device_class is not None: | |
| if authentication is None: | |
| dev = device_class(host) | |
| else: | |
| dev = device_class(host, authentication) | |
| await dev.update() | |
| return dev | |
| dev = device_class(host, authentication=authentication) | |
| await dev.update() |
It's better to keep the original code, and just add keyword-only authentication parameter to SmartDevice class.
| # In theory we should verify the hmac here too | ||
| return Padding.unpad(cipher.decrypt(payload[32:]), AES.block_size) | ||
|
|
||
| async def _ask(self, request: str) -> str: |
There was a problem hiding this comment.
This is not being called at all?
|
hey thanks for the PR! Thanks! |
|
Was this PR (and Simon's) reviewed to death? It's a shame it never went in, this is an incredibly useful tool. |
|
Is there anyone who can look into this readthedocs error? I'd really like to start using this library for a few things with newer devices, like a KL420V5 light strip... |
|
@mystcb Thanks a lot for your efforts! Are you still working on pushing this over the finish line? |
|
Hi all, Apologies for the delay in responding to this - between a move of a house and using a different set of remote plugs I lost track of this completely. What I do have is the broken plugs next to me, so I will take a look and see what needs to be done - I realise that a lot of the code is now very out of date from the main branch, so will need to look into that and merge. If someone else is already looking at this, please let me now - as I will need to remember how to get this up to date and such! But I will take another look, and see if I can this set of plugs I have sitting next to me working Colin |
|
Actually, just spotted that another PR is already open with some of the changes in Going to have a look through them now. |
Taking off where #117 left off, I have been making some additional amendments to make this work. As it stands this version is very rough, but completes some of the additions that @SimonWilkinson made from his branch.
During testing, I made a number of commits that went backwards and forwards in an attempt to get this working with HomeAssistant (only to find my dev setup was at fault). So I have tried to squash those changes into a single commit.
This version also merges changes made in the current master branch made yesterday, so hopefully as up to date as I can get it to that.
From the work Simon put in, this version should also fix the single device discovery issue as well.
Thank you Simon for doing most of the work, and apologies for the roughness of this PR!
Fixes #170 fixes #115