Deprecate ENGINE_cleanup() and make it a no-op

ENGINE_cleanup() should not be called expicitly - we should leave
auto-deinit to clean this up instead.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>

7 files changed, 37 insertions, 30 deletions

diff --git a/crypto/engine/eng_int.h b/crypto/engine/eng_int.h
index f6a54d8567..70b9517b7e 100644
--- a/crypto/engine/eng_int.h
+++ b/crypto/engine/eng_int.h
@@ -99,9 +99,9 @@ extern CRYPTO_RWLOCK *global_engine_lock;
 
 /*
  * Any code that will need cleanup operations should use these functions to
- * register callbacks. ENGINE_cleanup() will call all registered callbacks in
- * order. NB: both the "add" functions assume the engine lock to already be
- * held (in "write" mode).
+ * register callbacks. engine_cleanup_intern() will call all registered
+ * callbacks in order. NB: both the "add" functions assume the engine lock to
+ * already be held (in "write" mode).
  */
 typedef void (ENGINE_CLEANUP_CB) (void);
 typedef struct st_engine_cleanup_item {
diff --git a/crypto/engine/eng_lib.c b/crypto/engine/eng_lib.c
index dd4734202e..492a249361 100644
--- a/crypto/engine/eng_lib.c
+++ b/crypto/engine/eng_lib.c
@@ -148,8 +148,8 @@ int ENGINE_free(ENGINE *e)
 /* Cleanup stuff */
 
 /*
- * ENGINE_cleanup() is coded such that anything that does work that will need
- * cleanup can register a "cleanup" callback here. That way we don't get
+ * engine_cleanup_intern() is coded such that anything that does work that will
+ * need cleanup can register a "cleanup" callback here. That way we don't get
  * linker bloat by referring to all *possible* cleanups, but any linker bloat
  * into code "X" will cause X's cleanup function to end up here.
  */
@@ -200,7 +200,7 @@ static void engine_cleanup_cb_free(ENGINE_CLEANUP_ITEM *item)
     OPENSSL_free(item);
 }
 
