[kudu] branch master updated: [util] KUDU-3392: add validator for --trusted_certificate_file

[alexey]

This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 4734d6f16 [util] KUDU-3392: add validator for 
--trusted_certificate_file
4734d6f16 is described below

commit 4734d6f167333e67483d45bbeaf7509180641414
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Wed Jun 14 14:47:54 2023 -0700

    [util] KUDU-3392: add validator for --trusted_certificate_file
    
    While testing/troubleshooting the newly introduced JWT-based client
    authentication, I found that setting the flag to a non-existing file
    hadn't been handled as I expected.
    
    This patch addresses the issue, adding a validator for the flag.
    
    I didn't add any test to cover the newly added validator, but I have
    verified that it worked as expected in various scenarios w.r.t. the
    current value of the --trusted_certificate_file flag.
    
    This is a follow-up to 152211658ef9d33e0ad727ccba46f8af24cd45b0.
    
    Change-Id: I0d9d816a821a93037293d3985a2f577711d64ef2
    Reviewed-on: http://gerrit.cloudera.org:8080/20075
    Tested-by: Kudu Jenkins
    Reviewed-by: Attila Bukor <abu...@apache.org>
---
 src/kudu/util/curl_util.cc | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/src/kudu/util/curl_util.cc b/src/kudu/util/curl_util.cc
index 6dc936c98..08c32b759 100644
--- a/src/kudu/util/curl_util.cc
+++ b/src/kudu/util/curl_util.cc
@@ -19,6 +19,7 @@
 
 #include <cstdint>
 #include <cstring>
+#include <memory>
 #include <mutex>
 #include <ostream>
 #include <string>
@@ -29,17 +30,21 @@
 #include <glog/logging.h>
 
 #include "kudu/gutil/strings/substitute.h"
+#include "kudu/util/env.h"
 #include "kudu/util/faststring.h"
+#include "kudu/util/flag_validators.h"
 #include "kudu/util/openssl_util.h"
 #include "kudu/util/scoped_cleanup.h"
+#include "kudu/util/slice.h"
 
 using std::string;
 using std::vector;
 using strings::Substitute;
 
 DEFINE_string(trusted_certificate_file, "",
-              "Path to a PEM file that contains certificate(s) to be trusted 
when "
-              "Kudu acts as a client (e.g. when talking to a KMS service.");
+              "Path to a file that contains certificate(s) (in PEM format) "
+              "to trust when Kudu establishes a TLS-protected connection "
+              "to HTTP/HTTPS server (e.g., KMS service, JWKS server, etc.)");
 
 namespace kudu {
 
@@ -61,6 +66,43 @@ inline Status TranslateError(CURLcode code, const char* 
errbuf) {
   return Status::NetworkError("curl error", err_msg);
 }
 
+bool ValidateTrustedCertFile() {
+  const auto& fpath = FLAGS_trusted_certificate_file;
+  if (fpath.empty()) {
+    // No validation is needed.
+    return true;
+  }
+
+  // Make sure the file in question does exist, is readable, and non-empty.
+  // There might be extra verification to load the certificate(s) from the 
file,
+  // but since cURL could have some particular requirements on the contents
+  // of the file, let's skip that extra validation step and defer to the time
+  // when cURL loads the file on its own. Also, the validators might run
+  // at the time when the OpenSSL-based crypto runtime context isn't yet
+  // initialized, so it's safer to defer to TlsContext::Init() where the
+  // initialization is done in a proper way.
+  std::unique_ptr<RandomAccessFile> raf;
+  if (auto s = Env::Default()->NewRandomAccessFile(fpath, &raf); !s.ok()) {
+    LOG(ERROR) << Substitute("could not open file for reading: $0", 
s.ToString());
+    return false;
+  }
+  // Read just a single byte to make sure that the file is readable.
+  uint8_t scratch[1];
+  Slice data(scratch, sizeof(scratch));
+  if (auto s = raf->Read(0, data); !s.ok()) {
+    LOG(ERROR) << Substitute("could not read from file '$0': $1",
+                             fpath, s.ToString());
+    return false;
+  }
+  return true;
+}
+
+// The validator uses Env API, so it's necessary to use GROUP_FLAG_VALIDATOR()
+// instead of regular gflag's DEFINE_validator() macro to allow for custom
+// behavior of PosixEnv per settings of various flags defined in env_posix.cc.
+GROUP_FLAG_VALIDATOR(validate_trusted_certificate_file,
+                     ValidateTrustedCertFile);
+
 extern "C" {
 size_t WriteCallback(void* buffer, size_t size, size_t nmemb, void* user_ptr) {
   size_t real_size = size * nmemb;