Wire up --incompatible_disallow_unverified_http_downloads for maven_server Force usage of either HTTPS or HTTP w/ SHA-1. Note that SHA-1 is still susceptible to collision attacks, but this should reduce the exploitable surface of the current implementation that allows plain HTTP without checksums. Also see https://github.com/bazelbuild/bazel/issues/6799#issuecomment-524382068 Closes #9235. RELNOTES: `maven_jar` and `maven_server` now disallow using plain HTTP URLs without a specified checksum. If you are still using `maven_jar`, consider migrating to [`rules_jvm_external`](https://github.com/bazelbuild/rules_jvm_external) for transitive dependency management. See [#8607](https://github.com/bazelbuild/bazel/issues/8607) for more information. Change-Id: I61b96b1d797071dc84291fecbf05a45d927240a5 PiperOrigin-RevId: 265442213
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java index f3ede2c..37eba14 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java
@@ -14,6 +14,7 @@ package com.google.devtools.build.lib.bazel.repository; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Optional; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.RuleDefinition; @@ -129,10 +130,35 @@ return null; } + validateServerUrl( + rule, + serverValue.getUrl(), + starlarkSemantics.incompatibleDisallowUnverifiedHttpDownloads()); + Path outputDir = getExternalRepositoryDirectory(directories).getRelative(rule.getName()); return createOutputTree(rule, outputDir, serverValue, env.getListener()); } + @VisibleForTesting + void validateServerUrl(Rule rule, String serverUrl, boolean disallowUnverifiedHttpDownloads) + throws RepositoryFunctionException { + + boolean hasChecksum = + WorkspaceAttributeMapper.of(rule).isAttributeValueExplicitlySpecified("sha1"); + + if (disallowUnverifiedHttpDownloads && !hasChecksum && serverUrl.startsWith("http://")) { + throw new RepositoryFunctionException( + new EvalException( + rule.getLocation(), + "Plain HTTP URLs are not allowed without checksums in the maven_jar rule. Please " + + "use HTTPS for the maven_server rule for " + + serverUrl + + " or add a sha1 checksum to the maven_jar rule. To disable this check, pass " + + "--incompatible_disallow_unverified_http_downloads=false to your build"), + Transience.PERSISTENT); + } + } + private void createDirectory(Path path) throws RepositoryFunctionException { try { FileSystemUtils.createDirectoryAndParents(path);
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenServerFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenServerFunction.java index 29ec6d5..367a64f 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenServerFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenServerFunction.java
@@ -52,9 +52,7 @@ import org.apache.maven.settings.building.SettingsBuildingException; import org.apache.maven.settings.building.SettingsBuildingResult; -/** - * Implementation of maven_repository. - */ +/** Implementation of maven_server. */ public class MavenServerFunction implements SkyFunction { public static final SkyFunctionName NAME = SkyFunctionName.createHermetic("MAVEN_SERVER_FUNCTION");
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunctionTest.java index 9cc82dc..18bfaea 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunctionTest.java
@@ -25,6 +25,7 @@ import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException; import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper; import java.io.IOException; import org.apache.maven.settings.Server; @@ -135,4 +136,26 @@ assertThat(expected.getMessage()).contains("Failed to fetch Maven dependency:"); } } + + @Test + public void testValidateHttpServerUrlWithNoChecksum() throws Exception { + Rule rule = + scratchRule( + "external", + "foo", + "maven_jar(", + " name = 'foo',", + " artifact = 'x:y:z:1.1',", + ")"); + MavenDownloader downloader = new MavenDownloader(Mockito.mock(RepositoryCache.class)); + MavenJarFunction jarFunction = new MavenJarFunction(downloader); + + try { + jarFunction.validateServerUrl( + rule, TEST_SERVER.getUrl(), /*disallowUnverifiedHttpDownloads=*/ true); + } catch (RepositoryFunctionException expected) { + assertThat(expected.getMessage()) + .contains("Plain HTTP URLs are not allowed without checksums"); + } + } }
diff --git a/src/test/shell/bazel/maven_test.sh b/src/test/shell/bazel/maven_test.sh index 0c625ad..25833941 100755 --- a/src/test/shell/bazel/maven_test.sh +++ b/src/test/shell/bazel/maven_test.sh
@@ -100,8 +100,8 @@ ) EOF - bazel run //zoo:ball-pit >& $TEST_log || fail "Expected run to succeed" - expect_log "Tra-la!" + bazel run //zoo:ball-pit >& $TEST_log && fail "Expected run to fail" + expect_log "Plain HTTP URLs are not allowed without checksums" } # makes sure both jar and srcjar are downloaded @@ -114,6 +114,8 @@ name = 'endangered', artifact = "com.example.carnivore:carnivore:1.23", repository = 'http://127.0.0.1:$fileserver_port/', + sha1 = '$sha1', + sha1_src = '$sha1_src', ) EOF @@ -129,11 +131,13 @@ setup_zoo serve_not_found + some_sha1="0123456789012345678901234567890123456789" cat >> $(create_workspace_with_default_repos WORKSPACE) <<EOF maven_jar( name = 'endangered', artifact = "com.example.carnivore:carnivore:1.23", repository = 'http://127.0.0.1:$nc_port/', + sha1 = '$some_sha1', ) EOF @@ -172,6 +176,7 @@ maven_jar( name = "thing_a_ma_bop", artifact = "thing:amabop:1.9", + sha1 = '$sha1', ) EOF @@ -191,6 +196,7 @@ name = "thing_a_ma_bop", artifact = "thing:amabop:1.9", server = "x", + sha1 = '$sha1', ) EOF @@ -250,7 +256,7 @@ function test_auth() { startup_auth_server - create_artifact thing amabop 1.9 + create_artifact com.example.carnivore carnivore 1.23 cat >> $(create_workspace_with_default_repos WORKSPACE) <<EOF maven_server( name = "x", @@ -259,8 +265,9 @@ ) maven_jar( name = "good_auth", - artifact = "thing:amabop:1.9", + artifact = "com.example.carnivore:carnivore:1.23", server = "x", + sha1 = "$sha1", ) maven_server( @@ -270,8 +277,9 @@ ) maven_jar( name = "bad_auth", - artifact = "thing:amabop:1.9", + artifact = "com.example.carnivore:carnivore:1.23", server = "y", + sha1 = "$sha1", ) EOF @@ -292,11 +300,11 @@ </settings> EOF - bazel build @good_auth//jar &> $TEST_log \ + bazel build --repository_cache="" @good_auth//jar &> $TEST_log \ || fail "Expected correct password to work" expect_log "Target @good_auth//jar:jar up-to-date" - bazel build @bad_auth//jar &> $TEST_log \ + bazel build --repository_cache="" @bad_auth//jar &> $TEST_log \ && fail "Expected incorrect password to fail" expect_log "Unauthorized (401)" }