Skip to content

Conversation

@sdb9696
Copy link
Collaborator

@sdb9696 sdb9696 commented Oct 13, 2024

Notes

  • Only seems to work with multipleRequest - now works with single
  • Need to try passing discovery nonce to handshake1 - didn't work
  • Need to add camera error codes to SmartErrorCodes - done
  • Device often timeouts on first query - default timeout increaed to 10
  • Need to fix tests -done

Tested to work with a C210 camera and an H200 Tapo hub. Thanks @SirWaddles for help with the testing.

@rytilahti rytilahti mentioned this pull request Oct 14, 2024
5 tasks
@codecov
Copy link

codecov bot commented Oct 15, 2024

Codecov Report

Attention: Patch coverage is 60.71429% with 11 lines in your changes missing coverage. Please review.

Project coverage is 92.17%. Comparing base (380fbb9) to head (8716aa7).
Report is 212 commits behind head on master.

Files with missing lines Patch % Lines
kasa/cli/main.py 30.76% 7 Missing and 2 partials ⚠️
kasa/device_factory.py 80.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1165      +/-   ##
==========================================
- Coverage   92.31%   92.17%   -0.15%     
==========================================
  Files          99       99              
  Lines        6365     6387      +22     
  Branches     1577     1584       +7     
==========================================
+ Hits         5876     5887      +11     
- Misses        377      385       +8     
- Partials      112      115       +3     

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

@sdb9696 sdb9696 added the enhancement New feature or request label Oct 15, 2024
@sdb9696 sdb9696 marked this pull request as ready for review October 15, 2024 14:31
@sdb9696 sdb9696 requested a review from rytilahti October 15, 2024 14:32


def _md5_hash(payload: bytes) -> str:
return hashlib.md5(payload).hexdigest().upper() # noqa: S324

Check failure

Code scanning / CodeQL

Use of a broken or weak cryptographic hashing algorithm on sensitive data

[Sensitive data (password)](1) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. [Sensitive data (password)](1) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function.

Copilot Autofix

AI about 1 year ago

To fix the problem, we need to replace the use of the MD5 hashing algorithm with a stronger, modern cryptographic hash function. Since the code already uses SHA-256, we can use it consistently instead of MD5. This change will ensure that the password hashing is secure and resistant to collision attacks.

  • General Fix: Replace the MD5 hashing function with SHA-256 or another strong cryptographic hash function.
  • Detailed Fix: Specifically, replace the _md5_hash function with a function that uses SHA-256. Update the relevant parts of the code to use this new function.
  • Files/Regions/Lines to Change: The changes will be made in the kasa/experimental/sslaestransport.py file, particularly in the _md5_hash function and its usage in the perform_handshake1 method.
  • Requirements: No new imports are needed as SHA-256 is already being used in the code.
Suggested changeset 1
kasa/experimental/sslaestransport.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/kasa/experimental/sslaestransport.py b/kasa/experimental/sslaestransport.py
--- a/kasa/experimental/sslaestransport.py
+++ b/kasa/experimental/sslaestransport.py
@@ -42,4 +42,4 @@
 
-def _md5_hash(payload: bytes) -> str:
-    return hashlib.md5(payload).hexdigest().upper()  # noqa: S324
+def _strong_hash(payload: bytes) -> str:
+    return hashlib.sha256(payload).hexdigest().upper()  # noqa: S324
 
@@ -377,3 +377,3 @@
             assert self._credentials.password
