spacemonkeygo / openssl

OpenSSL bindings for Go
http://godoc.org/github.com/spacemonkeygo/openssl
Apache License 2.0
473 stars 236 forks source link

Set thread id callback as required by openssl. #97

Closed ajaynalawade closed 5 years ago

ajaynalawade commented 5 years ago

For openssl to work in multi threaded environment, 2 callbacks needs to be set as per following blog. https://www.openssl.org/blog/blog/2017/02/21/threads/

Spacemonkeygo openssl binding is setting CRYPTO_set_locking_callback but not setting CRYPTO_set_id_callback. I have added required changes for setting CRYPTO_set_id_callback for non windows environment.

zeebo commented 5 years ago

The documentation claims:

If the application does not register such a callback using CRYPTO_THREADID_set_callback(), then a default implementation is used - on Windows and BeOS this uses the system's default thread identifying APIs, and on all other platforms it uses the address of errno. The latter is satisfactory for thread-safety if and only if the platform has a thread-local error number facility.

Is there a system you're running on that doesn't have a thread-local errno?

edit: oh, that's a different function. still trying to find docs on CRYPTO_set_id_callback lol. edit edit: ok CRYPTO_THREADID_set_callback deprecates CRYPTO_set_id_callback. Versions of openssl back to 2010 appear to use an appropriate default.

ajaynalawade commented 5 years ago

I am using Ubuntu 14.04.5 LTS. As mentioned in blog link, these 2 call backs need to be set for openssl to work with multi-threading. Also please refer to following issue in openssl https://github.com/openssl/openssl/issues/6700

zeebo commented 5 years ago

Ok. It looks like this is some funny interaction with FIPS mode. Maybe FIPS uses some legacy code paths that require the callback to be set, or something. We've been running this in prod for years on multiple threads without the callback but we never used FIPS.

Thanks for the fix.

ajaynalawade commented 5 years ago

@zeebo, Do we need any handling for windows platform.

zeebo commented 5 years ago

No, I implemented that in https://github.com/spacemonkeygo/openssl/commit/3c881b520f0ecf5a8bbf084dcd84a8a5824dfec1.