-void ENGINE_cleanup(void)
+void engine_cleanup_intern(void)
 {
     if (int_cleanup_check(0)) {
         sk_ENGINE_CLEANUP_ITEM_pop_free(cleanup_stack,
diff --git a/crypto/engine/eng_list.c b/crypto/engine/eng_list.c
index 4883d14fff..26c6a6b077 100644
--- a/crypto/engine/eng_list.c
+++ b/crypto/engine/eng_list.c
@@ -79,7 +79,7 @@ static ENGINE *engine_list_tail = NULL;
 
 /*
  * This cleanup function is only needed internally. If it should be called,
- * we register it with the "ENGINE_cleanup()" stack to be called during
+ * we register it with the "engine_cleanup_intern()" stack to be called during
  * cleanup.
  */
 
diff --git a/crypto/include/internal/engine.h b/crypto/include/internal/engine.h
index abdb8cc49f..ee5394e96b 100644
--- a/crypto/include/internal/engine.h
+++ b/crypto/include/internal/engine.h
@@ -62,3 +62,4 @@ void engine_load_padlock_internal(void);
 void engine_load_capi_internal(void);
 void engine_load_dasync_internal(void);
 void engine_load_afalg_internal(void);
+void engine_cleanup_intern(void);
diff --git a/crypto/init.c b/crypto/init.c
index 170d11b91c..ca19408bef 100644
--- a/crypto/init.c
+++ b/crypto/init.c
@@ -460,7 +460,7 @@ void OPENSSL_cleanup(void)
                     "CONF_modules_free()\n");
 #ifndef OPENSSL_NO_ENGINE
     fprintf(stderr, "OPENSSL_INIT: OPENSSL_cleanup: "
-                    "ENGINE_cleanup()\n");
+                    "engine_cleanup_intern()\n");
 #endif
     fprintf(stderr, "OPENSSL_INIT: OPENSSL_cleanup: "
                     "crypto_cleanup_all_ex_data_intern()\n");
@@ -474,16 +474,16 @@ void OPENSSL_cleanup(void)
     /*
      * Note that cleanup order is important:
      * - rand_cleanup_intern could call an ENINGE's RAND cleanup function so
-     * must be called before ENGINE_cleanup()
+     * must be called before engine_cleanup_intern()
      * - ENGINEs use CRYPTO_EX_DATA and therefore, must be cleaned up
      * before the ex data handlers are wiped in CRYPTO_cleanup_all_ex_data().
      * - CONF_modules_free() can end up in ENGINE code so must be called before
-     * ENGINE_cleanup()
+     * engine_cleanup_intern()
      */
     rand_cleanup_intern();
     CONF_modules_free();
 #ifndef OPENSSL_NO_ENGINE
-    ENGINE_cleanup();
+    engine_cleanup_intern();
 #endif
     crypto_cleanup_all_ex_data_intern();
 #ifndef OPENSSL_NO_SOCK
diff --git a/doc/crypto/engine.pod b/doc/crypto/engine.pod
index c069fd778f..cb9308df83 100644
--- a/doc/crypto/engine.pod
+++ b/doc/crypto/engine.pod
@@ -23,8 +23,6 @@ engine - ENGINE cryptographic module support
 
  void ENGINE_load_builtin_engines(void);
 
- void ENGINE_cleanup(void);
-
  ENGINE *ENGINE_get_default_RSA(void);
  ENGINE *ENGINE_get_default_DSA(void);
  ENGINE *ENGINE_get_default_ECDH(void);
@@ -134,6 +132,12 @@ engine - ENGINE cryptographic module support
 
  void ENGINE_add_conf_module(void);
 
+Deprecated:
+
+ #if OPENSSL_API_COMPAT < 0x10100000L
+ # define ENGINE_cleanup()
+ #endif
+
 =head1 DESCRIPTION
 
 These functions create, manipulate, and use cryptographic modules in the
@@ -300,21 +304,23 @@ dynamically allocated and populated with these implementations and linked
 into OpenSSL's internal linked list. At this point it is important to
 mention an important API function;
 
- void ENGINE_cleanup(void);
+ ENGINE_cleanup()
 
 If no ENGINE API functions are called at all in an application, then there
-are no inherent memory leaks to worry about from the ENGINE functionality,
-however if any ENGINEs are loaded, even if they are never registered or
-used, it is necessary to use the ENGINE_cleanup() function to
-correspondingly cleanup before program exit, if the caller wishes to avoid
-memory leaks. This mechanism uses an internal callback registration table
+are no inherent memory leaks to worry about from the ENGINE functionality.
+However, prior to OpenSSL 1.1.0 if any ENGINEs are loaded, even if they are
+never registered or used, it was necessary to use the ENGINE_cleanup() function
+to correspondingly cleanup before program exit, if the caller wishes to avoid
+memory leaks. This mechanism used an internal callback registration table
 so that any ENGINE API functionality that knows it requires cleanup can
 register its cleanup details to be called during ENGINE_cleanup(). This
-approach allows ENGINE_cleanup() to clean up after any ENGINE functionality
+approach allowed ENGINE_cleanup() to clean up after any ENGINE functionality
 at all that your program uses, yet doesn't automatically create linker
 dependencies to all possible ENGINE functionality - only the cleanup
 callbacks required by the functionality you do use will be required by the
-linker.
+linker. From OpenSSL 1.1.0 it is no longer necessary to explicitly call
+ENGINE_cleanup and this function is deprecated. Cleanup automatically takes
+place at program exit.
 
 The fact that ENGINEs are made visible to OpenSSL (and thus are linked into
 the program and loaded into memory at run-time) does not mean they are
@@ -553,7 +559,8 @@ L<OPENSSL_init_crypto(3)>, L<rsa(3)>, L<dsa(3)>, L<dh(3)>, L<rand(3)>
 
 =head1 HISTORY
 
-ENGINE_load_openssl(), ENGINE_load_dynamic(), and ENGINE_load_cryptodev()
-were deprecated in OpenSSL 1.1.0 by OPENSSL_init_crypto().
+ENGINE_cleanup(), ENGINE_load_openssl(), ENGINE_load_dynamic(), and
+ENGINE_load_cryptodev() were deprecated in OpenSSL 1.1.0 by
+OPENSSL_init_crypto().
 
 =cut
diff --git a/include/openssl/engine.h b/include/openssl/engine.h
index c12e02755f..86a42933c5 100644
--- a/include/openssl/engine.h
+++ b/include/openssl/engine.h
@@ -406,8 +406,7 @@ void ENGINE_set_table_flags(unsigned int flags);
  *   ENGINE_register_***(e) - registers the implementation from 'e' (if it has one)
  *   ENGINE_unregister_***(e) - unregister the implementation from 'e'
  *   ENGINE_register_all_***() - call ENGINE_register_***() for each 'e' in the list
- * Cleanup is automatically registered from each table when required, so
- * ENGINE_cleanup() will reverse any "register" operations.
+ * Cleanup is automatically registered from each table when required.
  */
 
 int ENGINE_register_RSA(ENGINE *e);
@@ -549,13 +548,13 @@ int ENGINE_set_cmd_defns(ENGINE *e, const ENGINE_CMD_DEFN *defns);
 int ENGINE_set_ex_data(ENGINE *e, int idx, void *arg);
 void *ENGINE_get_ex_data(const ENGINE *e, int idx);
 
+#if OPENSSL_API_COMPAT < 0x10100000L
 /*
- * This function cleans up anything that needs it. Eg. the ENGINE_add()
- * function automatically ensures the list cleanup function is registered to
- * be called from ENGINE_cleanup(). Similarly, all ENGINE_register_***
- * functions ensure ENGINE_cleanup() will clean up after them.
+ * This function previously cleaned up anything that needs it. Auto-deinit will
+ * now take care of it so it is no longer required to call this function.
  */
-void ENGINE_cleanup(void);
+# define ENGINE_cleanup()
+#endif
 
 /*
  * These return values from within the ENGINE structure. These can be useful