Wipe output on failure: key generation (#586)

* 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.

* 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

* 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.

src/soter/openssl/soter_rsa_key_pair_gen.c

@@ -23,7 +23,7 @@
 
 #include "soter/openssl/soter_engine.h"
 
-static unsigned rsa_key_length(const int size)
+static int rsa_key_length(unsigned size)
 {
     switch (size) {
     case RSA_KEY_LENGTH_1024:
@@ -72,7 +72,7 @@ static soter_status_t soter_set_default_rsa_pub_exp(EVP_PKEY_CTX* pkey_ctx)
     return SOTER_SUCCESS;
 }
 
-static soter_status_t soter_set_rsa_key_length(EVP_PKEY_CTX* pkey_ctx, unsigned length)
+static soter_status_t soter_set_rsa_key_length(EVP_PKEY_CTX* pkey_ctx, int length)
 {
     if (EVP_PKEY_CTX_ctrl(pkey_ctx, -1, -1, EVP_PKEY_CTRL_RSA_KEYGEN_BITS, length, NULL) != 1) {
         return SOTER_FAIL;

src/themis/secure_keygen.c

@@ -24,6 +24,7 @@
 #include "soter/soter_rsa_key.h"
 #include "soter/soter_rsa_key_pair_gen.h"
 #include "soter/soter_t.h"
+#include "soter/soter_wipe.h"
 
 #include "themis/themis_portable_endian.h"
 
@@ -39,61 +40,92 @@
  */
 #define THEMIS_SYM_KEY_LENGTH 32
 
+static themis_status_t combine_key_generation_results(uint8_t* private_key,
+                                                      const size_t* private_key_length,
+                                                      themis_status_t private_result,
+                                                      uint8_t* public_key,
+                                                      const size_t* public_key_length,
+                                                      themis_status_t public_result)
+{
+    if (private_result == THEMIS_SUCCESS && public_result == THEMIS_SUCCESS) {
+        return THEMIS_SUCCESS;
+    }
+
+    if (private_result != THEMIS_BUFFER_TOO_SMALL) {
+        soter_wipe(private_key, *private_key_length);
+    }
+    if (public_result != THEMIS_BUFFER_TOO_SMALL) {
+        soter_wipe(public_key, *public_key_length);
+    }
+
+    if (private_result == THEMIS_BUFFER_TOO_SMALL || public_result == THEMIS_BUFFER_TOO_SMALL) {
+        return THEMIS_BUFFER_TOO_SMALL;
+    }
+
+    return (private_result != THEMIS_SUCCESS) ? private_result : public_result;
+}
+
 themis_status_t themis_gen_key_pair(soter_sign_alg_t alg,
                                     uint8_t* private_key,
                                     size_t* private_key_length,
                                     uint8_t* public_key,
                                     size_t* public_key_length)
 {
-    soter_sign_ctx_t* ctx = soter_sign_create(alg, NULL, 0, NULL, 0);
-    THEMIS_CHECK(ctx != NULL);
+    themis_status_t private_result = THEMIS_FAIL;
+    themis_status_t public_result = THEMIS_FAIL;
+    soter_sign_ctx_t* ctx = NULL;
 
-    soter_status_t res_private = soter_sign_export_key(ctx, private_key, private_key_length, true);
-    if (res_private != THEMIS_SUCCESS && res_private != THEMIS_BUFFER_TOO_SMALL) {
-        soter_sign_destroy(ctx);
-        return res_private;
+    if (!private_key_length || !public_key_length) {
+        return THEMIS_INVALID_PARAMETER;
     }
 
-    soter_status_t res_public = soter_sign_export_key(ctx, public_key, public_key_length, false);
-    if (res_public != THEMIS_SUCCESS && res_public != THEMIS_BUFFER_TOO_SMALL) {
-        soter_sign_destroy(ctx);
-        return res_public;
+    ctx = soter_sign_create(alg, NULL, 0, NULL, 0);
+    if (!ctx) {
+        return THEMIS_FAIL;
     }
 
+    private_result = soter_sign_export_key(ctx, private_key, private_key_length, true);
+    public_result = soter_sign_export_key(ctx, public_key, public_key_length, false);
+
     soter_sign_destroy(ctx);
-    if (res_private == THEMIS_BUFFER_TOO_SMALL || res_public == THEMIS_BUFFER_TOO_SMALL) {
-        return THEMIS_BUFFER_TOO_SMALL;
-    }
-    return THEMIS_SUCCESS;
+
+    return combine_key_generation_results(private_key,
+                                          private_key_length,
+                                          private_result,
+                                          public_key,
+                                          public_key_length,
+                                          public_result);
 }
 
 themis_status_t themis_gen_rsa_key_pair(uint8_t* private_key,
                                         size_t* private_key_length,
                                         uint8_t* public_key,
                                         size_t* public_key_length)
 {
-    soter_rsa_key_pair_gen_t* key_pair_ctx = soter_rsa_key_pair_gen_create(THEMIS_RSA_KEY_LENGTH);
-    THEMIS_CHECK(key_pair_ctx != NULL);
+    themis_status_t private_result = THEMIS_FAIL;
+    themis_status_t public_result = THEMIS_FAIL;
+    soter_rsa_key_pair_gen_t* ctx = NULL;
 
-    soter_status_t res_private =
-        soter_rsa_key_pair_gen_export_key(key_pair_ctx, private_key, private_key_length, true);
-    if (res_private != THEMIS_SUCCESS && res_private != THEMIS_BUFFER_TOO_SMALL) {
-        soter_rsa_key_pair_gen_destroy(key_pair_ctx);
-        return res_private;
+    if (!private_key_length || !public_key_length) {
+        return THEMIS_INVALID_PARAMETER;
     }
 
-    soter_status_t res_public =
-        soter_rsa_key_pair_gen_export_key(key_pair_ctx, public_key, public_key_length, false);
-    if (res_public != THEMIS_SUCCESS && res_public != THEMIS_BUFFER_TOO_SMALL) {
-        soter_rsa_key_pair_gen_destroy(key_pair_ctx);
-        return res_public;
+    ctx = soter_rsa_key_pair_gen_create(THEMIS_RSA_KEY_LENGTH);
+    if (!ctx) {
+        return THEMIS_FAIL;
     }
 
-    soter_rsa_key_pair_gen_destroy(key_pair_ctx);
-    if (res_private == THEMIS_BUFFER_TOO_SMALL || res_public == THEMIS_BUFFER_TOO_SMALL) {
-        return THEMIS_BUFFER_TOO_SMALL;
-    }
-    return THEMIS_SUCCESS;
+    private_result = soter_rsa_key_pair_gen_export_key(ctx, private_key, private_key_length, true);
+    public_result = soter_rsa_key_pair_gen_export_key(ctx, public_key, public_key_length, false);
+
+    soter_rsa_key_pair_gen_destroy(ctx);
+
+    return combine_key_generation_results(private_key,
+                                          private_key_length,
+                                          private_result,
+                                          public_key,
+                                          public_key_length,
+                                          public_result);
 }
 
 themis_status_t themis_gen_ec_key_pair(uint8_t* private_key,
@@ -179,6 +211,6 @@ themis_status_t themis_gen_sym_key(uint8_t* key, size_t* key_length)
         return THEMIS_BUFFER_TOO_SMALL;
     }
 
-    /* Soter error codes have the same value as Themis ones */
+    /* soter_rand() wipes the key on failure, soter_wipe() not needed */
     return soter_rand(key, *key_length);
 }