Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support for the new encryption protocol (updated version) #267

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
f6fc0de
Add support for the new encryption protocol
SimonWilkinson Nov 17, 2020
efcbece
Fix: Don't comment out old-style discovery
SimonWilkinson Nov 17, 2020
cacb996
Update poetry.lock file for new dependencies
SimonWilkinson Nov 18, 2020
901b0ce
Changes to pass commit checks
SimonWilkinson Nov 18, 2020
3c2709d
Teach pre-commit about pycryptodome
SimonWilkinson Nov 18, 2020
fbc7335
Add username and password authentication support for KLAP
SimonWilkinson Nov 18, 2020
2bd9976
Try forcing a newer mypy in pre-commit
SimonWilkinson Nov 18, 2020
b8df6ee
Add an authentication class
SimonWilkinson Nov 22, 2020
a74e60d
Move KLAP protocol implementation into its own file
SimonWilkinson Nov 22, 2020
75992bf
Address simpler review comments on the KLAPProtocol class
SimonWilkinson Nov 22, 2020
c3d1fc6
Add a base class for all protocols, and move shared logic there
SimonWilkinson Nov 22, 2020
9500d7b
Force a new handshake if the plug returns a 403 error
SimonWilkinson Nov 22, 2020
d6ef3cb
Use the specified timeout for KLAP queries, too
SimonWilkinson Nov 22, 2020
2d6ce06
Relax dependencies
SimonWilkinson Nov 22, 2020
d9a75e9
Fix tests
SimonWilkinson Nov 22, 2020
b69ca9e
Don't make emeter requests when they're not supported
SimonWilkinson Feb 28, 2021
f1a4b05
Added self declaration
mystcb Nov 16, 2021
3322dd6
Squashing updates: Added additional auth features throughout the code…
mystcb Nov 16, 2021
c4ea995
Initial upstream merge
mystcb Dec 7, 2021
f2aa66c
Merged latest changes from master branch
mystcb Dec 7, 2021
cef9e56
Initial run of pre-commit
mystcb Dec 7, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Binary file added .DS_Store
Binary file not shown.
2 changes: 2 additions & 0 deletions kasa/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"""
from importlib_metadata import version # type: ignore

from kasa.auth import Auth
from kasa.discover import Discover
from kasa.emeterstatus import EmeterStatus
from kasa.exceptions import SmartDeviceException
Expand All @@ -28,6 +29,7 @@


__all__ = [
"Auth",
"Discover",
"TPLinkSmartHomeProtocol",
"SmartBulb",
Expand Down
21 changes: 21 additions & 0 deletions kasa/auth.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
"""Authentication class for KASA username / passwords."""
from hashlib import md5


class Auth:
"""Authentication for Kasa KLAP authentication."""

def __init__(self, user: str = "", password: str = ""):
self.user = user
self.password = password
self.md5user = md5(user.encode()).digest()
self.md5password = md5(password.encode()).digest()
self.md5auth = md5(self.md5user + self.md5password).digest()

def authenticator(self):
"""Return the KLAP authenticator for these credentials."""
return self.md5auth

def owner(self):
"""Return the MD5 hash of the username in this object."""
return self.md5user
45 changes: 40 additions & 5 deletions kasa/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import asyncclick as click

from kasa import (
Auth,
Discover,
SmartBulb,
SmartDevice,
Expand Down Expand Up @@ -44,9 +45,24 @@
@click.option("--plug", default=False, is_flag=True)
@click.option("--lightstrip", default=False, is_flag=True)
@click.option("--strip", default=False, is_flag=True)
@click.option("--klap", default=False, is_flag=True)
@click.option(
"--user",
default="",
required=False,
help="Username/email address to authenticate to device.",
)
@click.option(
"--password",
default="",
required=False,
help="Password to use to authenticate to device.",
)
@click.version_option()
@click.pass_context
async def cli(ctx, host, alias, target, debug, bulb, plug, lightstrip, strip):
async def cli(
ctx, host, alias, target, debug, bulb, plug, lightstrip, strip, klap, user, password
):
"""A tool for controlling TP-Link smart home devices.""" # noqa
if debug:
logging.basicConfig(level=logging.DEBUG)
Expand All @@ -65,18 +81,27 @@ async def cli(ctx, host, alias, target, debug, bulb, plug, lightstrip, strip):
click.echo(f"No device with name {alias} found")
return

if password != "" and user == "":
click.echo("Using a password requires a username")
return

if klap or user != "":
authentication = Auth(user=user, password=password)
else:
authentication = None

if host is None:
click.echo("No host name given, trying discovery..")
await ctx.invoke(discover)
return
else:
if not bulb and not plug and not strip and not lightstrip:
click.echo("No --strip nor --bulb nor --plug given, discovering..")
dev = await Discover.discover_single(host)
dev = await Discover.discover_single(host, authentication)
elif bulb:
dev = SmartBulb(host)
elif plug:
dev = SmartPlug(host)
dev = SmartPlug(host, authentication)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

elif strip:
dev = SmartStrip(host)
elif lightstrip:
Expand Down Expand Up @@ -135,8 +160,18 @@ async def join(dev: SmartDevice, ssid, password, keytype):
async def discover(ctx, timeout, discover_only, dump_raw):
"""Discover devices in the network."""
target = ctx.parent.params["target"]
click.echo(f"Discovering devices on {target} for {timeout} seconds")
found_devs = await Discover.discover(target=target, timeout=timeout)
user = ctx.parent.params["user"]
password = ctx.parent.params["password"]

if user:
auth = Auth(user=user, password=password)
else:
auth = None

click.echo(f"Discovering devices for {timeout} seconds")
found_devs = await Discover.discover(
target=target, timeout=timeout, return_raw=dump_raw, authentication=auth
)
if not discover_only:
for ip, dev in found_devs.items():
if dump_raw:
Expand Down
114 changes: 99 additions & 15 deletions kasa/discover.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
"""Discovery module for TP-Link Smart Home devices."""
import asyncio
import binascii
import hashlib
import json
import logging
import socket
from typing import Awaitable, Callable, Dict, Optional, Type, cast
from typing import Awaitable, Callable, Dict, Mapping, Optional, Type, Union, cast

from kasa.auth import Auth
from kasa.klapprotocol import TPLinkKLAP
from kasa.protocol import TPLinkSmartHomeProtocol
from kasa.smartbulb import SmartBulb
from kasa.smartdevice import SmartDevice, SmartDeviceException
Expand Down Expand Up @@ -35,13 +39,17 @@ def __init__(
target: str = "255.255.255.255",
discovery_packets: int = 3,
interface: Optional[str] = None,
authentication: Optional[Auth] = None,
):
self.transport = None
self.discovery_packets = discovery_packets
self.interface = interface
self.on_discovered = on_discovered
self.target = (target, Discover.DISCOVERY_PORT)
self.new_target = (target, Discover.NEW_DISCOVERY_PORT)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.new_target = (target, Discover.NEW_DISCOVERY_PORT)
self.klap_target = (target, Discover.NEW_DISCOVERY_PORT)

self.discovered_devices = {}
self.authentication = authentication
self.emptyUser = hashlib.md5().digest()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.emptyUser = hashlib.md5().digest()

Not worth to introduce a new variable for this, it can be created when needed.


def connection_made(self, transport) -> None:
"""Set socket options for broadcasting."""
Expand All @@ -61,26 +69,49 @@ def do_discover(self) -> None:
req = json.dumps(Discover.DISCOVERY_QUERY)
_LOGGER.debug("[DISCOVERY] %s >> %s", self.target, Discover.DISCOVERY_QUERY)
encrypted_req = TPLinkSmartHomeProtocol.encrypt(req)
new_req = binascii.unhexlify("020000010000000000000000463cb5d3")
for i in range(self.discovery_packets):
self.transport.sendto(encrypted_req[4:], self.target) # type: ignore
self.transport.sendto(new_req, self.new_target) # type: ignore

def datagram_received(self, data, addr) -> None:
"""Handle discovery responses."""
ip, port = addr
if ip in self.discovered_devices:
return

info = json.loads(TPLinkSmartHomeProtocol.decrypt(data))
_LOGGER.debug("[DISCOVERY] %s << %s", ip, info)

try:
if port == 9999:
info = json.loads(TPLinkSmartHomeProtocol.decrypt(data))
device_class = Discover._get_device_class(info)
except SmartDeviceException as ex:
_LOGGER.debug("Unable to find device type from %s: %s", info, ex)
return
device = device_class(ip)
else:
info = json.loads(data[16:])
device_class = Discover._get_new_device_class(info)
owner = Discover._get_new_owner(info)
if owner is not None:
owner_bin = bytes.fromhex(owner)
Comment on lines +88 to +92
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please separate the klap handling into its own, internal method. Maybe this (and the related methods) should be part of the klapprotocol module, preferably just offering an interface like KLAPProtocol.get_device_for_auth(payload, auth)?

In the current form it is hard to write unittests for its behavior.


_LOGGER.debug(
"[DISCOVERY] Device owner is %s, empty owner is %s",
owner_bin,
self.emptyUser,
)
if owner is None or owner == "" or owner_bin == self.emptyUser:
_LOGGER.debug("[DISCOVERY] Device %s has no owner", ip)
device = device_class(ip, Auth())
elif (
self.authentication is not None
and owner_bin == self.authentication.owner()
):
_LOGGER.debug("[DISCOVERY] Device %s has authenticated owner", ip)
device = device_class(ip, self.authentication)
else:
_LOGGER.debug("[DISCOVERY] Found %s with unknown owner %s", ip, owner)
return

_LOGGER.debug("[DISCOVERY] %s << %s", ip, info)

device = device_class(ip)
device.update_from_discover_info(info)
asyncio.ensure_future(device.update())

self.discovered_devices[ip] = device

Expand Down Expand Up @@ -132,6 +163,8 @@ class Discover:

DISCOVERY_PORT = 9999

NEW_DISCOVERY_PORT = 20002

DISCOVERY_QUERY = {
"system": {"get_sysinfo": None},
}
Expand All @@ -144,7 +177,8 @@ async def discover(
timeout=5,
discovery_packets=3,
interface=None,
) -> DeviceDict:
authentication=None,
) -> Mapping[str, Union[SmartDevice, Dict]]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmh, why is this change necessary, what's wrong with DeviceDict? The interface should be as clear as possible to make, and that's the aim of returning that instead of some other structure.

"""Discover supported devices.

