Fix "cryptography_symmetric_encrypt" function and make sure it handles unicode values correctly #4528
Conversation
handle unicode (utf-8) data. Previously we didn't correctly convert string to bytes before padding data which resulted in incorrectly padded data (incorrect length) so the actual encryption step failed. Thanks to @dswebbthg for reporting this issue. Resolves #4513.
|
|
||
| def test_symmetric_encrypt_decrypt_utf8_character(self): | ||
| values = [ | ||
| u'£', |
There was a problem hiding this comment.
I don't think the u syntax is supported in Python 3. Better to use six.u() so we can run our tests with Python 3.
There was a problem hiding this comment.
It is - u as in prefix for strings which is also a default (e.g. u'foo).
Those tests also run under Python 2.x and 3.6 on Travis and I also ran them under both versions locally.
if isinstance(plaintext, (six.text_type, six.string_types)):
# Convert data to bytes
data = plaintext.encode('utf-8')This means that if I encrypt a string, I would get back bytes when I decrypt it, right? That's a little weird, but it's the previous behavior. |
|
@blag When you decrypt a string you always get back unicode - st2/st2common/st2common/util/crypto.py Line 302 in 35de7b6 Also see tests - they perform encrypt and decrypt round-trip and decrypted value is the same as the original one (unicode string). |
This pull request fixes an issue reported in #4513.
We didn't correctly handle unicode values so encryption didn't work for unicode values which were not of a correct AES block length (aka when padding was needed).
This pull request fixes that by making sure we convert data to bytes before we pass it to the padding function. It also includes a change in pad function so it handles bytes correctly.
Thanks to @dswebbthg and @nickbaum for reporting and working on this issue.
Resolves #4513.