soter: Fix misuse of OpenSSL API for EC key generation (and more)

.github/workflows/test-nodejs.yaml

@@ -48,10 +48,19 @@ jobs:
           node-version: ${{ matrix.node-version }}
       - name: Check out code
         uses: actions/checkout@v2
+        with:
+          submodules: true
       - name: Install Themis Core
+        env:
+          NODE_VERSION: ${{ matrix.node-version }}
         run: |
+          # Workaround for Node.js 8.x using OpenSSL 1.0.2 when Themis is built against OpenSSL 1.1
+          # https://github.com/cossacklabs/themis/issues/657
+          if [[ "$NODE_VERSION" = "8.x" ]]; then
+            export ENGINE=boringssl
+          fi
           make
-          sudo make install
+          sudo -E make install
       - name: Run test suite
         run: |
           echo Node.js: $(node --version)

CHANGELOG.md

@@ -69,6 +69,7 @@ _Code:_
   - Fixed cross-compilation on macOS by setting `ARCH` and `SDK` variables ([#849](https://github.com/cossacklabs/themis/pull/849)).
   - Updated embedded BoringSSL to the latest version ([#812](https://github.com/cossacklabs/themis/pull/812)).
   - Builds with OpenSSL 3.0 will result in a compilation error for the time being ([#872](https://github.com/cossacklabs/themis/pull/872)).
+  - Hardened EC/RSA key generation and Secure Message sign/verify code paths ([#875](https://github.com/cossacklabs/themis/pull/875)).
 
 - **Android**
 

src/soter/openssl/soter_ecdsa_common.c

@@ -22,28 +22,59 @@
 #include "soter/openssl/soter_engine.h"
 #include "soter/soter_ec_key.h"
 
-soter_status_t soter_ec_gen_key(EVP_PKEY_CTX* pkey_ctx)
+soter_status_t soter_ec_gen_key(EVP_PKEY** ppkey)
 {
-    EVP_PKEY* pkey;
-    EC_KEY* ec;
-    if (!pkey_ctx) {
+    soter_status_t res = SOTER_FAIL;
+    EVP_PKEY* param = NULL;
+    EVP_PKEY_CTX* param_ctx = NULL;
+    EVP_PKEY_CTX* pkey_ctx = NULL;
+
+    if (!ppkey) {
         return SOTER_INVALID_PARAMETER;
     }
-    pkey = EVP_PKEY_CTX_get0_pkey(pkey_ctx);
-    if (!pkey) {
-        return SOTER_INVALID_PARAMETER;
+
+    param_ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_EC, NULL);
+    if (!param_ctx) {
+        res = SOTER_NO_MEMORY;
+        goto err;
     }
-    if (EVP_PKEY_EC != EVP_PKEY_id(pkey)) {
-        return SOTER_INVALID_PARAMETER;
+
+    if (EVP_PKEY_paramgen_init(param_ctx) != 1) {
+        res = SOTER_FAIL;
+        goto err;
     }
-    ec = EVP_PKEY_get0(pkey);
-    if (NULL == ec) {
-        return SOTER_INVALID_PARAMETER;
+    if (EVP_PKEY_CTX_set_ec_paramgen_curve_nid(param_ctx, NID_X9_62_prime256v1) != 1) {
+        res = SOTER_FAIL;
+        goto err;
     }
-    if (1 == EC_KEY_generate_key(ec)) {
-        return SOTER_SUCCESS;
+    if (EVP_PKEY_paramgen(param_ctx, &param) != 1) {
+        res = SOTER_FAIL;
+        goto err;
     }
-    return SOTER_FAIL;
+
+    pkey_ctx = EVP_PKEY_CTX_new(param, NULL);
+    if (!pkey_ctx) {
+        res = SOTER_NO_MEMORY;
+        goto err;
+    }
+
+    if (EVP_PKEY_keygen_init(pkey_ctx) != 1) {
+        res = SOTER_FAIL;
+        goto err;
+    }
+    if (EVP_PKEY_keygen(pkey_ctx, ppkey) != 1) {
+        res = SOTER_FAIL;
+        goto err;
+    }
+
+    res = SOTER_SUCCESS;
+
+err:
+    EVP_PKEY_CTX_free(param_ctx);
+    EVP_PKEY_CTX_free(pkey_ctx);
+    EVP_PKEY_free(param);
+
+    return res;
 }
 
 soter_status_t soter_ec_import_key(EVP_PKEY* pkey, const void* key, const size_t key_length)
@@ -71,9 +102,8 @@ soter_status_t soter_ec_import_key(EVP_PKEY* pkey, const void* key, const size_t
     return SOTER_INVALID_PARAMETER;
 }
 
-soter_status_t soter_ec_export_key(soter_sign_ctx_t* ctx, void* key, size_t* key_length, bool isprivate)
+soter_status_t soter_ec_export_key(EVP_PKEY* pkey, void* key, size_t* key_length, bool isprivate)
 {
-    EVP_PKEY* pkey = EVP_PKEY_CTX_get0_pkey(ctx->pkey_ctx);
     if (!pkey) {
         return SOTER_INVALID_PARAMETER;
     }

src/soter/openssl/soter_ecdsa_common.h

@@ -21,8 +21,8 @@
 #include "soter/soter_ec_key.h"
 #include "soter/soter_error.h"
 
-soter_status_t soter_ec_gen_key(EVP_PKEY_CTX* pkey_ctx);
+soter_status_t soter_ec_gen_key(EVP_PKEY** ppkey);
 soter_status_t soter_ec_import_key(EVP_PKEY* pkey, const void* key, size_t key_length);
-soter_status_t soter_ec_export_key(soter_sign_ctx_t* ctx, void* key, size_t* key_length, bool isprivate);
+soter_status_t soter_ec_export_key(EVP_PKEY* pkey, void* key, size_t* key_length, bool isprivate);
 
 #endif /* SOTER_OPENSSL_ECDSA_COMMON_H */

src/soter/openssl/soter_engine.h

@@ -54,7 +54,7 @@ struct soter_asym_ka_type {
 };
 
 struct soter_sign_ctx_type {
-    EVP_PKEY_CTX* pkey_ctx;
+    EVP_PKEY* pkey;
     EVP_MD_CTX* md_ctx;
     soter_sign_alg_t alg;
 };

src/soter/openssl/soter_rsa_common.c

@@ -24,51 +24,66 @@
 #define SOTER_RSA_KEY_LENGTH 2048
 #endif
 
-soter_status_t soter_rsa_gen_key(EVP_PKEY_CTX* pkey_ctx)
+soter_status_t soter_rsa_gen_key(EVP_PKEY** ppkey)
 {
-    /* it is copy-paste from /src/soter/openssl/soter_asym_cipher.c */
-    BIGNUM* pub_exp;
-    EVP_PKEY* pkey = EVP_PKEY_CTX_get0_pkey(pkey_ctx);
-    if (!pkey) {
+    soter_status_t res = SOTER_FAIL;
+    BIGNUM* pub_exp = NULL;
+    EVP_PKEY_CTX* pkey_ctx = NULL;
+
+    if (!ppkey) {
         return SOTER_INVALID_PARAMETER;
     }
 
-    if (EVP_PKEY_RSA != EVP_PKEY_id(pkey)) {
-        return SOTER_INVALID_PARAMETER;
+    pkey_ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_RSA, NULL);
+    if (!pkey_ctx) {
+        res = SOTER_NO_MEMORY;
+        goto err;
     }
 
-    if (!EVP_PKEY_keygen_init(pkey_ctx)) {
-        return SOTER_INVALID_PARAMETER;
+    if (EVP_PKEY_keygen_init(pkey_ctx) != 1) {
+        res = SOTER_INVALID_PARAMETER;
+        goto err;
     }
 
     /* Although it seems that OpenSSL/LibreSSL use 0x10001 as default public exponent, we will set
      * it explicitly just in case */
     pub_exp = BN_new();
     if (!pub_exp) {
-        return SOTER_NO_MEMORY;
+        res = SOTER_NO_MEMORY;
+        goto err;
     }
 
     if (!BN_set_word(pub_exp, RSA_F4)) {
-        BN_free(pub_exp);
-        return SOTER_FAIL;
+        res = SOTER_FAIL;
+        goto err;
     }
 
+    /* Passing ownership over pub_exp to EVP_PKEY_CTX */
     if (1 > EVP_PKEY_CTX_ctrl(pkey_ctx, -1, -1, EVP_PKEY_CTRL_RSA_KEYGEN_PUBEXP, 0, pub_exp)) {
-        BN_free(pub_exp);
-        return SOTER_FAIL;
+        res = SOTER_FAIL;
+        goto err;
     }
+    pub_exp = NULL;
 
     /* Override default key size for RSA key. Currently OpenSSL has default key size of 1024.
      * LibreSSL has 2048. We will put 2048 explicitly */
     if (1 > EVP_PKEY_CTX_ctrl(pkey_ctx, -1, -1, EVP_PKEY_CTRL_RSA_KEYGEN_BITS, SOTER_RSA_KEY_LENGTH, NULL)) {
-        return SOTER_FAIL;
+        res = SOTER_FAIL;
+        goto err;
     }
 
-    if (!EVP_PKEY_keygen(pkey_ctx, &pkey)) {
-        return SOTER_FAIL;
+    if (EVP_PKEY_keygen(pkey_ctx, ppkey) != 1) {
+        res = SOTER_FAIL;
+        goto err;
     }
-    return SOTER_SUCCESS;
-    /* end of copy-paste from /src/soter/openssl/soter_asym_cipher.c*/
+
+    res = SOTER_SUCCESS;
+
+err:
+    BN_free(pub_exp);
+    EVP_PKEY_CTX_free(pkey_ctx);
+
+    return res;
 }
 
 soter_status_t soter_rsa_import_key(EVP_PKEY* pkey, const void* key, const size_t key_length)
@@ -94,10 +109,8 @@ soter_status_t soter_rsa_import_key(EVP_PKEY* pkey, const void* key, const size_
     return SOTER_INVALID_PARAMETER;
 }
 
-soter_status_t soter_rsa_export_key(soter_sign_ctx_t* ctx, void* key, size_t* key_length, bool isprivate)
+soter_status_t soter_rsa_export_key(EVP_PKEY* pkey, void* key, size_t* key_length, bool isprivate)
 {
-    EVP_PKEY* pkey = EVP_PKEY_CTX_get0_pkey(ctx->pkey_ctx);
-
     if (!pkey) {
         return SOTER_INVALID_PARAMETER;
     }

src/soter/openssl/soter_rsa_common.h

@@ -21,8 +21,8 @@
 #include "soter/soter_error.h"
 #include "soter/soter_rsa_key.h"
 
-soter_status_t soter_rsa_gen_key(EVP_PKEY_CTX* pkey_ctx);
+soter_status_t soter_rsa_gen_key(EVP_PKEY** ppkey);
 soter_status_t soter_rsa_import_key(EVP_PKEY* pkey, const void* key, size_t key_length);
-soter_status_t soter_rsa_export_key(soter_sign_ctx_t* ctx, void* key, size_t* key_length, bool isprivate);
+soter_status_t soter_rsa_export_key(EVP_PKEY* pkey, void* key, size_t* key_length, bool isprivate);
 
 #endif /* SOTER_OPENSSL_RSA_COMMON_H */