Skip to content

Add support for the new encryption protocol #117

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.

Sign up for GitHub

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

Closed
SimonWilkinson wants to merge 16 commits into from
Closed

Add support for the new encryption protocol #117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 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
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ repos:
additional_dependencies: [toml]

- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.740
rev: v0.790
hooks:
- id: mypy
additional_dependencies: [pycryptodome]
# args: [--no-strict-optional, --ignore-missing-imports]
2 changes: 2 additions & 0 deletions kasa/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
to be handled by the user of the library.
"""
from importlib_metadata import version # type: ignore
from kasa.auth import Auth
from kasa.discover import Discover
from kasa.exceptions import SmartDeviceException
from kasa.protocol import TPLinkSmartHomeProtocol
Expand All @@ -26,6 +27,7 @@


__all__ = [
"Auth",
"Discover",
"TPLinkSmartHomeProtocol",
"SmartBulb",
Expand Down
21 changes: 21 additions & 0 deletions kasa/auth.py
View file
Open in desktop
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()
Comment on lines +11 to +13
Copy link
Member

@rytilahti rytilahti Nov 22, 2020

Choose a reason for hiding this comment

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

Better do the calculations when using the accessors, use case: allow changing the creds without reconstructing the device instance.


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
41 changes: 37 additions & 4 deletions kasa/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import asyncclick as click

from kasa import (
Auth,
Discover,
SmartBulb,
SmartDevice,
Expand Down Expand Up @@ -46,9 +47,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 @@ -67,18 +83,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)
elif strip:
dev = SmartStrip(host)
elif lightstrip:
Expand Down Expand Up @@ -174,9 +199,17 @@ async def dump_discover(ctx, scrub):
async def discover(ctx, timeout, discover_only, dump_raw):
"""Discover devices in the network."""
target = ctx.parent.params["target"]
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
target=target, timeout=timeout, return_raw=dump_raw, authentication=auth
)
if not discover_only:
for ip, dev in found_devs.items():
Expand Down
79 changes: 72 additions & 7 deletions kasa/discover.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
"""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, Mapping, Optional, Type, Union, cast

from kasa.auth import Auth
from kasa.protocol import TPLinkSmartHomeProtocol
from kasa.smartbulb import SmartBulb
from kasa.smartdevice import SmartDevice, SmartDeviceException
Expand Down Expand Up @@ -35,15 +38,18 @@ 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.protocol = TPLinkSmartHomeProtocol()
self.target = (target, Discover.DISCOVERY_PORT)
self.new_target = (target, Discover.NEW_DISCOVERY_PORT)
self.discovered_devices = {}
self.discovered_devices_raw = {}
self.authentication = authentication
self.emptyUser = hashlib.md5().digest()
Copy link
Member

@rytilahti rytilahti Nov 22, 2020

Choose a reason for hiding this comment

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

empty_user could be a static method/property in Auth class.


def connection_made(self, transport) -> None:
"""Set socket options for broadcasting."""
Expand All @@ -62,21 +68,49 @@ def do_discover(self) -> None:
"""Send number of discovery datagrams."""
req = json.dumps(Discover.DISCOVERY_QUERY)
_LOGGER.debug("[DISCOVERY] %s >> %s", self.target, Discover.DISCOVERY_QUERY)
encrypted_req = self.protocol.encrypt(req)
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(self.protocol.decrypt(data))
if port == 9999:
Copy link
Contributor

@kirichkov kirichkov Nov 17, 2020

Choose a reason for hiding this comment

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

I would suggest that instead of relying on a hardcoded ports, to either have a command line option, or even better - attempt the old encryption mechanism, and if that fails - to try the new one.

In respect to this I've already opened a PR #109 that would extract the port and make it configurable and I think it'd quite handy for this implementation.

Copy link
Member

@rytilahti rytilahti Nov 21, 2020

Choose a reason for hiding this comment

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

Considering that there are only two discovery methods, I think it's fine to hard-code (or rather use a const from the class) for this use case.

I'd still separate the klap discovery handling into its own method that gets called only when necessary (so instead of else checking for the port for this, too), and log an error when something arrives from an unknown port.

Open question is how to handle the type detection, as the type information I saw for the new discovery responses does not give out enough information to decide between a smartplug and a strip, for example.

I'm personally for making a sysinfo query, which adds another round-trip but would allow direct reuse of the existing detection logic.

Copy link
Contributor Author

