Henry Robinson has submitted this change and it was merged. Change subject: IMPALA-5849: Remove compile-time checks for OpenSSL > 1.0.0 ......................................................................
IMPALA-5849: Remove compile-time checks for OpenSSL > 1.0.0 (Prerequisite knowledge: Impala must support building and linking against OpenSSL 1.0.0 and 1.0.1. 1.0.0 only supports TLS1.0. 1.0.1 supports TLS1.1 and 1.2 as well). Rather than check for OpenSSL w/TLSv1.1+ support at compile time, we altered Thrift-0.9.0-p11 to compile on all versions, and fail with an error if an unsupported version is requested at runtime. This patch is the Impala-side counterpart to that work, and also works around a bug. The bug is in OpenSSL. When disabling, say, TLS1.1 we pass an option bitmask to SSL_CTX_set_options() that includes SSL_OP_NO_TLSv1_1. In OpenSSL 1.0.1 this is defined as 0x10000000L but in 1.0.0 it is not defined. So to support compilation on all OpenSSL versions, we added definitions of that constant to Thrift if it was not found at compile time. However, in OpenSSL 1.0.0 *another* option for SSL_CTX_set_options() has the *same* value (0x10000000L). This means that passing SSL_OP_NO_TLSv1_1 to OpenSSL 1.0.0 actually does something completely different. This lack of backwards compatibility is the bug. To work around it, we use the SSLeay() function, which, although cryptically named, returns the value of OPENSSL_VERSION_NUMBER in the version of OpenSSL currently linked against (I have tested that it behaves differently when dynamically linked against different versions of OpenSSL, but compiled against the same one). We use that method to tell us which of the TLS protocol versions are actually supported, and raise errors if they are not. An alternative approach to doing this inside ThriftClient and ThriftServer would be to do so in Thrift; this might be a reasonable future option but for now it is too unwieldy to test a toolchain build that is linked against different OpenSSL versions. To reduce risk, and the number of ways things can go wrong, we do all the protocol version whitelisting in the Impala code. To simplify the code further, we also remove the values of SSLProtoVersions that allowed us to restrict the TLS protocol to exactly *one* value, rather than specify the minimum (e.g. we remove TLSv1_1 but retain TLSv1_1_plus). The more restrictive options were not intended for production use, and using them now will raise an error. Error messages -------------- Passing a valid, but unsupported version to --ssl_minimum_version: "TLS (6) version not supported (linked OpenSSL version is 1234567)" Passing an invalid version to --ssl_minimum_version: "Unknown TLS version: 'tls_4_2'" Testing ------- We assume that tests will be run on the same machine that built Impala, so still execute slightly different tests depending on the detected OpenSSL version. For thrift-server-test, we check to see if a protocol version is supported at runtime, and use that to determine the expected behaviour of a test which starts all combinations of clients and server versions. For the webserver-test, we determine at compile-time the set of supported TLS protocols, and change the test's expected behaviour based on that. Tests have been run when compiling and running against OpenSSL 1.0.0 and OpenSSL 1.0.1. Also include corresponding changes to Squeasel from this commit: https://github.com/henryr/squeasel/commit/eded53 Testing: New webserver and thrift-server tests. Change-Id: I20c5d39c0c4ae9c5445dd3ee3b175fe337a5728d Reviewed-on: http://gerrit.cloudera.org:8080/7866 Tested-by: Impala Public Jenkins Reviewed-by: Henry Robinson <he...@cloudera.com> --- M be/src/rpc/thrift-client.cc M be/src/rpc/thrift-server-test.cc M be/src/rpc/thrift-server.cc M be/src/rpc/thrift-server.h M be/src/service/impala-server.cc M be/src/thirdparty/squeasel/squeasel.c M be/src/util/webserver-test.cc M bin/impala-config.sh 8 files changed, 77 insertions(+), 35 deletions(-) Approvals: Impala Public Jenkins: Verified Henry Robinson: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/7866 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I20c5d39c0c4ae9c5445dd3ee3b175fe337a5728d Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com>
