IMPALA-5800: [Addendum] Fix bad import of squeasel.c The previous commit for IMPALA-5800 imported a slightly old version of squeasel.c from the upstream project. In particular, the strings used to choose the TLS version were incompatible with Impala's. This patch corrects the error.
Testing: Add tests to make sure that the expected flags are supported by Squeasel. Change-Id: I03b76f2d3b3e2e6133e5c57a4f33ac315c889e15 Reviewed-on: http://gerrit.cloudera.org:8080/7752 Reviewed-by: Bharath Vissapragada <[email protected]> Reviewed-by: Sailesh Mukil <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/fcf32b58 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/fcf32b58 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/fcf32b58 Branch: refs/heads/master Commit: fcf32b584b3afa9099f3553dc9506c97fac27275 Parents: 3b5c460 Author: Henry Robinson <[email protected]> Authored: Sun Aug 20 22:18:22 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Mon Aug 21 09:38:27 2017 +0000 ---------------------------------------------------------------------- be/src/thirdparty/squeasel/squeasel.c | 19 +++++++++---------- be/src/util/webserver-test.cc | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/fcf32b58/be/src/thirdparty/squeasel/squeasel.c ---------------------------------------------------------------------- diff --git a/be/src/thirdparty/squeasel/squeasel.c b/be/src/thirdparty/squeasel/squeasel.c index 3f957de..055a959 100644 --- a/be/src/thirdparty/squeasel/squeasel.c +++ b/be/src/thirdparty/squeasel/squeasel.c @@ -153,6 +153,8 @@ static const char *http_500_error = "Internal Server Error"; #include <openssl/ssl.h> #include <openssl/err.h> +#define OPENSSL_VERSION_HAS_TLS_1_1 0x10001000L + static const char *month_names[] = { "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec" @@ -4142,8 +4144,6 @@ static int set_uid_option(struct sq_context *ctx) { #if !defined(NO_SSL) -#define OPENSSL_VERSION_HAS_TLS_1_1 0x10001000L - static pthread_mutex_t *ssl_mutexes; static int sslize(struct sq_connection *conn, SSL_CTX *s, int (*func)(SSL *)) { @@ -4225,9 +4225,9 @@ static int set_ssl_option(struct sq_context *ctx) { if (sq_strcasecmp(ssl_version, "tlsv1") == 0) { // No-op - don't exclude any TLS protocols. #if OPENSSL_VERSION_NUMBER >= OPENSSL_VERSION_HAS_TLS_1_1 - } else if (sq_strcasecmp(ssl_version, "tlsv1_1") == 0) { + } else if (sq_strcasecmp(ssl_version, "tlsv1.1") == 0) { options |= SSL_OP_NO_TLSv1; - } else if (sq_strcasecmp(ssl_version, "tlsv1_2") == 0) { + } else if (sq_strcasecmp(ssl_version, "tlsv1.2") == 0) { options |= (SSL_OP_NO_TLSv1 | SSL_OP_NO_TLSv1_1); #endif } else { @@ -4272,12 +4272,11 @@ static int set_ssl_option(struct sq_context *ctx) { (void) SSL_CTX_use_certificate_chain_file(ctx->ssl_ctx, pem); } - if (ctx->config[SSL_CIPHERS] != NULL) { - if (SSL_CTX_set_cipher_list(ctx->ssl_ctx, ctx->config[SSL_CIPHERS]) == 0) { - cry(fc(ctx), "SSL_CTX_set_cipher_list: error setting ciphers (%s): %s", - ctx->config[SSL_CIPHERS], ssl_error()); - return 0; - } + if (ctx->config[SSL_CIPHERS] != NULL && + (SSL_CTX_set_cipher_list(ctx->ssl_ctx, ctx->config[SSL_CIPHERS]) == 0)) { + cry(fc(ctx), "SSL_CTX_set_cipher_list: error setting ciphers (%s): %s", + ctx->config[SSL_CIPHERS], ssl_error()); + return 0; } return 1; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/fcf32b58/be/src/util/webserver-test.cc ---------------------------------------------------------------------- diff --git a/be/src/util/webserver-test.cc b/be/src/util/webserver-test.cc index 7abaa37..ba6f276 100644 --- a/be/src/util/webserver-test.cc +++ b/be/src/util/webserver-test.cc @@ -20,6 +20,7 @@ #include <boost/bind.hpp> #include <boost/lexical_cast.hpp> #include <gutil/strings/substitute.h> +#include <openssl/ssl.h> #include "common/init.h" #include "testutil/gtest-util.h" @@ -286,6 +287,29 @@ TEST(Webserver, SslBadTlsVersion) { ASSERT_FALSE(webserver.Start().ok()); } +TEST(Webserver, SslGoodTlsVersion) { + auto cert = ScopedFlagSetter<string>::Make(&FLAGS_webserver_certificate_file, + Substitute("$0/be/src/testutil/server-cert.pem", getenv("IMPALA_HOME"))); + auto key = ScopedFlagSetter<string>::Make(&FLAGS_webserver_private_key_file, + Substitute("$0/be/src/testutil/server-key-password.pem", getenv("IMPALA_HOME"))); + auto cmd = ScopedFlagSetter<string>::Make( + &FLAGS_webserver_private_key_password_cmd, "echo password"); + +#if OPENSSL_VERSION_NUMBER >= 0x10001000L + auto versions = {"tlsv1", "tlsv1.1", "tlsv1.2"}; +#else + auto versions = {"tlsv1"}; +#endif + for (auto v: versions) { + auto ssl_version = ScopedFlagSetter<string>::Make( + &FLAGS_ssl_minimum_version, v); + + Webserver webserver(FLAGS_webserver_port); + ASSERT_OK(webserver.Start()); + } +} + + TEST(Webserver, StartWithPasswordFileTest) { stringstream password_file; password_file << getenv("IMPALA_HOME") << "/be/src/testutil/htpasswd";
