You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by sa...@apache.org on 2018/01/23 17:49:15 UTC

impala git commit: IMPALA-6418: Find a reliable way to detect supported TLS versions

Repository: impala
Updated Branches:
  refs/heads/master 56464e461 -> a3c0fffa1


IMPALA-6418: Find a reliable way to detect supported TLS versions

Due to a RHEL bug, finding the version of the dynamically linked
OpenSSL is unreliable on certain RHEL platforms. This means that
if Impala is built against OpenSSL 1.0.0 and run against OpenSSL
1.0.1, due to the bug, Impala will not be able to access the
capabilities of OpenSSL 1.0.1. The most significant drawback of
this is that in this scenario, Impala cannot use TLSv1.2, even though
it is available.

The RHEL bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1497859

This patch imitates Kudu's way of determining what TLS versions are
supported via this patch:
https://github.com/apache/kudu/commit/b88117415a02699c12a6eacbf065c4140ee0963c

This is tested and works on all supported platforms of Impala.

Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
Reviewed-on: http://gerrit.cloudera.org:8080/9060
Reviewed-by: Sailesh Mukil <sa...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/a3c0fffa
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/a3c0fffa
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/a3c0fffa

Branch: refs/heads/master
Commit: a3c0fffa1275c1d1d2770b9fc35885a22dc7edce
Parents: 56464e4
Author: Sailesh Mukil <sa...@cloudera.com>
Authored: Thu Jan 18 11:00:32 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Jan 23 01:59:41 2018 +0000

----------------------------------------------------------------------
 be/src/rpc/thrift-client.cc |  5 +++--
 be/src/rpc/thrift-server.cc | 25 ++++++++++++++++---------
 be/src/util/openssl-util.cc |  4 ++++
 be/src/util/openssl-util.h  | 25 +++++++++++++++++++++++--
 4 files changed, 46 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/a3c0fffa/be/src/rpc/thrift-client.cc
----------------------------------------------------------------------
diff --git a/be/src/rpc/thrift-client.cc b/be/src/rpc/thrift-client.cc
index 085e92e..1f8d99e 100644
--- a/be/src/rpc/thrift-client.cc
+++ b/be/src/rpc/thrift-client.cc
@@ -24,6 +24,7 @@
 #include <gutil/strings/substitute.h>
 
 #include "util/network-util.h"
+#include "util/openssl-util.h"
 #include "util/time.h"
 
 #include "common/names.h"
