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): ``` <html> <form enctype="application/x-www-form-urlencoded" method="POST" action="http://localhost:45000/set_glog_level"> <table> <tr> <td>glog</td> <td><input type="text" value="1" name="glog"></td> </tr> </table> <input type="submit" value="http://localhost:45000/set_glog_level"> </form> </html> ``` 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 <michael.sm...@cloudera.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>