You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bb...@apache.org on 2019/09/30 12:52:33 UTC
[mesos] 02/03: Fixed parsing of HTTP authentication headers.
This is an automated email from the ASF dual-hosted git repository.
bbannier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 19280bccfc9d8a02216ec7b912f59973882b74a7
Author: Benjamin Bannier <bb...@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 654bbc2..a0065db 100644
--- a/3rdparty/libprocess/include/process/http.hpp
+++ b/3rdparty/libprocess/include/process/http.hpp
@@ -434,7 +434,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 0ed41aa..b487ce2 100644
--- a/3rdparty/libprocess/src/http.cpp
+++ b/3rdparty/libprocess/src/http.cpp
@@ -637,28 +637,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] = *value;
+ 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] = *value;
}
// 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 b0f1272..1433f3d 100644
--- a/3rdparty/libprocess/src/tests/http_tests.cpp
+++ b/3rdparty/libprocess/src/tests/http_tests.cpp
@@ -1628,7 +1628,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>();
@@ -1676,6 +1676,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"]);
}