Skip to content

Commit

Permalink
Avoid extra KDF call in Secure Cell (#496)
Browse files Browse the repository at this point in the history
* Do not call KDF during size query (encryption)

Currently calling themis_auth_sym_encrypt_message() results in an
unnecessary call to themis_sym_kdf() when we only want to validate
input parameters and maybe return the correct output buffer length.

Move KDF calls to the helper function and leave only parameter checks in
the main one. While we're here, refactor the function to improve code
readability and clarity.

* Do not call KDF during size query (decryption)

themis_auth_sym_decrypt_message() has the same inefficient use of
themis_sym_kdf() as in themis_auth_sym_encrypt_message(). Update
and refactor it in a similar way.

Complexity here stems from the fact that decryption path contains
a workaround for a compatibility issue with Themis 0.9.6. We need
to preserve the workaround. In order to make it more readable,
move the compatibility code into a separate function that will be
compiled out entirely when not needed.

* Overflow checks for cell length

Secure Cell format does not allow messages longer than 4 GB. Container
length is limited by the length field in authentication context. This
was the source of the original compatibility issue in Themis 0.9.6.

However, currently we accept lengths over 4 GB, but silently truncate
them when constructing authentication context. This is not right and
we should check for the overflow instead of silently returning
incorrect data.

This code path is not tested because it's not very reliable to allocate
over 4 GB of memory in CI environment.

* Wipe derived keys after use

Currently we leave the resulting derived keys on the stack after use.
These keys are sensitive data so we should wipe them immediately after
they have been used and are no longer needed.

Move the derived key array into the parent function and wipe it there
to ensure that we always do so. There are alternatives, but they will
require to structure the code a bit differently or stop using these
THEMIS_CHECK macros.
  • Loading branch information
ilammy authored Jul 12, 2019
1 parent 258d0aa commit e6c36a6
Showing 1 changed file with 223 additions and 125 deletions.
Loading

0 comments on commit e6c36a6

Please sign in to comment.