@@ -46,8 +47,8 @@ ThriftClientImpl::ThriftClientImpl(const std::string& ipaddress, int port, bool
         SSLProtoVersions::StringToProtocol(FLAGS_ssl_minimum_version, &version);
     if (init_status_.ok() && !SSLProtoVersions::IsSupported(version)) {
       string err =
-          Substitute("TLS ($0) version not supported (linked OpenSSL version is $1)",
-              version, SSLeay());
+          Substitute("TLS ($0) version not supported (maximum supported version is $1)",
+              version, MaxSupportedTlsVersion());
       init_status_ = Status(err);
     }
     if (!init_status_.ok()) return;

http://git-wip-us.apache.org/repos/asf/impala/blob/a3c0fffa/be/src/rpc/thrift-server.cc
----------------------------------------------------------------------
diff --git a/be/src/rpc/thrift-server.cc b/be/src/rpc/thrift-server.cc
index eaca699..75ad424 100644
--- a/be/src/rpc/thrift-server.cc
+++ b/be/src/rpc/thrift-server.cc
@@ -40,6 +40,7 @@
 #include "util/debug-util.h"
 #include "util/metrics.h"
 #include "util/network-util.h"
+#include "util/openssl-util.h"
 #include "util/os-util.h"
 #include "util/uid-util.h"
 
@@ -86,14 +87,20 @@ Status SSLProtoVersions::StringToProtocol(const string& in, SSLProtocol* protoco
   return Status(Substitute("Unknown TLS version: '$0'", in));
 }
 
-#define OPENSSL_MIN_VERSION_WITH_TLS_1_1 0x10001000L
-
 bool SSLProtoVersions::IsSupported(const SSLProtocol& protocol) {
-  bool is_openssl_1_0_0_or_lower = (SSLeay() < OPENSSL_MIN_VERSION_WITH_TLS_1_1);
-  if (is_openssl_1_0_0_or_lower) return (protocol == TLSv1_0_plus);
-
-  // All other versions supported by OpenSSL 1.0.1 and later.
-  return true;
+  DCHECK_LE(protocol, TLSv1_2_plus);
+  int max_supported_tls_version = MaxSupportedTlsVersion();
+  DCHECK_GE(max_supported_tls_version, TLS1_VERSION);
+
+  switch (max_supported_tls_version) {
+    case TLS1_VERSION:
+      return protocol == TLSv1_0_plus || protocol == TLSv1_0;
+    case TLS1_1_VERSION:
+      return protocol != TLSv1_2_plus && protocol != TLSv1_2;
+    default:
+      DCHECK_GE(max_supported_tls_version, TLS1_2_VERSION);
+      return true;
+  }
 }
 
 bool EnableInternalSslConnections() {
@@ -376,8 +383,8 @@ Status ThriftServer::CreateSocket(boost::shared_ptr<TServerTransport>* socket) {
   if (ssl_enabled()) {
     if (!SSLProtoVersions::IsSupported(version_)) {
       return Status(TErrorCode::SSL_SOCKET_CREATION_FAILED,
-          Substitute("TLS ($0) version not supported (linked OpenSSL version is $1)",
-                        version_, SSLeay()));
+          Substitute("TLS ($0) version not supported (maximum supported version is $1)",
+                        version_, MaxSupportedTlsVersion()));
     }
     try {
       // This 'factory' is only called once, since CreateSocket() is only called from

http://git-wip-us.apache.org/repos/asf/impala/blob/a3c0fffa/be/src/util/openssl-util.cc
----------------------------------------------------------------------
diff --git a/be/src/util/openssl-util.cc b/be/src/util/openssl-util.cc
index a8ec976..264a49a 100644
--- a/be/src/util/openssl-util.cc
+++ b/be/src/util/openssl-util.cc
@@ -43,6 +43,10 @@ static const int RNG_RESEED_INTERVAL = 128;
 // Number of bytes of entropy to add at RNG_RESEED_INTERVAL.
 static const int RNG_RESEED_BYTES = 512;
 
+int MaxSupportedTlsVersion() {
+  return SSLv23_method()->version;
+}
+
 // Callback used by OpenSSLErr() - write the error given to us through buf to the
 // stringstream that's passed in through ctx.
 static int OpenSSLErrCallback(const char* buf, size_t len, void* ctx) {

http://git-wip-us.apache.org/repos/asf/impala/blob/a3c0fffa/be/src/util/openssl-util.h
----------------------------------------------------------------------
diff --git a/be/src/util/openssl-util.h b/be/src/util/openssl-util.h
index 22f8235..67d014d 100644
--- a/be/src/util/openssl-util.h
+++ b/be/src/util/openssl-util.h
@@ -21,12 +21,31 @@
 #include <openssl/aes.h>
 #include <openssl/evp.h>
 #include <openssl/sha.h>
+#include <openssl/ssl.h>
 
 #include "common/status.h"
 
 namespace impala {
 
-#define OPENSSL_VERSION_1_0_1 0x1000100L
+// From https://github.com/apache/kudu/commit/b88117415a02699c12a6eacbf065c4140ee0963c
+//
+// Hard code OpenSSL flag values from OpenSSL 1.0.1e[1][2] when compiling
+// against OpenSSL 1.0.0 and below. We detect when running against a too-old
+// version of OpenSSL using these definitions at runtime so that Kudu has full
+// functionality when run against a new OpenSSL version, even if it's compiled
+// against an older version.
+//
+// [1]: https://github.com/openssl/openssl/blob/OpenSSL_1_0_1e/ssl/ssl.h#L605-L609
+// [2]: https://github.com/openssl/openssl/blob/OpenSSL_1_0_1e/ssl/tls1.h#L166-L172
+#ifndef TLS1_1_VERSION
+#define TLS1_1_VERSION 0x0302
+#endif
+#ifndef TLS1_2_VERSION
+#define TLS1_2_VERSION 0x0303
+#endif
+
+/// Returns the maximum supported TLS version available in the linked OpenSSL library.
+int MaxSupportedTlsVersion();
 
 /// Add entropy from the system RNG to OpenSSL's global RNG. Called at system startup
 /// and again periodically to add new entropy.
@@ -67,7 +86,9 @@ class IntegrityHash {
 class EncryptionKey {
  public:
   EncryptionKey() : initialized_(false) {
-    mode_ = SSLeay() < OPENSSL_VERSION_1_0_1 ? AES_256_CFB : AES_256_CTR;
+    // If TLS1.2 is supported, then we're on a verison of OpenSSL that supports
+    // AES-256-CTR.
+    mode_ = MaxSupportedTlsVersion() < TLS1_2_VERSION ? AES_256_CFB : AES_256_CTR;
   }
 
   /// Initialize a key for temporary use with randomly generated data. Reinitializes with