On 2017-11-09 16:53:15 [+0000], Martin Simmons wrote:
> Hi Sebastian,
Hi Martin,

> Maybe openssl_init_threads and openssl_cleanup_threads (and helper functions)
> in src/lib/openssl.c should also be conditionalized to remove uses of the old
> threading and locking APIs?

oh boy. Yes, definitely. Something like that in the attached patch?
(this time not even compile tested).

> __Martin

Sebastian
>From 7eb1f97ca3a154818ff42e3259899fbc94140fbc Mon Sep 17 00:00:00 2001
From: Sebastian Andrzej Siewior <sebast...@breakpoint.cc>
Date: Thu, 9 Nov 2017 21:55:20 +0100
Subject: [PATCH] crypto: remove most of OpenSSL initcallbacks for 1.1

In OpenSSL 1.1 the thread model atomically initialized the library so
there is no need to invoke the init calls, like it was needed for 1.0.2
and earlier. The may be needed for non-standard inits like no-error
strings or so (not the case here). So the ifdef avoids them.

Also, in 1.1 there is no need to teach OpenSSL how to do locking and so
on. Infect, those functions are null-macros as for 1.0.2 compat. So
another ifdef avoids them, too.

I made four function static and removed them the header file since they
don't seem to be used outside of that openssl.c file.

This leaves us the 1.1 init part down to openssl_seed_prng(). I would
actually suggest to get rid of it. OpenSSL is able to gather some
entropy if needed. I don't think reading 2x 1024KiB from system's
entropy on each invocation of the program is wise. But I leave it to
the maintainer to make a decision, I just point it out.

Signed-off-by: Sebastian Andrzej Siewior <sebast...@breakpoint.cc>
---
 src/lib/openssl.c | 33 ++++++++++++++++++---------------
 src/lib/openssl.h |  4 ----
 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/src/lib/openssl.c b/src/lib/openssl.c
index 64b0e96009a7..d43b3c1344d5 100644
--- a/src/lib/openssl.c
+++ b/src/lib/openssl.c
@@ -41,15 +41,6 @@
 
 /* Are we initialized? */
 static int crypto_initialized = false;
-
-/* Array of mutexes for use with OpenSSL static locking */
-static pthread_mutex_t *mutexes;
-
-/* OpenSSL dynamic locking structure */
-struct CRYPTO_dynlock_value {
-   pthread_mutex_t mutex;
-};
-
 /*
  * ***FIXME*** this is a sort of dummy to avoid having to
  *   change all the existing code to pass either a jcr or
@@ -61,7 +52,6 @@ void openssl_post_errors(int code, const char *errstring)
    openssl_post_errors(NULL, code, errstring);
 }
 
-
 /*
  * Post all per-thread openssl errors
  */
@@ -79,6 +69,15 @@ void openssl_post_errors(JCR *jcr, int code, const char *errstring)
    }
 }
 
+#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
+/* Array of mutexes for use with OpenSSL static locking */
+static pthread_mutex_t *mutexes;
+
+/* OpenSSL dynamic locking structure */
+struct CRYPTO_dynlock_value {
+   pthread_mutex_t mutex;
+};
+
 /*
  * Return an OpenSSL thread ID
  *  Returns: thread ID
@@ -151,12 +150,11 @@ static void openssl_update_static_mutex (int mode, int i, const char *file, int
  *  Returns: 0 on success
  *           errno on failure
  */
-int openssl_init_threads (void)
+static int openssl_init_threads (void)
 {
    int i, numlocks;
    int stat;
 
-
    /* Set thread ID callback */
    CRYPTO_set_id_callback(get_openssl_thread_id);
 
@@ -185,7 +183,7 @@ int openssl_init_threads (void)
 /*
  * Clean up OpenSSL threading support
  */
-void openssl_cleanup_threads(void)
+static void openssl_cleanup_threads(void)
 {
    int i, numlocks;
    int stat;
@@ -216,13 +214,14 @@ void openssl_cleanup_threads(void)
    CRYPTO_set_dynlock_destroy_callback(NULL);
 }
 
+#endif
 
 /*
  * Seed OpenSSL PRNG
  *  Returns: 1 on success
  *           0 on failure
  */
-int openssl_seed_prng (void)
+static int openssl_seed_prng (void)
 {
    const char *names[]  = { "/dev/urandom", "/dev/random", NULL };
    int i;
@@ -247,7 +246,7 @@ int openssl_seed_prng (void)
  *  Returns: 1 on success
  *           0 on failure
  */
-int openssl_save_prng (void)
+static int openssl_save_prng (void)
 {
    // ***FIXME***
    // Implement PRNG state save
@@ -262,6 +261,7 @@ int openssl_save_prng (void)
  */
 int init_crypto (void)
 {
+#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
    int stat;
 
    if ((stat = openssl_init_threads()) != 0) {
@@ -278,6 +278,7 @@ int init_crypto (void)
 
    /* Register OpenSSL ciphers and digests */
    OpenSSL_add_all_algorithms();
+#endif
 
    if (!openssl_seed_prng()) {
       Jmsg0(NULL, M_ERROR_TERM, 0, _("Failed to seed OpenSSL PRNG\n"));
@@ -309,6 +310,7 @@ int cleanup_crypto (void)
       Jmsg0(NULL, M_ERROR, 0, _("Failed to save OpenSSL PRNG\n"));
    }
 
+#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
    openssl_cleanup_threads();
 
    /* Free libssl and libcrypto error strings */
@@ -319,6 +321,7 @@ int cleanup_crypto (void)
 
    /* Free memory used by PRNG */
    RAND_cleanup();
+#endif
 
    crypto_initialized = false;
 
diff --git a/src/lib/openssl.h b/src/lib/openssl.h
index 597517a4fd68..9374a581c735 100644
--- a/src/lib/openssl.h
+++ b/src/lib/openssl.h
@@ -39,10 +39,6 @@
 #ifdef HAVE_OPENSSL
 void             openssl_post_errors     (int code, const char *errstring);
 void             openssl_post_errors     (JCR *jcr, int code, const char *errstring);
-int              openssl_init_threads    (void);
-void             openssl_cleanup_threads (void);
-int              openssl_seed_prng       (void);
-int              openssl_save_prng       (void);
 #endif /* HAVE_OPENSSL */
 
 #endif /* __OPENSSL_H_ */
-- 
2.15.0

Reply via email to