@SimonWilkinson SimonWilkinson Nov 22, 2020

Choose a reason for hiding this comment

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

An example discovery payload is:

{'result': 
    {'ip': '<ip-address>',
      'mac': '<mac-address>', 
      'device_id': '<16 octet hex>', 
      'owner': '<16 octet hex>',
      'device_type': 'IOT.SMARTPLUGSWITCH', 
      'device_model': 'HS110(UK)', 
      'hw_ver': '4.1', 
      'factory_default': True, 
      'mgt_encrypt_schm': 
        {'is_support_https': False, 
         'encrypt_type': 'KLAP', 
         'http_port': 80}
    }, 
    'error_code': 0
}

So we could use the device_model as a way of determining the different between a plug (HS100 or HS110) and a strip (KP303)

info = json.loads(TPLinkSmartHomeProtocol.decrypt(data))
device_class = Discover._get_device_class(info)
device = device_class(ip)
else:
info = json.loads(data[16:])
device_class = Discover._get_new_device_class(info)
owner = Discover._get_new_owner(info)
Copy link
Member

@rytilahti rytilahti Nov 22, 2020

Choose a reason for hiding this comment

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

To simplify the code, maybe something like this:

try:
    owner = bytes.fromhex(info["result"]["owner"])
except KeyError as ex:
    raise SmartDeviceException("Unable to find 'owner' in discovery response") from ex

if owner is not None:
owner_bin = bytes.fromhex(owner)

_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:
Copy link
Member

@rytilahti rytilahti Nov 22, 2020

Choose a reason for hiding this comment

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

The owner cannot be none anymore at this point, so checking if the owner == Auth.empty_user should suffice?

_LOGGER.debug("[DISCOVERY] Device %s has no owner", ip)
device = device_class(ip, Auth())
elif (
self.authentication is not None
Copy link
Member

@rytilahti rytilahti Nov 22, 2020

Choose a reason for hiding this comment

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

The self.authentication needs to be already there if we get this far, so checking for the owner should be enough.

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_class = Discover._get_device_class(info)
device = device_class(ip)
asyncio.ensure_future(device.update())

self.discovered_devices[ip] = device
Expand Down Expand Up @@ -133,6 +167,8 @@ class Discover:

DISCOVERY_PORT = 9999

NEW_DISCOVERY_PORT = 20002

DISCOVERY_QUERY = {
"system": {"get_sysinfo": None},
"emeter": {"get_realtime": None},
Expand All @@ -150,6 +186,7 @@ async def discover(
discovery_packets=3,
return_raw=False,
interface=None,
authentication=None,
) -> Mapping[str, Union[SmartDevice, Dict]]:
"""Discover supported devices.

Expand All @@ -176,6 +213,7 @@ async def discover(
on_discovered=on_discovered,
discovery_packets=discovery_packets,
interface=interface,
authentication=authentication,
),
local_addr=("0.0.0.0", 0),
)
Expand All @@ -202,9 +240,9 @@ async def discover_single(host: str) -> SmartDevice:
:rtype: SmartDevice
:return: Object for querying/controlling found device.
"""
protocol = TPLinkSmartHomeProtocol()
protocol = TPLinkSmartHomeProtocol(host)

info = await protocol.query(host, Discover.DISCOVERY_QUERY)
info = await protocol.query(Discover.DISCOVERY_QUERY)

device_class = Discover._get_device_class(info)
if device_class is not None:
Expand Down Expand Up @@ -242,6 +280,33 @@ def _get_device_class(info: dict) -> Type[SmartDevice]:

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
Copy link
Member

@rytilahti rytilahti Nov 21, 2020

Choose a reason for hiding this comment

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

I would very much prefer to re-use the already existing mechanisms for device type detection, in order to keep the code cleaner & to support both types of devices without maintaining two separate code paths. The reason being that we will likely see devices from other device categories using this new encryption protocol, while we still want to keep on supporting the "legacy" devices.


raise SmartDeviceException("Unknown device type: %s", dtype)

@staticmethod
def _get_new_owner(info: dict) -> Optional[str]:
"""Find owner given new-style discovery payload."""
Copy link
Member

@rytilahti rytilahti Nov 22, 2020

Choose a reason for hiding this comment

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

I think this method should be removed. For incorrect payloads there is not much we can do, but we can wrap it inside a try-except block as I wrote above.

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)
Expand Down
Loading