[kudu-CR] KUDU-3210 Add lock before verifying signature

["Attila Bukor (Code Review)" <ge...@cloudera.org>]

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke, Wenzhe Zhou, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16659

to look at the new patch set (#8).

Change subject: KUDU-3210 Add lock before verifying signature
......................................................................

KUDU-3210 Add lock before verifying signature

It seems there is a race condition somewhere in OpenSSL FIPS Object
Module, or at least in SafeLogic CryptoComply for Servers, as a
certificate can get corrupted when multiple certificates are being
verified in the same time, throwing an error during TLS handshake
similar to this:

error:0407006A:rsa routines:RSA_padding_check_PKCS1_type_1:block type is not 01:rsa_pk1.c:102 error:04067072:rsa routines:RSA_EAY_PUBLIC_DECRYPT:padding check failed:rsa_eay.c:786 error:1408D07B:SSL routines:ssl3_get_key_exchange:bad signature:s3_clnt.c:2038

This means OpenSSL is trying to verify a signature with a public key
that doesn't match the private key, resulting in an invalid message.
According to a mailing list thread[1] this can happen in multi-threaded
applications, such as ours, even though we seem to have proper locking
in our verify signature function and the OpenSSL locking callbacks are
properly registered too.

Unfortunately, OpenSSL 1.0.2 doesn't guarantee complete thread
safety[2]:

"OpenSSL can generally be used safely in multi-threaded applications
provided that at least two callback functions are set, the
locking_function and threadid_func. Note that OpenSSL is not completely
thread-safe, and unfortunately not all global resources have the
necessary locks. Further, the thread-safety does not extend to things
like multiple threads using the same SSL object at the same time."

In addition to the TLS negotiation this can also get triggered when
verifying authn token signatures, further suggesting a race condition in
the underlying OpenSSL library. This commits adds additional locking to
crypto and TLS context/handshake which seems to prevent this from
happening in both places.

[1] https://groups.google.com/u/1/g/mailing.openssl.users/c/u0JyAuogrc0
[2] https://www.openssl.org/docs/man1.0.2/man3/threads.html

Change-Id: Ifafc7dcf91db910123276b657515e410bb7f6fcd
---
M src/kudu/security/crypto.cc
M src/kudu/security/tls_context.cc
M src/kudu/security/tls_handshake.cc
3 files changed, 35 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/16659/8
-- 
To view, visit http://gerrit.cloudera.org:8080/16659
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifafc7dcf91db910123276b657515e410bb7f6fcd
Gerrit-Change-Number: 16659
Gerrit-PatchSet: 8
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>