Skip to content

Commit

Permalink
Wire up --incompatible_disallow_unverified_http_downloads for maven_s…
Browse files Browse the repository at this point in the history
…erver

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 #6799 (comment)

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](#8607) for more information.

Change-Id: I61b96b1d797071dc84291fecbf05a45d927240a5
PiperOrigin-RevId: 265442213
  • Loading branch information
jin authored and copybara-github committed Aug 26, 2019
1 parent 532778d commit 06d79dd
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -129,10 +130,35 @@ public RepositoryDirectoryValue.Builder fetch(
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -135,4 +136,26 @@ public void testNoSha1WithCache() throws Exception {
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");
}
}
}
22 changes: 15 additions & 7 deletions src/test/shell/bazel/maven_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ maven_jar(
)
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
Expand All @@ -114,6 +114,8 @@ maven_jar(
name = 'endangered',
artifact = "com.example.carnivore:carnivore:1.23",
repository = 'http://127.0.0.1:$fileserver_port/',
sha1 = '$sha1',
sha1_src = '$sha1_src',
)
EOF

Expand All @@ -129,11 +131,13 @@ function test_maven_jar_404() {
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

Expand Down Expand Up @@ -172,6 +176,7 @@ maven_server(
maven_jar(
name = "thing_a_ma_bop",
artifact = "thing:amabop:1.9",
sha1 = '$sha1',
)
EOF

Expand All @@ -191,6 +196,7 @@ maven_jar(
name = "thing_a_ma_bop",
artifact = "thing:amabop:1.9",
server = "x",
sha1 = '$sha1',
)
EOF

Expand Down Expand Up @@ -250,7 +256,7 @@ EOF

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",
Expand All @@ -259,8 +265,9 @@ maven_server(
)
maven_jar(
name = "good_auth",
artifact = "thing:amabop:1.9",
artifact = "com.example.carnivore:carnivore:1.23",
server = "x",
sha1 = "$sha1",
)
maven_server(
Expand All @@ -270,8 +277,9 @@ maven_server(
)
maven_jar(
name = "bad_auth",
artifact = "thing:amabop:1.9",
artifact = "com.example.carnivore:carnivore:1.23",
server = "y",
sha1 = "$sha1",
)
EOF

Expand All @@ -292,11 +300,11 @@ EOF
</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)"
}
Expand Down

0 comments on commit 06d79dd

Please sign in to comment.