This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 26ad5ad96 [webserver] add security-related HTTP headers
26ad5ad96 is described below
commit 26ad5ad962fe05af4b69730ffded9414fd13a5e5
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Thu Feb 17 17:54:11 2022 -0800
[webserver] add security-related HTTP headers
To please various security scanners, this patch adds the following
HTTP headers into Kudu embedded webserver's responses:
* Cache-Control (set to 'no-store' by default, see [1])
* X-Content-Type-Options (set to 'nosniff' by default, see [2])
* Strict-Transport-Security (see [3] and below for details)
The embedded webserver adds the HTTP strict transport security
(HSTS) header 'Strict-Transport-Security' for responses sent from HTTPS
(i.e. TLS-protected) endpoints if --webserver_hsts_max_age_seconds is
set to a non-negative value. The header contains the 'max-age'
attribute as specified by the flag, and adds the optional
'includeSubDomains' attribute as per set setting of the
--webserver_hsts_include_sub_domains flag. The HSTS header isn't added
to the responses sent from plain HTTP endpoints (BTW, it seems most
browsers simply ignore the HSTS header anyway if it's received from
an HTTP, not an HTTPS endpoint).
Essentially, the HSTS header for Kudu is a no-op since the embedded
webserver doesn't serve both HTTP and HTTPS endpoints at the same time:
one can enable one or the other, but never both. However, many security
scanners almost cry "Security breach" if they don't see the header :)
Adding the HSTS header isn't enabled by default since it could make
other plain HTTP endpoints at the same node/hostname inaccessible.
To enable adding the HSTS header for HTTPS responses, set the
--webserver_hsts_max_age_seconds flag to a non-negative integer.
Enable it with care and only if you know what you are doing!
One extra test added and a few existing ones updated correspondingly
to cover the newly introduced functionality.
[1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control
[2]
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Content-Type-Options
[3]
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Strict-Transport-Security
Change-Id: Id844b9588196b3d608765d0f16f5caec1c414d41
Reviewed-on: http://gerrit.cloudera.org:8080/18253
Tested-by: Alexey Serbin <ale...@apache.org>
Reviewed-by: Attila Bukor <abu...@apache.org>
---
src/kudu/server/webserver-test.cc | 76 ++++++++++++++++++++++++++++++++++++++-
src/kudu/server/webserver.cc | 74 ++++++++++++++++++++++++++++++++++----
2 files changed, 142 insertions(+), 8 deletions(-)
diff --git a/src/kudu/server/webserver-test.cc
b/src/kudu/server/webserver-test.cc
index 472a64e62..924e11128 100644
--- a/src/kudu/server/webserver-test.cc
+++ b/src/kudu/server/webserver-test.cc
@@ -24,6 +24,7 @@
#include <cstdlib>
#include <functional>
+#include <initializer_list>
#include <iosfwd>
#include <memory>
#include <string>
@@ -61,8 +62,13 @@ using std::vector;
using std::unique_ptr;
using strings::Substitute;
+DECLARE_bool(webserver_hsts_include_sub_domains);
+DECLARE_int32(webserver_hsts_max_age_seconds);
DECLARE_int32(webserver_max_post_length_bytes);
DECLARE_string(trusted_certificate_file);
+DECLARE_string(webserver_cache_control_options);
+DECLARE_string(webserver_x_content_type_options);
+DECLARE_string(webserver_x_frame_options);
DEFINE_bool(test_sensitive_flag, false, "a sensitive flag");
TAG_FLAG(test_sensitive_flag, sensitive);
@@ -360,14 +366,42 @@ TEST_F(SpnegoWebserverTest,
TestAuthNotRequiredForOptions) {
TEST_F(WebserverTest, TestIndexPage) {
curl_.set_return_headers(true);
ASSERT_OK(curl_.FetchURL(url_, &buf_));
- // Check expected header.
+
+ // Check for the expected headers.
+ ASSERT_STR_CONTAINS(buf_.ToString(), "Cache-Control: no-store");
ASSERT_STR_CONTAINS(buf_.ToString(), "X-Frame-Options: DENY");
+ ASSERT_STR_CONTAINS(buf_.ToString(), "X-Content-Type-Options: nosniff");
+
+ FLAGS_webserver_hsts_max_age_seconds = 1000;
+ // The HTTP strict transport security policy (HSTS) header should be absent
+ // in the response sent from the plain HTTP (i.e. non-TLS) endpoint.
+ ASSERT_STR_NOT_CONTAINS(buf_.ToString(), "Strict-Transport-Security");
// Should have expected title.
ASSERT_STR_CONTAINS(buf_.ToString(), "Kudu");
// Should have link to default path handlers (e.g memz)
ASSERT_STR_CONTAINS(buf_.ToString(), "memz");
+
+ // Check that particular headers are generated as expected when customizing
+ // the Cache-Control and X-Content-Type-Options headers.
+ FLAGS_webserver_cache_control_options = "no-cache";
+ FLAGS_webserver_x_frame_options = "SAMEORIGIN";
+ FLAGS_webserver_x_content_type_options = "nosnuff";
+ ASSERT_OK(curl_.FetchURL(url_, &buf_));
+ ASSERT_STR_CONTAINS(buf_.ToString(), "Cache-Control: no-cache");
+ ASSERT_STR_CONTAINS(buf_.ToString(), "X-Frame-Options: SAMEORIGIN");
+ ASSERT_STR_CONTAINS(buf_.ToString(), "X-Content-Type-Options: nosnuff");
+
+ // Check that particular headers aren't added when corresponding flags
+ // have empty settings.
+ FLAGS_webserver_cache_control_options = "";
+ FLAGS_webserver_x_frame_options = "";
+ FLAGS_webserver_x_content_type_options = "";
+ ASSERT_OK(curl_.FetchURL(url_, &buf_));
+ ASSERT_STR_NOT_CONTAINS(buf_.ToString(), "Cache-Control");
+ ASSERT_STR_NOT_CONTAINS(buf_.ToString(), "X-Frame-Options");
+ ASSERT_STR_NOT_CONTAINS(buf_.ToString(), "X-Content-Type-Options");
}
TEST_F(WebserverTest, TestHttpCompression) {
@@ -435,6 +469,46 @@ TEST_F(SslWebserverTest, TestSSL) {
ASSERT_STR_CONTAINS(buf_.ToString(), "Kudu");
}
+TEST_F(SslWebserverTest, StrictTransportSecurtyPolicyHeaders) {
+ constexpr const char* const kHstsHeader = "Strict-Transport-Security";
+ // Since the server uses a self-signed TLS certificate, disable the cert
+ // validation in curl.
+ curl_.set_verify_peer(false);
+ curl_.set_return_headers(true);
+
+ ASSERT_OK(curl_.FetchURL(url_, &buf_));
+ // Basic sanity check: the page should have the expected title.
+ ASSERT_STR_CONTAINS(buf_.ToString(), "Kudu");
+
+ // The HSTS policy is disabled by default for the embedded Kudu webserver.
+ ASSERT_OK(curl_.FetchURL(url_, &buf_));
+ ASSERT_STR_NOT_CONTAINS(buf_.ToString(), kHstsHeader);
+
+ // There should be the HTTP strict transport security policy (HSTS) header
+ // in the response sent from the HTTPS (i.e. TLS-protected) endpoint.
+ for (auto max_age : {0, 1, 1000, 31536000}) {
+ FLAGS_webserver_hsts_max_age_seconds = max_age;
+ ASSERT_OK(curl_.FetchURL(url_, &buf_));
+ ASSERT_STR_CONTAINS(buf_.ToString(), Substitute(
+ "$0: max-age=$1", kHstsHeader, max_age));
+ ASSERT_STR_NOT_CONTAINS(buf_.ToString(), "includeSubDomains");
+ }
+
+ // Setting the flag to a negative value should result in no HSTS headers.
+ for (auto max_age : {-31536000, -100, -1 }) {
+ FLAGS_webserver_hsts_max_age_seconds = max_age;
+ ASSERT_OK(curl_.FetchURL(url_, &buf_));
+ ASSERT_STR_NOT_CONTAINS(buf_.ToString(), kHstsHeader);
+ }
+
+ constexpr auto kMaxAge = 888;
+ FLAGS_webserver_hsts_max_age_seconds = kMaxAge;
+ FLAGS_webserver_hsts_include_sub_domains = true;
+ ASSERT_OK(curl_.FetchURL(url_, &buf_));
+ ASSERT_STR_CONTAINS(buf_.ToString(), Substitute(
+ "$0: max-age=$1; includeSubDomains", kHstsHeader, kMaxAge));
+}
+
TEST_F(WebserverTest, TestDefaultPaths) {
// Test memz
ASSERT_OK(curl_.FetchURL(Substitute("$0/memz?raw=1", url_), &buf_));
diff --git a/src/kudu/server/webserver.cc b/src/kudu/server/webserver.cc
index 3ee571dd6..07dd1f45e 100644
--- a/src/kudu/server/webserver.cc
+++ b/src/kudu/server/webserver.cc
@@ -88,16 +88,53 @@ 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_cache_control_options, "no-store",
+ "The webserver adds 'Cache-Control' HTTP header with this value "
+ "to all responses.");
+TAG_FLAG(webserver_cache_control_options, advanced);
+TAG_FLAG(webserver_cache_control_options, 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);
+TAG_FLAG(webserver_x_frame_options, runtime);
DEFINE_bool(webserver_enable_csp, true,
"The webserver adds the Content-Security-Policy header to response
when enabled.");
TAG_FLAG(webserver_enable_csp, advanced);
TAG_FLAG(webserver_enable_csp, runtime);
+DEFINE_int32(webserver_hsts_max_age_seconds, -1,
+ "The time, in seconds, that a browser should remember that the "
+ "webserver must only be accessed using HTTPS. The HTTP Strict "
+ "Transport Security (HSTS) policy is implemented by adding a "
+ "'Strict-Transport-Security' header. If greater or equal to zero,
"
+ "a TLS-enabled webserver adds the HSTS header with corresponding "
+ "'max-age' setting to the response. If the setting is negative, "
+ "the HSTS header is not present in the response. As an option, it
"
+ "is possible to add 'includeSubDomains' directive as specified "
+ "by the --webserver_hsts_include_sub_domains flag. "
+ "WARNING: once enabled, the HSTS policy affects *everything* at "
+ "the node/domain, so any HTTP endpoint at the node/domain "
+ "effectively becomes unreachable to the browser which has reached
"
+ "the embedded webserver at this node via HTTPS");
+TAG_FLAG(webserver_hsts_max_age_seconds, advanced);
+TAG_FLAG(webserver_hsts_max_age_seconds, runtime);
+
+DEFINE_bool(webserver_hsts_include_sub_domains, false,
+ "Whether to add 'includeSubDomains' directive into the "
+ "'Strict-Transport-Security' header when the latter is enabled. "
+ "See --webserver_hsts_max_age_seconds's description for details.");
+TAG_FLAG(webserver_hsts_include_sub_domains, advanced);
+TAG_FLAG(webserver_hsts_include_sub_domains, runtime);
+
+DEFINE_string(webserver_x_content_type_options, "nosniff",
+ "The webserver adds 'X-Content-Type-Options' HTTP header with "
+ "this value to all responses.");
+TAG_FLAG(webserver_x_content_type_options, advanced);
+TAG_FLAG(webserver_x_content_type_options, runtime);
+
namespace kudu {
@@ -692,7 +729,7 @@ void Webserver::SendResponse(struct sq_connection*
connection,
oss << Substitute("HTTP/1.1 $0\r\n",
HttpStatusCodeToString(resp->status_code));
// The "Content-Type" is defined by the value of the 'mode' parameter.
- const char* content_type = nullptr;
+ const char* content_type = "text/plain";
switch (mode) {
case StyleMode::STYLED:
content_type = "text/html";
@@ -704,8 +741,8 @@ void Webserver::SendResponse(struct sq_connection*
connection,
content_type = "application/octet-stream";
break;
default:
- LOG(FATAL) << "unexpected style mode: " << static_cast<uint32_t>(mode);
- break; // unreachable
+ LOG(DFATAL) << "unexpected style mode: " << static_cast<uint32_t>(mode);
+ break;
}
oss << Substitute("Content-Type: $0\r\n", content_type);
@@ -713,6 +750,11 @@ void Webserver::SendResponse(struct sq_connection*
connection,
if (is_compressed) {
oss << "Content-Encoding: gzip\r\n";
}
+ const auto& cache_control_options = FLAGS_webserver_cache_control_options;
+ if (!cache_control_options.empty()) {
+ oss << Substitute("Cache-Control: $0\r\n", cache_control_options);
+ }
+
if (PREDICT_TRUE(FLAGS_webserver_enable_csp)) {
// TODO(aserbin): add information on when to update the SHA hash and
// how to do so (ideally, the exact command line)
@@ -720,19 +762,37 @@ void Webserver::SendResponse(struct sq_connection*
connection,
<< "style-src 'self' 'unsafe-hashes'
'sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=';"
<< "img-src 'self' data:;\r\n";
}
- oss << Substitute("X-Frame-Options: $0\r\n",
FLAGS_webserver_x_frame_options);
+ const auto& x_frame_options = FLAGS_webserver_x_frame_options;
+ if (!x_frame_options.empty()) {
+ oss << Substitute("X-Frame-Options: $0\r\n", x_frame_options);
+ }
+ if (IsSecure() && FLAGS_webserver_hsts_max_age_seconds >= 0) {
+ oss << Substitute("Strict-Transport-Security: max-age=$0",
+ FLAGS_webserver_hsts_max_age_seconds);
+ if (FLAGS_webserver_hsts_include_sub_domains) {
+ oss << "; includeSubDomains";
+ }
+ oss << "\r\n";
+ }
+ const auto& x_content_type_options = FLAGS_webserver_x_content_type_options;
+ if (!x_content_type_options.empty()) {
+ oss << Substitute("X-Content-Type-Options: $0\r\n",
x_content_type_options);
+ }
static const unordered_set<string> kInvalidHeaders = {
+ "Cache-Control",
"Content-Encoding",
"Content-Length",
- "Content-Type",
"Content-Security-Policy",
+ "Content-Type",
+ "Strict-Transport-Security",
+ "X-Content-Type-Options",
"X-Frame-Options",
};
for (const auto& entry : resp->response_headers) {
// It's forbidden to override the above headers.
if (PREDICT_FALSE(ContainsKey(kInvalidHeaders, entry.first))) {
- LOG(FATAL) << Substitute("Reserved header $0 was overridden by handler",
- entry.first);
+ LOG(DFATAL) << Substitute("reserved header $0 was overridden by handler",
+ entry.first);
}
oss << Substitute("$0: $1\r\n", entry.first, entry.second);
}
