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 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/24511/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/24511/1//COMMIT_MSG@9
PS1, Line 9: d4ec371 introduced a new tool to debug TLS connections (diagnose
           : tls_debug), but unfortunately, it didn't work with Kerberos-enabled
           : servers, which makes it less practical in real-life environments.
Can you add a test scenario to demonstrate this changelist is more fortunate 
than d4ec371 and catch future regressions when working with Kerberos-enabled 
servers?


http://gerrit.cloudera.org:8080/#/c/24511/1/src/kudu/rpc/client_negotiation.cc
File src/kudu/rpc/client_negotiation.cc:

http://gerrit.cloudera.org:8080/#/c/24511/1/src/kudu/rpc/client_negotiation.cc@221
PS1, Line 221:   // Diagnostic shortcut: when the caller only wants to observe 
the negotiated
             :   // TLS parameters (e.g. `kudu diagnose tls_debug`), skip 
SASL/token/JWT
             :   // authentication. The server will log an aborted negotiation 
when we
             :   // disconnect, but the TLS-level info on the socket is fully 
populated.
             :   if (tls_only_) {
             :     TRACE("Stopping negotiation after TLS handshake (tls_only 
mode)");
             :     return Status::OK();
             :   }
Is this necessary only for the diagnostic tool?  This doesn't smell good.

Did you explore alternative approaches, e.g., handling an error at the client 
side?

My concerns with this shortcut is an unexpected security vulnerability when a 
client might assume it has authenticated a malicious server, but in fact it 
didn't.



--
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: 1
Gerrit-Owner: Attila Bukor <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Comment-Date: Thu, 25 Jun 2026 17:09:38 +0000
Gerrit-HasComments: Yes

Reply via email to