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.

Sign up for GitHub

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

Jump to bottom

Wipe output on failure: key generation #586

Merged
merged 3 commits into from
Feb 3, 2020
Merged

Wipe output on failure: key generation #586

merged 3 commits into from
Feb 3, 2020

Commits on Jan 30, 2020

  1. Refactor error handling: key generation 

    Current code is hard to follow and has complicated management for key
generation contexts. However, this is not essential complexity.

The core insight here is that while the context is shared, exporting
private and public key are two independent operations. Therefore we can
try exporting both keys, free the context, and only then deal with the
results. It then becomes really natural to introduce a helper function
to combine two results into one.

While we're here, make sure that all local variables are initialized.
This too makes error handling path easier to write and understand.
    @ilammy
    ilammy committed Jan 30, 2020
    Configuration menu
    Copy the full SHA
    d11cf03 View commit details
    Browse the repository at this point in the history

  2. Wipe output on failure: key generation 

    Now, let's make sure that we don't leak any information if key
generation fails. For example, if the private key has been extracted
just fine but extracting public key fails then the private key has
already been written out into the user buffer. This is not nice.

We can easily insert soter_wipe() calls to remove results from output
buffers if key generation is not succesful. Note that we *do not* wipe
the buffer if it is reported to be too small. In this case the length
is now equal to the desired buffer length, not actual one provided by
the caller. We'd probably overrun the buffer if we touch it. Instead,
we can simply not wipe it because in this case we have not written
anything into it yet.

Symmetric key generation simply calls soter_rand() which wipes after
itself like an adult. We leave a comment just in case the code gets
more complex in the future, so that the developers don't forget to add
a soter_wipe() call if necessary.

This covers Themis code for key generation API:

  - themis_gen_sym_key
  - themis_gen_rsa_key_pair
  - themis_gen_ec_key_pair
    @ilammy
    ilammy committed Jan 30, 2020
    Configuration menu
    Copy the full SHA
    9b51de3 View commit details
    Browse the repository at this point in the history

  3. Fix clang-tidy warnings for OpenSSL 

    In OpenSSL EVP_PKEY_CTX_ctrl() penultimate argument "p1" has "int" type.
We use this function to set RSA key length in bits, and we incorrectly
use "unsigned" type when interfacing with EVP_PKEY_CTX_ctrl(). Update
the type to just "int" to avoid warnings by clang-tidy about narrowing
casts. We can use "int" in rsa_key_length() because it's wide enough
for current maximum length of 4096 bits.
    @ilammy
    ilammy committed Jan 30, 2020
    Configuration menu
    Copy the full SHA
    4535fa8 View commit details
    Browse the repository at this point in the history