webserver: add X-Frame-Options header This adds a default 'DENY' header in order to prevent Kudu web pages from being put into cross-domain iframes. This can prevent clickjacking attacks, and generally considered a good idea for web security.
See: https://www.owasp.org/index.php/Clickjacking Change-Id: Ie43ec476712c2574a4dc746dae6218f0a4195e09 Reviewed-on: http://gerrit.cloudera.org:8080/6215 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert <[email protected]> (cherry picked from commit f6a1a60760296e7014d5d7b04ce68d0835721da8) Reviewed-on: http://gerrit.cloudera.org:8080/6233 Reviewed-by: Todd Lipcon <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/6de257e5 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/6de257e5 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/6de257e5 Branch: refs/heads/branch-1.3.x Commit: 6de257e529ed44949806cc2a5e895ce1815117db Parents: 3f97ef6 Author: Todd Lipcon <[email protected]> Authored: Wed Mar 1 14:53:28 2017 -0800 Committer: Todd Lipcon <[email protected]> Committed: Fri Mar 3 01:09:03 2017 +0000 ---------------------------------------------------------------------- src/kudu/server/webserver-test.cc | 4 ++++ src/kudu/server/webserver.cc | 27 +++++++++++++++------------ src/kudu/util/curl_util.cc | 3 +++ src/kudu/util/curl_util.h | 7 +++++++ 4 files changed, 29 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/6de257e5/src/kudu/server/webserver-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/server/webserver-test.cc b/src/kudu/server/webserver-test.cc index 3638141..82f68e2 100644 --- a/src/kudu/server/webserver-test.cc +++ b/src/kudu/server/webserver-test.cc @@ -95,8 +95,12 @@ class SslWebserverTest : public WebserverTest { }; TEST_F(WebserverTest, TestIndexPage) { + curl_.set_return_headers(true); ASSERT_OK(curl_.FetchURL(strings::Substitute("http://$0/";, addr_.ToString()), &buf_)); + // Check expected header. + ASSERT_STR_CONTAINS(buf_.ToString(), "X-Frame-Options: DENY"); + // Should have expected title. ASSERT_STR_CONTAINS(buf_.ToString(), "Kudu"); http://git-wip-us.apache.org/repos/asf/kudu/blob/6de257e5/src/kudu/server/webserver.cc ---------------------------------------------------------------------- diff --git a/src/kudu/server/webserver.cc b/src/kudu/server/webserver.cc index 06b6f6a..6d493b5 100644 --- a/src/kudu/server/webserver.cc +++ b/src/kudu/server/webserver.cc @@ -64,6 +64,11 @@ DEFINE_int32(webserver_max_post_length_bytes, 1024 * 1024, TAG_FLAG(webserver_max_post_length_bytes, advanced); TAG_FLAG(webserver_max_post_length_bytes, runtime); +DEFINE_string(webserver_x_frame_options, "DENY", + "The webserver will add an 'X-Frame-Options' HTTP header with this value " + "to all responses. This can help prevent clickjacking attacks."); +TAG_FLAG(webserver_x_frame_options, advanced); + namespace kudu { Webserver::Webserver(const WebserverOptions& opts) @@ -385,19 +390,17 @@ int Webserver::RunPathHandler(const PathHandler& handler, string str = output.str(); // Without styling, render the page as plain text - if (!use_style) { - sq_printf(connection, "HTTP/1.1 200 OK\r\n" - "Content-Type: text/plain\r\n" - "Content-Length: %zd\r\n" - "\r\n", str.length()); - } else { - sq_printf(connection, "HTTP/1.1 200 OK\r\n" - "Content-Type: text/html\r\n" - "Content-Length: %zd\r\n" - "\r\n", str.length()); - } - + string headers = strings::Substitute( + "HTTP/1.1 200 OK\r\n" + "Content-Type: $0\r\n" + "Content-Length: $1\r\n" + "X-Frame-Options: $2\r\n" + "\r\n", + use_style ? "text/html" : "text/plain", + str.length(), + FLAGS_webserver_x_frame_options); // Make sure to use sq_write for printing the body; sq_printf truncates at 8kb + sq_write(connection, headers.c_str(), headers.length()); sq_write(connection, str.c_str(), str.length()); return 1; } http://git-wip-us.apache.org/repos/asf/kudu/blob/6de257e5/src/kudu/util/curl_util.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/curl_util.cc b/src/kudu/util/curl_util.cc index 133e2e6..6211834 100644 --- a/src/kudu/util/curl_util.cc +++ b/src/kudu/util/curl_util.cc @@ -78,6 +78,9 @@ Status EasyCurl::DoRequest(const std::string& url, curl_, CURLOPT_SSL_VERIFYPEER, static_cast<long>(verify_peer_)))); // NOLINT(runtime/int) RETURN_NOT_OK(TranslateError(curl_easy_setopt(curl_, CURLOPT_URL, url.c_str()))); + if (return_headers_) { + RETURN_NOT_OK(TranslateError(curl_easy_setopt(curl_, CURLOPT_HEADER, 1))); + } RETURN_NOT_OK(TranslateError(curl_easy_setopt(curl_, CURLOPT_WRITEFUNCTION, WriteCallback))); RETURN_NOT_OK(TranslateError(curl_easy_setopt(curl_, CURLOPT_WRITEDATA, static_cast<void *>(dst)))); http://git-wip-us.apache.org/repos/asf/kudu/blob/6de257e5/src/kudu/util/curl_util.h ---------------------------------------------------------------------- diff --git a/src/kudu/util/curl_util.h b/src/kudu/util/curl_util.h index d758975..797c8a6 100644 --- a/src/kudu/util/curl_util.h +++ b/src/kudu/util/curl_util.h @@ -54,6 +54,10 @@ class EasyCurl { verify_peer_ = verify; } + void set_return_headers(bool v) { + return_headers_ = v; + } + private: // Do a request. If 'post_data' is non-NULL, does a POST. // Otherwise, does a GET. @@ -65,6 +69,9 @@ class EasyCurl { // Whether to verify the server certificate. bool verify_peer_ = true; + // Whether to return the HTTP headers with the response. + bool return_headers_ = false; + DISALLOW_COPY_AND_ASSIGN(EasyCurl); };
