This is an automated email from the ASF dual-hosted git repository. dbecker pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 40a72e05d7cf011a6525a69d2b52d19aa5c6477f Author: jasonmfehr <[email protected]> AuthorDate: Mon Feb 13 21:12:48 2023 -0800 IMPALA-14066 (Part 4): Re-applying IMPALA-11922 Verify JWKS URL server TLS certificate by default after Kudu rebase This commit re-applies IMPALA-11922 to the Kudu files after the Kudu rebase to v1.17.1. The original commit message is below: **** BREAKING CHANGE **** If using JWT authentication to the Impala engine and the JWKS is retrieved from a URL, Impala now verifies the server's TLS certificate. Before, Impala did not verify the trust chain nor did it verify the CN/SAN. JWT Auth has an option to specify the location of the JSON Web Key Set (JWKS) using a URL. If that URL is accessed over HTTPS, the TLS certificate presented by the server is not verified. This means that Impala only requires the server to return a TLS certificate, whether or not Impala trusts the signing certificate chain. The implications of this setup is that a fully secure chain of trust cannot be established throughout the entire JWT authentication lifecycle and thus creates an attack vector where a bad actor could trick Impala into trusting an actor-controlled JWKS. The bad actor can then generate a JWT with any claims they chose and Impala will accept it. This change introduces: 1. verification of JWKS server TLS certificate by default 2. jwks_verify_server_certificate Impala startup flag 3. jwks_ca_certificate Impala startup flag 1. While previously, the JWKS URL was always called without verifying its TLS certificate, the default is to now to verify that cert. Thus, any cases where the JWKS was retrieved from an untrusted URL will now cause Impala to fail to start. 2. The new flag jwks_verify_server_certificate controls whether or not Impala verifies the TLS certificate presented by the JWKS server. It defaults to "false" meaning that the certificate will be verified. Setting this value to "false" will restore the previous behavior where untrusted TLS certificates are accepted. 3. The new flag jwks_ca_certificate enables specifying a PEM certificate bundle that contains certificates to trust when calling to the JWKS URL. Testing was achieved in the front-end Java custom cluster tests. An existing test was modified and three new tests were created. The following test cases are covered: 1. Insecurely retrieve a JWKS from a server with an untrusted TLS certificate. This test case is expected to pass. 2. Securely retrieve a JWKS from a server with an untrusted TLS certificate. This test case is expected to fail. The Impala coordinator logs are checked to ensure the cause was an untrusted certificate presented by the JWKS server. 3. Retrieve a JWKS from a server where the root CA is trusted, but the cert contains the wrong CN. This test is expected to fail. The Impala logs are checked to ensure the cause was a certificate with an incorrect CN. 4. Securely retrieve a JWKS from a server with a trusted TLS certificate. This test case is expected to pass. Change-Id: Ia7a648da9d65cc50caeedda75ca8b98912db4ae0 Reviewed-on: http://gerrit.cloudera.org:8080/19503 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> Reviewed-on: http://gerrit.cloudera.org:8080/22970 Reviewed-by: Daniel Becker <[email protected]> Tested-by: Daniel Becker <[email protected]> --- be/src/kudu/util/curl_util.cc | 29 +++++++++++++++++++++-------- be/src/kudu/util/curl_util.h | 10 ++++++++++ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/be/src/kudu/util/curl_util.cc b/be/src/kudu/util/curl_util.cc index 08c32b759..3e4872809 100644 --- a/be/src/kudu/util/curl_util.cc +++ b/be/src/kudu/util/curl_util.cc @@ -133,11 +133,8 @@ EasyCurl::EasyCurl() curl_ = curl_easy_init(); CHECK(curl_) << "Could not init curl"; - // Set the error buffer to enhance error messages with more details, when - // available. + // Ensure the curl error buffer is large enough. static_assert(kErrBufSize >= CURL_ERROR_SIZE, "kErrBufSize is too small"); - const auto code = curl_easy_setopt(curl_, CURLOPT_ERRORBUFFER, errbuf_); - CHECK_EQ(CURLE_OK, code); } EasyCurl::~EasyCurl() { @@ -190,15 +187,31 @@ Status EasyCurl::DoRequest(const string& url, const vector<string>& headers) { CHECK_NOTNULL(dst)->clear(); + // Reset all options to default values to ensure settings do not leak + // across calls. + curl_easy_reset(curl_); + + // Set the error buffer to enhance error messages with more details, when + // available. + CURL_RETURN_NOT_OK(curl_easy_setopt(curl_, CURLOPT_ERRORBUFFER, errbuf_)); + // Mark the error buffer as cleared. errbuf_[0] = 0; - if (!verify_peer_) { + if (verify_peer_) { + CURL_RETURN_NOT_OK(curl_easy_setopt(curl_, CURLOPT_SSL_VERIFYHOST, 2)); + CURL_RETURN_NOT_OK(curl_easy_setopt(curl_, CURLOPT_SSL_VERIFYPEER, 1)); + + if (!FLAGS_trusted_certificate_file.empty()) { + CURL_RETURN_NOT_OK(curl_easy_setopt(curl_, CURLOPT_CAINFO, + FLAGS_trusted_certificate_file.c_str())); + } + } else { CURL_RETURN_NOT_OK(curl_easy_setopt(curl_, CURLOPT_SSL_VERIFYHOST, 0)); CURL_RETURN_NOT_OK(curl_easy_setopt(curl_, CURLOPT_SSL_VERIFYPEER, 0)); - } else if (!FLAGS_trusted_certificate_file.empty()) { - CURL_RETURN_NOT_OK(curl_easy_setopt(curl_, CURLOPT_CAINFO, - FLAGS_trusted_certificate_file.c_str())); + } + if (!ca_certificates_.empty()) { + CURL_RETURN_NOT_OK(curl_easy_setopt(curl_, CURLOPT_CAINFO, ca_certificates_.c_str())); } switch (auth_type_) { diff --git a/be/src/kudu/util/curl_util.h b/be/src/kudu/util/curl_util.h index 3910d6736..1738d7fed 100644 --- a/be/src/kudu/util/curl_util.h +++ b/be/src/kudu/util/curl_util.h @@ -88,6 +88,12 @@ class EasyCurl { verify_peer_ = verify; } + // Sets a file path for a PEM bundle of certificates to trust when making a request over + // HTTPS. Can be either CA certificates or the actual server certificate. + void set_ca_certificates(const std::string& ca_certificates) { + ca_certificates_ = ca_certificates; + } + void set_return_headers(bool v) { return_headers_ = v; } @@ -176,6 +182,10 @@ class EasyCurl { // Whether to verify the server certificate. bool verify_peer_ = true; + // File path to a pem encoded bundle of certs to trust when calling to a server + // over https + std::string ca_certificates_; + // Whether to return the HTTP headers with the response. bool return_headers_ = false;
