[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. IMPALA-11856: Use POST requests to set log level Set and reset loglevel handlers now require a POST. Implements Cross-Site Request Forgery (CSRF) prevention in Impala's webserver using the Double Submit Cookie pattern - where POST requests must include a csrf_token field in their post with the random value from the cookie - or a custom header. CSRF attacks rely on the browser always sending a cookie or 'Authorization: Basic' header. - With cookies, attacks don't have access to default form values or the original cookie, so we can include the cookie's random value in the form as a cross-check. As the cookie is cryptographically signed, they also can't be replaced with one that would match an attack's forms. - When not using cookies, a custom header (X-Requested-By) is required as CSRFs are unable to add custom headers. This approach is also used by Jersey; see http://blog.alutam.com/2011/09/14/jersey-and-cross-site-request-forgery-csrf In a broader implementation this would require a separate cookie so it can be used to protect logins as well, but login is handled external to Impala so we re-use the cookie the page already has. Cookies are now generated for the HTPASSWD authentication method. Authenticating via JWT still omits cookies because the JWT is already provided via custom header (preventing CSRF) and disabling authentication (NONE) means anyone could directly send a request so CSRF protection is meaningless. We also start an additional Webserver instance with authentication NONE when metrics_webserver_port > 0, and the Webserver metric "impala.webserver.total-cookie-auth-success" can only be registered once. Additional changes would be necessary to make metric names unique in Webserver (based on port); for the moment we avoid that by ensuring all metrics counters are only instantiated for Webservers that use authentication. Cookie generation and authentication were updated to provide access to the random value. Adds flag to enable SameSite=Strict for defense in depth as mentioned in https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis. This can be enabled if another CSRF attack method is found. Verified that this prevents CSRF attacks by disabling SameSite=Strict and visiting (via https://security.love/CSRF-PoC-Genorator): ``` http://localhost:45000/set_glog_level;> glog http://localhost:45000/set_glog_level;> ``` Adds tests for the webserver with basic authentication, LDAP, and SPNEGO that authorization fails on POST unless - using a cookie and csrf_token is correctly set in the POST body - the X-Requested-By header is set Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Reviewed-on: http://gerrit.cloudera.org:8080/19199 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/rpc/authentication-util.cc M be/src/rpc/authentication-util.h M be/src/util/logging-support.cc M be/src/util/webserver-test.cc M be/src/util/webserver.cc M be/src/util/webserver.h M fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java M fe/src/test/java/org/apache/impala/customcluster/JwtWebserverTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapImpylaHttpTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapJdbcTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java M fe/src/test/java/org/apache/impala/service/JdbcTest.java A fe/src/test/java/org/apache/impala/testutil/WebClient.java D fe/src/test/java/org/apache/impala/util/Metrics.java M tests/webserver/test_web_pages.py M www/form-hidden-inputs.tmpl M www/log_level.tmpl 19 files changed, 900 insertions(+), 369 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 25 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 24: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 24 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 31 Jan 2023 14:40:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 23: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 23 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 31 Jan 2023 09:29:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 24: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9006/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 24 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 31 Jan 2023 09:31:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 24: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 24 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 31 Jan 2023 09:31:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 23: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 23 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Mon, 30 Jan 2023 23:44:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 23: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/12266/ : 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/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 23 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Mon, 30 Jan 2023 18:51:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 23: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8999/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 23 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Mon, 30 Jan 2023 18:32:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Hello Andrew Sherman, Attila Bukor, Wenzhe Zhou, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/19199 to look at the new patch set (#23). Change subject: IMPALA-11856: Use POST requests to set log level .. IMPALA-11856: Use POST requests to set log level Set and reset loglevel handlers now require a POST. Implements Cross-Site Request Forgery (CSRF) prevention in Impala's webserver using the Double Submit Cookie pattern - where POST requests must include a csrf_token field in their post with the random value from the cookie - or a custom header. CSRF attacks rely on the browser always sending a cookie or 'Authorization: Basic' header. - With cookies, attacks don't have access to default form values or the original cookie, so we can include the cookie's random value in the form as a cross-check. As the cookie is cryptographically signed, they also can't be replaced with one that would match an attack's forms. - When not using cookies, a custom header (X-Requested-By) is required as CSRFs are unable to add custom headers. This approach is also used by Jersey; see http://blog.alutam.com/2011/09/14/jersey-and-cross-site-request-forgery-csrf In a broader implementation this would require a separate cookie so it can be used to protect logins as well, but login is handled external to Impala so we re-use the cookie the page already has. Cookies are now generated for the HTPASSWD authentication method. Authenticating via JWT still omits cookies because the JWT is already provided via custom header (preventing CSRF) and disabling authentication (NONE) means anyone could directly send a request so CSRF protection is meaningless. We also start an additional Webserver instance with authentication NONE when metrics_webserver_port > 0, and the Webserver metric "impala.webserver.total-cookie-auth-success" can only be registered once. Additional changes would be necessary to make metric names unique in Webserver (based on port); for the moment we avoid that by ensuring all metrics counters are only instantiated for Webservers that use authentication. Cookie generation and authentication were updated to provide access to the random value. Adds flag to enable SameSite=Strict for defense in depth as mentioned in https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis. This can be enabled if another CSRF attack method is found. Verified that this prevents CSRF attacks by disabling SameSite=Strict and visiting (via https://security.love/CSRF-PoC-Genorator): ``` http://localhost:45000/set_glog_level;> glog http://localhost:45000/set_glog_level;> ``` Adds tests for the webserver with basic authentication, LDAP, and SPNEGO that authorization fails on POST unless - using a cookie and csrf_token is correctly set in the POST body - the X-Requested-By header is set Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 --- M be/src/rpc/authentication-util.cc M be/src/rpc/authentication-util.h M be/src/util/logging-support.cc M be/src/util/webserver-test.cc M be/src/util/webserver.cc M be/src/util/webserver.h M fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java M fe/src/test/java/org/apache/impala/customcluster/JwtWebserverTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapImpylaHttpTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapJdbcTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java M fe/src/test/java/org/apache/impala/service/JdbcTest.java A fe/src/test/java/org/apache/impala/testutil/WebClient.java D fe/src/test/java/org/apache/impala/util/Metrics.java M tests/webserver/test_web_pages.py M www/form-hidden-inputs.tmpl M www/log_level.tmpl 19 files changed, 900 insertions(+), 369 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/19199/23 -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 23 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 21: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/8993/ -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 21 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Sat, 28 Jan 2023 03:02:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 21: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8993/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 21 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 27 Jan 2023 21:54:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 21: Code-Review+2 Carrying +2 after addressing test and comment nits. -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 21 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 27 Jan 2023 21:54:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 21: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/12249/ : 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/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 21 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 27 Jan 2023 21:52:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Hello Andrew Sherman, Attila Bukor, Wenzhe Zhou, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/19199 to look at the new patch set (#21). Change subject: IMPALA-11856: Use POST requests to set log level .. IMPALA-11856: Use POST requests to set log level Set and reset loglevel handlers now require a POST. Implements Cross-Site Request Forgery (CSRF) prevention in Impala's webserver using the Double Submit Cookie pattern - where POST requests must include a csrf_token field in their post with the random value from the cookie - or a custom header. CSRF attacks rely on the browser always sending a cookie or 'Authorization: Basic' header. - With cookies, attacks don't have access to default form values or the original cookie, so we can include the cookie's random value in the form as a cross-check. As the cookie is cryptographically signed, they also can't be replaced with one that would match an attack's forms. - When not using cookies, a custom header (X-Requested-By) is required as CSRFs are unable to add custom headers. This approach is also used by Jersey; see http://blog.alutam.com/2011/09/14/jersey-and-cross-site-request-forgery-csrf In a broader implementation this would require a separate cookie so it can be used to protect logins as well, but login is handled external to Impala so we re-use the cookie the page already has. Cookies are now generated for the HTPASSWD authentication method. Authenticating via JWT still omits cookies because the JWT is already provided via custom header (preventing CSRF) and disabling authentication (NONE) means anyone could directly send a request so CSRF protection is meaningless. We also start an additional Webserver instance with authentication NONE when metrics_webserver_port > 0, and the Webserver metric "impala.webserver.total-cookie-auth-success" can only be registered once. Additional changes would be necessary to make metric names unique in Webserver (based on port); for the moment we avoid that by ensuring all metrics counters are only instantiated for Webservers that use authentication. Cookie generation and authentication were updated to provide access to the random value. Adds flag to enable SameSite=Strict for defense in depth as mentioned in https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis. This can be enabled if another CSRF attack method is found. Verified that this prevents CSRF attacks by disabling SameSite=Strict and visiting (via https://security.love/CSRF-PoC-Genorator): ``` http://localhost:45000/set_glog_level;> glog http://localhost:45000/set_glog_level;> ``` Adds tests for the webserver with basic authentication, LDAP, and SPNEGO that authorization fails on POST unless - using a cookie and csrf_token is correctly set in the POST body - the X-Requested-By header is set Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 --- M be/src/rpc/authentication-util.cc M be/src/rpc/authentication-util.h M be/src/util/logging-support.cc M be/src/util/webserver-test.cc M be/src/util/webserver.cc M be/src/util/webserver.h M fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java M fe/src/test/java/org/apache/impala/customcluster/JwtWebserverTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapImpylaHttpTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapJdbcTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java M fe/src/test/java/org/apache/impala/service/JdbcTest.java A fe/src/test/java/org/apache/impala/testutil/WebClient.java D fe/src/test/java/org/apache/impala/util/Metrics.java M www/form-hidden-inputs.tmpl M www/log_level.tmpl 18 files changed, 848 insertions(+), 337 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/19199/21 -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 21 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Michael Smith has posted comments on this change. (
http://gerrit.cloudera.org:8080/19199 )
Change subject: IMPALA-11856: Use POST requests to set log level
..
Patch Set 20:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/19199/20/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
File fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java:
http://gerrit.cloudera.org:8080/#/c/19199/20/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java@475
PS20, Line 475:* Test that we can set glog level.
> Nit: java log level
Ack
http://gerrit.cloudera.org:8080/#/c/19199/20/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java@563
PS20, Line 563: assertTrue("Expected cookie to contain random number",
false);
> Nit: or just use fail()
Ack
http://gerrit.cloudera.org:8080/#/c/19199/20/fe/src/test/java/org/apache/impala/testutil/WebClient.java
File fe/src/test/java/org/apache/impala/testutil/WebClient.java:
http://gerrit.cloudera.org:8080/#/c/19199/20/fe/src/test/java/org/apache/impala/testutil/WebClient.java@107
PS20, Line 107:* @param data Parameters to include in the POST
> Nit: I know you are just copying code but this parameter is wrong
Just an earlier version of my function.
--
To view, visit http://gerrit.cloudera.org:8080/19199
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693
Gerrit-Change-Number: 19199
Gerrit-PatchSet: 20
Gerrit-Owner: Michael Smith
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Comment-Date: Fri, 27 Jan 2023 21:32:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Andrew Sherman has posted comments on this change. (
http://gerrit.cloudera.org:8080/19199 )
Change subject: IMPALA-11856: Use POST requests to set log level
..
Patch Set 20: Code-Review+2
(3 comments)
LGTM, thanks for this complex change
http://gerrit.cloudera.org:8080/#/c/19199/20/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
File fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java:
http://gerrit.cloudera.org:8080/#/c/19199/20/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java@475
PS20, Line 475:* Test that we can set glog level.
Nit: java log level
http://gerrit.cloudera.org:8080/#/c/19199/20/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java@563
PS20, Line 563: assertTrue("Expected cookie to contain random number",
false);
Nit: or just use fail()
http://gerrit.cloudera.org:8080/#/c/19199/20/fe/src/test/java/org/apache/impala/testutil/WebClient.java
File fe/src/test/java/org/apache/impala/testutil/WebClient.java:
http://gerrit.cloudera.org:8080/#/c/19199/20/fe/src/test/java/org/apache/impala/testutil/WebClient.java@107
PS20, Line 107:* @param data Parameters to include in the POST
Nit: I know you are just copying code but this parameter is wrong
--
To view, visit http://gerrit.cloudera.org:8080/19199
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693
Gerrit-Change-Number: 19199
Gerrit-PatchSet: 20
Gerrit-Owner: Michael Smith
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Comment-Date: Fri, 27 Jan 2023 20:29:25 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 20: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/12247/ : 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/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 20 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 27 Jan 2023 17:16:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Hello Andrew Sherman, Attila Bukor, Wenzhe Zhou, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/19199 to look at the new patch set (#20). Change subject: IMPALA-11856: Use POST requests to set log level .. IMPALA-11856: Use POST requests to set log level Set and reset loglevel handlers now require a POST. Implements Cross-Site Request Forgery (CSRF) prevention in Impala's webserver using the Double Submit Cookie pattern - where POST requests must include a csrf_token field in their post with the random value from the cookie - or a custom header. CSRF attacks rely on the browser always sending a cookie or 'Authorization: Basic' header. - With cookies, attacks don't have access to default form values or the original cookie, so we can include the cookie's random value in the form as a cross-check. As the cookie is cryptographically signed, they also can't be replaced with one that would match an attack's forms. - When not using cookies, a custom header (X-Requested-By) is required as CSRFs are unable to add custom headers. This approach is also used by Jersey; see http://blog.alutam.com/2011/09/14/jersey-and-cross-site-request-forgery-csrf In a broader implementation this would require a separate cookie so it can be used to protect logins as well, but login is handled external to Impala so we re-use the cookie the page already has. Cookies are now generated for the HTPASSWD authentication method. Authenticating via JWT still omits cookies because the JWT is already provided via custom header (preventing CSRF) and disabling authentication (NONE) means anyone could directly send a request so CSRF protection is meaningless. We also start an additional Webserver instance with authentication NONE when metrics_webserver_port > 0, and the Webserver metric "impala.webserver.total-cookie-auth-success" can only be registered once. Additional changes would be necessary to make metric names unique in Webserver (based on port); for the moment we avoid that by ensuring all metrics counters are only instantiated for Webservers that use authentication. Cookie generation and authentication were updated to provide access to the random value. Adds flag to enable SameSite=Strict for defense in depth as mentioned in https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis. This can be enabled if another CSRF attack method is found. Verified that this prevents CSRF attacks by disabling SameSite=Strict and visiting (via https://security.love/CSRF-PoC-Genorator): ``` http://localhost:45000/set_glog_level;> glog http://localhost:45000/set_glog_level;> ``` Adds tests for the webserver with basic authentication, LDAP, and SPNEGO that authorization fails on POST unless - using a cookie and csrf_token is correctly set in the POST body - the X-Requested-By header is set Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 --- M be/src/rpc/authentication-util.cc M be/src/rpc/authentication-util.h M be/src/util/logging-support.cc M be/src/util/webserver-test.cc M be/src/util/webserver.cc M be/src/util/webserver.h M fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java M fe/src/test/java/org/apache/impala/customcluster/JwtWebserverTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapImpylaHttpTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapJdbcTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java M fe/src/test/java/org/apache/impala/service/JdbcTest.java A fe/src/test/java/org/apache/impala/testutil/WebClient.java D fe/src/test/java/org/apache/impala/util/Metrics.java M www/form-hidden-inputs.tmpl M www/log_level.tmpl 18 files changed, 844 insertions(+), 337 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/19199/20 -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 20 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 19: (3 comments) http://gerrit.cloudera.org:8080/#/c/19199/19//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19199/19//COMMIT_MSG@11 PS19, Line 11: where POST requests must include a : csrf_token field in their post with the random value from the cookie - : or a custom header. > This means that POST requests will always need csrf_token or X-requested-by Someone POSTing just to read a page would have been possible, but seems unlikely. curl requires a body when POSTing, and browsers require a form, so they would have had to jump through hoops to do it. http://gerrit.cloudera.org:8080/#/c/19199/19/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java File fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java: http://gerrit.cloudera.org:8080/#/c/19199/19/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java@414 PS19, Line 414: does nothing > This also returns an error like the previous block, right? Yeah, I'll update the comment to match. http://gerrit.cloudera.org:8080/#/c/19199/19/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java@425 PS19, Line 425: new WebClient(TEST_USER_1, TEST_PASSWORD_1); > just an idea to reduce noise: WebClient could have a reset function or a re Doing that correctly turned out to be tricky. I had to recreate the internal client anyway because it doesn't have public methods that allow sufficient cleanup. I think this is clear enough. -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 19 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 27 Jan 2023 16:54:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 19: Code-Review+1 (3 comments) Went through the tests too, looks great! http://gerrit.cloudera.org:8080/#/c/19199/19//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19199/19//COMMIT_MSG@11 PS19, Line 11: where POST requests must include a : csrf_token field in their post with the random value from the cookie - : or a custom header. This means that POST requests will always need csrf_token or X-requested-by, even if it is not POST with side-effects (e.g. changing logs, just reading a page), right? Is it possible that some tools query the Impala webui with POST while they could use GET? http://gerrit.cloudera.org:8080/#/c/19199/19/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java File fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java: http://gerrit.cloudera.org:8080/#/c/19199/19/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java@414 PS19, Line 414: does nothing This also returns an error like the previous block, right? http://gerrit.cloudera.org:8080/#/c/19199/19/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java@425 PS19, Line 425: new WebClient(TEST_USER_1, TEST_PASSWORD_1); just an idea to reduce noise: WebClient could have a reset function or a resetAfterRequest property that leads to dropping cookies + opening new connection -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 19 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 27 Jan 2023 08:16:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 19: (1 comment) Looks good to me in general, but I couldn't process all the tests yet. http://gerrit.cloudera.org:8080/#/c/19199/15/be/src/util/webserver.cc File be/src/util/webserver.cc: http://gerrit.cloudera.org:8080/#/c/19199/15/be/src/util/webserver.cc@703 PS15, Line 703: LOG(INFO) << "Inv > I think I get it now - what confused me was the handling of AuthMode::HTPAS Oops. I didn't see the latest patches when writing this. The HTPASSWD handling looks much clearer now. -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 19 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Thu, 26 Jan 2023 16:53:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 19: (1 comment) http://gerrit.cloudera.org:8080/#/c/19199/15/be/src/util/webserver.cc File be/src/util/webserver.cc: http://gerrit.cloudera.org:8080/#/c/19199/15/be/src/util/webserver.cc@703 PS15, Line 703: LOG(INFO) << "Inv > This is a relocation of lines 665-667. It didn't have a comment before; I'l I think I get it now - what confused me was the handling of AuthMode::HTPASSWD. In that case the authentication is handled by Squeasel based on a password file, so I guess we won't even get here if the password was wrong. If it was correct, then it is true that the authentication is finished. and the cookie should be added / checked. I still think that the code is quite confusing, but I don't mind if it is kept as it is. -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 19 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Thu, 26 Jan 2023 13:43:33 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 19: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/12235/ : 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/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 19 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Wed, 25 Jan 2023 20:06:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Hello Andrew Sherman, Attila Bukor, Wenzhe Zhou, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/19199 to look at the new patch set (#19). Change subject: IMPALA-11856: Use POST requests to set log level .. IMPALA-11856: Use POST requests to set log level Set and reset loglevel handlers now require a POST. Implements Cross-Site Request Forgery (CSRF) prevention in Impala's webserver using the Double Submit Cookie pattern - where POST requests must include a csrf_token field in their post with the random value from the cookie - or a custom header. CSRF attacks rely on the browser always sending a cookie or 'Authorization: Basic' header. - With cookies, attacks don't have access to default form values or the original cookie, so we can include the cookie's random value in the form as a cross-check. As the cookie is cryptographically signed, they also can't be replaced with one that would match an attack's forms. - When not using cookies, a custom header (X-Requested-By) is required as CSRFs are unable to add custom headers. This approach is also used by Jersey; see http://blog.alutam.com/2011/09/14/jersey-and-cross-site-request-forgery-csrf In a broader implementation this would require a separate cookie so it can be used to protect logins as well, but login is handled external to Impala so we re-use the cookie the page already has. Cookies are now generated for the HTPASSWD authentication method. Authenticating via JWT still omits cookies because the JWT is already provided via custom header (preventing CSRF) and disabling authentication (NONE) means anyone could directly send a request so CSRF protection is meaningless. We also start an additional Webserver instance with authentication NONE when metrics_webserver_port > 0, and the Webserver metric "impala.webserver.total-cookie-auth-success" can only be registered once. Additional changes would be necessary to make metric names unique in Webserver (based on port); for the moment we avoid that by ensuring all metrics counters are only instantiated for Webservers that use authentication. Cookie generation and authentication were updated to provide access to the random value. Adds flag to enable SameSite=Strict for defense in depth as mentioned in https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis. This can be enabled if another CSRF attack method is found. Verified that this prevents CSRF attacks by disabling SameSite=Strict and visiting (via https://security.love/CSRF-PoC-Genorator): ``` http://localhost:45000/set_glog_level;> glog http://localhost:45000/set_glog_level;> ``` Adds tests for the webserver with basic authentication, LDAP, and SPNEGO that authorization fails on POST unless - using a cookie and csrf_token is correctly set in the POST body - the X-Requested-By header is set Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 --- M be/src/rpc/authentication-util.cc M be/src/rpc/authentication-util.h M be/src/util/logging-support.cc M be/src/util/webserver-test.cc M be/src/util/webserver.cc M be/src/util/webserver.h M fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java M fe/src/test/java/org/apache/impala/customcluster/JwtWebserverTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapImpylaHttpTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapJdbcTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java M fe/src/test/java/org/apache/impala/service/JdbcTest.java A fe/src/test/java/org/apache/impala/testutil/WebClient.java D fe/src/test/java/org/apache/impala/util/Metrics.java M www/form-hidden-inputs.tmpl M www/log_level.tmpl 18 files changed, 844 insertions(+), 337 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/19199/19 -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 19 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19199 ) Change subject: IMPALA-11856: Use POST requests to set log level .. Patch Set 18: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/12234/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 18 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Wed, 25 Jan 2023 19:42:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11856: Use POST requests to set log level
Michael Smith has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19199 Change subject: IMPALA-11856: Use POST requests to set log level .. IMPALA-11856: Use POST requests to set log level Set and reset loglevel handlers now require a POST. Implements Cross-Site Request Forgery (CSRF) prevention in Impala's webserver using the Double Submit Cookie pattern - where POST requests must include a csrf_token field in their post with the random value from the cookie - or a custom header. CSRF attacks rely on the browser always sending a cookie or 'Authorization: Basic' header. - With cookies, attacks don't have access to default form values or the original cookie, so we can include the cookie's random value in the form as a cross-check. As the cookie is cryptographically signed, they also can't be replaced with one that would match an attack's forms. - When not using cookies, a custom header (X-Requested-By) is required as CSRFs are unable to add custom headers. This approach is also used by Jersey; see http://blog.alutam.com/2011/09/14/jersey-and-cross-site-request-forgery-csrf In a broader implementation this would require a separate cookie so it can be used to protect logins as well, but login is handled external to Impala so we re-use the cookie the page already has. Cookies are now generated for the HTPASSWD authentication method. Authenticating via JWT still omits cookies because the JWT is already provided via custom header (preventing CSRF) and disabling authentication (NONE) means anyone could directly send a request so CSRF protection is meaningless. We also start an additional Webserver instance with authentication NONE when metrics_webserver_port > 0, and the Webserver metric "impala.webserver.total-cookie-auth-success" can only be registered once. Additional changes would be necessary to make metric names unique in Webserver (based on port); for the moment we avoid that by ensuring all metrics counters are only instantiated for Webservers that use authentication. Cookie generation and authentication were updated to provide access to the random value. Adds flag to enable SameSite=Strict for defense in depth as mentioned in https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis. This can be enabled if another CSRF attack method is found. Verified that this prevents CSRF attacks by disabling SameSite=Strict and visiting (via https://security.love/CSRF-PoC-Genorator): ``` http://localhost:45000/set_glog_level;> glog http://localhost:45000/set_glog_level;> ``` Adds tests for the webserver with basic authentication, LDAP, and SPNEGO that authorization fails on POST unless - using a cookie and csrf_token is correctly set in the POST body - the X-Requested-By header is set Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 --- M be/src/rpc/authentication-util.cc M be/src/rpc/authentication-util.h M be/src/util/logging-support.cc M be/src/util/webserver-test.cc M be/src/util/webserver.cc M be/src/util/webserver.h M fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java M fe/src/test/java/org/apache/impala/customcluster/JwtWebserverTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapImpylaHttpTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapJdbcTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java M fe/src/test/java/org/apache/impala/service/JdbcTest.java A fe/src/test/java/org/apache/impala/testutil/WebClient.java D fe/src/test/java/org/apache/impala/util/Metrics.java M www/form-hidden-inputs.tmpl M www/log_level.tmpl 18 files changed, 844 insertions(+), 337 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/19199/18 -- To view, visit http://gerrit.cloudera.org:8080/19199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4be8694492b8ba16737f644ac8c56d8124f19693 Gerrit-Change-Number: 19199 Gerrit-PatchSet: 18 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou
