Fix encoding of plaintext length indicator for secrets (fixes #2473)

The encoding of the secret length indicator has likely been broken
since commit b21833f79c which introduced
Py3 support for secrets: the indicator was switched to using UTF-8 byte
strings and this caused the length indicator to consume 3 bytes instead
of just 2 under certain circumstances. For example when the plaintext
secret length is between 128 and 256 bytes long. This is because contrary
to latin-1 encoding, UTF-8 byte sting encoding consumes 2 bytes for
code points > 80. See the table at https://en.wikipedia.org/wiki/UTF-8#Description

The fix is to explicitely use 'latin-1' encoding for the length indicator.
This makes the code behave exactly as it did with the original Python2
implemenation while remaining compatible w/ Py3.

A test case which uses a 171 byte long plaintext string has also been added.
This test triggers the bug when the fix is not present.

This fixes #2473
This commit is contained in:
Marc Heckmann 2018-10-19 10:03:22 -04:00
parent 0bb5d229e8
commit 11b85e5247
2 changed files with 25 additions and 2 deletions

View File

@ -400,8 +400,8 @@ class Secret(ChangeLoggedModel, CustomFieldModel):
else: else:
pad_length = 0 pad_length = 0
return ( return (
chr(len(s) >> 8).encode() + chr(len(s) >> 8).encode('latin-1') +
chr(len(s) % 256).encode() + chr(len(s) % 256).encode('latin-1') +
s + s +
os.urandom(pad_length) os.urandom(pad_length)
) )

View File

@ -131,3 +131,26 @@ class SecretTestCase(TestCase):
self.assertEqual(duplicate_ivs, [], "One or more duplicate IVs found!") self.assertEqual(duplicate_ivs, [], "One or more duplicate IVs found!")
duplicate_ciphertexts = [i for i, x in enumerate(ciphertexts) if ciphertexts.count(x) > 1] duplicate_ciphertexts = [i for i, x in enumerate(ciphertexts) if ciphertexts.count(x) > 1]
self.assertEqual(duplicate_ciphertexts, [], "One or more duplicate ciphertexts (first blocks) found!") self.assertEqual(duplicate_ciphertexts, [], "One or more duplicate ciphertexts (first blocks) found!")
def test_03_encrypt_longer_plaintext(self):
"""
Test basic encryption and decryption using a plaintext string with
more than 128 and less than 256 characters to trigger any potential
padding bugs which would prevent decryption
"""
plaintext = "ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBFDpfiHbXIB/J9Gpds6Ad3qkd7+nRWFjIzT/njvE3LZsz+ZIMtMS1bZeD16grrDPvEVPdj3TpIAHaBMUsdCq4g8= root@test"
secret_key = generate_random_key()
s = Secret(plaintext=plaintext)
s.encrypt(secret_key)
# Enforce minimum ciphertext length
self.assertGreaterEqual(len(s.ciphertext), 80, "Ciphertext must be at least 80 bytes (16B IV + 64B+ ciphertext")
# Test hash validation
self.assertTrue(s.validate(plaintext), "Plaintext does not validate against the generated hash")
self.assertFalse(s.validate(""), "Empty plaintext validated against hash")
self.assertFalse(s.validate("Invalid plaintext"), "Invalid plaintext validated against hash")
# Test decryption
s.decrypt(secret_key)
self.assertEqual(plaintext, s.plaintext, "Decrypting Secret returned incorrect plaintext")