Support subresource integrity format for `download()` checksums. Closes #4881 Closes #7208. PiperOrigin-RevId: 258577605
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/debug/WorkspaceRuleEvent.java b/src/main/java/com/google/devtools/build/lib/bazel/debug/WorkspaceRuleEvent.java index 8518488..5c17598 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/debug/WorkspaceRuleEvent.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/debug/WorkspaceRuleEvent.java
@@ -84,6 +84,7 @@ List<URL> urls, String output, String sha256, + String integrity, Boolean executable, String ruleLabel, Location location) { @@ -91,6 +92,7 @@ WorkspaceLogProtos.DownloadEvent.newBuilder() .setOutput(output) .setSha256(sha256) + .setIntegrity(integrity) .setExecutable(executable); for (URL u : urls) { e.addUrl(u.toString()); @@ -135,6 +137,7 @@ List<URL> urls, String output, String sha256, + String integrity, String type, String stripPrefix, String ruleLabel, @@ -143,6 +146,7 @@ WorkspaceLogProtos.DownloadAndExtractEvent.newBuilder() .setOutput(output) .setSha256(sha256) + .setIntegrity(integrity) .setType(type) .setStripPrefix(stripPrefix); for (URL u : urls) {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/debug/workspace_log.proto b/src/main/java/com/google/devtools/build/lib/bazel/debug/workspace_log.proto index a90f7d9..d6ce5b8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/debug/workspace_log.proto +++ b/src/main/java/com/google/devtools/build/lib/bazel/debug/workspace_log.proto
@@ -44,10 +44,12 @@ repeated string url = 1; // Output file string output = 2; - // sha256, if speficied + // sha256, if specified string sha256 = 3; // whether to make the resulting file executable bool executable = 4; + // checksum in Subresource Integrity format, if specified + string integrity = 5; } message ExtractEvent { @@ -70,6 +72,8 @@ string type = 4; // A directory prefix to strip from extracted files. string strip_prefix = 5; + // checksum in Subresource Integrity format, if specified + string integrity = 6; } // Information on "file" event in repository_ctx.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCache.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCache.java index 54de121..5b16cd83 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCache.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCache.java
@@ -37,7 +37,9 @@ /** The types of cache keys used. */ public enum KeyType { SHA1("SHA-1", "\\p{XDigit}{40}", "sha1", Hashing.sha1()), - SHA256("SHA-256", "\\p{XDigit}{64}", "sha256", Hashing.sha256()); + SHA256("SHA-256", "\\p{XDigit}{64}", "sha256", Hashing.sha256()), + SHA384("SHA-384", "\\p{XDigit}{96}", "sha384", Hashing.sha384()), + SHA512("SHA-512", "\\p{XDigit}{128}", "sha512", Hashing.sha512()); private final String stringRepr; private final String regexp; @@ -64,6 +66,10 @@ return hashFunction.newHasher(); } + public String getHashName() { + return hashName; + } + @Override public String toString() { return stringRepr;
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/Checksum.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/Checksum.java index 07ca16d..0c992bf 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/Checksum.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/Checksum.java
@@ -16,6 +16,7 @@ import com.google.common.hash.HashCode; import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache.KeyType; +import java.util.Base64; /** The content checksum for an HTTP download, which knows its own type. */ public class Checksum { @@ -35,6 +36,49 @@ return new Checksum(keyType, HashCode.fromString(hash)); } + /** Constructs a new Checksum from a hash in Subresource Integrity format. */ + public static Checksum fromSubresourceIntegrity(String integrity) { + Base64.Decoder decoder = Base64.getDecoder(); + KeyType keyType = null; + byte[] hash = null; + int expectedLength = 0; + + if (integrity.startsWith("sha256-")) { + keyType = KeyType.SHA256; + expectedLength = 32; + hash = decoder.decode(integrity.substring(7)); + } + if (integrity.startsWith("sha384-")) { + keyType = KeyType.SHA384; + expectedLength = 48; + hash = decoder.decode(integrity.substring(7)); + } + if (integrity.startsWith("sha512-")) { + keyType = KeyType.SHA512; + expectedLength = 64; + hash = decoder.decode(integrity.substring(7)); + } + + if (keyType == null) { + throw new IllegalArgumentException( + "Unsupported checksum algorithm: '" + + integrity + + "' (expected SHA-256, SHA-384, or SHA-512)"); + } + + if (hash.length != expectedLength) { + throw new IllegalArgumentException( + "Invalid " + keyType + " SRI checksum '" + integrity + "'"); + } + + return Checksum.fromString(keyType, HashCode.fromBytes(hash).toString()); + } + + public String toSubresourceIntegrity() { + String encoded = Base64.getEncoder().encodeToString(hashCode.asBytes()); + return keyType.getHashName() + "-" + encoded; + } + @Override public String toString() { return hashCode.toString();
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java index 2ed70f6..c9b7d83 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
@@ -525,7 +525,7 @@ return null; } - private void warnAboutSha256Error(List<URL> urls, String sha256) { + private void warnAboutChecksumError(List<URL> urls, String errorMessage) { // Inform the user immediately, even though the file will still be downloaded. // This cannot be done by a regular error event, as all regular events are recorded // and only shown once the execution of the repository rule is finished. @@ -534,7 +534,7 @@ if (urls.size() > 0) { url = urls.get(0).toString(); } - reportProgress("Will fail after download of " + url + ". Invalid SHA256 '" + sha256 + "'"); + reportProgress("Will fail after download of " + url + ". " + errorMessage); } @Override @@ -546,27 +546,32 @@ Boolean allowFail, String canonicalId, SkylarkDict<String, SkylarkDict<Object, Object>> auth, + String integrity, Location location) throws RepositoryFunctionException, EvalException, InterruptedException { Map<URI, Map<String, String>> authHeaders = getAuthHeaders(auth); List<URL> urls = getUrls(url, /* ensureNonEmpty= */ !allowFail, env, !Strings.isNullOrEmpty(sha256)); - RepositoryFunctionException sha256Validation = validateSha256(sha256, location); - if (sha256Validation != null) { - warnAboutSha256Error(urls, sha256); - sha256 = ""; - } Optional<Checksum> checksum; - if (sha256.isEmpty()) { - checksum = Optional.absent(); - } else { - checksum = Optional.of(Checksum.fromString(KeyType.SHA256, sha256)); + RepositoryFunctionException checksumValidation = null; + try { + checksum = validateChecksum(sha256, integrity, urls, location); + } catch (RepositoryFunctionException e) { + checksum = Optional.<Checksum>absent(); + checksumValidation = e; } + SkylarkPath outputPath = getPath("download()", output); WorkspaceRuleEvent w = WorkspaceRuleEvent.newDownloadEvent( - urls, output.toString(), sha256, executable, rule.getLabel().toString(), location); + urls, + output.toString(), + sha256, + integrity, + executable, + rule.getLabel().toString(), + location); env.getListener().post(w); Path downloadedPath; try { @@ -597,20 +602,11 @@ throw new RepositoryFunctionException(e, Transience.TRANSIENT); } } - if (sha256Validation != null) { - throw sha256Validation; + if (checksumValidation != null) { + throw checksumValidation; } - String finalSha256; - try { - finalSha256 = calculateSha256(sha256, downloadedPath); - } catch (IOException e) { - throw new RepositoryFunctionException( - new IOException( - "Couldn't hash downloaded file (" + downloadedPath.getPathString() + ")", e), - Transience.PERSISTENT); - } - SkylarkDict<String, Object> dict = SkylarkDict.of(null, "sha256", finalSha256, "success", true); - return StructProvider.STRUCT.createStruct(dict, null); + + return calculateDownloadResult(checksum, downloadedPath); } @Override @@ -658,22 +654,20 @@ Boolean allowFail, String canonicalId, SkylarkDict<String, SkylarkDict<Object, Object>> auth, + String integrity, Location location) throws RepositoryFunctionException, InterruptedException, EvalException { Map<URI, Map<String, String>> authHeaders = getAuthHeaders(auth); List<URL> urls = getUrls(url, /* ensureNonEmpty= */ !allowFail, env, !Strings.isNullOrEmpty(sha256)); - RepositoryFunctionException sha256Validation = validateSha256(sha256, location); - if (sha256Validation != null) { - warnAboutSha256Error(urls, sha256); - sha256 = ""; - } Optional<Checksum> checksum; - if (sha256.isEmpty()) { - checksum = Optional.absent(); - } else { - checksum = Optional.of(Checksum.fromString(KeyType.SHA256, sha256)); + RepositoryFunctionException checksumValidation = null; + try { + checksum = validateChecksum(sha256, integrity, urls, location); + } catch (RepositoryFunctionException e) { + checksum = Optional.<Checksum>absent(); + checksumValidation = e; } WorkspaceRuleEvent w = @@ -681,6 +675,7 @@ urls, output.toString(), sha256, + integrity, type, stripPrefix, rule.getLabel().toString(), @@ -717,8 +712,8 @@ throw new RepositoryFunctionException(e, Transience.TRANSIENT); } } - if (sha256Validation != null) { - throw sha256Validation; + if (checksumValidation != null) { + throw checksumValidation; } env.getListener().post(w); DecompressorValue.decompress( @@ -729,15 +724,8 @@ .setRepositoryPath(outputPath.getPath()) .setPrefix(stripPrefix) .build()); - String finalSha256 = null; - try { - finalSha256 = calculateSha256(sha256, downloadedPath); - } catch (IOException e) { - throw new RepositoryFunctionException( - new IOException( - "Couldn't hash downloaded file (" + downloadedPath.getPathString() + ")", e), - Transience.PERSISTENT); - } + + StructImpl downloadResult = calculateDownloadResult(checksum, downloadedPath); try { if (downloadedPath.exists()) { downloadedPath.delete(); @@ -748,33 +736,84 @@ "Couldn't delete temporary file (" + downloadedPath.getPathString() + ")", e), Transience.TRANSIENT); } - SkylarkDict<String, Object> dict = SkylarkDict.of(null, "sha256", finalSha256, "success", true); - return StructProvider.STRUCT.createStruct(dict, null); + return downloadResult; } - private String calculateSha256(String originalSha, Path path) + private Checksum calculateChecksum(Optional<Checksum> originalChecksum, Path path) throws IOException, InterruptedException { - if (!Strings.isNullOrEmpty(originalSha)) { - // The sha is checked on download, so if we got here, the user provided sha is good - return originalSha; + if (originalChecksum.isPresent()) { + // The checksum is checked on download, so if we got here, the user provided checksum is good + return originalChecksum.get(); } - return RepositoryCache.getChecksum(KeyType.SHA256, path); + return Checksum.fromString(KeyType.SHA256, RepositoryCache.getChecksum(KeyType.SHA256, path)); } - private RepositoryFunctionException validateSha256(String sha256, Location loc) { - if (!sha256.isEmpty() && !KeyType.SHA256.isValid(sha256)) { - return new RepositoryFunctionException( + private Optional<Checksum> validateChecksum( + String sha256, String integrity, List<URL> urls, Location loc) + throws RepositoryFunctionException, EvalException { + if (!sha256.isEmpty()) { + if (!integrity.isEmpty()) { + throw new EvalException(loc, "Expected either 'sha256' or 'integrity', but not both"); + } + try { + return Optional.of(Checksum.fromString(KeyType.SHA256, sha256)); + } catch (IllegalArgumentException e) { + warnAboutChecksumError(urls, e.getMessage()); + throw new RepositoryFunctionException( + new EvalException( + loc, + "Definition of repository " + + rule.getName() + + ": " + + e.getMessage() + + " at " + + rule.getLocation()), + Transience.PERSISTENT); + } + } + + if (integrity.isEmpty()) { + return Optional.absent(); + } + + try { + return Optional.of(Checksum.fromSubresourceIntegrity(integrity)); + } catch (IllegalArgumentException e) { + warnAboutChecksumError(urls, e.getMessage()); + throw new RepositoryFunctionException( new EvalException( loc, "Definition of repository " + rule.getName() - + ": Syntactically invalid SHA256 checksum: '" - + sha256 - + "' at " + + ": " + + e.getMessage() + + " at " + rule.getLocation()), Transience.PERSISTENT); } - return null; + } + + private StructImpl calculateDownloadResult(Optional<Checksum> checksum, Path downloadedPath) + throws EvalException, InterruptedException, RepositoryFunctionException { + Checksum finalChecksum; + try { + finalChecksum = calculateChecksum(checksum, downloadedPath); + } catch (IOException e) { + throw new RepositoryFunctionException( + new IOException( + "Couldn't hash downloaded file (" + downloadedPath.getPathString() + ")", e), + Transience.PERSISTENT); + } + + ImmutableMap.Builder<String, Object> out = new ImmutableMap.Builder<>(); + out.put("success", true); + out.put("integrity", finalChecksum.toSubresourceIntegrity()); + + // For compatibility with older Bazel versions that don't support non-SHA256 checksums. + if (finalChecksum.getKeyType() == KeyType.SHA256) { + out.put("sha256", finalChecksum.toString()); + } + return StructProvider.STRUCT.createStruct(SkylarkDict.copyOf(null, out.build()), null); } private static ImmutableList<String> checkAllUrls(Iterable<?> urlList) throws EvalException {
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java index 31743b3..79c5020 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java
@@ -343,8 +343,9 @@ @SkylarkCallable( name = "download", doc = - "Downloads a file to the output path for the provided url and returns a struct containing" - + " a hash of the file with the field <code>sha256</code>.", + "Downloads a file to the output path for the provided url and returns a struct" + + " containing a hash of the file with the fields <code>sha256</code> and" + + " <code>integrity</code>.", useLocation = true, parameters = { @Param( @@ -404,6 +405,18 @@ defaultValue = "{}", named = true, doc = "An optional dict specifying authentication information for some of the URLs."), + @Param( + name = "integrity", + type = String.class, + defaultValue = "''", + named = true, + positional = false, + doc = + "Expected checksum of the file downloaded, in Subresource Integrity format." + + " This must match the checksum of the file downloaded. It is a security" + + " risk to omit the checksum as remote files can change. At best omitting this" + + " field will make your build non-hermetic. It is optional to make development" + + " easier but should be set before shipping."), }) public StructApi download( Object url, @@ -413,6 +426,7 @@ Boolean allowFail, String canonicalId, SkylarkDict<String, SkylarkDict<Object, Object>> auth, + String integrity, Location location) throws RepositoryFunctionExceptionT, EvalException, InterruptedException; @@ -463,8 +477,8 @@ name = "download_and_extract", doc = "Downloads a file to the output path for the provided url, extracts it, and returns" - + " a struct containing a hash of the downloaded file with the field" - + " <code>sha256</code>.", + + " a struct containing a hash of the downloaded file with the fields" + + " <code>sha256</code> and <code>integrity</code>.", useLocation = true, parameters = { @Param( @@ -546,6 +560,18 @@ defaultValue = "{}", named = true, doc = "An optional dict specifying authentication information for some of the URLs."), + @Param( + name = "integrity", + type = String.class, + defaultValue = "''", + named = true, + positional = false, + doc = + "Expected checksum of the file downloaded, in Subresource Integrity format." + + " This must match the checksum of the file downloaded. It is a security" + + " risk to omit the checksum as remote files can change. At best omitting this" + + " field will make your build non-hermetic. It is optional to make development" + + " easier but should be set before shipping."), }) public StructApi downloadAndExtract( Object url, @@ -556,6 +582,7 @@ Boolean allowFail, String canonicalId, SkylarkDict<String, SkylarkDict<Object, Object>> auth, + String integrity, Location location) throws RepositoryFunctionExceptionT, InterruptedException, EvalException; }
diff --git a/src/test/shell/bazel/bazel_workspaces_test.sh b/src/test/shell/bazel/bazel_workspaces_test.sh index 90a19f6..ef2b84e 100755 --- a/src/test/shell/bazel/bazel_workspaces_test.sh +++ b/src/test/shell/bazel/bazel_workspaces_test.sh
@@ -223,6 +223,62 @@ ensure_contains_exactly 'output: "out_for_list.txt"' 1 } +function test_download_integrity_sha256() { + # Prepare HTTP server with Python + local server_dir="${TEST_TMPDIR}/server_dir" + mkdir -p "${server_dir}" + local file="${server_dir}/file.txt" + echo "file contents here" > "${file}" + + # Use Python for hashing and encoding due to cross-platform differences in + # presence + behavior of `shasum` and `base64`. + sha256_py='import base64, hashlib, sys; print(base64.b64encode(hashlib.sha256(sys.stdin.read()).digest()))' + file_integrity="sha256-$(cat "${file}" | python -c "${sha256_py}")" + + # Start HTTP server with Python + startup_server "${server_dir}" + + set_workspace_command "repository_ctx.download(\"http://localhost:${fileserver_port}/file.txt\", \"file.txt\", integrity=\"${file_integrity}\")" + + build_and_process_log --exclude_rule "//external:local_config_cc" + + ensure_contains_exactly 'location: .*repos.bzl:2:3' 1 + ensure_contains_atleast 'rule: "//external:repo"' 1 + ensure_contains_exactly 'download_event' 1 + ensure_contains_exactly "url: \"http://localhost:${fileserver_port}/file.txt\"" 1 + ensure_contains_exactly 'output: "file.txt"' 1 + ensure_contains_exactly "sha256: " 0 + ensure_contains_exactly "integrity: \"${file_integrity}\"" 1 +} + +function test_download_integrity_sha512() { + # Prepare HTTP server with Python + local server_dir="${TEST_TMPDIR}/server_dir" + mkdir -p "${server_dir}" + local file="${server_dir}/file.txt" + echo "file contents here" > "${file}" + + # Use Python for hashing and encoding due to cross-platform differences in + # presence + behavior of `shasum` and `base64`. + sha512_py='import base64, hashlib, sys; print(base64.b64encode(hashlib.sha512(sys.stdin.read()).digest()))' + file_integrity="sha512-$(cat "${file}" | python -c "${sha512_py}")" + + # Start HTTP server with Python + startup_server "${server_dir}" + + set_workspace_command "repository_ctx.download(\"http://localhost:${fileserver_port}/file.txt\", \"file.txt\", integrity=\"${file_integrity}\")" + + build_and_process_log --exclude_rule "//external:local_config_cc" + + ensure_contains_exactly 'location: .*repos.bzl:2:3' 1 + ensure_contains_atleast 'rule: "//external:repo"' 1 + ensure_contains_exactly 'download_event' 1 + ensure_contains_exactly "url: \"http://localhost:${fileserver_port}/file.txt\"" 1 + ensure_contains_exactly 'output: "file.txt"' 1 + ensure_contains_exactly "sha256: " 0 + ensure_contains_exactly "integrity: \"${file_integrity}\"" 1 +} + function test_download_then_extract() { # Prepare HTTP server with Python local server_dir="${TEST_TMPDIR}/server_dir"
diff --git a/src/test/shell/bazel/external_integration_test.sh b/src/test/shell/bazel/external_integration_test.sh index bfa4582..708eeb8 100755 --- a/src/test/shell/bazel/external_integration_test.sh +++ b/src/test/shell/bazel/external_integration_test.sh
@@ -905,7 +905,7 @@ ) EOF bazel build @repo//... &> $TEST_log && fail "Expected to fail" - expect_log "[Ii]nvalid SHA256 checksum" + expect_log "[Ii]nvalid SHA-256 checksum" shutdown_server }