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..ae592aafac8981 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,13 @@ 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 +166,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,13 +178,13 @@ 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). */ @@ -192,7 +194,7 @@ public abstract static class UnresolvedScopedCredentialHelper { 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 = @@ -200,7 +202,11 @@ public static final class UnresolvedScopedCredentialHelperConverter @Override public String getTypeDescription() { - return "An (unresolved) path to a credential helper for a scope."; + return "Path to a credential helper. It may be absolute, relative to the PATH environment" + + " variable, or %workspace%-relative. The path be optionally prefixed by a scope " + + " followed by an '='. The scope is a domain name, optionally with a single leading '*'" + + " wildcard component. A helper applies to URIs matching its scope, with more specific" + + " scopes preferred. If a helper has no scope, it applies to every URI."; } @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/authandtls/credentialhelper/BUILD b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/BUILD index 8a8a511068fbcc..1c95c60cdaf7da 100644 --- a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/BUILD +++ b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/BUILD @@ -17,6 +17,7 @@ java_library( deps = [ "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib/authandtls", + "//src/main/java/com/google/devtools/common/options", "//third_party:caffeine", "//third_party:guava", ], diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialModule.java b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialModule.java index f18aa2d338579e..ba880430b79bb4 100644 --- a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialModule.java +++ b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialModule.java @@ -21,6 +21,7 @@ import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions; import com.google.devtools.build.lib.runtime.BlazeModule; import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.common.options.OptionsBase; import java.net.URI; import java.time.Duration; @@ -37,6 +38,11 @@ public Cache>> getCredentialCach return credentialCache; } + @Override + public Iterable> getCommonCommandOptions() { + return ImmutableList.of(AuthAndTLSOptions.class); + } + @Override public void beforeCommand(CommandEnvironment env) { // Update the cache expiration policy according to the command options. 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 22c366230dbd89..3c29ea712d2909 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/BUILD @@ -26,6 +26,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", @@ -59,6 +62,7 @@ java_library( "//src/main/java/com/google/devtools/common/options", "//src/main/protobuf:failure_details_java_proto", "//third_party:guava", + "//third_party:jsr305", ], ) 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 26d05bbbef501f..5e87645a450f50 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,8 @@ 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 +274,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 +386,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..f8c88defa99f73 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 @@ -61,6 +61,12 @@ public class DownloadManager { private int retries = 0; private boolean urlsAsDefaultCanonicalId; @Nullable private Credentials netrcCreds; + private CredentialFactory credentialFactory = StaticCredentials::new; + + /** Creates {@code Credentials} from a map of per-{@code URI} authentication headers. */ + public interface CredentialFactory { + Credentials create(Map>> authHeaders); + } public DownloadManager(RepositoryCache repositoryCache, Downloader downloader) { this.repositoryCache = repositoryCache; @@ -92,6 +98,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 +267,7 @@ public Path download( try { downloader.download( rewrittenUrls, - new StaticCredentials(rewrittenAuthHeaders), + credentialFactory.create(rewrittenAuthHeaders), checksum, canonicalId, destination, @@ -338,7 +348,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; diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index 99c919d2e4cdf4..2bced000af26af 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/java/com/google/devtools/build/lib/buildtool/util/BUILD b/src/test/java/com/google/devtools/build/lib/buildtool/util/BUILD index 09ba9771ffbd1f..bd188c69f5740e 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/util/BUILD +++ b/src/test/java/com/google/devtools/build/lib/buildtool/util/BUILD @@ -50,6 +50,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:top_level_artifact_context", "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_collection", "//src/main/java/com/google/devtools/build/lib/analysis:workspace_status_action", + "//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module", "//src/main/java/com/google/devtools/build/lib/bazel:main", "//src/main/java/com/google/devtools/build/lib/bazel:repository_module", "//src/main/java/com/google/devtools/build/lib/bugreport", diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java b/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java index 1d4cea1c13df02..2ba922485a8942 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java @@ -53,6 +53,7 @@ import com.google.devtools.build.lib.analysis.util.AnalysisMock; import com.google.devtools.build.lib.analysis.util.AnalysisTestUtil; import com.google.devtools.build.lib.analysis.util.AnalysisTestUtil.DummyWorkspaceStatusActionContext; +import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialModule; import com.google.devtools.build.lib.bazel.BazelRepositoryModule; import com.google.devtools.build.lib.bugreport.BugReport; import com.google.devtools.build.lib.bugreport.BugReporter; @@ -530,7 +531,8 @@ protected BlazeRuntime.Builder getRuntimeBuilder() throws Exception { .addBlazeModule(new BuildIntegrationTestCommandsModule()) .addBlazeModule(new OutputFilteringModule()) .addBlazeModule(connectivityModule) - .addBlazeModule(getMockBazelRepositoryModule()); + .addBlazeModule(getMockBazelRepositoryModule()) + .addBlazeModule(new CredentialModule()); getSpawnModules().forEach(builder::addBlazeModule); builder .addBlazeModule(getBuildInfoModule()) diff --git a/src/test/shell/bazel/remote_helpers.sh b/src/test/shell/bazel/remote_helpers.sh index 78031a66a30a98..aaad923b164443 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 1f526efcbdeb4c..84f933fb1b6011 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,51 @@ genrule( cmd = "cp $< $@", ) EOF +} + +function test_auth_from_starlark() { + setup_auth foo bar + bazel build //:it \ - || fail "Expected success despite needing a file behind basic auth" + || fail "Expected success when downloading repo with basic auth" +} + +function test_auth_from_credential_helper() { + if "$is_windows"; then + # Skip on Windows: credential helper is a Python script. + return + fi + + 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() { + if "$is_windows"; then + # Skip on Windows: credential helper is a Python script. + return + fi + + 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 +2147,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 @@ -2118,6 +2187,87 @@ EOF || fail "Expected success despite needing a file behind basic auth" } +function test_http_archive_credential_helper() { + if "$is_windows"; then + # Skip on Windows: credential helper is a Python script. + return + fi + + 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() { + if "$is_windows"; then + # Skip on Windows: credential helper is a Python script. + return + fi + + 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..00c714792d2d95 100644 --- a/src/test/shell/bazel/testing_server.py +++ b/src/test/shell/bazel/testing_server.py @@ -94,7 +94,9 @@ 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:]