Initializing IV to ''

[nijave]

Is there a reason you're explicitly setting the IV to ''?

Like cipher = AES.new(self.key, mode=AES.MODE_ECB, IV='')

I was having some issues on homeassistant since it appears setting IV has been removed because it didn't used to do anything.

[clach04]

@nijave I made it empty for two reasons:

1) matches Node/js implementation https://github.com/codetheweb/tuyapi 2) its explicit clear what the IV is

Can you go into more details about HA and IV being removed? This doesn't have anything to do with HA (it relies on either PyCrypto or https://github.com/ricmoo/pyaes - I've not got around to integrating with HA - but if someone has already done that work, that's awesome and its saves me some work! ;)

[nijave]

Someone else started working on it here: https://github.com/home-assistant/home-assistant/pull/11000

I've been trying to get it working better by making status polling work correctly since it seems like the outlet terminates the TCP connection randomly (sometimes it works from HA and sometimes the device sends a RST,ACK right after HA asks for the status). Here's what I'm seeing from HA

Traceback (most recent call last):
  File "/usr/lib/python3.6/asyncio/tasks.py", line 182, in _step
    result = coro.throw(exc)
  File "/usr/lib/python3.6/site-packages/homeassistant/core.py", line 1031, in _event_to_service_call
    yield from service_handler.func(service_call)
  File "/usr/lib/python3.6/site-packages/homeassistant/components/switch/__init__.py", line 113, in async_handle_switch_service
    yield from switch.async_turn_on()
  File "/usr/lib/python3.6/asyncio/futures.py", line 332, in __iter__
    yield self  # This tells Task to wait for completion.
  File "/usr/lib/python3.6/asyncio/tasks.py", line 250, in _wakeup
    future.result()
  File "/usr/lib/python3.6/asyncio/futures.py", line 245, in result
    raise self._exception
  File "/usr/lib/python3.6/concurrent/futures/thread.py", line 56, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/config/custom_components/switch/tuya.py", line 66, in turn_on
    self._device.set_status(True, self._switchid)
  File "/config/deps/lib/python3.6/site-packages/pytuya/__init__.py", line 228, in set_status
    payload = self.generate_payload(command, dps_id=switch)
  File "/config/deps/lib/python3.6/site-packages/pytuya/__init__.py", line 163, in generate_payload
    json_payload = self.cipher.encrypt(json_payload)
  File "/config/deps/lib/python3.6/site-packages/pytuya/__init__.py", line 42, in encrypt
    cipher = AES.new(self.key, mode=AES.MODE_ECB, IV='')
  File "/usr/lib/python3.6/site-packages/Crypto/Cipher/AES.py", line 202, in new
    return _create_cipher(sys.modules[__name__], key, mode, *args, **kwargs)
  File "/usr/lib/python3.6/site-packages/Crypto/Cipher/__init__.py", line 55, in _create_cipher
    return modes[mode](factory, **kwargs)
  File "/usr/lib/python3.6/site-packages/Crypto/Cipher/_mode_ecb.py", line 177, in _create_ecb_cipher
    raise TypeError("Unknown parameters for ECB: %s" % str(kwargs))
TypeError: Unknown parameters for ECB: {'IV': ''}

Which led me to this https://github.com/Legrandin/pycryptodome/issues/28 so it seems like that might not be doing anything (although I didn't research any further to see if that's the case here)? It seems to work fine after removing that. I was also running it an issue with _pad where is tries to concatenate bytes with a string and leads to an exception

Edit, looks like IV isn't used with AES ECB https://stackoverflow.com/questions/1789709/is-it-possible-to-use-aes-with-an-iv-in-ecb-mode

[clach04]

@nijave The original padding code was not great but it did/does work. As a diagnostic, how about you hack the python-tuya on your machine to use the pure python version (make sure you download that) that would avoid the IV issue.

From my very quick scan its looks like a PyCrypto (API/ABI) problem. So remove it from the equation whilst experimenting with it. Sadly I don't have time to check in to HA support to confirm or deny my suspicion.

[clach04]

@nijave obviously you could try hacking python-tuya to remove the IV param (possibly with a comment about IV not used) ;-) I don't have a strong technical reason to keep it.

