Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13774 )
Change subject: Support SPNEGO for Impala webserver ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/13774/1/be/src/util/kudu-status-util.h File be/src/util/kudu-status-util.h: http://gerrit.cloudera.org:8080/#/c/13774/1/be/src/util/kudu-status-util.h@26 PS1, Line 26: \ > Might as well fix the formatting while you're here Done http://gerrit.cloudera.org:8080/#/c/13774/1/be/src/util/webserver.cc File be/src/util/webserver.cc: http://gerrit.cloudera.org:8080/#/c/13774/1/be/src/util/webserver.cc@203 PS1, Line 203: // is invalid, a bad Status will be returned (and the other out-parameters left untouched). > line too long (91 > 90) Done http://gerrit.cloudera.org:8080/#/c/13774/1/be/src/util/webserver.cc@214 PS1, Line 214: RETURN_NOT_OK(kudu::gssapi::SpnegoStep(neg_token, &resp_token_b64, &is_complete, authn_user)); > line too long (96 > 90) Done http://gerrit.cloudera.org:8080/#/c/13774/1/be/src/util/webserver.cc@370 PS1, Line 370: We assume that security::InitKerberosForServer() I don't think the --principal is actually necessary -- the way SPNEGO works is that the client provides a principal, and the server will look in the configured keytab for any matching principal. This is what allows users to have multiple DNS names, and clients can connect to them via any name (useful for L4 load balancing or CNAMEs to work, for example). If we don't have KRB5_KTNAME set at this point, it'll be null, and we'll get the expected bad Status below. Does that seem right? We actually do have some test case for this on the equivalent Kudu code, but it wasn't really easy to port it over to Impala's tests. > What about the opposite case - where --principal is set but > --webserver_require_spnego=false? Right now I think we just silently allow > this, but users may find it surprising that setting up kerberos doesn't > automatically secure the webserver. Maybe worth logging a warning in that > case? The problem with this is that, while SPNEGO is a standard, it's a huge pain in the ass for users to set up, and in fact I think even on secure clusters, most users don't enable SPNEGO on their web UIs. If we started WARNING about being insecure if you don't enable SPNEGO I think people will get pretty grouchy. That said, this is still useful because we'll use this to allow access via Knox Trusted Proxy support, which uses SPNEGO to authenticate the Knox service to Impala (and Knox itself gets authentication via pluggable modules, including SSO integration) -- To view, visit http://gerrit.cloudera.org:8080/13774 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife2b04310e1571d231bf8ee1bcfd3b7afc2edd8f Gerrit-Change-Number: 13774 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Wed, 10 Jul 2019 04:45:14 +0000 Gerrit-HasComments: Yes
