Refactor ENGINE API and memory around METHOD structs (#1776)

### Description of changes: 
1. Breaking changes to the ENGINE API - made the function signatures
match OpenSSL. Removed method_size as a parameter since the method can
no longer be statically allocated.
2. Got rid of old code supporting reference counting for METHOD structs
as it never existed in OpenSSL and was no-ops/unused in BoringSSL
3. Removed static allocation enforcement for METHOD structs. Now
consumers can only dynamically allocate these structs via funcs like
EC_KEY_METHOD_new and RSA_METHOD_new which are added
[here](#1785) and
[here](#1790).

### Call-outs:
First PR in a series of 3 to refactor and expand support for ENGINE,
RSA_METHOD, and EC_KEY_METHOD structs

### Testing:
No significant tests can be written yet. The memory model will be tested
further in the PRs that follow.
 
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.

crypto/engine/engine.c

@@ -34,58 +34,34 @@ struct engine_st {
 ENGINE *ENGINE_new(void) { return OPENSSL_zalloc(sizeof(ENGINE)); }
 
 int ENGINE_free(ENGINE *engine) {
-  // Methods are currently required to be static so are not unref'ed.
   OPENSSL_free(engine);
   return 1;
 }
 
-// set_method takes a pointer to a method and its given size and sets
-// |*out_member| to point to it. This function might want to be extended in the
-// future to support making a copy of the method so that a stable ABI for
-// ENGINEs can be supported. But, for the moment, all *_METHODS must be
-// static.
-static int set_method(void **out_member, const void *method, size_t method_size,
-                      size_t compiled_size) {
-  const struct openssl_method_common_st *common = method;
-  if (method_size != compiled_size || !common->is_static) {
-    return 0;
+int ENGINE_set_RSA(ENGINE *engine, const RSA_METHOD *method) {
+  if(engine) {
+    engine->rsa_method = (RSA_METHOD *)method;
+    return 1;
   }
 
-  *out_member = (void*) method;
-  return 1;
-}
-
-int ENGINE_set_RSA_method(ENGINE *engine, const RSA_METHOD *method,
-                         size_t method_size) {
-  return set_method((void **)&engine->rsa_method, method, method_size,
-                    sizeof(RSA_METHOD));
+  return 0;
 }
 
-RSA_METHOD *ENGINE_get_RSA_method(const ENGINE *engine) {
+const RSA_METHOD *ENGINE_get_RSA(const ENGINE *engine) {
   return engine->rsa_method;
 }
 
-int ENGINE_set_ECDSA_method(ENGINE *engine, const ECDSA_METHOD *method,
-                            size_t method_size) {
-  return set_method((void **)&engine->ecdsa_method, method, method_size,
-                    sizeof(ECDSA_METHOD));
-}
-
-ECDSA_METHOD *ENGINE_get_ECDSA_method(const ENGINE *engine) {
-  return engine->ecdsa_method;
-}
+int ENGINE_set_ECDSA(ENGINE *engine, const ECDSA_METHOD *method) {
+  if(engine) {
+    engine->ecdsa_method = (ECDSA_METHOD *)method;
+    return 1;
+  }
 
-void METHOD_ref(void *method_in) {
-  assert(((struct openssl_method_common_st*) method_in)->is_static);
+  return 0;
 }
 
-void METHOD_unref(void *method_in) {
-  struct openssl_method_common_st *method = method_in;
-
-  if (method == NULL) {
-    return;
-  }
-  assert(method->is_static);
+const ECDSA_METHOD *ENGINE_get_ECDSA(const ENGINE *engine) {
+  return engine->ecdsa_method;
 }
 
 OPENSSL_DECLARE_ERROR_REASON(ENGINE, OPERATION_NOT_SUPPORTED)

crypto/fipsmodule/ec/ec_key.c

@@ -111,10 +111,7 @@ EC_KEY *EC_KEY_new_method(const ENGINE *engine) {
   }
 
   if (engine) {
-    ret->ecdsa_meth = ENGINE_get_ECDSA_method(engine);
-  }
-  if (ret->ecdsa_meth) {
-    METHOD_ref(ret->ecdsa_meth);
+    ret->ecdsa_meth = ENGINE_get_ECDSA(engine);
   }
 
   ret->conv_form = POINT_CONVERSION_UNCOMPRESSED;
@@ -124,9 +121,6 @@ EC_KEY *EC_KEY_new_method(const ENGINE *engine) {
 
   if (ret->ecdsa_meth && ret->ecdsa_meth->init && !ret->ecdsa_meth->init(ret)) {
     CRYPTO_free_ex_data(g_ec_ex_data_class_bss_get(), ret, &ret->ex_data);
-    if (ret->ecdsa_meth) {
-      METHOD_unref(ret->ecdsa_meth);
-    }
     OPENSSL_free(ret);
     return NULL;
   }
@@ -156,11 +150,8 @@ void EC_KEY_free(EC_KEY *r) {
     return;
   }
 
-  if (r->ecdsa_meth) {
-    if (r->ecdsa_meth->finish) {
-      r->ecdsa_meth->finish(r);
-    }
-    METHOD_unref(r->ecdsa_meth);
+  if (r->ecdsa_meth && r->ecdsa_meth->finish) {
+    r->ecdsa_meth->finish(r);
   }
 
   CRYPTO_free_ex_data(g_ec_ex_data_class_bss_get(), r, &r->ex_data);

crypto/fipsmodule/ec/internal.h

@@ -766,7 +766,7 @@ struct ec_key_st {
 
   CRYPTO_refcount_t references;
 
-  ECDSA_METHOD *ecdsa_meth;
+  const ECDSA_METHOD *ecdsa_meth;
 
   CRYPTO_EX_DATA ex_data;
 } /* EC_KEY */;

crypto/fipsmodule/rsa/internal.h

@@ -70,7 +70,7 @@ extern "C" {
 typedef struct bn_blinding_st BN_BLINDING;
 
 struct rsa_st {
-  RSA_METHOD *meth;
+  const RSA_METHOD *meth;
 
   BIGNUM *n;
   BIGNUM *e;
@@ -128,6 +128,8 @@ struct rsa_st {
 
 // Default implementations of RSA operations.
 
+// RSA_default_method returns a zero initialized |RSA_METHOD| object. The
+// wrapper functions will select the appropriate |rsa_default_*| implementation.
 const RSA_METHOD *RSA_default_method(void);
 
 size_t rsa_default_size(const RSA *rsa);

crypto/fipsmodule/rsa/rsa.c

@@ -217,13 +217,12 @@ RSA *RSA_new_method(const ENGINE *engine) {
   }
 
   if (engine) {
-    rsa->meth = ENGINE_get_RSA_method(engine);
+    rsa->meth = ENGINE_get_RSA(engine);
   }
 
   if (rsa->meth == NULL) {
     rsa->meth = (RSA_METHOD *) RSA_default_method();
   }
-  METHOD_ref(rsa->meth);
 
   rsa->references = 1;
   rsa->flags = rsa->meth->flags;
@@ -233,7 +232,6 @@ RSA *RSA_new_method(const ENGINE *engine) {
   if (rsa->meth->init && !rsa->meth->init(rsa)) {
     CRYPTO_free_ex_data(g_rsa_ex_data_class_bss_get(), rsa, &rsa->ex_data);
     CRYPTO_MUTEX_cleanup(&rsa->lock);
-    METHOD_unref(rsa->meth);
     OPENSSL_free(rsa);
     return NULL;
   }
@@ -263,10 +261,9 @@ void RSA_free(RSA *rsa) {
     return;
   }
 
-  if (rsa->meth->finish) {
+  if (rsa->meth && rsa->meth->finish) {
     rsa->meth->finish(rsa);
   }
-  METHOD_unref(rsa->meth);
 
   CRYPTO_free_ex_data(g_rsa_ex_data_class_bss_get(), rsa, &rsa->ex_data);
 

crypto/fipsmodule/rsa/rsa_impl.c

@@ -1278,5 +1278,4 @@ DEFINE_METHOD_FUNCTION(RSA_METHOD, RSA_default_method) {
   // drop unused functions. The wrapper functions will select the appropriate
   // |rsa_default_*| implementation.
   OPENSSL_memset(out, 0, sizeof(RSA_METHOD));
-  out->common.is_static = 1;
 }

include/openssl/ec_key.h

@@ -289,8 +289,6 @@ OPENSSL_EXPORT void *EC_KEY_get_ex_data(const EC_KEY *r, int idx);
 // ecdsa_method_st is a structure of function pointers for implementing ECDSA.
 // See engine.h.
 struct ecdsa_method_st {
-  struct openssl_method_common_st common;
-
   void *app_data;
 
   int (*init)(EC_KEY *key);

include/openssl/engine.h

@@ -46,37 +46,23 @@ OPENSSL_EXPORT int ENGINE_free(ENGINE *engine);
 
 // Method accessors.
 //
-// Method accessors take a method pointer and the size of the structure. The
-// size allows for ABI compatibility in the case that the method structure is
-// extended with extra elements at the end. Methods are always copied by the
-// set functions.
+// Method accessors take a method pointer and set it on the |ENGINE| object.
+// AWS-LC does not take ownership of the |method| pointer. The consumer
+// must free the |method| pointer after all objects referencing it are
+// freed.
 //
-// Set functions return one on success and zero on allocation failure.
+// Set functions return one on success and zero for failure when
+// |engine| is NULL.
 
-OPENSSL_EXPORT int ENGINE_set_RSA_method(ENGINE *engine,
-                                         const RSA_METHOD *method,
-                                         size_t method_size);
-OPENSSL_EXPORT RSA_METHOD *ENGINE_get_RSA_method(const ENGINE *engine);
+OPENSSL_EXPORT int ENGINE_set_RSA(ENGINE *engine,
+                                         const RSA_METHOD *method);
 
-OPENSSL_EXPORT int ENGINE_set_ECDSA_method(ENGINE *engine,
-                                           const ECDSA_METHOD *method,
-                                           size_t method_size);
-OPENSSL_EXPORT ECDSA_METHOD *ENGINE_get_ECDSA_method(const ENGINE *engine);
+OPENSSL_EXPORT const RSA_METHOD *ENGINE_get_RSA(const ENGINE *engine);
 
+OPENSSL_EXPORT int ENGINE_set_ECDSA(ENGINE *engine,
+                                           const ECDSA_METHOD *method);
 
-// Generic method functions.
-//
-// These functions take a void* type but actually operate on all method
-// structures.
-
-// METHOD_ref increments the reference count of |method|. This is a no-op for
-// now because all methods are currently static.
-void METHOD_ref(void *method);
-
-// METHOD_unref decrements the reference count of |method| and frees it if the
-// reference count drops to zero. This is a no-op for now because all methods
-// are currently static.
-void METHOD_unref(void *method);
+OPENSSL_EXPORT const ECDSA_METHOD *ENGINE_get_ECDSA(const ENGINE *engine);
 
 
 // Deprecated functions.
@@ -86,16 +72,6 @@ void METHOD_unref(void *method);
 OPENSSL_EXPORT void ENGINE_cleanup(void);
 
 
-// Private functions.
-
-// openssl_method_common_st contains the common part of all method structures.
-// This must be the first member of all method structures.
-struct openssl_method_common_st {
-  int references;  // dummy – not used.
-  char is_static;
-};
-
-
 #if defined(__cplusplus)
 }  // extern C
 

include/openssl/rsa.h

@@ -830,8 +830,6 @@ OPENSSL_EXPORT RSA *RSA_new_method_no_e(const ENGINE *engine, const BIGNUM *n);
 
 
 struct rsa_meth_st {
-  struct openssl_method_common_st common;
-
   void *app_data;
 
   int (*init)(RSA *rsa);