diff mbox series

Fix compile with OpenSSL 1.1.0 and deprecated APIs

Message ID 20180728023109.3230-1-rosenp@gmail.com
State Accepted
Headers show
Series Fix compile with OpenSSL 1.1.0 and deprecated APIs | expand

Commit Message

Rosen Penev July 28, 2018, 2:31 a.m. UTC
SSL_session_reused is the same as SSL_cache_hit. The engine load stuff is
now handled by OPENSSL_init.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 src/crypto/tls_openssl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Jouni Malinen Aug. 21, 2018, 5:33 p.m. UTC | #1
On Fri, Jul 27, 2018 at 07:31:09PM -0700, Rosen Penev wrote:
> SSL_session_reused is the same as SSL_cache_hit. The engine load stuff is
> now handled by OPENSSL_init.

Thanks, applied.
David Woodhouse March 14, 2019, 12:38 a.m. UTC | #2
On Fri, 2018-07-27 at 19:31 -0700, Rosen Penev wrote:
> SSL_session_reused is the same as SSL_cache_hit. The engine load stuff is
> now handled by OPENSSL_init.

um...

 $ git grep OPENSSL_init
 $
Rosen Penev March 14, 2019, 12:40 a.m. UTC | #3
On Wed, Mar 13, 2019 at 5:38 PM David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Fri, 2018-07-27 at 19:31 -0700, Rosen Penev wrote:
> > SSL_session_reused is the same as SSL_cache_hit. The engine load stuff is
> > now handled by OPENSSL_init.
>
> um...
>
>  $ git grep OPENSSL_init
>  $
OPENSSL_init is implicit with OpenSSL 1.1. There's no need to call it
unless some non-default setting is needed (not loading error strings
for example).
David Woodhouse March 14, 2019, 12:48 a.m. UTC | #4
On Wed, 2019-03-13 at 17:40 -0700, Rosen Penev wrote:
> OPENSSL_init is implicit with OpenSSL 1.1. There's no need to call it
> unless some non-default setting is needed (not loading error strings
> for example).

My empirical observation is that with OpenSSL 1.1.0g, engines aren't
working unless I remove the #ifdef and let ENGINE_load_dynamic() get
called. How did you test this, and with which version(s) of OpenSSL?

With the #ifdef:

ENGINE: Loading dynamic engine
ENGINE: Loading OpenSC Engine from /home/dwmw/git/openssl_tpm2_engine/.libs/libtpm2.so
ENGINE: Can't find engine dynamic [error:2606A074:engine routines:ENGINE_by_id:no such engine]
SSL: Failed to initialize TLS context.

Reverting it:

ENGINE: Loading dynamic engine
ENGINE: Loading OpenSC Engine from /home/dwmw/git/openssl_tpm2_engine/.libs/libtpm2.so
ENGINE: engine 'tpm2' is already available
ENGINE: Loading dynamic engine
ENGINE: Loading OpenSC Engine from /home/dwmw/git/openssl_tpm2_engine/.libs/libtpm2.so
ENGINE: engine 'tpm2' is already available
Rosen Penev March 14, 2019, 1:01 a.m. UTC | #5
On Wed, Mar 13, 2019 at 5:48 PM David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Wed, 2019-03-13 at 17:40 -0700, Rosen Penev wrote:
> > OPENSSL_init is implicit with OpenSSL 1.1. There's no need to call it
> > unless some non-default setting is needed (not loading error strings
> > for example).
>
> My empirical observation is that with OpenSSL 1.1.0g, engines aren't
> working unless I remove the #ifdef and let ENGINE_load_dynamic() get
> called. How did you test this, and with which version(s) of OpenSSL?
1.1.0 and 1.1.1 on OpenWrt. When deprecated APIs are disabled, this
will not compile.

From the OpenSSL source, ENGINE_load_dynamic() is only defined when
deprecated APIs are enabled. It defines to:
OPENSSL_init_crypto(OPENSSL_INIT_ENGINE_DYNAMIC, NULL)

looks like an #else section is needed.

