Implement a stricter ECX_KEY type

Add ref counting and control how we allocate storage for the private key.
We will need this type in following commits where we move the ecx code
to be provider aware.

Reviewed-by: Patrick Steuer <patrick.steuer@de.ibm.com>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/10964)

10 files changed, 160 insertions, 75 deletions

diff --git a/crypto/ec/build.info b/crypto/ec/build.info
index ba16e088fa..a8828c5102 100644
--- a/crypto/ec/build.info
+++ b/crypto/ec/build.info
@@ -54,8 +54,8 @@ $COMMON=ec_lib.c ecp_smpl.c ecp_mont.c ecp_nist.c ec_cvt.c ec_mult.c \
         curve448/arch_32/f_impl.c curve448/f_generic.c curve448/scalar.c \
         curve448/curve448_tables.c curve448/eddsa.c curve448/curve448.c \
         $ECASM
-SOURCE[../../libcrypto]=$COMMON ec_ameth.c ec_pmeth.c ecx_meth.c ec_err.c \
-                        ecdh_kdf.c eck_prn.c
+SOURCE[../../libcrypto]=$COMMON ec_ameth.c ec_pmeth.c ecx_meth.c ecx_key.c \
+                        ec_err.c ecdh_kdf.c eck_prn.c
 SOURCE[../../providers/libfips.a]=$COMMON
 
 # Implementations are now spread across several libraries, so the defines
diff --git a/crypto/ec/curve25519.c b/crypto/ec/curve25519.c
index 6672f5d249..024f7fe169 100644
--- a/crypto/ec/curve25519.c
+++ b/crypto/ec/curve25519.c
@@ -14,6 +14,7 @@
 #include "internal/deprecated.h"
 
 #include <string.h>
+#include "crypto/ecx.h"
 #include "ec_local.h"
 #include <openssl/evp.h>
 #include <openssl/sha.h>
diff --git a/crypto/ec/curve448/curve448.c b/crypto/ec/curve448/curve448.c
index e3dffd09c9..ecc98d884c 100644
--- a/crypto/ec/curve448/curve448.c
+++ b/crypto/ec/curve448/curve448.c
@@ -15,6 +15,7 @@
 
 #include "point_448.h"
 #include "ed448.h"
+#include "crypto/ecx.h"
 #include "curve448_local.h"
 
 #define COFACTOR 4
diff --git a/crypto/ec/curve448/curve448_local.h b/crypto/ec/curve448/curve448_local.h
index 197627f6b8..36f960ec0e 100644
--- a/crypto/ec/curve448/curve448_local.h
+++ b/crypto/ec/curve448/curve448_local.h
@@ -10,12 +10,6 @@
 # define OSSL_CRYPTO_EC_CURVE448_LOCAL_H
 # include "curve448utils.h"
 
