Skip to content
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.

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

Fix pyaes to be reinitialised before encrypt/decrypt. #276

Open
wants to merge 2 commits into
base: master
from

Conversation

@webworxshop
Copy link

commented Aug 7, 2019

This fixes #274

Based upon a bisect I determined that the RM Mini broke in 38a40c5. It looks like the AESModeOfOperationCBC object needs to be re-initialised for every encrypt/decrypt operation.

I've restored something like the state in the previous working version, but maintained the update_aes method.

@webworxshop

This comment has been minimized.

Copy link
Author

commented Aug 8, 2019

Updated with fix when using cryptography module rather than pyaes.

@marcelodavanzo

This comment has been minimized.

Copy link

commented Aug 12, 2019

When will this fix make its way into hassio?

@webworxshop

This comment has been minimized.

Copy link
Author

commented Aug 12, 2019

When will this fix make its way into hassio?

In the simple case, once this is merged and released (which isn't up to me), I can put in an PR to Home Assistant for it to be included in the next release.

However, I've been doing some testing with this version of the library driving the Home Assistant integration and it looks like it's still encrypting the payload incorrectly (even though it works outside of HASS). I need to get all my debugging info assembled and then I'm going to submit an issue in HASS.

return b"".join([self.aes.decrypt(bytes(payload[i:i + 16])) for i in range(0, len(payload), 16)])

def update_aes_crypto(self, key):
self.aes = Cipher(algorithms.AES(key), modes.CBC(self.iv),
backend=default_backend())

def encrypt_crypto(self, payload):
self.update_aes(self.key)

This comment has been minimized.

Copy link
@Danielhiversen

Danielhiversen Aug 15, 2019

Collaborator

We should not reinitialize aes for crypto each time.

This comment has been minimized.

Copy link
@webworxshop

webworxshop Aug 16, 2019

Author

Hi Daniel,

Thanks for taking a look at this.

That is the exact fix we are making here, since the device seems not to be accepting later packets encrypted in CBC mode. It works fine if you reinitialise the object to go back to the beginning of the chain.

I'm happy to do this differently, if you can suggest an alternative. This change is basically just reverting to the original approach from 0.10 and before where the object was reinitialised on every call.

This comment has been minimized.

Copy link
@Danielhiversen

Danielhiversen Aug 17, 2019

Collaborator

But it is not necessary when using the crypto library.
So we should only do it when using pyaes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.