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 e07405e398e9aa932395709a2b4b6b2ab5808eb9 Author: Andrei Sekretenko <asekrete...@mesosphere.com> AuthorDate: Thu Jan 30 14:26:38 2020 +0100 Added workaround for Docker repositories not providing scope/service. This patch adds a fallback Docker authorization server URI generation mechanism (see MESOS-10092) for repository servers that provide no "scope"/"service" params in the "WWW-Authenticate" header of the initial "401 Unathorized" response. Review: https://reviews.apache.org/r/72079 --- src/uri/fetchers/docker.cpp | 163 ++++++++++++++++++++++++++++++++------------ src/uri/fetchers/docker.hpp | 9 ++- 2 files changed, 128 insertions(+), 44 deletions(-) diff --git a/src/uri/fetchers/docker.cpp b/src/uri/fetchers/docker.cpp index 9a64021..fbacabf 100644 --- a/src/uri/fetchers/docker.cpp +++ b/src/uri/fetchers/docker.cpp @@ -450,6 +450,12 @@ static URI constructRegistryUri(const URI& imageUri, string&& path) } +static URI getRegistryRootUri(const URI& imageUri) +{ + return constructRegistryUri(imageUri, "/v2"); +} + + static URI getManifestUri(const URI& imageUri) { return constructRegistryUri( @@ -467,56 +473,40 @@ static URI getBlobUri(const URI& imageUri) } -// Obtains URI of the V2 authorization service based on the -// initial "401 unauthorized" response of the Docker V2 registry. -static Future<string> getAuthServiceUri(const http::Response& initialResponse) +// Validates that the response contains WWW-Authenticate header with +// a scheme BEARER and, if so, extracts parameters from the header. +// Otherwise, returns Error. +static Try<hashmap<string, string>> getBearerAuthParam( + const URI& uri, + const http::Response& response) { Result<http::header::WWWAuthenticate> header = - initialResponse.headers.get<http::header::WWWAuthenticate>(); + response.headers.get<http::header::WWWAuthenticate>(); if (header.isError()) { - return Failure("Failed to get WWW-Authenticate header: " + header.error()); + return Error( + "Failed to get WWW-Authenticate header from " + stringify(uri) + + ": " + header.error()); } else if (header.isNone()) { - return Failure("Unexpected empty WWW-Authenticate header"); + return Error( + "Got unexpected empty WWW-Authenticate header from " + stringify(uri)); } // According to RFC, auth scheme should be case insensitive. const string authScheme = strings::upper(header->authScheme()); - - if (authScheme == "BASIC") { - return Failure( - "Unexpected BASIC Authorization response status: " + - initialResponse.status); + if (authScheme == "BASIC"){ + return Error( + "Got unexpected BASIC Authorization response status: " + + response.status + " from " + stringify(uri)); } if (authScheme != "BEARER") { - return Failure("Unsupported auth-scheme: " + authScheme); + return Error( + "Got unsupported auth-scheme: " + authScheme + + " from " + stringify(uri)); } - hashmap<string, string> authParam = header->authParam(); - - // `authParam` is supposed to contain the 'realm', 'service' - // and 'scope' information for bearer authentication. - // - // TODO(asekretenko): Fall back to querying repository root and - // constructing scope if one of these is missing (see MESOS-10092). - if (!authParam.contains("realm")) { - return Failure("Missing 'realm' in WWW-Authenticate header"); - } - - if (!authParam.contains("service")) { - return Failure("Missing 'service' in WWW-Authenticate header"); - } - - if (!authParam.contains("scope")) { - return Failure("Missing 'scope' in WWW-Authenticate header"); - } - - // TODO(jieyu): Currently, we don't expect the auth server to return - // a service or a scope that needs encoding. - return authParam.at("realm") + "?" + - "service=" + authParam.at("service") + "&" + - "scope=" + authParam.at("scope"); + return header->authParam(); } @@ -529,10 +519,12 @@ class DockerFetcherPluginProcess : public Process<DockerFetcherPluginProcess> public: DockerFetcherPluginProcess( const hashmap<string, spec::Config::Auth>& _auths, - const Option<Duration>& _stallTimeout) + const Option<Duration>& _stallTimeout, + bool _enableAuthServiceUriFallback) : ProcessBase(process::ID::generate("docker-fetcher-plugin")), auths(_auths), - stallTimeout(_stallTimeout) {} + stallTimeout(_stallTimeout), + enableAuthServiceUriFallback(_enableAuthServiceUriFallback) {} Future<Nothing> fetch( const URI& uri, @@ -585,9 +577,16 @@ private: vector<string> urls); #endif + Future<string> getAuthServiceUri( + const string& repository, + const URI& initialUri, + const http::Response& initialResponse, + const http::Headers& basicAuthHeaders) const; + // Returns a token-based authorization header. Basic authorization // header may be required to get a proper authorization token. Future<http::Headers> getAuthHeader( + const string& repository, const URI& uri, const http::Headers& basicAuthHeaders, const http::Response& response); @@ -599,6 +598,10 @@ private: // Timeout for curl to wait when a net download stalls. const Option<Duration> stallTimeout; + + // Disables auth server URI generation (see MESOS-10092). + // Used for tests only. + const bool enableAuthServiceUriFallback; }; @@ -619,7 +622,9 @@ DockerFetcherPlugin::Flags::Flags() const char DockerFetcherPlugin::NAME[] = "docker"; -Try<Owned<Fetcher::Plugin>> DockerFetcherPlugin::create(const Flags& flags) +Try<Owned<Fetcher::Plugin>> DockerFetcherPlugin::create( + const Flags& flags, + bool enableAuthServiceUriFallback) { // TODO(jieyu): Make sure curl is available. @@ -637,7 +642,8 @@ Try<Owned<Fetcher::Plugin>> DockerFetcherPlugin::create(const Flags& flags) Owned<DockerFetcherPluginProcess> process(new DockerFetcherPluginProcess( hashmap<string, spec::Config::Auth>(auths), - flags.docker_stall_timeout)); + flags.docker_stall_timeout, + enableAuthServiceUriFallback)); return Owned<Fetcher::Plugin>(new DockerFetcherPlugin(process)); } @@ -801,7 +807,7 @@ Future<Nothing> DockerFetcherPluginProcess::_fetch( { if (response.code == http::Status::UNAUTHORIZED) { // Use the 'Basic' credential to request an auth token by default. - return getAuthHeader(manifestUri, basicAuthHeaders, response) + return getAuthHeader(uri.path(), manifestUri, basicAuthHeaders, response) .then(defer(self(), [=]( const http::Headers& authHeaders) -> Future<Nothing> { return curl(manifestUri, manifestHeaders + authHeaders, stallTimeout) @@ -1020,7 +1026,7 @@ Future<Nothing> DockerFetcherPluginProcess::_fetchBlob( "but get '" + response.status + "' instead"); } - return getAuthHeader(blobUri, basicAuthHeaders, response) + return getAuthHeader(uri.path(), blobUri, basicAuthHeaders, response) .then(defer(self(), [=]( const http::Headers& authHeaders) -> Future<Nothing> { return download( @@ -1120,6 +1126,76 @@ Future<Nothing> DockerFetcherPluginProcess::_urlFetchBlob( } #endif +// Tries to obtain URI of the V2 authorization service based on the +// "realm", "scope" and "service" auth params from the +// initial "401 unauthorized" response of the Docker V2 registry +// (see details in https://docs.docker.com/registry/spec/auth/token). +// +// If any of the params are missing from the "WWW-Authenticate" header, this +// function falls back to the scheme implemented in Docker image puller (see +// MESOS-10092): it queries the registry root URI (as opposed to manifest/blob +// URI) to get the "WWW-Authenticate" header with "realm", and composes the +// repository scope on its own. Scope grammar and semantics are documented in +// https://docs.docker.com/registry/spec/auth/scope . +Future<string> DockerFetcherPluginProcess::getAuthServiceUri( + const string& repository, + const URI& initialUri, + const http::Response& initialResponse, + const http::Headers& basicAuthHeaders) const +{ + const Try<hashmap<string, string>> authParam = + getBearerAuthParam(initialUri, initialResponse); + + if (authParam.isError()) { + LOG(WARNING) << authParam.error(); + return Failure(authParam.error()); + } + + // `authParam` is supposed to contain the 'realm', 'service' + // and 'scope' information for bearer authentication. + if (authParam->contains("realm") && + authParam->contains("service") && + authParam->contains("scope")) { + // TODO(jieyu): Currently, we don't expect the auth server to return + // a service or a scope that needs encoding. + return authParam->at("realm") + "?" + + "service=" + authParam->at("service") + "&" + + "scope=" + authParam->at("scope"); + } + + const string msg = + "Missing 'realm', 'service' or 'scope' in header WWW-Authenticate: " + + initialResponse.headers.at("WWW-Authenticate"); + + if (!enableAuthServiceUriFallback) { + return Failure(msg); + } + + LOG(WARNING) << msg; + + const URI registryRootUri = getRegistryRootUri(initialUri); + return curl(registryRootUri, basicAuthHeaders, stallTimeout) + .then([repository, registryRootUri](const http::Response& rootResponse) + -> Future<string> { + const Try<hashmap<string, string>> authParam = + getBearerAuthParam(registryRootUri, rootResponse); + + if (authParam.isError()) { + LOG(WARNING) << authParam.error(); + return Failure(authParam.error()); + } + + if (!authParam->contains("realm")) { + return Failure( + "Missing 'realm' in WWW-Authenticate header obtained from " + + stringify(registryRootUri)); + } + + return authParam->at("realm") + "?scope=repository:" + repository + + ":pull"; + }); +} + // If a '401 Unauthorized' response is received and the auth-scheme // is 'Bearer', we expect a header 'Www-Authenticate' containing the @@ -1131,13 +1207,14 @@ Future<Nothing> DockerFetcherPluginProcess::_urlFetchBlob( // See details here: // https://docs.docker.com/registry/spec/auth/token/ Future<http::Headers> DockerFetcherPluginProcess::getAuthHeader( + const string& repository, const URI& uri, const http::Headers& basicAuthHeaders, const http::Response& response) { const auto stallTimeout = this->stallTimeout; - return getAuthServiceUri(response) + return getAuthServiceUri(repository, uri, response, basicAuthHeaders) .then([basicAuthHeaders, stallTimeout](const string& authServiceUri) { return curl(authServiceUri, basicAuthHeaders, stallTimeout) .then([authServiceUri](const http::Response& response) diff --git a/src/uri/fetchers/docker.hpp b/src/uri/fetchers/docker.hpp index 2abe735..03a91af 100644 --- a/src/uri/fetchers/docker.hpp +++ b/src/uri/fetchers/docker.hpp @@ -45,7 +45,14 @@ public: static const char NAME[]; - static Try<process::Owned<Fetcher::Plugin>> create(const Flags& flags); + // `enableAuthServiceUriFallback` switches on the fallback auth service URI + // generation scheme for V2 registries that do not provide 'service'/'scope' + // parameters in the initial 'WWW-Authenticate' header. + // + // NOTE: switching the fallback off is required for testing purposes. + static Try<process::Owned<Fetcher::Plugin>> create( + const Flags& flags, + bool enableAuthServiceUriFallback = true); static std::string getBlobPath( const std::string& directory,