diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/AuthAndTLSOptions.java b/src/main/java/com/google/devtools/build/lib/authandtls/AuthAndTLSOptions.java index 7423b555a13e9c..fcdea3e176c72d 100644 --- a/src/main/java/com/google/devtools/build/lib/authandtls/AuthAndTLSOptions.java +++ b/src/main/java/com/google/devtools/build/lib/authandtls/AuthAndTLSOptions.java @@ -148,11 +148,12 @@ public class AuthAndTLSOptions extends OptionsBase { documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, effectTags = {OptionEffectTag.UNKNOWN}, help = - "Configures Credential Helpers to use for retrieving credentials for the provided scope" - + " (domain).\n\n" - + "Credentials from Credential Helpers take precedence over credentials from" - + " --google_default_credentials, `--google_credentials`, or" - + " .netrc.\n\n" + "Configures a credential helper to use for retrieving authorization credentials for " + + " repository fetching, remote caching and execution, and the build event service.\n\n" + + "Credentials supplied by a helper take precedence over credentials supplied by" + + " --google_default_credentials, --google_credentials, a .netrc file, or the auth" + + " parameter to repository_ctx.download and repository_ctx.download_and_extract.\n\n" + + "May be specified multiple times to set up multiple helpers.\n\n" + "See https://github.com/bazelbuild/proposals/blob/main/designs/2022-06-07-bazel-credential-helpers.md" + " for details.") public List credentialHelpers; @@ -164,8 +165,8 @@ public class AuthAndTLSOptions extends OptionsBase { documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, effectTags = {OptionEffectTag.UNKNOWN}, help = - "Configures the timeout for the Credential Helper.\n\n" - + "Credential Helpers failing to respond within this timeout will fail the" + "Configures the timeout for a credential helper.\n\n" + + "Credential helpers failing to respond within this timeout will fail the" + " invocation.") public Duration credentialHelperTimeout; @@ -176,31 +177,43 @@ public class AuthAndTLSOptions extends OptionsBase { documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, effectTags = {OptionEffectTag.UNKNOWN}, help = - "Configures the duration for which credentials from Credential Helpers are cached.\n\n" + "The duration for which credentials supplied by a credential helper are cached.\n\n" + "Invoking with a different value will adjust the lifetime of preexisting entries;" + " pass zero to clear the cache. A clean command always clears the cache, regardless" + " of this flag.") public Duration credentialHelperCacheTimeout; - /** One of the values of the `--credential_helper` flag. */ + /** + * One of the values of the `--experimental_credential_helper` flag. + */ @AutoValue public abstract static class UnresolvedScopedCredentialHelper { - /** Returns the scope of the credential helper (if any). */ + + /** + * Returns the scope of the credential helper (if any). + */ public abstract Optional getScope(); - /** Returns the (unparsed) path of the credential helper. */ + /** + * Returns the (unparsed) path of the credential helper. + */ public abstract String getPath(); } - /** A {@link Converter} for the `--credential_helper` flag. */ + /** + * A {@link Converter} for the `--experimental_credential_helper` flag. + */ public static final class UnresolvedScopedCredentialHelperConverter extends Converter.Contextless { + public static final UnresolvedScopedCredentialHelperConverter INSTANCE = new UnresolvedScopedCredentialHelperConverter(); @Override public String getTypeDescription() { - return "An (unresolved) path to a credential helper for a scope."; + return "Path to a credential helper, optionally prefixed by a scope (domain name) followed" + + " an '='. The path may be absolute or relative. If no scope is provided, the helper" + + " has universal scope."; } @Override diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java b/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java index e226e6b069db20..409263929c7cdc 100644 --- a/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java +++ b/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java @@ -372,7 +372,6 @@ static Optional newCredentialsFromNetrc( } } - @VisibleForTesting public static CredentialHelperProvider newCredentialHelperProvider( CredentialHelperEnvironment environment, CommandLinePathFactory pathFactory, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/BUILD index c66da5a1ec31a4..5cdf2e3c847476 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/BUILD @@ -27,6 +27,9 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories", "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration", + "//src/main/java/com/google/devtools/build/lib/authandtls", + "//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper", + "//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:inspection", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:inspection_impl", @@ -55,6 +58,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository", "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception", "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code", + "//src/main/java/com/google/devtools/build/lib/util:exit_code", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/common/options", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java index da982ee79aacd2..078fc8094bf97a 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.bazel; +import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; @@ -25,6 +26,13 @@ import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; +import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions; +import com.google.devtools.build.lib.authandtls.GoogleAuthUtils; +import com.google.devtools.build.lib.authandtls.StaticCredentials; +import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperCredentials; +import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperEnvironment; +import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperProvider; +import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialModule; import com.google.devtools.build.lib.bazel.bzlmod.AttributeValues; import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphFunction; import com.google.devtools.build.lib.bazel.bzlmod.BazelLockFileFunction; @@ -110,6 +118,7 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; +import javax.annotation.Nullable; /** Adds support for fetching external code. */ public class BazelRepositoryModule extends BlazeModule { @@ -146,6 +155,9 @@ public class BazelRepositoryModule extends BlazeModule { private List allowedYankedVersions = ImmutableList.of(); private SingleExtensionEvalFunction singleExtensionEvalFunction; + @Nullable + private CredentialModule credentialModule; + public BazelRepositoryModule() { this.starlarkRepositoryFunction = new StarlarkRepositoryFunction(downloadManager); this.repositoryHandlers = repositoryRules(); @@ -263,6 +275,8 @@ SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(directories.getWorkspace .addSkyFunction(SkyFunctions.SINGLE_EXTENSION_EVAL, singleExtensionEvalFunction) .addSkyFunction(SkyFunctions.SINGLE_EXTENSION_USAGES, new SingleExtensionUsagesFunction()); filesystem = runtime.getFileSystem(); + + credentialModule = Preconditions.checkNotNull(runtime.getBlazeModule(CredentialModule.class)); } @Override @@ -373,6 +387,41 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { Code.BAD_DOWNLOADER_CONFIG)); } + try { + AuthAndTLSOptions authAndTlsOptions = env.getOptions().getOptions(AuthAndTLSOptions.class); + var credentialHelperEnvironment = + CredentialHelperEnvironment.newBuilder() + .setEventReporter(env.getReporter()) + .setWorkspacePath(env.getWorkspace()) + .setClientEnvironment(env.getClientEnv()) + .setHelperExecutionTimeout(authAndTlsOptions.credentialHelperTimeout) + .build(); + CredentialHelperProvider credentialHelperProvider = + GoogleAuthUtils.newCredentialHelperProvider( + credentialHelperEnvironment, + env.getCommandLinePathFactory(), + authAndTlsOptions.credentialHelpers); + + downloadManager.setCredentialFactory(headers -> { + Preconditions.checkNotNull(headers); + + return new CredentialHelperCredentials( + credentialHelperProvider, + credentialHelperEnvironment, + credentialModule.getCredentialCache(), + Optional.of(new StaticCredentials(headers))); + }); + } catch (IOException e) { + env.getReporter().handle(Event.error(e.getMessage())); + env.getBlazeModuleEnvironment() + .exit( + new AbruptExitException( + detailedExitCode( + "Error initializing credential helper", + Code.CREDENTIALS_INIT_FAILURE))); + return; + } + if (repoOptions.experimentalDistdir != null) { downloadManager.setDistdir( repoOptions.experimentalDistdir.stream() diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java index 05250dc08608a1..0749772f877283 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java @@ -20,6 +20,7 @@ import com.google.auth.Credentials; import com.google.common.base.MoreObjects; import com.google.common.base.Optional; +import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -61,6 +62,7 @@ public class DownloadManager { private int retries = 0; private boolean urlsAsDefaultCanonicalId; @Nullable private Credentials netrcCreds; + private CredentialFactory credentialFactory = new DefaultCredentialFactory(); public DownloadManager(RepositoryCache repositoryCache, Downloader downloader) { this.repositoryCache = repositoryCache; @@ -92,6 +94,10 @@ public void setNetrcCreds(Credentials netrcCreds) { this.netrcCreds = netrcCreds; } + public void setCredentialFactory(CredentialFactory credentialFactory) { + this.credentialFactory = credentialFactory; + } + /** * Downloads file to disk and returns path. * @@ -257,7 +263,7 @@ public Path download( try { downloader.download( rewrittenUrls, - new StaticCredentials(rewrittenAuthHeaders), + credentialFactory.create(rewrittenAuthHeaders), checksum, canonicalId, destination, @@ -338,7 +344,7 @@ public byte[] downloadAndReadOneUrl( for (int attempt = 0; attempt <= retries; ++attempt) { try { return httpDownloader.downloadAndReadOneUrl( - rewrittenUrls.get(0), new StaticCredentials(authHeaders), eventHandler, clientEnv); + rewrittenUrls.get(0), credentialFactory.create(authHeaders), eventHandler, clientEnv); } catch (ContentLengthMismatchException e) { if (attempt == retries) { throw e; @@ -427,4 +433,17 @@ public boolean isFinished() { return isFinished; } } + + public interface CredentialFactory { + Credentials create(Map>> authHeaders); + } + + private static final class DefaultCredentialFactory implements CredentialFactory { + @Override + public Credentials create(Map>> authHeaders) { + Preconditions.checkNotNull(authHeaders); + + return new StaticCredentials(authHeaders); + } + } } diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index ec3c67ea41e940..8646a14e54b7da 100644 --- a/src/main/protobuf/failure_details.proto +++ b/src/main/protobuf/failure_details.proto @@ -249,6 +249,7 @@ message ExternalRepository { OVERRIDE_DISALLOWED_MANAGED_DIRECTORIES = 1 [(metadata) = { exit_code: 2 }]; BAD_DOWNLOADER_CONFIG = 2 [(metadata) = { exit_code: 2 }]; REPOSITORY_MAPPING_RESOLUTION_FAILED = 3 [(metadata) = { exit_code: 37 }]; + CREDENTIALS_INIT_FAILURE = 4 [(metadata) = { exit_code: 2 }]; } Code code = 1; // Additional data could include external repository names. diff --git a/src/test/shell/bazel/remote_helpers.sh b/src/test/shell/bazel/remote_helpers.sh index b9819a7c76b039..6a66398d3eabd9 100755 --- a/src/test/shell/bazel/remote_helpers.sh +++ b/src/test/shell/bazel/remote_helpers.sh @@ -38,8 +38,7 @@ function serve_file() { cd - } -# Serves $1 as a file on localhost:$nc_port insisting on authentication (but -# accepting any credentials. +# Serves $1 as a file on localhost:$nc_port expecting authentication. # * nc_port - the port nc is listening on. # * nc_log - the path to nc's log. # * nc_pid - the PID of nc. diff --git a/src/test/shell/bazel/starlark_repository_test.sh b/src/test/shell/bazel/starlark_repository_test.sh index 9ffc42deebf531..c816ffe9e9c076 100755 --- a/src/test/shell/bazel/starlark_repository_test.sh +++ b/src/test/shell/bazel/starlark_repository_test.sh @@ -1713,38 +1713,64 @@ EOF expect_log "//:b.bzl" } -function test_auth_provided() { - mkdir x +# Common setup logic for test_auth_*. +# Receives as arguments the username and password for basic authentication. +# If no arguments are provided, no authentication is used. +function setup_auth() { + if [[ $# -ne 0 && $# -ne 2 ]]; then + fail "setup_auth expects exactly zero or two arguments" + fi + + mkdir -p x echo 'exports_files(["file.txt"])' > x/BUILD echo 'Hello World' > x/file.txt tar cvf x.tar x sha256="$(sha256sum x.tar | head -c 64)" serve_file_auth x.tar - cat >> $(create_workspace_with_default_repos WORKSPACE) < auth_attrs.bzl < auth_attrs.bzl <<'EOF' +AUTH_ATTRS = None +EOF + fi + cat > auth.bzl <<'EOF' +load("//:auth_attrs.bzl", "AUTH_ATTRS") def _impl(ctx): ctx.download_and_extract( url = ctx.attr.url, sha256 = ctx.attr.sha256, - # Use the username/password pair hard-coded - # in the testing server. - auth = {ctx.attr.url : { "type": "basic", - "login" : "foo", - "password" : "bar"}} + auth = { ctx.attr.url: AUTH_ATTRS } if AUTH_ATTRS else {}, ) -with_auth = repository_rule( +maybe_with_auth = repository_rule( implementation = _impl, - attrs = { "url" : attr.string(), "sha256" : attr.string() } + attrs = { + "url" : attr.string(), + "sha256" : attr.string(), + } ) EOF + + cat >> $(create_workspace_with_default_repos WORKSPACE) < BUILD <<'EOF' genrule( name = "it", @@ -1753,8 +1779,46 @@ genrule( cmd = "cp $< $@", ) EOF +} + +function test_auth_from_starlark() { + setup_auth # no auth + bazel build //:it \ - || fail "Expected success despite needing a file behind basic auth" + && fail "Expected failure when downloading repo without basic auth" + + setup_auth foo bar + + bazel build //:it \ + || fail "Expected success when downloading repo with basic auth" +} + +function test_auth_from_credential_helper() { + setup_credential_helper + + setup_auth # no auth + + bazel build //:it \ + && fail "Expected failure when downloading repo without credential helper" + + bazel build --experimental_credential_helper="${TEST_TMPDIR}/credhelper" //:it \ + || fail "Expected success when downloading repo with credential helper" + + expect_credential_helper_calls 1 + + bazel build --experimental_credential_helper="${TEST_TMPDIR}/credhelper" //:it \ + || fail "Expected success when downloading repo with credential helper" + + expect_credential_helper_calls 1 # expect credentials to have been cached +} + +function test_auth_from_credential_helper_overrides_starlark() { + setup_credential_helper + + setup_auth baduser badpass + + bazel build --experimental_credential_helper="${TEST_TMPDIR}/credhelper" //:it \ + || fail "Expected success when downloading repo with credential helper overriding basic auth" } function test_netrc_reading() { @@ -2078,7 +2142,7 @@ EOF || fail "Expected success despite needing a file behind bearer auth" } -function test_implicit_netrc() { +function test_http_archive_implicit_netrc() { mkdir x echo 'exports_files(["file.txt"])' > x/BUILD echo 'Hello World' > x/file.txt @@ -2093,7 +2157,7 @@ function test_implicit_netrc() { cat > .netrc <<'EOF' machine 127.0.0.1 login foo -password bar +password baz EOF mkdir main @@ -2118,6 +2182,77 @@ EOF || fail "Expected success despite needing a file behind basic auth" } +function test_http_archive_credential_helper() { + setup_credential_helper + + mkdir x + echo 'exports_files(["file.txt"])' > x/BUILD + echo 'Hello World' > x/file.txt + tar cvf x.tar x + sha256=$(sha256sum x.tar | head -c 64) + serve_file_auth x.tar + cat > WORKSPACE < BUILD <<'EOF' +genrule( + name = "it", + srcs = ["@ext//x:file.txt"], + outs = ["it.txt"], + cmd = "cp $< $@", +) +EOF + bazel build --experimental_credential_helper="${TEST_TMPDIR}/credhelper" //:it \ + || fail "Expected success despite needing a file behind credential helper" +} + +function test_http_archive_credential_helper_overrides_netrc() { + setup_credential_helper + + mkdir x + echo 'exports_files(["file.txt"])' > x/BUILD + echo 'Hello World' > x/file.txt + tar cvf x.tar x + sha256=$(sha256sum x.tar | head -c 64) + serve_file_auth x.tar + + export HOME=`pwd` + if "$is_windows"; then + export USERPROFILE="$(cygpath -m ${HOME})" + fi + cat > .netrc <<'EOF' +machine 127.0.0.1 +login badusername +password badpassword +EOF + + mkdir main + cd main + cat > WORKSPACE < BUILD <<'EOF' +genrule( + name = "it", + srcs = ["@ext//x:file.txt"], + outs = ["it.txt"], + cmd = "cp $< $@", +) +EOF + bazel build --experimental_credential_helper="${TEST_TMPDIR}/credhelper" //:it \ + || fail "Expected success despite needing a file behind credential helper" +} + function test_disable_download_should_prevent_downloading() { mkdir x echo 'exports_files(["file.txt"])' > x/BUILD diff --git a/src/test/shell/bazel/testing_server.py b/src/test/shell/bazel/testing_server.py index 9d6cd23a57ee88..d311ef1ff729df 100644 --- a/src/test/shell/bazel/testing_server.py +++ b/src/test/shell/bazel/testing_server.py @@ -94,7 +94,7 @@ def do_GET(self): # pylint: disable=invalid-name self.serve_file() else: self.do_AUTHHEAD() - self.wfile.write(b'Login required.' + str(auth_header)) + self.wfile.write("Bad authorization header: {}".format(auth_header).encode("ascii")) def serve_file(self): path_to_serve = self.path[1:]