Conversation
A credentials_hash is specific to the transport that made it. Klap stores the base64 of a raw digest, aes and ssltransport store base64 json of hashed credentials, and sslaestransport stores base64 json of the plaintext. Nothing checked that the hash a transport was handed was its own. A device can change its encryption type without the credentials changing. Toggling Third-Party Compatibility on a Tapo device moves it between tpap and klap, and discovery then hands the transport a stored hash the other transport wrote. Klap decodes any base64 successfully, so it built _local_auth_hash out of another transport's json and failed handshake1, reporting "Device response did not match our challenge". Callers cannot tell that from a wrong password; Home Assistant acts on it by deleting the stored hash and asking the user to reauthenticate. aestransport and ssltransport are worse and raise UnicodeDecodeError from the constructor, and sslaestransport raises KeyError. Check the hash has the shape the transport produces and treat a foreign one as absent instead.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1749 +/- ##
==========================================
+ Coverage 93.29% 93.36% +0.06%
==========================================
Files 157 158 +1
Lines 9932 10032 +100
Branches 1022 1030 +8
==========================================
+ Hits 9266 9366 +100
- Misses 471 472 +1
+ Partials 195 194 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
sslaestransport and the tpap transport on python-kasa#1592 store a credentials_hash that is base64 json of the plaintext credentials, so the credentials can be read back out of it. klap and aes hashes are one way and cannot. When a device changes its encryption type the stored hash is the one the old transport wrote. The previous commit stops that being mistaken for a bad password, but the connection still has no credentials to offer and the caller has to prompt for them again. Home Assistant stores the hash as the only copy of the credentials for a device, so that prompt loses them. Read the credentials back out when the hash is one of the plaintext forms and let the transport derive its own. A tpap device that reverts to klap because Third-Party Compatibility was turned on then reconnects without a reauth. This only works in that direction.
b8d98f3 to
147b350
Compare
rytilahti
left a comment
rytilahti
left a comment
There was a problem hiding this comment.
I really like the idea, thanks for the PR @nopoz!
We should take a look how this behaves in the bigger picture (and I have wanted to contain other logic, e.g., the get_credentias/defaultcreds logic properly), but this is a good starter to make the Credentials a bit more self-contained. I added a couple of suggestions inline, please take a look and let me know what you think!
| password: str = field(default="", repr=False) | ||
|
|
||
|
|
||
| def _credentials_from_plaintext_hash(credentials_hash: str) -> Credentials | None: |
There was a problem hiding this comment.
Instead of having a "free" top-level private function, I think this would be better contained within the Credentials class.
There was a problem hiding this comment.
Now Credentials._from_plaintext_hash. Kept it private: Credentials is autodoc'd with :members: in reference.md, so a public classmethod lands in the API reference, and unlike the transport check it is not an override point. Can make it public if you would rather the two match.
Give BaseTransport an is_transport_credentials_hash hook that defaults to accepting the hash, and do the discard and plaintext recovery once in its constructor rather than in each transport. Recovering the credentials is now a Credentials classmethod instead of a module level function. The ssl transport gains the recovery it did not have before, so it is covered by a new test.
2 participants
A
credentials_hashis specific to the transport that produced it, but nothing checked that the hash a transport was handed was its own.klapv1 / v2aesusernameandpassword2, both sha1'dssltransportusernameandpassword, password md5'dsslaesunandpwd, plaintextThis matters because a device can change its encryption type without the credentials changing. Toggling Third-Party Compatibility in the Tapo app moves a plug or strip between TPAP and KLAP, as described in #1590. Discovery correctly rewrites the encryption type, and the transport is then handed a stored hash that a different transport wrote.
KlapTransportdecodes any base64 successfully, so it built_local_auth_hashout of another transport's json and failed handshake1 with:That is indistinguishable from a wrong password, and it is acted on as one.
homeassistant/components/tplink/__init__.pycatchesAuthenticationError, deletesCONF_CREDENTIALS_HASHfrom the config entry and raisesConfigEntryAuthFailed. Where there is no separate credential store, that hash is the only copy of the credentials, so the encryption change costs the user their credentials and a reauth.AesTransportandSslTransportfail less quietly and raiseUnicodeDecodeErrorfrom the constructor when handed a klap hash;SslAesTransportraisesKeyError.What this changes
First commit adds a per-transport check that the hash has the shape that transport produces, and treats a foreign hash as absent rather than as a bad password. This makes the failure honest but still ends in a reauth.
Second commit reads the credentials back out when the hash is one of the plaintext forms and lets the transport derive its own, so the change of encryption type needs no reauth.
klapandaeshashes are one way, so this only works in that direction, and the first commit's behaviour is what applies otherwise.Notes
aesatlogin_version=1andssltransportproduce structurally identical hashes, so they will still accept each other's. Distinguishing them would need the hash to record its transport, which would not help any hash already stored.Testing
New
tests/transports/test_credentials_hash.pycovers, per transport, that a foreign hash is not used to authenticate, that the transport's own hash still is, and that the plaintext forms are recovered. Full suite passes.Also verified against hardware: an L920 strip that reverted from TPAP to KLAP authenticates over KLAP when given the TPAP-format hash, and rederives its own klap hash.