[Impala-ASF-CR] Support SPNEGO for Impala webserver
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13774 ) Change subject: Support SPNEGO for Impala webserver .. Support SPNEGO for Impala webserver This ports over changes from kudu commit 1f291b77ef0868ac888a850678adc2d7cce65529 which implemented SPNEGO for the Kudu webserver. Unfortunately, thorough testing of this is difficult given that curl isn't currently in the toolchain. I was able to manually test this by adding a 'sleep(1000)' call into the newly added test case, then setting up $KRB5_CONFIG in my shell to point to the temporary KDC's environment, and using 'curl -u : --negotiate http://...' to authenticate. Strangely, using the version of curl on el7 didn't seem to work properly (perhaps an el7 curl bug) but using curl on my Ubuntu 18 laptop I was able to authenticate with SPNEGO. Change-Id: Ife2b04310e1571d231bf8ee1bcfd3b7afc2edd8f Reviewed-on: http://gerrit.cloudera.org:8080/13774 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/exec/kudu-util.h M be/src/gutil/strings/escaping.cc M be/src/util/CMakeLists.txt A be/src/util/kudu-status-util.h M be/src/util/webserver-test.cc M be/src/util/webserver.cc M be/src/util/webserver.h 7 files changed, 267 insertions(+), 42 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- 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: merged Gerrit-Change-Id: Ife2b04310e1571d231bf8ee1bcfd3b7afc2edd8f Gerrit-Change-Number: 13774 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] Support SPNEGO for Impala webserver
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13774 ) Change subject: Support SPNEGO for Impala webserver .. Patch Set 5: Verified+1 -- 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: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 15 Jul 2019 23:05:15 + Gerrit-HasComments: No
[Impala-ASF-CR] Support SPNEGO for Impala webserver
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13774 ) Change subject: Support SPNEGO for Impala webserver .. Patch Set 5: Code-Review+2 -- 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: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 15 Jul 2019 16:45:50 + Gerrit-HasComments: No
[Impala-ASF-CR] Support SPNEGO for Impala webserver
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13774 ) Change subject: Support SPNEGO for Impala webserver .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4599/ DRY_RUN=false -- 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: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 15 Jul 2019 16:45:51 + Gerrit-HasComments: No
[Impala-ASF-CR] Support SPNEGO for Impala webserver
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13774 ) Change subject: Support SPNEGO for Impala webserver .. Patch Set 4: Code-Review+2 -- 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: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 12 Jul 2019 17:24:09 + Gerrit-HasComments: No
[Impala-ASF-CR] Support SPNEGO for Impala webserver
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13774 ) Change subject: Support SPNEGO for Impala webserver .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3865/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 11 Jul 2019 21:11:29 + Gerrit-HasComments: No
[Impala-ASF-CR] Support SPNEGO for Impala webserver
Hello Thomas Tauber-Marshall, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13774 to look at the new patch set (#4). Change subject: Support SPNEGO for Impala webserver .. Support SPNEGO for Impala webserver This ports over changes from kudu commit 1f291b77ef0868ac888a850678adc2d7cce65529 which implemented SPNEGO for the Kudu webserver. Unfortunately, thorough testing of this is difficult given that curl isn't currently in the toolchain. I was able to manually test this by adding a 'sleep(1000)' call into the newly added test case, then setting up $KRB5_CONFIG in my shell to point to the temporary KDC's environment, and using 'curl -u : --negotiate http://...' to authenticate. Strangely, using the version of curl on el7 didn't seem to work properly (perhaps an el7 curl bug) but using curl on my Ubuntu 18 laptop I was able to authenticate with SPNEGO. Change-Id: Ife2b04310e1571d231bf8ee1bcfd3b7afc2edd8f --- M be/src/exec/kudu-util.h M be/src/gutil/strings/escaping.cc M be/src/util/CMakeLists.txt A be/src/util/kudu-status-util.h M be/src/util/webserver-test.cc M be/src/util/webserver.cc M be/src/util/webserver.h 7 files changed, 267 insertions(+), 42 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/13774/4 -- 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: newpatchset Gerrit-Change-Id: Ife2b04310e1571d231bf8ee1bcfd3b7afc2edd8f Gerrit-Change-Number: 13774 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] Support SPNEGO for Impala webserver
Thomas Tauber-Marshall has posted comments on this change. (
http://gerrit.cloudera.org:8080/13774 )
Change subject: Support SPNEGO for Impala webserver
..
Patch Set 3:
(1 comment)
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@370
PS1, Line 370: LAGS_webserver_require_spnego) {
> I don't think the --principal is actually necessary
My point was that InitKerberosForServer() won't have actually been called
unless --principal is set (see authentication.cc). Maybe my confusion is the
word "assume" and it would be clearer to me if this was phrased as "checking"
whether all of the necessary set up has been done? Unless we want to support a
configuration where the webserver is secured with Kerberos but regular client
connections aren't (which seems weird)
> I think people will get pretty grouchy
Works for me. Of course, we'll probably want to update the docs around
Impala/Kerberos to mention this stuff
--
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: 3
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Wed, 10 Jul 2019 17:43:43 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] Support SPNEGO for Impala webserver
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13774 ) Change subject: Support SPNEGO for Impala webserver .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3855/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 10 Jul 2019 05:25:13 + Gerrit-HasComments: No
[Impala-ASF-CR] Support SPNEGO for Impala webserver
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 10 Jul 2019 04:45:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Support SPNEGO for Impala webserver
Hello Thomas Tauber-Marshall, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13774 to look at the new patch set (#2). Change subject: Support SPNEGO for Impala webserver .. Support SPNEGO for Impala webserver This ports over changes from kudu commit 1f291b77ef0868ac888a850678adc2d7cce65529 which implemented SPNEGO for the Kudu webserver. Unfortunately, thorough testing of this is difficult given that curl isn't currently in the toolchain. I was able to manually test this by adding a 'sleep(1000)' call into the newly added test case, then setting up $KRB5_CONFIG in my shell to point to the temporary KDC's environment, and using 'curl -u : --negotiate http://...' to authenticate. Strangely, using the version of curl on el7 didn't seem to work properly (perhaps an el7 curl bug) but using curl on my Ubuntu 18 laptop I was able to authenticate with SPNEGO. Change-Id: Ife2b04310e1571d231bf8ee1bcfd3b7afc2edd8f --- M be/src/exec/kudu-util.h M be/src/gutil/strings/escaping.cc M be/src/util/CMakeLists.txt A be/src/util/kudu-status-util.h M be/src/util/webserver-test.cc M be/src/util/webserver.cc M be/src/util/webserver.h 7 files changed, 265 insertions(+), 42 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/13774/2 -- 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: newpatchset Gerrit-Change-Id: Ife2b04310e1571d231bf8ee1bcfd3b7afc2edd8f Gerrit-Change-Number: 13774 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] Support SPNEGO for Impala webserver
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13774 ) Change subject: Support SPNEGO for Impala webserver .. Patch Set 1: (2 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 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@370 PS1, Line 370: We assume that security::InitKerberosForServer() What if this isn't the case, eg. because --principal isn't set? Should we check for that? 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? -- 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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 09 Jul 2019 17:52:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Support SPNEGO for Impala webserver
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13774 ) Change subject: Support SPNEGO for Impala webserver .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3798/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 02 Jul 2019 00:42:34 + Gerrit-HasComments: No
[Impala-ASF-CR] Support SPNEGO for Impala webserver
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13774 ) Change subject: Support SPNEGO for Impala webserver .. Patch Set 1: (2 comments) 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) 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) -- 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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 02 Jul 2019 00:03:16 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Support SPNEGO for Impala webserver
Todd Lipcon has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13774 Change subject: Support SPNEGO for Impala webserver .. Support SPNEGO for Impala webserver This ports over changes from kudu commit 1f291b77ef0868ac888a850678adc2d7cce65529 which implemented SPNEGO for the Kudu webserver. Unfortunately, thorough testing of this is difficult given that curl isn't currently in the toolchain. I was able to manually test this by adding a 'sleep(1000)' call into the newly added test case, then setting up $KRB5_CONFIG in my shell to point to the temporary KDC's environment, and using 'curl -u : --negotiate http://...' to authenticate. Strangely, using the version of curl on el7 didn't seem to work properly (perhaps an el7 curl bug) but using curl on my Ubuntu 18 laptop I was able to authenticate with SPNEGO. Change-Id: Ife2b04310e1571d231bf8ee1bcfd3b7afc2edd8f --- M be/src/exec/kudu-util.h M be/src/gutil/strings/escaping.cc M be/src/util/CMakeLists.txt A be/src/util/kudu-status-util.h M be/src/util/webserver-test.cc M be/src/util/webserver.cc M be/src/util/webserver.h 7 files changed, 263 insertions(+), 42 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/13774/1 -- 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: newchange Gerrit-Change-Id: Ife2b04310e1571d231bf8ee1bcfd3b7afc2edd8f Gerrit-Change-Number: 13774 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon
