diff --git a/be/src/rpc/authentication-util.cc b/be/src/rpc/authentication-util.cc index b02a97ed8..36c67a6fa 100644 --- a/be/src/rpc/authentication-util.cc +++ b/be/src/rpc/authentication-util.cc @@ -35,6 +35,8 @@ DEFINE_bool(cookie_require_secure, true, DEFINE_int64(max_cookie_lifetime_s, 24 * 60 * 60, "Maximum amount of time in seconds that an authentication cookie will remain valid. " "Setting to 0 disables use of cookies. Defaults to 1 day."); +DEFINE_bool(samesite_strict, false, + "(Advanced) If true, authentication cookies will include SameSite=Strict."); using namespace strings; @@ -59,13 +61,14 @@ static const int SHA256_BASE64_LEN = CalculateBase64EscapedLen(AuthenticationHash::HashLen(), /* do_padding */ true); // Since we only return cookies with a single name, well behaved clients should only ever -// return one cookie to us. To accomodate non-malicious but poorly behaved clients, we +// return one cookie to us. To accommodate non-malicious but poorly behaved clients, we // allow for checking a limited number of cookies, up to MAX_COOKIES_TO_CHECK or until we // find the first one with COOKIE_NAME. static const int MAX_COOKIES_TO_CHECK = 5; Status AuthenticateCookie( - const AuthenticationHash& hash, const string& cookie_header, string* username) { + const AuthenticationHash& hash, const string& cookie_header, + string* username, string* rand) { // The 'Cookie' header allows sending multiple name/value pairs separated by ';'. vector cookies = strings::Split(cookie_header, ";"); if (cookies.size() > MAX_COOKIES_TO_CHECK) { @@ -125,6 +128,11 @@ Status AuthenticateCookie( if (!TryStripPrefixString(cookie_value_split[0], USERNAME_KEY, username)) { return Status("The cookie username value has an invalid format."); } + if (rand != nullptr) { + if (!TryStripPrefixString(cookie_value_split[2], RAND_KEY, rand)) { + return Status("The cookie rand value has an invalid format."); + } + } // We've successfully authenticated. return Status::OK(); } else { @@ -135,12 +143,18 @@ Status AuthenticateCookie( return Status(Substitute("Did not find expected cookie name: $0", COOKIE_NAME)); } -string GenerateCookie(const string& username, const AuthenticationHash& hash) { +string GenerateCookie(const string& username, const AuthenticationHash& hash, + std::string* srand) { // Its okay to use rand() here even though its a weak RNG because being able to guess // the random numbers generated won't help an attacker. The important thing is that // we're using a strong RNG to create the key and a strong HMAC function. + int cookie_rand = rand(); + string cookie_rand_s = std::to_string(cookie_rand); + if (srand != nullptr) { + *srand = cookie_rand_s; + } string cookie_value = StrCat(USERNAME_KEY, username, COOKIE_SEPARATOR, TIMESTAMP_KEY, - MonotonicMillis(), COOKIE_SEPARATOR, RAND_KEY, rand()); + MonotonicMillis(), COOKIE_SEPARATOR, RAND_KEY, cookie_rand_s); uint8_t signature[AuthenticationHash::HashLen()]; Status compute_status = hash.Compute(reinterpret_cast(cookie_value.data()), @@ -155,12 +169,13 @@ string GenerateCookie(const string& username, const AuthenticationHash& hash) { SHA256_BASE64_LEN, /* do_padding */ true); base64_signature[SHA256_BASE64_LEN] = '\0'; - const char* secure_flag = ";Secure"; - if (!FLAGS_cookie_require_secure) { - secure_flag = ""; - } - return Substitute("$0=$1$2$3;HttpOnly;Max-Age=$4$5", COOKIE_NAME, base64_signature, - COOKIE_SEPARATOR, cookie_value, FLAGS_max_cookie_lifetime_s, secure_flag); + const char* secure_flag = FLAGS_cookie_require_secure ? ";Secure" : ""; + const char* samesite_flag = FLAGS_samesite_strict ? ";SameSite=Strict" : ""; + // Add SameSite=Strict to notify the browser it should avoid sending the cookie with + // requests from other domains. + return Substitute("$0=$1$2$3;HttpOnly;Max-Age=$4$5$6", COOKIE_NAME, base64_signature, + COOKIE_SEPARATOR, cookie_value, FLAGS_max_cookie_lifetime_s, secure_flag, + samesite_flag); } string GetDeleteCookie() { diff --git a/be/src/rpc/authentication-util.h b/be/src/rpc/authentication-util.h index c07b7cc50..c901beea8 100644 --- a/be/src/rpc/authentication-util.h +++ b/be/src/rpc/authentication-util.h @@ -25,13 +25,15 @@ class AuthenticationHash; // Takes a single 'key=value' pair from a 'Cookie' header and attempts to verify its // signature with 'hash'. If verification is successful and the cookie is still valid, -// sets 'username' to the corresponding username and returns OK. -Status AuthenticateCookie(const AuthenticationHash& hash, - const std::string& cookie_header, std::string* username); +// sets 'username' and 'rand' (if specified) to the corresponding values and returns OK. +Status AuthenticateCookie( + const AuthenticationHash& hash, const std::string& cookie_header, + std::string* username, std::string* rand = nullptr); // Generates and returns a cookie containing the username set on 'connection_context' and -// a signature generated with 'hash'. -std::string GenerateCookie(const std::string& username, const AuthenticationHash& hash); +// a signature generated with 'hash'. If specified, sets 'rand' to the 'r=' cookie value. +std::string GenerateCookie(const std::string& username, const AuthenticationHash& hash, + std::string* rand = nullptr); // Returns a empty cookie. Returned in a 'Set-Cookie' when cookie auth fails to indicate // to the client that the cookie should be deleted. @@ -51,4 +53,6 @@ bool IsTrustedDomain(const std::string& origin, const std::string& trusted_domai Status BasicAuthExtractCredentials( const string& token, string& username, string& password); +constexpr int RAND_MAX_LENGTH = 10; + } // namespace impala diff --git a/be/src/util/logging-support.cc b/be/src/util/logging-support.cc index 459f913d2..9e2e103ad 100644 --- a/be/src/util/logging-support.cc +++ b/be/src/util/logging-support.cc @@ -149,7 +149,12 @@ void GetJavaLogLevels(Document* document) { // Callback handler for /set_java_loglevel. void SetJavaLogLevelCallback(const Webserver::WebRequest& req, Document* document) { - const auto& args = req.parsed_args; + if (req.request_method != "POST") { + AddDocumentMember("Use form input to update java log levels", "error", document); + return; + } + + Webserver::ArgumentMap args = Webserver::GetVars(req.post_data); Webserver::ArgumentMap::const_iterator classname = args.find("class"); Webserver::ArgumentMap::const_iterator level = args.find("level"); if (classname == args.end() || classname->second.empty() || @@ -179,6 +184,11 @@ void SetJavaLogLevelCallback(const Webserver::WebRequest& req, Document* documen // Callback handler for /reset_java_loglevel. void ResetJavaLogLevelCallback(const Webserver::WebRequest& req, Document* document) { + if (req.request_method != "POST") { + AddDocumentMember("Use form input to reset java log levels", "error", document); + return; + } + Status status = ResetJavaLogLevels(); if (!status.ok()) { AddDocumentMember(status.GetDetail(), "error", document); @@ -202,7 +212,12 @@ void GetGlogLevel(Document* document) { // Callback handler for /set_glog_level void SetGlogLevelCallback(const Webserver::WebRequest& req, Document* document) { - const auto& args = req.parsed_args; + if (req.request_method != "POST") { + AddDocumentMember("Use form input to update glog level", "error", document); + return; + } + + Webserver::ArgumentMap args = Webserver::GetVars(req.post_data); Webserver::ArgumentMap::const_iterator glog_level = args.find("glog"); if (glog_level == args.end() || glog_level->second.empty()) { AddDocumentMember("Bad glog level input. Valid inputs are integers in the " @@ -221,6 +236,11 @@ void SetGlogLevelCallback(const Webserver::WebRequest& req, Document* document) // Callback handler for /reset_glog_level void ResetGlogLevelCallback(const Webserver::WebRequest& req, Document* document) { + if (req.request_method != "POST") { + AddDocumentMember("Use form input to reset glog level", "error", document); + return; + } + string new_log_level = google::SetCommandLineOption("v", to_string(FLAGS_v_original_value).data()); VLOG(1) << "New glog level set: " << new_log_level; diff --git a/be/src/util/webserver-test.cc b/be/src/util/webserver-test.cc index 65e18f640..26ee1ee28 100644 --- a/be/src/util/webserver-test.cc +++ b/be/src/util/webserver-test.cc @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -65,49 +66,74 @@ const string TO_ESCAPE_KEY = "ToEscape"; const string TO_ESCAPE_VALUE = "