Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24383 )
Change subject: KUDU-3781: add TLS transport support for HMS client ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/24383/3/src/kudu/hms/hms_catalog.cc File src/kudu/hms/hms_catalog.cc: http://gerrit.cloudera.org:8080/#/c/24383/3/src/kudu/hms/hms_catalog.cc@76 PS3, Line 76: DEFINE_string(hive_metastore_tls_trusted_ca_cert_file, "", > I guess the more important separation is that one controls the internal Kud Alright, then we should be good to go with the --hive_metastore_tls_use_https_trusted_ca_cert flag. http://gerrit.cloudera.org:8080/#/c/24383/5/src/kudu/hms/hms_catalog.cc File src/kudu/hms/hms_catalog.cc: http://gerrit.cloudera.org:8080/#/c/24383/5/src/kudu/hms/hms_catalog.cc@146 PS5, Line 146: ; : } : : Status HmsCatalog::Start(HmsClientVerifyK > aren't the args reversed? this would say: "flag is set to 'CA flag name' an Good catch! Fixed. http://gerrit.cloudera.org:8080/#/c/24383/3/src/kudu/hms/hms_client-test.cc File src/kudu/hms/hms_client-test.cc: http://gerrit.cloudera.org:8080/#/c/24383/3/src/kudu/hms/hms_client-test.cc@615 PS3, Line 615: TEST_F(HmsClientTest, TlsEnabledOnlyOnClient) { > Would it make sense to use a tri-state flag with 'optional' default similar You mean using tri-state behavior the newly introduced flag --hive_metastore_tls_enabled? I don't clearly see now to implement this given we don't have direct control over the TLS/Thrift negotiation between Kudu HMS client and Hive Metastore server. A simple trial-and-error approach on top of Thrift's TSSLSocket is possible, but it's a bit brittle and still requires patching the Thrift library to get closer to OpenSSL's error codes: we'd want to differentiate between intermittent network I/O errors and a case when the server side isn't talking TLS. Even if doing so, I'd rather do that in a separate changelist, not here. http://gerrit.cloudera.org:8080/#/c/24383/5/src/kudu/thrift/client.cc File src/kudu/thrift/client.cc: http://gerrit.cloudera.org:8080/#/c/24383/5/src/kudu/thrift/client.cc@89 PS5, Line 89: if (options.tls_trusted_ca_cert_file.empty()) { : sf.authenticate(false); : } else { : > What happens in a scenario, where enable_tls is true, sf.authenticate(true) Thank you pointing on this. Yes -- the idea is to use the system-wide CA trust chain when options.tls_trusted_ca_cert_file is empty. The intended behavior is similar to running 'curl <URL>' compared with 'curl --cacert <trusted_ca_certs_file> <URL>'. I.e., by default, the system-wide CA certs are used, but with specifying '--cacert', the system-wide CA trust chain is completely replaced with custom trusted CA certificates from the specified file. The Thrift client uses the API of the OpenSSL library, and for some reason I assumed it would load the system-wide CA trust chain implicitly if TSSLSocketFactory::loadTrustedCertificates() hasn't been called. It turned out not to be the case. So, I updated the code here to behave as expected. I verified that with PS6 the TLS cert verification works as intended (pointed the Thrift client to a few HTTPS servers with legit certificates signed by designated CAs). -- To view, visit http://gerrit.cloudera.org:8080/24383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9658323ed9aebdfdf8e6847c9c3129f5c8517c65 Gerrit-Change-Number: 24383 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Wed, 03 Jun 2026 18:59:12 +0000 Gerrit-HasComments: Yes