[nijave]

Strangely, it originally worked flawlessly on my local machine and I only started running into issues when messing with it integrated with HA. As for the RST,ACK issues, I just put a loop in to try 3 times from the HA side and it always seems to succeed on the second. I sort of suspect it's an issue with HA making frequent connections to the outlet and it possibly not cleaning them up

Current working version is here https://github.com/nijave/home-assistant/tree/component-tuya You can take the component from home-assistant/homeassistant/components/switch/tuya.py and drop it on HA in /config/custom_components/switch/tuya.py (of course you might run into the issues above--I'll try to figure out what's going on there)

[clach04]

@nijave the IV issue appears to be separate from the status issue (where you added a retry). But just in case they are related.

If this is failing (but sometimes succeeding) with HA installation. I wonder if the PyCrypto versions are different? It makes zero sense that it would sometimes work when setting status. Unless HA is now using pycryptodome? (I don't have a current HA installation).

Also weird is that if this is happening in get status, why does the traceback show set? Again, if status problem was a side comment to the IV for this ticket, ignore these questions/comments :-)

I have seen issues in the past where a device times out (I think this happened to me when the Android app had a connection open). So a retry like you added to https://github.com/nijave/home-assistant/blob/component-tuya/homeassistant/components/switch/tuya.py makes sense.

On my todo list is adding some logging (of calls, payloads, errors, etc.) - sadly I don't have time to do this at the moment. That might help figure out the sequence of events as I'm not clear what's going on if this only happens on a get status (the trace back above makes no sense).

[nijave]

For clarity, there seem to be 3 separate issues:

1. The IV for AES-ECB (only a problem for set)
2. The padding issue (only a problem for set)
3. The TCP reset issue (this seems to be an issue with the outlet--only a problem when getting status)

I'll have to debug more with HA. It seemed to work fine out of the box originally, but after a while stopped working (possibly when I updated to HA 0.60)

[clach04]

Lets focus on #1 in this issue, if you can dump PyCrypto version/details that would be helpful (hacking python-tuya).

The others can be dealt with in #2 and #3.

[clach04]

@nijave I just added the most naive version dumping possible. If you edit the script that imports pytuya, before the import it will start dumping. E.g.:

#.....
import logging
log = logging.getLogger('pytuya')
log.setLevel(level=logging.DEBUG)  # Debug hack!

import pytuya
#.....

[nijave]

I'm just ssh-ing to my raspberry pi then Docker exec-ing into the container homeassistant runs in

So it looks like it's probably using pycryptodome

>>> import Crypto
>>> Crypto.version_info
(3, 4, 7)

Although I'm not sure how to verify exactly. The version number matches up but pip isn't installed. I'm looking at the HA build to try to find info--possibly another component has installed pycryptodome and pytuya is picking it up

Edit:

bash-4.4# grep -rni pycryptodome *
dev/disk/by-uuid/eb5caab2-89c0-4c69-8060-34690ee273a4:67622052:!pycryptodome-3.4.7-py3.6.egg-info
dev/disk/by-uuid/eb5caab2-89c0-4c69-8060-34690ee273a4:77464176:!pycryptodome-3.4.7-py3.6.egg-info
dev/disk/by-uuid/eb5caab2-89c0-4c69-8060-34690ee273a4:80094002:"pycryptodomex-3.4.7-py3.6.egg-info
dev/disk/by-uuid/eb5caab2-89c0-4c69-8060-34690ee273a4:93082584:"pycryptodomex-3.4.7-py3.6.egg-info

I'll let that finish and see what comes up

[nijave]

I installed all the home-assistant Python modules then ran pipdeptree and it looks like multiple things are requiring pycryptodome which pytuya is then picking up. Do you have any opinions on how to address? Probably easiest is just removing the IV parameter since it doesn't appear to be needed

[clach04]

Thanks @nijave. How about you remove the IV usage and send me a pull request?

I'm happy to trust your testing with pycryptodome. I'll then re-confirm it works with pyaes and PyCrypto before merging.

[nijave]

Created a new pull request (made a mess out of the last one) and this includes the padding fix you committed