8000 Decrypt KLAP data from PCAP files by clstrickland · Pull Request #1041 · python-kasa/python-kasa · GitHub
[go: up one dir, main page]
Skip to content

Decrypt KLAP data from PCAP files #1041

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

Jump to bottom
Merged
merged 13 commits into from
Jul 15, 2024
Merged

Decrypt KLAP data from PCAP files #1041

merged 13 commits into from
Jul 15, 2024

Conversation

clstrickland
Copy link
Contributor

A tool to allow KLAP data to be exported, in JSON, from a PCAP file of encrypted requests.

@clstrickland
Decrypt KLAP data from PCAP files
4d5bbfe 
A tool to allow KLAP data to be exported, in JSON, from a PCAP file of encrypted requests.
@clstrickland
Copy link
Contributor Author

It looks like it's only unhappy about pyshark being untyped. I would think that, especially since this is just a dev tool, this is something that can safely be ignored?

Aside from that, I think I've convinced Ruff that the code is well-formatted, so it should be good to go.

@clstrickland clstrickland marked this pull request as ready for review July 3, 2024 23:55
rytilahti
rytilahti previously requested changes Jul 11, 2024
View reviewed changes
Copy link
Member
@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

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

This is really cool, thanks for working on it! I don't currently have any pcaps to test, but I added some comments to make the script easier to use. On using click, you can take a look into parse_pcap.py and dump_devinfo.py for some inspiration.

@rytilahti rytilahti added the enhancement New feature or request label Jul 11, 2024
sdb9696
sdb9696 requested changes Jul 11, 2024
View reviewed changes
Copy link
Collaborator
@sdb9696 sdb9696 left a comment

Choose a reason for hiding this comment

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

This looks great, I've ordered a wifi dongle with monitor mode so I can generate a PCAP file to play with. @rytilahti maybe we should add some of the devtools dependencies as a package extra? Or maybe even devtools itself as a package that can be installed as an extra?

Minor change requested to internal variables in additions to @rytilahti comments

@clstrickland
Copy link
Contributor Author

@sdb9696 if you have Android you can use PCAPdroid. That's what I used.

There is an issue with TP-Link's app where, when using PCAPdroid, it thinks it no longer has local access to smart devices. The trick is to open the TP-Link app, establish a connection to the smart device, then start up PCAPdroid. Finally, you need to physically restart the smart device so that the existing handshake is invalidated and it goes through it again now that the packets are being captured.

@sdb9696
Copy link
Collaborator
sdb9696 commented Jul 12, 2024 •
edited
Loading

@sdb9696 if you have Android you can use PCAPdroid. That's what I used.

There is an issue with TP-Link's app where, when using PCAPdroid, it thinks it no longer has local access to smart devices. The trick is to open the TP-Link app, establish a connection to the smart device, then start up PCAPdroid. Finally, you need to physically restart the smart device so that the existing handshake is invalidated and it goes through it again now that the packets are being captured.

Hi @clstrickland, I got this working in the end with an android emulator and wireshark so no need to restart the device, just the emulator.

I didn't get any issues with odd characters and I'm wondering if it's because you're getting the sequence numbers from all the devices rather than the just the device_ip you want. I would try moving the

                    if packet.ip.dst != device_ip:
                        continue

further up in the logic and only getting the sequence number for the device you're targeting.

Really great output though, super helpful!

sdb9696
sdb9696 reviewed Jul 12, 2024
View reviewed changes
@sdb9696 sdb9696 mentioned this pull request Jul 12, 2024
Merged
@clstrickland
Copy link
Contributor Author

I didn't get any issues with odd characters and I'm wondering if it's because you're getting the sequence numbers from all the devices rather than the just the device_ip you want.

I was able to get that working before making the PR. I had tried using seq numbers to no avail when I made that original comment about the weird characters. It turns out that, in my original script, I was setting the seq number but then it was being overridden by internal logic for the encryption. Once I fixed that, it worked perfectly.

@sdb9696
Copy link
Collaborator
sdb9696 commented Jul 12, 2024

I was able to get that working before making the PR. I had tried using seq numbers to no avail when I made that original comment about the weird characters. It turns out that, in my original script, I was setting the seq number but then it was being overridden by internal logic for the encryption. Once I fixed that, it worked perfectly.

Cool, I would still move the:

 if packet.ip.dst != device_ip:
                        continue

up above the sequence number bit though no?

Are you good with all the review comments? It'd be nice to get this merged soonish as we could ask a few people to run it against some of their more interesting devices once the cli parameters are added.

@clstrickland
Copy link
Contributor Author

I was able to get that working before making the PR. I had tried using seq numbers to no avail when I made that original comment about the weird characters. It turns out that, in my original script, I was setting the seq number but then it was being overridden by internal logic for the encryption. Once I fixed that, it worked perfectly.

Cool, I would still move the:

 if packet.ip.dst != device_ip:
                        continue

up above the sequence number bit though no?

I could not put it any higher up because in handshake1 we need to grab the response, which will not have a matching device IP. If I put it any further above, it would strip out that packet completely.

Are you good with all the review comments? It'd be nice to get this merged soonish as we could ask a few people to run it against some of their more interesting devices once the cli parameters are added.

I'm going to see what I can do right now, but my power (and therefore internet connection) has fallen victim to Hurricane Beryl, so I'm kinda at the mercy of Mother Nature right now 🥲

rytilahti
rytilahti reviewed Jul 14, 2024
View reviewed changes
Copy link
Member
@rytilahti rytilahti left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Some more comments, this looks pretty much ready to go, thanks @clstrickland!

