Symmetric keygen: Themis Core (#560)

* Cleanup existing Doxygen comments for key generation

Properly group and document all members of this file.

The definition of themis_key_kind_t is tweaked a bit so that Doxygen
does not generate two entries for both the enum and the typedef.
However, we still keep the "enum themis_key_kind" tag available.

* Symmetric keygen: Themis Core

Implement themis_gen_sym_key() function in Themis Core. Add tests.
It's pretty much documented in doxygen comments. This is the actual
key generation implementation which will be used by all wrappers.

* Additional tests with sanity checks

Verify that we can generate big keys too, up to MAX_KEY_SIZE.

Verify that key generation routines actually fill in the key buffer
when they return THEMIS_SUCCESS, and that every invocation returns
a different key.

* Handle zero length consistently

Asymmetric key generation API considers zero length "too small",
not an error, and outputs a proper length while returning approriate
error code. Update symmetric key generation behavior for consistency.

* More strict check for default key length

After fierce discussion we have reached a compromise: we should do
a more strict check than "returns length greater that zero", but
we do not want to export THEMIS_SYM_KEY_LENGTH constant. Copying
its value for tests is acceptable.

CHANGELOG.md

@@ -4,8 +4,17 @@
 
 Changes that are currently in development and have not been released yet.
 
+**TL;DR:**
+- Added API for generating symmetric keys for use with Secure Cell;
+
 _Code:_
 
+- **Core**
+
+  - **Key generation**
+
+    - New function `themis_gen_sym_key()` can be used to securely generate symmetric keys for Secure Cell ([#560](https://github.com/cossacklabs/themis/pull/560)).
+
 - **Java**
 
   - JDK location is now detected automatically in most cases, you should not need to set JAVA_HOME or JDK_INCLUDE_PATH manually ([#551](https://github.com/cossacklabs/themis/pull/551)).

src/themis/secure_keygen.c

@@ -20,6 +20,7 @@
 
 #include "soter/soter_container.h"
 #include "soter/soter_ec_key.h"
+#include "soter/soter_rand.h"
 #include "soter/soter_rsa_key.h"
 #include "soter/soter_rsa_key_pair_gen.h"
 #include "soter/soter_t.h"
@@ -30,6 +31,14 @@
 #define THEMIS_RSA_KEY_LENGTH RSA_KEY_LENGTH_2048
 #endif
 
+/*
+ * This is the default key length recommended for use with Secure Cell.
+ * It will have enough randomness for AES-256 (normally used by Themis)
+ * and is consistent with NIST recommendations for the next ten years,
+ * as of 2020. See: https://www.keylength.com/en/4/
+ */
+#define THEMIS_SYM_KEY_LENGTH 32
+
 themis_status_t themis_gen_key_pair(soter_sign_alg_t alg,
                                     uint8_t* private_key,
                                     size_t* private_key_length,
@@ -158,3 +167,18 @@ themis_status_t themis_is_valid_asym_key(const uint8_t* key, size_t length)
 
     return THEMIS_INVALID_PARAMETER;
 }
+
+themis_status_t themis_gen_sym_key(uint8_t* key, size_t* key_length)
+{
+    if (key_length == NULL) {
+        return THEMIS_INVALID_PARAMETER;
+    }
+
+    if (key == NULL || *key_length == 0) {
+        *key_length = THEMIS_SYM_KEY_LENGTH;
+        return THEMIS_BUFFER_TOO_SMALL;
+    }
+
+    /* Soter error codes have the same value as Themis ones */
+    return soter_rand(key, *key_length);
+}

src/themis/secure_keygen.h

@@ -15,8 +15,8 @@
  */
 
 /**
- * @file secure_keygen.h
- * @brief secure key generation
+ * Securely generating random keys.
+ * @file themis/secure_keygen.h
  */
 
 #ifndef THEMIS_SECURE_KEYGEN_H
@@ -32,24 +32,83 @@ extern "C" {
 /**
  * @addtogroup THEMIS
  * @{
- * @defgroup THEMIS_KEYS secure key generation
- * @brief securely generating random key pairs
+ * @defgroup THEMIS_KEYS Secure key generation
+ * Securely generating random symmetric keys and asymmetric key pairs.
  * @{
  */
 
 /**
- * @brief generate RSA key pair
- * @param [out] private_key buffer for private key to store. May be set to NULL for private key
- * length determination
- * @param [in, out] private_key_length length of private_key
- * @param [out] public_key buffer for public key to store. May be set to NULL for public key length
- * determination
- * @param [in, out] public_key_length length of public key
- * @return THEMIS_SUCCESS on success or THEMIS_FAIL on failure
- * @note If private_key==NULL or public_key==NULL or private_key_length is not enought for private
- * key storage or public_key_length is not enought for public key storage then
- * THEMIS_BUFFER_TOO_SMALL will return and private_key_length and public_key_length will store
- * lengths of buffers needed for private key and public key store respectively
+ * Generates a symmetric key.
+ *
+ * @param [out]     key         buffer for generated key
+ * @param [in,out]  key_length  length of generated key in bytes
+ *
+ * New symmetric key is generated and written into `key` buffer which
+ * must have at least `key_length` bytes available. Note that length
+ * parameter is a _pointer_ to actual length value.
+ *
+ * You can pass NULL for `key` in order to determine appropriate buffer
+ * length. In this case the length is written into provided location,
+ * no key data is generated, and THEMIS_BUFFER_TOO_SMALL is returned.
+ *
+ * @returns THEMIS_SUCCESS if the key has been generated successfully
+ * and written into `key`.
+ *
+ * @returns THEMIS_BUFFER_TOO_SMALL if the key length has been written
+ * to `key_length`.
+ *
+ * @exception THEMIS_INVALID_PARAM if `key_length` is NULL.
+ *
+ * @exception THEMIS_INVALID_PARAM if `key_length` is too big
+ * for cryptographic backend to handle.
+ *
+ * @exception THEMIS_BUFFER_TOO_SMALL if `key_length` is too small
+ * to hold a generated key.
+ *
+ * @exception THEMIS_FAIL if cryptographic backend was unable
+ * to generate enough randomness to fill the entire buffer.
+ *
+ * @exception THEMIS_NOT_SUPPORTED if cryptographic backend
+ * does not support strong random number generation.
+ *
+ * @note Some backends might abort the process instead of returning
+ * error codes if they are unable to generate random data.
+ */
+THEMIS_API
+themis_status_t themis_gen_sym_key(uint8_t* key, size_t* key_length);
+
+/**
+ * Generates an RSA key pair.
+ *
+ * @param [out]     private_key         buffer for private key
+ * @param [in,out]  private_key_length  length of private key in bytes
+ * @param [out]     public_key          buffer for public key
+ * @param [in,out]  public_key_length   length of public key in bytes
+ *
+ * New RSA key pair is generated and written into `private_key` and
+ * `public_key` buffers which must have at least `private_key_length`
+ * and `public_key_length` bytes available respectively. Note that
+ * length parameters are _pointers_ to actual length values.
+ *
+ * You can pass NULL for `private_key` and `public_key` in order to
+ * determine appropriate buffer length. In this case the lengths are
+ * written into provided locations, no key data is generated, and
+ * THEMIS_BUFFER_TOO_SMALL is returned.
+ *
+ * @returns THEMIS_SUCCESS if the keys have been generated successfully
+ * and written to `private_key` and `public_key`.
+ *
+ * @returns THEMIS_BUFFER_TOO_SMALL if the key lengths have been written
+ * to `private_key_length` and `public_key_length`.
+ *
+ * @exception THEMIS_FAIL if key generation has failed.
+ *
+ * @exception THEMIS_INVALID_PARAM if `private_key_length` or
+ * `public_key_length` is NULL.
+ *
+ * @exception THEMIS_BUFFER_TOO_SMALL if `private_key` and `public_key`
+ * are not NULL, but `private_key_length` or `public_key_length` in not
+ * sufficient to hold a generated key.
  */
 THEMIS_API
 themis_status_t themis_gen_rsa_key_pair(uint8_t* private_key,
@@ -58,49 +117,84 @@ themis_status_t themis_gen_rsa_key_pair(uint8_t* private_key,
                                         size_t* public_key_length);
 
 /**
- * @brief generate EC key pair
- * @param [out] private_key buffer for private key to store. May be set to NULL for private key
- * length determination
- * @param [in, out] private_key_length length of private_key
- * @param [out] public_key buffer for public key to store. May be set to NULL for public key length
- * determination
- * @param [in, out] public_key_length length of public key
- * @return THEMIS_SUCCESS on success or THEMIS_FAIL on failure
- * @note If private_key==NULL or public_key==NULL or private_key_length is not enought for private
- * key storage or public_key_length is not enought for public key storage then
- * THEMIS_BUFFER_TOO_SMALL will return and private_key_length and public_key_length will store
- * lengths of buffers needed for private key and public key store respectively
+ * Generates an EC key pair.
+ *
+ * @param [out]     private_key         buffer for private key
+ * @param [in,out]  private_key_length  length of private key in bytes
+ * @param [out]     public_key          buffer for public key
+ * @param [in,out]  public_key_length   length of public key in bytes
+ *
+ * New EC key pair is generated and written into `private_key` and
+ * `public_key` buffers which must have at least `private_key_length`
+ * and `public_key_length` bytes available respectively. Note that
+ * length parameters are _pointers_ to actual length values.
+ *
+ * You can pass NULL for `private_key` and `public_key` in order to
+ * determine appropriate buffer length. In this case the lengths are
+ * written into provided locations, no key data is generated, and
+ * THEMIS_BUFFER_TOO_SMALL is returned.
+ *
+ * @returns THEMIS_SUCCESS if the keys have been generated successfully
+ * and written to `private_key` and `public_key`.
+ *
+ * @returns THEMIS_BUFFER_TOO_SMALL if the key lengths have been written
+ * to `private_key_length` and `public_key_length`.
+ *
+ * @exception THEMIS_FAIL if key generation has failed.
+ *
+ * @exception THEMIS_INVALID_PARAM if `private_key_length` or
+ * `public_key_length` is NULL.
+ *
+ * @exception THEMIS_BUFFER_TOO_SMALL if `private_key` and `public_key`
+ * are not NULL, but `private_key_length` or `public_key_length` in not
+ * sufficient to hold a generated key.
  */
 THEMIS_API
 themis_status_t themis_gen_ec_key_pair(uint8_t* private_key,
                                        size_t* private_key_length,
                                        uint8_t* public_key,
                                        size_t* public_key_length);
 
-enum themis_key_kind {
+/**
+ * Kind of an asymmetric Themis key.
+ */
+typedef enum themis_key_kind {
+    /** Invalid key buffer. */
     THEMIS_KEY_INVALID,
+    /** Private RSA key. */
     THEMIS_KEY_RSA_PRIVATE,
+    /** Public RSA key. */
     THEMIS_KEY_RSA_PUBLIC,
+    /** Private EC key. */
     THEMIS_KEY_EC_PRIVATE,
+    /** Public EC key. */
     THEMIS_KEY_EC_PUBLIC,
-};
-
-typedef enum themis_key_kind themis_key_kind_t;
+} themis_key_kind_t;
 
 /**
- * @brief get Themis key kind
+ * Returns kind of an asymmetric Themis key.
+ *
  * @param [in]  key     key buffer
- * @param [in]  length  length of key
- * @return corresponding key kind if the buffer contains a key, or THEMIS_KEY_INVALID otherwise
+ * @param [in]  length  length of key in bytes
+ *
+ * @return corresponding key kind if the buffer contains a key,
+ * or THEMIS_KEY_INVALID otherwise.
+ *
+ * @exception THEMIS_KEY_INVALID if `key` is NULL.
  */
 THEMIS_API
 themis_key_kind_t themis_get_asym_key_kind(const uint8_t* key, size_t length);
 
 /**
- * @brief validate a Themis key
+ * Validates an asymmetric Themis key.
+ *
  * @param [in]  key     key buffer to validate
- * @param [in]  length  length of key
- * @return THEMIS_SUCCESS if the buffer contains a valid Themis key, or an error code otherwise
+ * @param [in]  length  length of key in bytes
+ *
+ * @return THEMIS_SUCCESS if the buffer contains a valid Themis key,
+ * or THEMIS_INVALID_PARAMETER if it does not.
+ *
+ * @exception THEMIS_INVALID_PARAMETER if `key` is NULL.
  */
 THEMIS_API
 themis_status_t themis_is_valid_asym_key(const uint8_t* key, size_t length);

tests/themis/themis_secure_cell.c

@@ -16,6 +16,7 @@
 
 #include "themis/themis_test.h"
 
+#include <stdbool.h>
 #include <string.h>
 
 #include <soter/soter_rand.h>
@@ -28,10 +29,103 @@
 /* Keep it under 2^31 to support 32-bit systems. */
 #define CORRUPTED_LENGTH 0x5AFE
 
+#define DEFAULT_SYM_KEY_LENGTH 32
+
 static char passwd[] = "password";
 static char message[] = "secure cell test message by Mnatsakanov Andrey from Cossack Labs";
 static char user_context[] = "secure cell user context";
 
+static bool non_zero_buffer(const uint8_t* buffer, size_t length)
+{
+    for (size_t i = 0; i < length; i++) {
+        if (buffer[i] != 0) {
+            return true;
+        }
+    }
+    return false;
+}
+
+static bool different_buffers(const uint8_t* buffer1, size_t length1, const uint8_t* buffer2, size_t length2)
+{
+    size_t min = (length1 < length2) ? length1 : length2;
+    if (memcmp(buffer1, buffer2, min) != 0) {
+        return true;
+    }
+    return length1 != length2;
+}
+
+static void secure_cell_key_generation(void)
+{
+    themis_status_t res;
+    uint8_t master_key_1[MAX_KEY_SIZE];
+    uint8_t master_key_2[MAX_KEY_SIZE];
+    size_t master_key_1_length = 0;
+    size_t master_key_2_length = 0;
+
+    /*
+     * Normal usage of key generation API.
+     */
+
+    res = themis_gen_sym_key(NULL, &master_key_1_length);
+    testsuite_fail_unless(res == THEMIS_BUFFER_TOO_SMALL, "keygen: query key size");
+    testsuite_fail_unless(master_key_1_length == DEFAULT_SYM_KEY_LENGTH,
+                          "keygen: recommends default key size");
+
+    master_key_1_length = 0;
+    res = themis_gen_sym_key(master_key_1, &master_key_1_length);
+    testsuite_fail_unless(res == THEMIS_BUFFER_TOO_SMALL, "keygen: query key size (with buffer)");
+    testsuite_fail_unless(master_key_1_length == DEFAULT_SYM_KEY_LENGTH,
+                          "keygen: recommends default key size");
+
+    size_t old_value = master_key_1_length;
+    res = themis_gen_sym_key(master_key_1, &master_key_1_length);
+    testsuite_fail_unless(res == THEMIS_SUCCESS, "keygen: generate recommended key");
+    testsuite_fail_unless(master_key_1_length == old_value, "keygen: key length unchanged");
+
+    master_key_1_length = MAX_KEY_SIZE;
+    res = themis_gen_sym_key(master_key_1, &master_key_1_length);
+    testsuite_fail_unless(res == THEMIS_SUCCESS, "keygen: generate extra-big key");
+    testsuite_fail_unless(master_key_1_length == MAX_KEY_SIZE, "keygen: key length unchanged");
+
+    master_key_1_length = 8;
+    res = themis_gen_sym_key(master_key_1, &master_key_1_length);
+    testsuite_fail_unless(res == THEMIS_SUCCESS, "keygen: generate small key");
+    testsuite_fail_unless(master_key_1_length == 8, "keygen: key length unchanged");
+
+    /*
+     * Verify that key generation is not bogus and returns new keys.
+     */
+
+    memset(master_key_1, 0, sizeof(master_key_1));
+    memset(master_key_2, 0, sizeof(master_key_2));
+
+    master_key_1_length = MAX_KEY_SIZE;
+    master_key_2_length = MAX_KEY_SIZE;
+
+    res = themis_gen_sym_key(master_key_1, &master_key_1_length);
+    testsuite_fail_unless(res == THEMIS_SUCCESS, "keygen: generates key 1");
+    res = themis_gen_sym_key(master_key_2, &master_key_2_length);
+    testsuite_fail_unless(res == THEMIS_SUCCESS, "keygen: generates key 2");
+
+    testsuite_fail_unless(non_zero_buffer(master_key_1, master_key_1_length),
+                          "keygen: returns nonzero key 1");
+    testsuite_fail_unless(non_zero_buffer(master_key_2, master_key_2_length),
+                          "keygen: returns nonzero key 2");
+
+    testsuite_fail_unless(different_buffers(master_key_1, master_key_1_length, master_key_2, master_key_2_length),
+                          "keygen: generates different keys");
+
+    /*
+     * Verify invalid input handling and error conditions.
+     */
+
+    res = themis_gen_sym_key(NULL, NULL);
+    testsuite_fail_unless(res == THEMIS_INVALID_PARAMETER, "keygen: missing key length");
+
+    res = themis_gen_sym_key(master_key_1, NULL);
+    testsuite_fail_unless(res == THEMIS_INVALID_PARAMETER, "keygen: missing key length");
+}
+
 static int secure_cell_seal(void)
 {
     uint8_t* encrypted_message;
@@ -1363,6 +1457,9 @@ static void secure_cell_context_corruption(void)
 
 void run_secure_cell_test(void)
 {
+    testsuite_enter_suite("secure cell: key generation");
+    testsuite_run_test(secure_cell_key_generation);
+
     testsuite_enter_suite("secure cell: basic flow");
     testsuite_run_test(secure_cell_test);
     testsuite_run_test(secure_cell_test_lengths);