This is an automated email from the ASF dual-hosted git repository. asekretenko pushed a commit to branch 1.7.x in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 38c292862cfe0911ddab0b8119380df97b4b465c Author: Benjamin Bannier <bbann...@apache.org> AuthorDate: Mon Sep 23 10:24:50 2019 +0200 Fixed parsing of HTTP authentication headers. This patch adds support for quoted strings in `Www-Authenticate` headers and allows to use spaces when delimiting authentication attributes of the form `WWW-Autenticate: a=b, c=d`, both of with are allowed by RFC2617. Review: https://reviews.apache.org/r/71534 --- 3rdparty/libprocess/include/process/http.hpp | 2 +- 3rdparty/libprocess/src/http.cpp | 93 ++++++++++++++++++++++++---- 3rdparty/libprocess/src/tests/http_tests.cpp | 31 +++++++++- 3 files changed, 113 insertions(+), 13 deletions(-) diff --git a/3rdparty/libprocess/include/process/http.hpp b/3rdparty/libprocess/include/process/http.hpp index fa66b0f..2e36419 100644 --- a/3rdparty/libprocess/include/process/http.hpp +++ b/3rdparty/libprocess/include/process/http.hpp @@ -431,7 +431,7 @@ public: : authScheme_(authScheme), authParam_(authParam) {} - static Try<WWWAuthenticate> create(const std::string& value); + static Try<WWWAuthenticate> create(const std::string& input); std::string authScheme(); hashmap<std::string, std::string> authParam(); diff --git a/3rdparty/libprocess/src/http.cpp b/3rdparty/libprocess/src/http.cpp index e9d4392..0726a8b 100644 --- a/3rdparty/libprocess/src/http.cpp +++ b/3rdparty/libprocess/src/http.cpp @@ -635,28 +635,99 @@ Future<Nothing> Pipe::Writer::readerClosed() const namespace header { -Try<WWWAuthenticate> WWWAuthenticate::create(const string& value) +Try<WWWAuthenticate> WWWAuthenticate::create(const string& input) { // Set `maxTokens` as 2 since auth-param quoted string may // contain space (e.g., "Basic realm="Registry Realm"). - vector<string> tokens = strings::tokenize(value, " ", 2); + vector<string> tokens = strings::tokenize(input, " ", 2); if (tokens.size() != 2) { - return Error("Unexpected WWW-Authenticate header format: '" + value + "'"); + return Error("Unexpected WWW-Authenticate header format: '" + input + "'"); } + // Since the authentication parameters can contain quote values, we + // do not use `strings::split` here since the delimiter may occur in + // a quoted value which should not be split. hashmap<string, string> authParam; - foreach (const string& token, strings::split(tokens[1], ",")) { - vector<string> split = strings::split(token, "="); - if (split.size() != 2) { - return Error( - "Unexpected auth-param format: '" + - token + "' in '" + tokens[1] + "'"); - } + Option<string> key, value; + bool inQuotes = false; + foreach (char c, tokens[1]) { // Auth-param values can be a quoted-string or directive values. // Please see section "3.2.2.4 Directive values and quoted-string": // https://tools.ietf.org/html/rfc2617. - authParam[split[0]] = strings::trim(split[1], strings::ANY, "\""); + // + // If we see a quote we know we must already be parsing `value` + // since `key` cannot be a quoted-string. + if (c != '"' && inQuotes) { + if (value.isNone()) { + return Error("Unexpected auth-param format: '" + tokens[1] + "'"); + } + + value->append({c}); + continue; + } + + // If we have not yet parsed `key` this character must belong to + // it if it is not a space, and cannot be a `,` delimiter. + if (key.isNone()) { + if (c == ',') { + return Error("Unexpected auth-param format: '" + tokens[1] + "'"); + } + + if (c == ' ') { + continue; + } + + key = string({c}); + continue; + } + + // If the current character is `=` we must start parsing a new + // `value`. Since we have already handled `=` in quotes above we + // cannot already have started parsing `value`. + if (c == '=') { + if (value.isSome()) { + return Error("Unexpected auth-param format: '" + tokens[1] + "'"); + } + value = ""; + continue; + } + + // If the current character is a quote, drop the + // character and toogle the quote state. + if (c == '"') { + inQuotes = !inQuotes; + continue; + } + + // If the current character is a record delimiter and we are not + // in quotes, we should have parsed both a key and a value. Store + // them, drop the delimiter, and restart parsing. + if (c == ',' && !inQuotes) { + if (key.isNone() || value.isNone()) { + return Error("Unexpected auth-param format: '" + tokens[1] + "'"); + } + authParam[key.get()] = value.get(); + key = None(); + value = None(); + + continue; + } + + // If we have not started parsing `value` we are still parsing `key`. + if (value.isNone()) { + key->append({c}); + } else { + value->append({c}); + } + } + + // Record the last parsed `(key, value)` pair. + if (key.isSome()) { + if (value.isNone() || inQuotes) { + return Error("Unexpected auth-param format: '" + tokens[1] + "'"); + } + authParam[key.get()] = value.get(); } // The realm directive (case-insensitive) is required for all diff --git a/3rdparty/libprocess/src/tests/http_tests.cpp b/3rdparty/libprocess/src/tests/http_tests.cpp index 9451fed..7caaa2b 100644 --- a/3rdparty/libprocess/src/tests/http_tests.cpp +++ b/3rdparty/libprocess/src/tests/http_tests.cpp @@ -1580,7 +1580,7 @@ TEST_P(HTTPTest, WWWAuthenticateHeader) header = http::Headers( {{"Www-Authenticate", - "Bearer realm=\"https://auth.docker.io/token\"," + "Bearer realm=\"https://auth.docker.io/token\", " "service=\"registry.docker.io\"," "scope=\"repository:gilbertsong/inky:pull\""}}) .get<http::header::WWWAuthenticate>(); @@ -1628,6 +1628,35 @@ TEST_P(HTTPTest, WWWAuthenticateHeader) {{"Www-Authenticate", "Digest uri=\"/dir/index.html\",qop=auth"}}) .get<http::header::WWWAuthenticate>()); + + EXPECT_ERROR( + http::Headers( + {{"Www-Authenticate", + "Bearer =\"https://https://example.com\""}}) + .get<http::header::WWWAuthenticate>()); + + // authParam keys cannot be quoted strings. + EXPECT_ERROR( + http::Headers( + {{"Www-Authenticate", + "Bearer \"realm\"=\"https://example.com\""}}) + .get<http::header::WWWAuthenticate>()); + + // We do not incorrectly split if authParam values contain `=` + // delimiters inside quotes. This is a regression test for MESOS-9968. + header = http::Headers( + {{"Www-Authenticate", + "Bearer realm=" + "\"https://nvcr.io/proxy_auth?scope=" + "repository:nvidia/tensorflow:pull,push\""}}) + .get<http::header::WWWAuthenticate>(); + + ASSERT_SOME(header); + EXPECT_EQ("Bearer", header->authScheme()); + ASSERT_EQ(hashset<string>{"realm"}, header->authParam().keys()); + EXPECT_EQ( + "https://nvcr.io/proxy_auth?scope=repository:nvidia/tensorflow:pull,push", + header->authParam()["realm"]); }