THRIFT-4084: Add a SSL/TLS negotiation check to crossfeature to verify SSLv3 is not active and that at least one of TLSv1.0 through 1.2 are accepted.
Client: csharp, d, go, nodejs, perl
This closes #1197
diff --git a/compiler/cpp/src/thrift/generate/t_cpp_generator.cc b/compiler/cpp/src/thrift/generate/t_cpp_generator.cc
index 869d802..c7eec48 100644
--- a/compiler/cpp/src/thrift/generate/t_cpp_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_cpp_generator.cc
@@ -4452,4 +4452,4 @@
" include_prefix: Use full include paths in generated files.\n"
" moveable_types: Generate move constructors and assignment operators.\n"
" no_ostream_operators:\n"
- " Omit generation of ostream definitions.\n");
+ " Omit generation of ostream definitions.\n")
diff --git a/configure.ac b/configure.ac
index a889f75..bb25495 100755
--- a/configure.ac
+++ b/configure.ac
@@ -911,8 +911,8 @@
echo "C++ Library:"
echo " Build TZlibTransport ...... : $have_zlib"
echo " Build TNonblockingServer .. : $have_libevent"
- echo " Build TQTcpServer (Qt4) .... : $have_qt"
- echo " Build TQTcpServer (Qt5) .... : $have_qt5"
+ echo " Build TQTcpServer (Qt4) ... : $have_qt"
+ echo " Build TQTcpServer (Qt5) ... : $have_qt5"
fi
if test "$have_java" = "yes" ; then
echo
@@ -1004,7 +1004,7 @@
if test "$have_lua" = "yes" ; then
echo
echo "Lua Library:"
- echo " Using Lua .............. : $LUA"
+ echo " Using Lua ................. : $LUA"
fi
if test "$have_rs" = "yes" ; then
echo
diff --git a/lib/csharp/src/Transport/TTLSServerSocket.cs b/lib/csharp/src/Transport/TTLSServerSocket.cs
index 86a4494..d51c217 100644
--- a/lib/csharp/src/Transport/TTLSServerSocket.cs
+++ b/lib/csharp/src/Transport/TTLSServerSocket.cs
@@ -108,7 +108,7 @@
X509Certificate2 certificate,
RemoteCertificateValidationCallback clientCertValidator = null,
LocalCertificateSelectionCallback localCertificateSelectionCallback = null,
- // TODO: Enable Tls1 and Tls2 (TLS 1.1 and 1.2) by default once we start using .NET 4.5+.
+ // TODO: Enable Tls11 and Tls12 (TLS 1.1 and 1.2) by default once we start using .NET 4.5+.
SslProtocols sslProtocols = SslProtocols.Tls)
{
if (!certificate.HasPrivateKey)
@@ -126,7 +126,7 @@
try
{
// Create server socket
- this.server = TSocketVersionizer.CreateTcpListener(port);
+ this.server = TSocketVersionizer.CreateTcpListener(port);
this.server.Server.NoDelay = true;
}
catch (Exception)
diff --git a/lib/d/src/thrift/transport/ssl.d b/lib/d/src/thrift/transport/ssl.d
index a78a2ed..fbcb6ee 100644
--- a/lib/d/src/thrift/transport/ssl.d
+++ b/lib/d/src/thrift/transport/ssl.d
@@ -249,7 +249,9 @@
}
count_++;
- ctx_ = SSL_CTX_new(TLSv1_method());
+ ctx_ = SSL_CTX_new(SSLv23_method());
+ SSL_CTX_set_options(ctx_, SSL_OP_NO_SSLv2);
+ SSL_CTX_set_options(ctx_, SSL_OP_NO_SSLv3); // THRIFT-3164
enforce(ctx_, getSSLException("SSL_CTX_new"));
SSL_CTX_set_mode(ctx_, SSL_MODE_AUTO_RETRY);
}
diff --git a/lib/go/thrift/ssl_server_socket.go b/lib/go/thrift/ssl_server_socket.go
index 58f859b..907afca 100644
--- a/lib/go/thrift/ssl_server_socket.go
+++ b/lib/go/thrift/ssl_server_socket.go
@@ -20,9 +20,9 @@
package thrift
import (
+ "crypto/tls"
"net"
"time"
- "crypto/tls"
)
type TSSLServerSocket struct {
@@ -38,6 +38,9 @@
}
func NewTSSLServerSocketTimeout(listenAddr string, cfg *tls.Config, clientTimeout time.Duration) (*TSSLServerSocket, error) {
+ if cfg.MinVersion == 0 {
+ cfg.MinVersion = tls.VersionTLS10
+ }
addr, err := net.ResolveTCPAddr("tcp", listenAddr)
if err != nil {
return nil, err
diff --git a/lib/go/thrift/ssl_socket.go b/lib/go/thrift/ssl_socket.go
index 04d3850..2395986 100644
--- a/lib/go/thrift/ssl_socket.go
+++ b/lib/go/thrift/ssl_socket.go
@@ -48,6 +48,9 @@
// NewTSSLSocketTimeout creates a net.Conn-backed TTransport, given a host and port
// it also accepts a tls Configuration and a timeout as a time.Duration
func NewTSSLSocketTimeout(hostPort string, cfg *tls.Config, timeout time.Duration) (*TSSLSocket, error) {
+ if cfg.MinVersion == 0 {
+ cfg.MinVersion = tls.VersionTLS10
+ }
return &TSSLSocket{hostPort: hostPort, timeout: timeout, cfg: cfg}, nil
}
diff --git a/lib/nodejs/lib/thrift/connection.js b/lib/nodejs/lib/thrift/connection.js
index 575d516..608e552 100644
--- a/lib/nodejs/lib/thrift/connection.js
+++ b/lib/nodejs/lib/thrift/connection.js
@@ -18,6 +18,7 @@
*/
var util = require('util');
var EventEmitter = require("events").EventEmitter;
+var constants = require('constants');
var net = require('net');
var tls = require('tls');
var thrift = require('./thrift');
@@ -221,7 +222,7 @@
this.retry_timer = setTimeout(function () {
if (self._debug) {
console.log("Retrying connection...");
- }
+ }
self.retry_totaltime += self.retry_delay;
@@ -247,6 +248,11 @@
};
exports.createSSLConnection = function(host, port, options) {
+ if (!('secureProtocol' in options) && !('secureOptions' in options)) {
+ options.secureProtocol = "SSLv23_method";
+ options.secureOptions = constants.SSL_OP_NO_SSLv2 | constants.SSL_OP_NO_SSLv3;
+ }
+
var stream = tls.connect(port, host, options);
var connection = new Connection(stream, options);
connection.host = host;
diff --git a/lib/nodejs/lib/thrift/server.js b/lib/nodejs/lib/thrift/server.js
index 921bb86..e124acc 100644
--- a/lib/nodejs/lib/thrift/server.js
+++ b/lib/nodejs/lib/thrift/server.js
@@ -16,6 +16,8 @@
* specific language governing permissions and limitations
* under the License.
*/
+
+var constants = require('constants');
var net = require('net');
var tls = require('tls');
@@ -87,6 +89,10 @@
}
if (options && options.tls) {
+ if (!('secureProtocol' in options.tls) && !('secureOptions' in options.tls)) {
+ options.tls.secureProtocol = "SSLv23_method";
+ options.tls.secureOptions = constants.SSL_OP_NO_SSLv2 | constants.SSL_OP_NO_SSLv3;
+ }
return tls.createServer(options.tls, serverImpl);
} else {
return net.createServer(serverImpl);
diff --git a/lib/perl/lib/Thrift/SSLServerSocket.pm b/lib/perl/lib/Thrift/SSLServerSocket.pm
index e885ede..a8dfa56 100644
--- a/lib/perl/lib/Thrift/SSLServerSocket.pm
+++ b/lib/perl/lib/Thrift/SSLServerSocket.pm
@@ -60,13 +60,15 @@
Proto => 'tcp',
ReuseAddr => 1};
+ my $verify = IO::Socket::SSL::SSL_VERIFY_PEER | IO::Socket::SSL::SSL_VERIFY_FAIL_IF_NO_PEER_CERT | IO::Socket::SSL::SSL_VERIFY_CLIENT_ONCE;
+
$opts->{SSL_ca_file} = $self->{ca} if defined $self->{ca};
$opts->{SSL_cert_file} = $self->{cert} if defined $self->{cert};
$opts->{SSL_cipher_list} = $self->{ciphers} if defined $self->{ciphers};
$opts->{SSL_key_file} = $self->{key} if defined $self->{key};
$opts->{SSL_use_cert} = (defined $self->{cert}) ? 1 : 0;
- $opts->{SSL_verify_mode} = (defined $self->{ca}) ? IO::Socket::SSL::SSL_VERIFY_PEER : IO::Socket::SSL::SSL_VERIFY_NONE;
- $opts->{SSL_version} = (defined $self->{version}) ? $self->{version} : 'SSLv23:!SSLv2:!SSLv3';
+ $opts->{SSL_verify_mode} = (defined $self->{ca}) ? $verify : IO::Socket::SSL::SSL_VERIFY_NONE;
+ $opts->{SSL_version} = (defined $self->{version}) ? $self->{version} : 'SSLv23:!SSLv3:!SSLv2';
return IO::Socket::SSL->new(%$opts);
}
diff --git a/lib/perl/lib/Thrift/SSLSocket.pm b/lib/perl/lib/Thrift/SSLSocket.pm
index 046692e..99a4107 100644
--- a/lib/perl/lib/Thrift/SSLSocket.pm
+++ b/lib/perl/lib/Thrift/SSLSocket.pm
@@ -71,13 +71,15 @@
Proto => 'tcp',
Timeout => $self->{sendTimeout} / 1000};
+ my $verify = IO::Socket::SSL::SSL_VERIFY_PEER | IO::Socket::SSL::SSL_VERIFY_FAIL_IF_NO_PEER_CERT | IO::Socket::SSL::SSL_VERIFY_CLIENT_ONCE;
+
$opts->{SSL_ca_file} = $self->{ca} if defined $self->{ca};
$opts->{SSL_cert_file} = $self->{cert} if defined $self->{cert};
$opts->{SSL_cipher_list} = $self->{ciphers} if defined $self->{ciphers};
$opts->{SSL_key_file} = $self->{key} if defined $self->{key};
$opts->{SSL_use_cert} = (defined $self->{cert}) ? 1 : 0;
- $opts->{SSL_verify_mode} = (defined $self->{ca}) ? IO::Socket::SSL::SSL_VERIFY_PEER : IO::Socket::SSL::SSL_VERIFY_NONE;
- $opts->{SSL_version} = (defined $self->{version}) ? $self->{version} : 'SSLv23:!SSLv2:!SSLv3';
+ $opts->{SSL_verify_mode} = (defined $self->{ca}) ? $verify : IO::Socket::SSL::SSL_VERIFY_NONE;
+ $opts->{SSL_version} = (defined $self->{version}) ? $self->{version} : 'SSLv23:!SSLv3:!SSLv2';
return IO::Socket::SSL->new(%$opts);
}
diff --git a/test/cpp/src/TestClient.cpp b/test/cpp/src/TestClient.cpp
index da20b89..a918bfb 100644
--- a/test/cpp/src/TestClient.cpp
+++ b/test/cpp/src/TestClient.cpp
@@ -136,8 +136,11 @@
int ERR_EXCEPTIONS = 8;
int ERR_UNKNOWN = 64;
- string testDir = boost::filesystem::system_complete(argv[0]).parent_path().parent_path().parent_path().string();
- string pemPath = testDir + "/keys/CA.pem";
+ string testDir = boost::filesystem::system_complete(argv[0]).parent_path().parent_path().parent_path().string();
+ string caPath = testDir + "/keys/CA.pem";
+ string certPath = testDir + "/keys/client.crt";
+ string keyPath = testDir + "/keys/client.key";
+
#if _WIN32
transport::TWinsockSingleton::create();
#endif
@@ -232,9 +235,15 @@
boost::shared_ptr<TSSLSocketFactory> factory;
if (ssl) {
+ cout << "Client Certificate File: " << certPath << endl;
+ cout << "Client Key File: " << keyPath << endl;
+ cout << "CA File: " << caPath << endl;
+
factory = boost::shared_ptr<TSSLSocketFactory>(new TSSLSocketFactory());
factory->ciphers("ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH");
- factory->loadTrustedCertificates(pemPath.c_str());
+ factory->loadTrustedCertificates(caPath.c_str());
+ factory->loadCertificate(certPath.c_str());
+ factory->loadPrivateKey(keyPath.c_str());
factory->authenticate(true);
socket = factory->createSocket(host, port);
} else {
diff --git a/test/features/Makefile.am b/test/features/Makefile.am
index f8d6538..337d789 100644
--- a/test/features/Makefile.am
+++ b/test/features/Makefile.am
@@ -21,8 +21,10 @@
index.html \
known_failures_Linux.json \
Makefile.am \
+ nosslv3.sh \
string_limit.py \
tests.json \
theader_binary.py \
setup.cfg \
+ tls.sh \
util.py
diff --git a/test/features/nosslv3.sh b/test/features/nosslv3.sh
new file mode 100755
index 0000000..e550d51
--- /dev/null
+++ b/test/features/nosslv3.sh
@@ -0,0 +1,52 @@
+#!/bin/bash
+
+#
+# Checks to make sure SSLv3 is not allowed by a server.
+#
+
+THRIFTHOST=localhost
+THRIFTPORT=9090
+
+while [[ $# -ge 1 ]]; do
+ arg="$1"
+ argIN=(${arg//=/ })
+
+ case ${argIN[0]} in
+ -h|--host)
+ THRIFTHOST=${argIN[1]}
+ shift # past argument
+ ;;
+ -p|--port)
+ THRIFTPORT=${argIN[1]}
+ shift # past argument
+ ;;
+ *)
+ # unknown option ignored
+ ;;
+ esac
+
+ shift # past argument or value
+done
+
+function nosslv3
+{
+ local nego
+ local negodenied
+
+ # echo "openssl s_client -connect $THRIFTHOST:$THRIFTPORT -CAfile ../keys/CA.pem -ssl3 2>&1 < /dev/null"
+ nego=$(openssl s_client -connect $THRIFTHOST:$THRIFTPORT -CAfile ../keys/CA.pem -ssl3 2>&1 < /dev/null)
+ negodenied=$?
+
+ if [[ $negodenied -ne 0 ]]; then
+ echo "[pass] SSLv3 negotiation disabled"
+ echo $nego
+ return 0
+ fi
+
+ echo "[fail] SSLv3 negotiation enabled! stdout:"
+ echo $nego
+ return 1
+}
+
+nosslv3
+exit $?
diff --git a/test/features/tests.json b/test/features/tests.json
index cfcb4b6..3ab3b68 100644
--- a/test/features/tests.json
+++ b/test/features/tests.json
@@ -90,5 +90,27 @@
"transports": ["buffered"],
"sockets": ["ip"],
"workdir": "features"
+ },
+ {
+ "name": "nosslv3",
+ "comment": "check to make sure SSLv3 is not supported",
+ "command": [
+ "nosslv3.sh"
+ ],
+ "protocols": ["binary"],
+ "transports": ["buffered"],
+ "sockets": ["ip-ssl"],
+ "workdir": "features"
+ },
+ {
+ "name": "tls",
+ "comment": "check to make sure TLSv1.0 or later is supported",
+ "command": [
+ "tls.sh"
+ ],
+ "protocols": ["binary"],
+ "transports": ["buffered"],
+ "sockets": ["ip-ssl"],
+ "workdir": "features"
}
]
diff --git a/test/features/tls.sh b/test/features/tls.sh
new file mode 100755
index 0000000..dada6ab
--- /dev/null
+++ b/test/features/tls.sh
@@ -0,0 +1,71 @@
+#!/bin/bash
+
+#
+# Checks to make sure TLSv1.0 or later is allowed by a server.
+#
+
+THRIFTHOST=localhost
+THRIFTPORT=9090
+
+while [[ $# -ge 1 ]]; do
+ arg="$1"
+ argIN=(${arg//=/ })
+
+ case ${argIN[0]} in
+ -h|--host)
+ THRIFTHOST=${argIN[1]}
+ shift # past argument
+ ;;
+ -p|--port)
+ THRIFTPORT=${argIN[1]}
+ shift # past argument
+ ;;
+ *)
+ # unknown option ignored
+ ;;
+ esac
+
+ shift # past argument or value
+done
+
+declare -A EXPECT_NEGOTIATE
+EXPECT_NEGOTIATE[tls1]=1
+EXPECT_NEGOTIATE[tls1_1]=1
+EXPECT_NEGOTIATE[tls1_2]=1
+
+failures=0
+
+function tls
+{
+ for PROTO in "${!EXPECT_NEGOTIATE[@]}"; do
+
+ local nego
+ local negodenied
+ local res
+
+ echo "openssl s_client -connect $THRIFTHOST:$THRIFTPORT -CAfile ../keys/CA.pem -$PROTO 2>&1 < /dev/null"
+ nego=$(openssl s_client -connect $THRIFTHOST:$THRIFTPORT -CAfile ../keys/CA.pem -$PROTO 2>&1 < /dev/null)
+ negodenied=$?
+ echo "result of command: $negodenied"
+
+ res="enabled"; if [[ ${EXPECT_NEGOTIATE[$PROTO]} -eq 0 ]]; then res="disabled"; fi
+
+ if [[ $negodenied -ne ${EXPECT_NEGOTIATE[$PROTO]} ]]; then
+ echo "$PROTO negotiation allowed"
+ else
+ echo "[warn] $PROTO negotiation did not work"
+ echo $nego
+ ((failures++))
+ fi
+ done
+}
+
+tls
+
+if [[ $failures -eq 3 ]]; then
+ echo "[fail] At least one of TLSv1.0, TLSv1.1, or TLSv1.2 needs to work, but does not"
+ exit $failures
+fi
+
+echo "[pass] At least one of TLSv1.0, TLSv1.1, or TLSv1.2 worked"
+exit 0
diff --git a/test/known_failures_Linux.json b/test/known_failures_Linux.json
index d60997d..5ca6d4e 100644
--- a/test/known_failures_Linux.json
+++ b/test/known_failures_Linux.json
@@ -47,6 +47,9 @@
"csharp-d_binary_buffered-ip-ssl",
"csharp-d_compact_buffered-ip-ssl",
"csharp-d_json_buffered-ip-ssl",
+ "csharp-d_binary_framed-ip-ssl",
+ "csharp-d_compact_framed-ip-ssl",
+ "csharp-d_json_framed-ip-ssl",
"csharp-erl_binary_buffered-ip-ssl",
"csharp-erl_binary_framed-ip-ssl",
"csharp-erl_compact_buffered-ip-ssl",
@@ -105,6 +108,7 @@
"d-cpp_json_http-ip-ssl",
"d-d_binary_http-ip",
"d-d_compact_http-ip",
+ "d-d_json_http-ip",
"d-dart_binary_framed-ip",
"d-dart_binary_http-ip",
"d-dart_compact_framed-ip",
@@ -220,9 +224,5 @@
"hs-py3_json_framed-ip",
"java-d_compact_buffered-ip",
"java-d_compact_buffered-ip-ssl",
- "java-d_compact_framed-ip",
- "perl-erl_binary_buffered-ip-ssl",
- "perl-erl_binary_framed-ip-ssl",
- "perl-perl_binary_buffered-ip-ssl",
- "perl-perl_binary_framed-ip-ssl"
+ "java-d_compact_framed-ip"
]
diff --git a/test/tests.json b/test/tests.json
index e228928..f1d6a47 100644
--- a/test/tests.json
+++ b/test/tests.json
@@ -121,8 +121,8 @@
"framed:fastframed"
],
"sockets": [
- "ip-ssl",
- "ip"
+ "ip",
+ "ip-ssl"
],
"protocols": [
"compact",
@@ -415,7 +415,7 @@
"-I../../lib/perl/lib/",
"TestClient.pl",
"--ca=../keys/CA.pem",
- "--cert=../keys/client.pem",
+ "--cert=../keys/client.crt",
"--key=../keys/client.key"
]
},
@@ -425,8 +425,7 @@
"-Igen-perl/",
"-I../../lib/perl/lib/",
"TestServer.pl",
- "--ca=../keys/CA.pem",
- "--cert=../keys/server.pem",
+ "--cert=../keys/server.crt",
"--key=../keys/server.key"
]
},