As far as ERR_load_ENGINE_strings() is concerned, this is default in 1.1.
OPENSSL_INIT_NO_LOAD_CRYPTO_STRINGS must be passed to
OPENSSL_init_crypto to avoid it.
>
> With the #ifdef:
>
> ENGINE: Loading dynamic engine
> ENGINE: Loading OpenSC Engine from /home/dwmw/git/openssl_tpm2_engine/.libs/libtpm2.so
> ENGINE: Can't find engine dynamic [error:2606A074:engine routines:ENGINE_by_id:no such engine]
> SSL: Failed to initialize TLS context.
>
> Reverting it:
>
> ENGINE: Loading dynamic engine
> ENGINE: Loading OpenSC Engine from /home/dwmw/git/openssl_tpm2_engine/.libs/libtpm2.so
> ENGINE: engine 'tpm2' is already available
> ENGINE: Loading dynamic engine
> ENGINE: Loading OpenSC Engine from /home/dwmw/git/openssl_tpm2_engine/.libs/libtpm2.so
> ENGINE: engine 'tpm2' is already available
David Woodhouse March 14, 2019, 1:20 a.m. UTC | #6
On Wed, 2019-03-13 at 18:01 -0700, Rosen Penev wrote:
> > My empirical observation is that with OpenSSL 1.1.0g, engines aren't
> > working unless I remove the #ifdef and let ENGINE_load_dynamic() get
> > called. How did you test this, and with which version(s) of OpenSSL?
> 
> 1.1.0 and 1.1.1 on OpenWrt. When deprecated APIs are disabled, this
> will not compile.

But only compile-tested?

> From the OpenSSL source, ENGINE_load_dynamic() is only defined when
> deprecated APIs are enabled. It defines to:
> OPENSSL_init_crypto(OPENSSL_INIT_ENGINE_DYNAMIC, NULL)
> 
> looks like an #else section is needed.

Right.

> As far as ERR_load_ENGINE_strings() is concerned, this is default in 1.1.
> OPENSSL_INIT_NO_LOAD_CRYPTO_STRINGS must be passed to
> OPENSSL_init_crypto to avoid it.

Yeah, I think that part is fine. It's just that the dynamic engine
doesn't get auto-initialised.
Rosen Penev March 14, 2019, 1:26 a.m. UTC | #7
On Wed, Mar 13, 2019 at 6:20 PM David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Wed, 2019-03-13 at 18:01 -0700, Rosen Penev wrote:
> > > My empirical observation is that with OpenSSL 1.1.0g, engines aren't
> > > working unless I remove the #ifdef and let ENGINE_load_dynamic() get
> > > called. How did you test this, and with which version(s) of OpenSSL?
> >
> > 1.1.0 and 1.1.1 on OpenWrt. When deprecated APIs are disabled, this
> > will not compile.
>
> But only compile-tested?
OpenWrt does not enable ENGINE support by default. So it has been run
tested. Yet it has not.
>
> > From the OpenSSL source, ENGINE_load_dynamic() is only defined when
> > deprecated APIs are enabled. It defines to:
> > OPENSSL_init_crypto(OPENSSL_INIT_ENGINE_DYNAMIC, NULL)
> >
> > looks like an #else section is needed.
>
> Right.
>
> > As far as ERR_load_ENGINE_strings() is concerned, this is default in 1.1.
> > OPENSSL_INIT_NO_LOAD_CRYPTO_STRINGS must be passed to
> > OPENSSL_init_crypto to avoid it.
>
> Yeah, I think that part is fine. It's just that the dynamic engine
> doesn't get auto-initialised.
>
David Woodhouse March 14, 2019, 5:20 p.m. UTC | #8
On Wed, 2019-03-13 at 18:01 -0700, Rosen Penev wrote:
> As far as ERR_load_ENGINE_strings() is concerned, this is default in 1.1.
> OPENSSL_INIT_NO_LOAD_CRYPTO_STRINGS must be passed to
> OPENSSL_init_crypto to avoid it.

Even before 1.1, we're calling SSL_load_error_strings() from tls_init()
and that should suffice. We can drop the explicit
ERR_load_ENGINE_strings() and just call ENGINE_load_builtin_engines()
as you suggest.
diff mbox series

Patch

diff --git a/src/crypto/tls_openssl.c b/src/crypto/tls_openssl.c
index b4bfc9b73..79ac909d0 100644
--- a/src/crypto/tls_openssl.c
+++ b/src/crypto/tls_openssl.c
@@ -1024,8 +1024,10 @@  void * tls_init(const struct tls_config *conf)
 
 #ifndef OPENSSL_NO_ENGINE
 	wpa_printf(MSG_DEBUG, "ENGINE: Loading dynamic engine");
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
 	ERR_load_ENGINE_strings();
 	ENGINE_load_dynamic();
+#endif /* OPENSSL_VERSION_NUMBER */
 
 	if (conf &&
 	    (conf->opensc_engine_path || conf->pkcs11_engine_path ||
@@ -3874,7 +3876,7 @@  struct wpabuf * tls_connection_decrypt(void *tls_ctx,
 
 int tls_connection_resumed(void *ssl_ctx, struct tls_connection *conn)
 {
-	return conn ? SSL_cache_hit(conn->ssl) : 0;
+	return conn ? SSL_session_reused(conn->ssl) : 0;
 }