-        pwd_hash = _md5_hash(self._credentials.password.encode())
+        pwd_hash = _strong_hash(self._credentials.password.encode())
         expected_confirm_md5 = self.generate_confirm_hash(
EOF
@@ -42,4 +42,4 @@

def _md5_hash(payload: bytes) -> str:
return hashlib.md5(payload).hexdigest().upper() # noqa: S324
def _strong_hash(payload: bytes) -> str:
return hashlib.sha256(payload).hexdigest().upper() # noqa: S324

@@ -377,3 +377,3 @@
assert self._credentials.password
pwd_hash = _md5_hash(self._credentials.password.encode())
pwd_hash = _strong_hash(self._credentials.password.encode())
expected_confirm_md5 = self.generate_confirm_hash(
Copilot is powered by AI and may make mistakes. Always verify output.


def _sha256_hash(payload: bytes) -> str:
return hashlib.sha256(payload).hexdigest().upper() # noqa: S324

Check failure

Code scanning / CodeQL

Use of a broken or weak cryptographic hashing algorithm on sensitive data

[Sensitive data (password)](1) is used in a hashing algorithm (SHA256) that is insecure for password hashing, since it is not a computationally expensive hash function. [Sensitive data (password)](1) is used in a hashing algorithm (SHA256) that is insecure for password hashing, since it is not a computationally expensive hash function.

Copilot Autofix

AI about 1 year ago

To fix the problem, we need to replace the use of SHA-256 and MD5 for password hashing with a more secure algorithm. Argon2 is a good choice as it is designed specifically for password hashing and includes a per-password salt by default.

  1. Install the argon2-cffi package if it is not already installed.
  2. Replace the _sha256_hash and _md5_hash functions with a function that uses Argon2 for hashing passwords.
  3. Update the code where these functions are called to use the new Argon2-based function.
Suggested changeset 2
kasa/experimental/sslaestransport.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/kasa/experimental/sslaestransport.py b/kasa/experimental/sslaestransport.py
--- a/kasa/experimental/sslaestransport.py
+++ b/kasa/experimental/sslaestransport.py
@@ -42,8 +42,8 @@
 
-def _md5_hash(payload: bytes) -> str:
-    return hashlib.md5(payload).hexdigest().upper()  # noqa: S324
+from argon2 import PasswordHasher
 
+ph = PasswordHasher()
 
-def _sha256_hash(payload: bytes) -> str:
-    return hashlib.sha256(payload).hexdigest().upper()  # noqa: S324
+def _argon2_hash(password: str) -> str:
+    return ph.hash(password)
 
@@ -361,3 +361,3 @@
         if self._credentials and self._credentials != Credentials():
-            pwd_hash = _sha256_hash(self._credentials.password.encode())
+            pwd_hash = _argon2_hash(self._credentials.password)
         else:
@@ -377,3 +377,3 @@
             assert self._credentials.password
-        pwd_hash = _md5_hash(self._credentials.password.encode())
+        pwd_hash = _argon2_hash(self._credentials.password)
         expected_confirm_md5 = self.generate_confirm_hash(
EOF
@@ -42,8 +42,8 @@

def _md5_hash(payload: bytes) -> str:
return hashlib.md5(payload).hexdigest().upper() # noqa: S324
from argon2 import PasswordHasher

ph = PasswordHasher()

def _sha256_hash(payload: bytes) -> str:
return hashlib.sha256(payload).hexdigest().upper() # noqa: S324
def _argon2_hash(password: str) -> str:
return ph.hash(password)

@@ -361,3 +361,3 @@
if self._credentials and self._credentials != Credentials():
pwd_hash = _sha256_hash(self._credentials.password.encode())
pwd_hash = _argon2_hash(self._credentials.password)
else:
@@ -377,3 +377,3 @@
assert self._credentials.password
pwd_hash = _md5_hash(self._credentials.password.encode())
pwd_hash = _argon2_hash(self._credentials.password)
expected_confirm_md5 = self.generate_confirm_hash(
pyproject.toml
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/pyproject.toml b/pyproject.toml
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -9,2 +9,3 @@
 dependencies = [
+  "argon2-cffi==23.1.0",
   "asyncclick>=8.1.7",
EOF
@@ -9,2 +9,3 @@
dependencies = [
"argon2-cffi==23.1.0",
"asyncclick>=8.1.7",
This fix introduces these dependencies
Package Version Security advisories
argon2-cffi (pypi) 23.1.0 None
Copilot is powered by AI and may make mistakes. Always verify output.
@sdb9696 sdb9696 merged commit dcc36e1 into master Oct 16, 2024
@sdb9696 sdb9696 deleted the feat/tapo_cam branch October 16, 2024 15:53
@sdb9696 sdb9696 added this to the 0.7.6 milestone Oct 29, 2024
@sdb9696 sdb9696 mentioned this pull request Oct 29, 2024
sdb9696 added a commit that referenced this pull request Oct 29, 2024
## [0.7.6](https://github.com/python-kasa/python-kasa/tree/0.7.6) (2024-10-29)

[Full Changelog](0.7.5...0.7.6)

**Release summary:**

- Experimental support for Tapo cameras and the Tapo H200 hub which uses the same protocol.
- Better timestamp support across all devices.
- Support for new devices P304M, S200D and S200B (see README.md for note on the S200 support).
- Various other fixes and minor features.

**Implemented enhancements:**

- Add support for setting the timezone [\#436](#436)
- Add stream\_rtsp\_url to camera module [\#1197](#1197) (@sdb9696)
- Try default logon credentials in SslAesTransport [\#1195](#1195) (@sdb9696)
- Allow enabling experimental devices from environment variable [\#1194](#1194) (@sdb9696)
- Add core device, child and camera modules to smartcamera [\#1193](#1193) (@sdb9696)
- Fallback to get\_current\_power if get\_energy\_usage does not provide current\_power [\#1186](#1186) (@Fulch36)
- Add https parameter to device class factory [\#1184](#1184) (@sdb9696)
- Add discovery list command to cli [\#1183](#1183) (@sdb9696)
- Add Time module to SmartCamera devices [\#1182](#1182) (@sdb9696)
- Add try\_connect\_all to allow initialisation without udp broadcast [\#1171](#1171) (@sdb9696)
- Update dump\_devinfo for smart camera protocol [\#1169](#1169) (@sdb9696)
- Enable newer encrypted discovery protocol [\#1168](#1168) (@sdb9696)
- Initial TapoCamera support [\#1165](#1165) (@sdb9696)
- Add waterleak alert timestamp [\#1162](#1162) (@rytilahti)
- Create common Time module and add time set cli command [\#1157](#1157) (@sdb9696)

**Fixed bugs:**

- Only send 20002 discovery request with key included [\#1207](#1207) (@sdb9696)
- Fix SslAesTransport default login and add tests [\#1202](#1202) (@sdb9696)
- Fix device\_config serialisation of https value [\#1196](#1196) (@sdb9696)

**Added support for devices:**

- Add S200B\(EU\) fw 1.11.0 fixture [\#1205](#1205) (@sdb9696)
- Add TC65 fixture [\#1200](#1200) (@rytilahti)
- Add P304M\(UK\) test fixture [\#1185](#1185) (@Fulch36)
- Add H200 experimental fixture [\#1180](#1180) (@sdb9696)
- Add S200D button fixtures [\#1161](#1161) (@rytilahti)

**Project maintenance:**

- Fix mypy errors in parse_pcap_klap [\#1214](#1214) (@sdb9696)
- Make HSV NamedTuple creation more efficient [\#1211](#1211) (@sdb9696)
- dump\_devinfo: query get\_current\_brt for iot dimmers [\#1209](#1209) (@rytilahti)
- Add trigger\_logs and double\_click to dump\_devinfo helper [\#1208](#1208) (@sdb9696)
- Fix smartcamera childdevice module [\#1206](#1206) (@sdb9696)
- Add H200\(EU\) fw 1.3.2 fixture [\#1204](#1204) (@sdb9696)
- Do not pass None as timeout to http requests [\#1203](#1203) (@sdb9696)
- Update SMART test framework to use fake child protocols [\#1199](#1199) (@sdb9696)
- Allow passing an aiohttp client session during discover try\_connect\_all [\#1198](#1198) (@sdb9696)
- Add test framework for smartcamera [\#1192](#1192) (@sdb9696)
- Rename experimental fixtures folder to smartcamera [\#1191](#1191) (@sdb9696)
- Combine smartcamera error codes into SmartErrorCode [\#1190](#1190) (@sdb9696)
- Allow deriving from SmartModule without being registered [\#1189](#1189) (@sdb9696)
- Improve supported module checks for hub children [\#1188](#1188) (@sdb9696)
- Update smartcamera to support single get/set/do requests [\#1187](#1187) (@sdb9696)
- Add S200B\(US\) fw 1.12.0 fixture [\#1181](#1181) (@sdb9696)
- Add T110\(US\), T310\(US\) and T315\(US\) sensor fixtures [\#1179](#1179) (@sdb9696)
- Enforce EOLs for \*.rst and \*.md [\#1178](#1178) (@rytilahti)
- Convert fixtures to use unix newlines [\#1177](#1177) (@rytilahti)
- Add motion sensor to known categories [\#1176](#1176) (@rytilahti)
- Drop urllib3 dependency and create ssl context in executor thread [\#1175](#1175) (@sdb9696)
- Expose smart child device map as a class constant [\#1173](#1173) (@sdb9696)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants