Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24511 )
Change subject: KUDU-3787 Make tls_debug support Kerberos ...................................................................... Patch Set 2: Code-Review+2 (3 comments) LGTM http://gerrit.cloudera.org:8080/#/c/24511/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24511/2//COMMIT_MSG@15 PS2, Line 15: It also fixes a bug (which was only present in debug build) that : surfaced while testing skipping authentication where a malformed : negotiation could crash a server It's nice to have an extra fix like this. Thanks a lot! http://gerrit.cloudera.org:8080/#/c/24511/2/src/kudu/integration-tests/security-itest.cc File src/kudu/integration-tests/security-itest.cc: http://gerrit.cloudera.org:8080/#/c/24511/2/src/kudu/integration-tests/security-itest.cc@496 PS2, Line 496: is refused Refused by the server side, right? But what if a client talks to a malicious server that doesn't refuse RPCs in such a case? Even if hypothetical, but was the point in my comment at https://gerrit.cloudera.org/#/c/24511/1/src/kudu/rpc/client_negotiation.cc@228 So far it seems the client side wouldn't send its authn token to a server in this case (because authn step is skipped), but it would send its authz tokens. However, it's a hypothetical situation because: (1) Kudu C++ client API doesn't expose ClientNegotiation class (2) even if somebody mindlessly messes up with ClientNegotiation class, it requires additional wiring to call set_skip_authn() that would affect connections created by KuduClient (3) it's would require additional meddling to call set_skip_authn() only on connections to malicious servers, but not calling it on connections to valid servers :) http://gerrit.cloudera.org:8080/#/c/24511/2/src/kudu/integration-tests/security-itest.cc@504 PS2, Line 504: TestSkipAuthnSocketIsUseless Thank you for adding this test -- it's nice to have an explicit test scenario like this even if such a situation is implicitly covered by already existing test scenario (I haven't check for the latter, though). -- To view, visit http://gerrit.cloudera.org:8080/24511 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6c6c9ac1cebeab8cb48bd22aea852c93c0252a4 Gerrit-Change-Number: 24511 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Fri, 26 Jun 2026 15:40:49 +0000 Gerrit-HasComments: Yes