-int X448(uint8_t out_shared_key[56], const uint8_t private_key[56],
-         const uint8_t peer_public_value[56]);
-
-void X448_public_from_private(uint8_t out_public_value[56],
-                              const uint8_t private_key[56]);
-
 int ED448_sign(OPENSSL_CTX *ctx, uint8_t *out_sig, const uint8_t *message,
                size_t message_len, const uint8_t public_key[57],
                const uint8_t private_key[57], const uint8_t *context,
diff --git a/crypto/ec/ec_local.h b/crypto/ec/ec_local.h
index a523ab6422..c0eacc9ce5 100644
--- a/crypto/ec/ec_local.h
+++ b/crypto/ec/ec_local.h
@@ -683,11 +683,6 @@ int ED25519_verify(const uint8_t *message, size_t message_len,
 void ED25519_public_from_private(uint8_t out_public_key[32],
                                  const uint8_t private_key[32]);
 
-int X25519(uint8_t out_shared_key[32], const uint8_t private_key[32],
-           const uint8_t peer_public_value[32]);
-void X25519_public_from_private(uint8_t out_public_value[32],
-                                const uint8_t private_key[32]);
-
 /*-
  * This functions computes a single point multiplication over the EC group,
  * using, at a high level, a Montgomery ladder with conditional swaps, with
diff --git a/crypto/ec/ecx_key.c b/crypto/ec/ecx_key.c
new file mode 100644
index 0000000000..59643cc6ad
--- /dev/null
+++ b/crypto/ec/ecx_key.c
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2020 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License 2.0 (the "License").  You may not use
+ * this file except in compliance with the License.  You can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ */
+
+#include <openssl/err.h>
+#include "crypto/ecx.h"
+
+ECX_KEY *ecx_key_new(size_t keylen, int haspubkey)
+{
+    ECX_KEY *ret = OPENSSL_zalloc(sizeof(*ret));
+
+    if (ret == NULL)
+        return NULL;
+
+    ret->haspubkey = haspubkey;
+    ret->keylen = keylen;
+    ret->references = 1;
+
+    ret->lock = CRYPTO_THREAD_lock_new();
+    if (ret->lock == NULL) {
+        ERR_raise(ERR_LIB_EC, ERR_R_MALLOC_FAILURE);
+        OPENSSL_free(ret);
+        return NULL;
+    }
+
+    return ret;
+}
+
+void ecx_key_free(ECX_KEY *key)
+{
+    int i;
+
+    if (key == NULL)
+        return;
+
+    CRYPTO_DOWN_REF(&key->references, &i, key->lock);
+    REF_PRINT_COUNT("ECX_KEY", r);
+    if (i > 0)
+        return;
+    REF_ASSERT_ISNT(i < 0);
+
+    OPENSSL_secure_clear_free(key->privkey, key->keylen);
+    CRYPTO_THREAD_lock_free(key->lock);
+    OPENSSL_free(key);
+}
+
+int ecx_key_up_ref(ECX_KEY *key)
+{
+    int i;
+
+    if (CRYPTO_UP_REF(&key->references, &i, key->lock) <= 0)
+        return 0;
+
+    REF_PRINT_COUNT("ECX_KEY", key);
+    REF_ASSERT_ISNT(i < 2);
+    return ((i > 1) ? 1 : 0);
+}
+
+unsigned char *ecx_key_allocate_privkey(ECX_KEY *key)
+{
+    key->privkey = OPENSSL_secure_zalloc(key->keylen);
+
+    return key->privkey;
+}
diff --git a/crypto/ec/ecx_meth.c b/crypto/ec/ecx_meth.c
index 525fcd343f..15b902ec1d 100644
--- a/crypto/ec/ecx_meth.c
+++ b/crypto/ec/ecx_meth.c
@@ -20,20 +20,10 @@
 #include <openssl/rand.h>
 #include "crypto/asn1.h"
 #include "crypto/evp.h"
+#include "crypto/ecx.h"
 #include "ec_local.h"
 #include "curve448/curve448_local.h"
 
-#define X25519_BITS          253
-#define X25519_SECURITY_BITS 128
-
-#define ED25519_SIGSIZE      64
-
-#define X448_BITS            448
-#define ED448_BITS           456
-#define X448_SECURITY_BITS   224
-
-#define ED448_SIGSIZE        114
-
 #define ISX448(id)      ((id) == EVP_PKEY_X448)
 #define IS25519(id)     ((id) == EVP_PKEY_X25519 || (id) == EVP_PKEY_ED25519)
 #define KEYLENID(id)    (IS25519(id) ? X25519_KEYLEN \
@@ -73,7 +63,7 @@ static int ecx_key_op(EVP_PKEY *pkey, int id, const X509_ALGOR *palg,
         }
     }
 
-    key = OPENSSL_zalloc(sizeof(*key));
+    key = ecx_key_new(KEYLENID(id), 1);
     if (key == NULL) {
         ECerr(EC_F_ECX_KEY_OP, ERR_R_MALLOC_FAILURE);
         return 0;
@@ -83,17 +73,14 @@ static int ecx_key_op(EVP_PKEY *pkey, int id, const X509_ALGOR *palg,
     if (op == KEY_OP_PUBLIC) {
         memcpy(pubkey, p, plen);
     } else {
-        privkey = key->privkey = OPENSSL_secure_malloc(KEYLENID(id));
+        privkey = ecx_key_allocate_privkey(key);
         if (privkey == NULL) {
             ECerr(EC_F_ECX_KEY_OP, ERR_R_MALLOC_FAILURE);
             goto err;
         }
         if (op == KEY_OP_KEYGEN) {
-            if (RAND_priv_bytes(privkey, KEYLENID(id)) <= 0) {
-                OPENSSL_secure_free(privkey);
-                key->privkey = NULL;
+            if (RAND_priv_bytes(privkey, KEYLENID(id)) <= 0)
                 goto err;
-            }
             if (id == EVP_PKEY_X25519) {
                 privkey[0] &= 248;
                 privkey[X25519_KEYLEN - 1] &= 127;
@@ -128,7 +115,7 @@ static int ecx_key_op(EVP_PKEY *pkey, int id, const X509_ALGOR *palg,
     EVP_PKEY_assign(pkey, id, key);
     return 1;
  err:
-    OPENSSL_free(key);
+    ecx_key_free(key);
     return 0;
 }
 
@@ -264,9 +251,7 @@ static int ecx_security_bits(const EVP_PKEY *pkey)
 
 static void ecx_free(EVP_PKEY *pkey)
 {
-    if (pkey->pkey.ecx != NULL)
-        OPENSSL_secure_clear_free(pkey->pkey.ecx->privkey, KEYLEN(pkey));
-    OPENSSL_free(pkey->pkey.ecx);
+    ecx_key_free(pkey->pkey.ecx);
 }
 
 /* "parameters" are always equal */
@@ -1067,10 +1052,9 @@ static int s390x_pkey_ecx_keygen25519(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
         0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
         0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
     };
-    ECX_KEY *key;
+    ECX_KEY *key = ecx_key_new(X25519_KEYLEN, 1);
     unsigned char *privkey = NULL, *pubkey;
 
-    key = OPENSSL_zalloc(sizeof(*key));
     if (key == NULL) {
         ECerr(EC_F_S390X_PKEY_ECX_KEYGEN25519, ERR_R_MALLOC_FAILURE);
         goto err;
@@ -1078,7 +1062,7 @@ static int s390x_pkey_ecx_keygen25519(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
 
     pubkey = key->pubkey;
 
-    privkey = key->privkey = OPENSSL_secure_malloc(X25519_KEYLEN);
+    privkey = ecx_key_allocate_privkey(key);
     if (privkey == NULL) {
         ECerr(EC_F_S390X_PKEY_ECX_KEYGEN25519, ERR_R_MALLOC_FAILURE);
         goto err;
@@ -1097,9 +1081,7 @@ static int s390x_pkey_ecx_keygen25519(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
     EVP_PKEY_assign(pkey, ctx->pmeth->pkey_id, key);
     return 1;
  err:
-    OPENSSL_secure_clear_free(privkey, X25519_KEYLEN);
-    key->privkey = NULL;
-    OPENSSL_free(key);
+    ecx_key_free(key);
     return 0;
 }
 
@@ -1112,10 +1094,9 @@ static int s390x_pkey_ecx_keygen448(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
         0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
         0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
     };
-    ECX_KEY *key;
+    ECX_KEY *key = ecx_key_new(X448_KEYLEN, 1);
     unsigned char *privkey = NULL, *pubkey;
 
-    key = OPENSSL_zalloc(sizeof(*key));
     if (key == NULL) {
         ECerr(EC_F_S390X_PKEY_ECX_KEYGEN448, ERR_R_MALLOC_FAILURE);
         goto err;
@@ -1123,7 +1104,7 @@ static int s390x_pkey_ecx_keygen448(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
 
     pubkey = key->pubkey;
 
-    privkey = key->privkey = OPENSSL_secure_malloc(X448_KEYLEN);
+    privkey = ecx_key_allocate_privkey(key);
     if (privkey == NULL) {
         ECerr(EC_F_S390X_PKEY_ECX_KEYGEN448, ERR_R_MALLOC_FAILURE);
         goto err;
@@ -1141,9 +1122,7 @@ static int s390x_pkey_ecx_keygen448(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
     EVP_PKEY_assign(pkey, ctx->pmeth->pkey_id, key);
     return 1;
  err:
-    OPENSSL_secure_clear_free(privkey, X448_KEYLEN);
-    key->privkey = NULL;
-    OPENSSL_free(key);
+    ecx_key_free(key);
     return 0;
 }
 
@@ -1160,11 +1139,10 @@ static int s390x_pkey_ecd_keygen25519(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
         0x66, 0x66, 0x66, 0x66, 0x66, 0x66, 0x66, 0x66,
     };
     unsigned char x_dst[32], buff[SHA512_DIGEST_LENGTH];
-    ECX_KEY *key;
+    ECX_KEY *key = ecx_key_new(ED25519_KEYLEN, 1);
     unsigned char *privkey = NULL, *pubkey;
     unsigned int sz;
 
-    key = OPENSSL_zalloc(sizeof(*key));
     if (key == NULL) {
         ECerr(EC_F_S390X_PKEY_ECD_KEYGEN25519, ERR_R_MALLOC_FAILURE);
         goto err;
@@ -1172,7 +1150,7 @@ static int s390x_pkey_ecd_keygen25519(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
 
     pubkey = key->pubkey;
 
-    privkey = key->privkey = OPENSSL_secure_malloc(ED25519_KEYLEN);
+    privkey = ecx_key_allocate_privkey(key);
     if (privkey == NULL) {
         ECerr(EC_F_S390X_PKEY_ECD_KEYGEN25519, ERR_R_MALLOC_FAILURE);
         goto err;
@@ -1197,9 +1175,7 @@ static int s390x_pkey_ecd_keygen25519(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
     EVP_PKEY_assign(pkey, ctx->pmeth->pkey_id, key);
     return 1;
  err:
-    OPENSSL_secure_clear_free(privkey, ED25519_KEYLEN);
-    key->privkey = NULL;
-    OPENSSL_free(key);
+    ecx_key_free(key);
     return 0;
 }
 
@@ -1220,11 +1196,10 @@ static int s390x_pkey_ecd_keygen448(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
         0x24, 0xbc, 0xb6, 0x6e, 0x71, 0x46, 0x3f, 0x69, 0x00
     };
     unsigned char x_dst[57], buff[114];
-    ECX_KEY *key;
+    ECX_KEY *key = ecx_key_new(ED448_KEYLEN, 1);
     unsigned char *privkey = NULL, *pubkey;
     EVP_MD_CTX *hashctx = NULL;
 
-    key = OPENSSL_zalloc(sizeof(*key));
     if (key == NULL) {
         ECerr(EC_F_S390X_PKEY_ECD_KEYGEN448, ERR_R_MALLOC_FAILURE);
         goto err;
@@ -1232,7 +1207,7 @@ static int s390x_pkey_ecd_keygen448(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
 
     pubkey = key->pubkey;
 
-    privkey = key->privkey = OPENSSL_secure_malloc(ED448_KEYLEN);
+    privkey = ecx_key_allocate_privkey(key);
     if (privkey == NULL) {
         ECerr(EC_F_S390X_PKEY_ECD_KEYGEN448, ERR_R_MALLOC_FAILURE);
         goto err;
@@ -1265,9 +1240,7 @@ static int s390x_pkey_ecd_keygen448(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
     EVP_MD_CTX_free(hashctx);
     return 1;
  err:
-    OPENSSL_secure_clear_free(privkey, ED448_KEYLEN);
-    key->privkey = NULL;
-    OPENSSL_free(key);
+    ecx_key_free(key);
     EVP_MD_CTX_free(hashctx);
     return 0;
 }
diff --git a/include/crypto/ecx.h b/include/crypto/ecx.h
new file mode 100644
index 0000000000..3f0bbe1bb9
--- /dev/null
+++ b/include/crypto/ecx.h
@@ -0,0 +1,67 @@
+/*
+ * Copyright 2020 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License 2.0 (the "License").  You may not use
+ * this file except in compliance with the License.  You can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ */
+
+/* Internal EC functions for other submodules: not for application use */
+
+#ifndef OSSL_CRYPTO_ECX_H
+# define OSSL_CRYPTO_ECX_H
+# include <openssl/opensslconf.h>
+
+# ifndef OPENSSL_NO_EC
+
+#  include <openssl/e_os2.h>
+#  include <openssl/crypto.h>
+#  include "internal/refcount.h"
+
+#  define X25519_KEYLEN        32
+#  define X448_KEYLEN          56
+#  define ED25519_KEYLEN       32
+#  define ED448_KEYLEN         57
+
+#  define MAX_KEYLEN  ED448_KEYLEN
+
+#  define X25519_BITS          253
+#  define X25519_SECURITY_BITS 128
+
+#  define ED25519_SIGSIZE      64
+
+#  define X448_BITS            448
+#  define ED448_BITS           456
+#  define X448_SECURITY_BITS   224
+
+#  define ED448_SIGSIZE        114
+
+struct ecx_key_st {
+    unsigned int haspubkey:1;
+    unsigned char pubkey[MAX_KEYLEN];
+    unsigned char *privkey;
+    size_t keylen;
+    CRYPTO_REF_COUNT references;
+    CRYPTO_RWLOCK *lock;
+};
+
+typedef struct ecx_key_st ECX_KEY;
+
+ECX_KEY *ecx_key_new(size_t keylen, int haspubkey);
+unsigned char *ecx_key_allocate_privkey(ECX_KEY *key);
+void ecx_key_free(ECX_KEY *key);
+int ecx_key_up_ref(ECX_KEY *key);
+
+int X25519(uint8_t out_shared_key[32], const uint8_t private_key[32],
+           const uint8_t peer_public_value[32]);
+void X25519_public_from_private(uint8_t out_public_value[32],
+                                const uint8_t private_key[32]);
+
+int X448(uint8_t out_shared_key[56], const uint8_t private_key[56],
+         const uint8_t peer_public_value[56]);
+void X448_public_from_private(uint8_t out_public_value[56],
+                              const uint8_t private_key[56]);
+
+# endif /* OPENSSL_NO_EC */
+#endif
diff --git a/include/crypto/evp.h b/include/crypto/evp.h
index 6903cc656a..65889ae812 100644
--- a/include/crypto/evp.h
+++ b/include/crypto/evp.h
@@ -10,6 +10,7 @@
 #include <openssl/evp.h>
 #include <openssl/core_numbers.h>
 #include "internal/refcount.h"
+#include "crypto/ecx.h"
 
 /*
  * Don't free up md_ctx->pctx in EVP_MD_CTX_reset, use the reserved flag
@@ -495,23 +496,6 @@ const EVP_CIPHER *EVP_##cname##_ecb(void) { return &cname##_ecb; }
                              (fl)|EVP_CIPH_FLAG_DEFAULT_ASN1, \
                              cipher##_init_key, NULL, NULL, NULL, NULL)
 
-
-# ifndef OPENSSL_NO_EC
-
-#define X25519_KEYLEN        32
-#define X448_KEYLEN          56
-#define ED25519_KEYLEN       32
-#define ED448_KEYLEN         57
-
-#define MAX_KEYLEN  ED448_KEYLEN
-
-typedef struct {
-    unsigned char pubkey[MAX_KEYLEN];
-    unsigned char *privkey;
-} ECX_KEY;
-
-#endif
-
 /*
  * Type needs to be a bit field Sub-type needs to be for variations on the
  * method, as in, can it do arbitrary encryption....
diff --git a/test/curve448_internal_test.c b/test/curve448_internal_test.c
index 054948d23b..953b56c012 100644
--- a/test/curve448_internal_test.c
+++ b/test/curve448_internal_test.c
@@ -10,6 +10,7 @@
 #include <string.h>
 #include <openssl/e_os2.h>
 #include <openssl/evp.h>
+#include "crypto/ecx.h"
 #include "curve448_local.h"
 #include "testutil.h"