Please run pre-commit run -a, it will likely fix most of the whitespace issues & the linting errors.

clstrickland and others added 6 commits July 14, 2024 19:09
@clstrickland
Copy link
Contributor Author

@rytilahti I renamed symbols. I just used the pre-commit command, so I assume it automatically fixed all those spacing issues. I think I stopped running it at some point because (part of) it always failed due to pyshark being untyped.

As far as dependencies go, I agree with @sdb9696 (#1041 (review)) that it should somehow be included in the existing package management solution. I removed the requirements file and put a note in the readme, but it would still be really cool to see it in a devtools extra.

@codecov Codecov
Copy link
codecov bot commented Jul 15, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.08%. Comparing base (9cffbe9) to head (dfd776d).
Report is 88 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1041      +/-   ##
==========================================
+ Coverage   91.98%   92.08%   +0.09%     
==========================================
  Files          84       85       +1     
  Lines        5762     5885     +123     
  Branches     1395     1426      +31     
==========================================
+ Hits         5300     5419     +119     
- Misses        356      359       +3     
- Partials      106      107       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sdb9696
sdb9696 approved these changes Jul 15, 2024
View reviewed changes
Copy link
Collaborator
@sdb9696 sdb9696 left a comment

Choose a reason for hiding this comment

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

LGTM, fixed the CI and made some tweaks to get it working on linux and with asyncclick

Thanks for the great PR @clstrickland !

@sdb9696 sdb9696 dismissed rytilahti’s stale review July 15, 2024 12:17

Review comments all addressed

@sdb9696 sdb9696 merged commit 7e9b168 into python-kasa:master Jul 15, 2024
25 checks passed
@sdb9696 sdb9696 added this to the 0.7.1 milestone Jul 24, 2024
@sdb9696 sdb9696 mentioned this pull request Jul 31, 2024
Merged
sdb9696 added a commit that referenced this pull request Jul 31, 2024
@sdb9696
Prepare 0.7.1 (#1094)
145a16d 
## [0.7.1](https://github.com/python-kasa/python-kasa/tree/0.7.1) (2024-07-31)

[Full Changelog](0.7.0.5...0.7.1)

**Release highlights:**
- This release consists mainly of bugfixes and project improvements.
- There is also new support for Tapo T100 motion sensors.
- The CLI now supports child devices on all applicable commands.

**Implemented enhancements:**

- Expose reboot action [\#1073](#1073) (@rytilahti)
- Decrypt KLAP data from PCAP files [\#1041](#1041) (@clstrickland)
- Support child devices in all applicable cli commands [\#1020](#1020) (@sdb9696)

**Fixed bugs:**

- Fix iot light effect brightness [\#1092](#1092) (@sdb9696)
- Enable setting brightness with color temp for smart devices [\#1091](#1091) (@sdb9696)
- Do not send light\_on value to iot bulb set\_state [\#1090](#1090) (@sdb9696)
- Allow erroring modules to recover [\#1080](#1080) (@sdb9696)
- Raise KasaException on decryption errors [\#1078](#1078) (@sdb9696)
- Update smart request parameter handling [\#1061](#1061) (@sdb9696)
- Fix light preset module when list contains lighting effects [\#1048](#1048) (@sdb9696)
- Handle module errors more robustly and add query params to light preset and transition [\#1036](#1036) (@sdb9696)
- Fix credential hash to return None on empty credentials [\#1029](#1029) (@sdb9696)

**Added support for devices:**

- Add support for T100 motion sensor [\#1079](#1079) (@rytilahti)

**Project maintenance:**

- Bump project version to 0.7.0.5 [\#1087](#1087) (@sdb9696)
- Fix generate\_supported pre commit to run in venv [\#1085](#1085) (@sdb9696)
- Fix intermittently failing decryption error test [\#1082](#1082) (@sdb9696)
- Fix mypy pre-commit hook on windows [\#1081](#1081) (@sdb9696)
- Update RELEASING.md for patch releases [\#1076](#1076) (@sdb9696)
- Use monotonic time for query timing [\#1070](#1070) (@sdb9696)
- Fix parse\_pcap\_klap on windows and support default credentials [\#1068](#1068) (@sdb9696)
- Add fixture file for KP405 fw 1.0.6 [\#1063](#1063) (@daleye)
- Bump project version to 0.7.0.3 [\#1053](#1053) (@sdb9696)
- Add KP400\(US\) v1.0.4 fixture [\#1051](#1051) (@gimpy88)
- Add new HS220 kasa aes fixture [\#1050](#1050) (@sdb9696)
- Add KS205\(US\) v1.1.0 fixture [\#1049](#1049) (@gimpy88)
- Add KS200M\(US\) v1.0.11 fixture [\#1047](#1047) (@sdb9696)
- Add KS225\(US\) v1.1.0 fixture [\#1046](#1046) (@sdb9696)
- Split out main cli module into lazily loaded submodules [\#1039](#1039) (@sdb9696)
- Structure cli into a package [\#1038](#1038) (@sdb9696)
- Add KP400 v1.0.3 fixture [\#1037](#1037) (@gimpy88)
- Add L920\(EU\) v1.1.3 fixture [\#1031](#1031) (@rytilahti)
- Update changelog generator config [\#1030](#1030) (@sdb9696)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rytilahti rytilahti rytilahti left review comments

@sdb9696 sdb9696 sdb9696 approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
0.7.1
Development

Successfully merging this pull request may close these issues.
3 participants
@clstrickland @sdb9696 @rytilahti
0