Sends discovery message to 255.255.255.255:9999 in order
Expand All @@ -171,6 +205,7 @@ async def discover(
on_discovered=on_discovered,
discovery_packets=discovery_packets,
interface=interface,
authentication=authentication,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The authentication needs to be documented in the docstring.

),
local_addr=("0.0.0.0", 0),
)
Expand All @@ -187,20 +222,29 @@ async def discover(
return protocol.discovered_devices

@staticmethod
async def discover_single(host: str) -> SmartDevice:
async def discover_single(host: str, authentication=None) -> SmartDevice:
"""Discover a single device by the given IP address.

:param host: Hostname of device to query
:rtype: SmartDevice
:return: Object for querying/controlling found device.
"""
protocol = TPLinkSmartHomeProtocol(host)
if authentication is None:
protocol = TPLinkSmartHomeProtocol(host)
else:
protocol = TPLinkKLAP(host, authentication)
# protocol = TPLinkSmartHomeProtocol(host)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# protocol = TPLinkSmartHomeProtocol(host)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove.


info = await protocol.query(Discover.DISCOVERY_QUERY)

device_class = Discover._get_device_class(info)
dev = device_class(host)
await dev.update()
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
Comment on lines +241 to +247
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.


return dev

Expand Down Expand Up @@ -231,3 +275,43 @@ def _get_device_class(info: dict) -> Type[SmartDevice]:
return SmartBulb

raise SmartDeviceException("Unknown device type: %s" % type_)

@staticmethod
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)
Comment on lines +280 to +293
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


@staticmethod
def _get_new_owner(info: dict) -> Optional[str]:
"""Find owner given new-style discovery payload."""
if "result" not in info:
raise SmartDeviceException("No 'result' in discovery response")

if "owner" not in info["result"]:
return None

return info["result"]["owner"]


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}")
Comment on lines +307 to +317
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.