Permalink
Browse files

CDH-30961: Allow Thrift to accept TLSv1.1 and v1.2

Thrift 0.9.0 forces all connections to be TLSv1. However, TLSv1.1 and
v1.2 also exist and are better, and should be allowed.

By patching Thrift's TSSLSocket, we pass different options to
OpenSSL. The way this is done is counter-intuitive - we tell OpenSSL to
accept any protocol, and then blacklist SSLv2 and v3 (which are known to
be bad).

Testing:

Manual testing using openssl s_client -connect localhost:21000 -tls1_1
etc. (including checks that -sslv3 does *not* work).

Connectivity testing is already covered in thrift-server-test.cc,
although there's no attempt to programmatically identify the negotiated
protocol.

Change-Id: I2e98a1055fb7171a823a1cb87d1d67e2a7edb325
Reviewed-on: http://gerrit.cloudera.org:8080/731
Reviewed-by: Mike Yoder <myoder@cloudera.com>
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
  • Loading branch information...
henryr authored and Internal Jenkins committed Sep 1, 2015
1 parent 9880145 commit fc2c96feb49e10af0a9ae8098f0acea7b64b063e
@@ -0,0 +1,42 @@
+From c28bbabcee45c59b78e26f268ebb5132cabfb823 Mon Sep 17 00:00:00 2001
+From: Henry Robinson <henry@cloudera.com>
+Date: Tue, 1 Sep 2015 10:13:33 -0700
+Subject: [PATCH] CDH-30961: Allow Thrift to accept TLSv1.1 and v1.2
+
+Change-Id: I2e98a1055fb7171a823a1cb87d1d67e2a7edb325
+---
+ thirdparty/thrift-0.9.0/lib/cpp/src/thrift/transport/TSSLSocket.cpp | 5 +++--
+ 1 file changed, 3 insertions(+), 2 deletions(-)
+
+diff --git a/thirdparty/thrift-0.9.0/lib/cpp/src/thrift/transport/TSSLSocket.cpp b/thirdparty/thrift-0.9.0/lib/cpp/src/thrift/transport/TSSLSocket.cpp
+index 14c13dc..6234ad3 100644
+--- a/thirdparty/thrift-0.9.0/lib/cpp/src/thrift/transport/TSSLSocket.cpp
++++ b/thirdparty/thrift-0.9.0/lib/cpp/src/thrift/transport/TSSLSocket.cpp
+@@ -57,13 +57,14 @@ static char uppercase(char c);
+
+ // SSLContext implementation
+ SSLContext::SSLContext() {
+- ctx_ = SSL_CTX_new(TLSv1_method());
++ ctx_ = SSL_CTX_new(SSLv23_method());
+ if (ctx_ == NULL) {
+ string errors;
+ buildErrors(errors);
+ throw TSSLException("SSL_CTX_new: " + errors);
+ }
+ SSL_CTX_set_mode(ctx_, SSL_MODE_AUTO_RETRY);
++ SSL_CTX_set_options(ctx_, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
+ }
+
+ SSLContext::~SSLContext() {
+@@ -598,7 +599,7 @@ void buildErrors(string& errors, int errno_copy) {
+ * Default implementation of AccessManager
+ */
+ Decision DefaultClientAccessManager::verify(const sockaddr_storage& sa)
+- throw() {
++ throw() {
+ (void) sa;
+ return SKIP;
+ }
+--
+2.4.5
+
@@ -57,13 +57,14 @@ static char uppercase(char c);
// SSLContext implementation
SSLContext::SSLContext() {
- ctx_ = SSL_CTX_new(TLSv1_method());
+ ctx_ = SSL_CTX_new(SSLv23_method());
if (ctx_ == NULL) {
string errors;
buildErrors(errors);
throw TSSLException("SSL_CTX_new: " + errors);
}
SSL_CTX_set_mode(ctx_, SSL_MODE_AUTO_RETRY);
+ SSL_CTX_set_options(ctx_, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
}
SSLContext::~SSLContext() {
@@ -598,7 +599,7 @@ void buildErrors(string& errors, int errno_copy) {
* Default implementation of AccessManager
*/
Decision DefaultClientAccessManager::verify(const sockaddr_storage& sa)
- throw() {
+ throw() {
(void) sa;
return SKIP;
}

0 comments on commit fc2c96f

Please sign in to comment.