Skip to content

Commit a9ca4b9

Browse files
committed
Add checksum verification for rustfmt downloads
1 parent 81f511c commit a9ca4b9

File tree

4 files changed

+83
-10
lines changed

4 files changed

+83
-10
lines changed

Cargo.lock

+2
Original file line numberDiff line numberDiff line change
@@ -216,13 +216,15 @@ dependencies = [
216216
"cmake",
217217
"filetime",
218218
"getopts",
219+
"hex 0.4.2",
219220
"ignore",
220221
"libc",
221222
"once_cell",
222223
"opener",
223224
"pretty_assertions 0.7.2",
224225
"serde",
225226
"serde_json",
227+
"sha2",
226228
"sysinfo",
227229
"tar",
228230
"toml",

src/bootstrap/Cargo.toml

+2
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,10 @@ filetime = "0.2"
4040
getopts = "0.2.19"
4141
cc = "1.0.69"
4242
libc = "0.2"
43+
hex = "0.4"
4344
serde = { version = "1.0.8", features = ["derive"] }
4445
serde_json = "1.0.2"
46+
sha2 = "0.10"
4547
tar = "0.4"
4648
toml = "0.5"
4749
ignore = "0.4.10"

src/bootstrap/builder.rs

+22-1
Original file line numberDiff line numberDiff line change
@@ -879,7 +879,6 @@ impl<'a> Builder<'a> {
879879
) {
880880
// Use a temporary file in case we crash while downloading, to avoid a corrupt download in cache/.
881881
let tempfile = self.tempdir().join(dest_path.file_name().unwrap());
882-
// FIXME: support `do_verify` (only really needed for nightly rustfmt)
883882
self.download_with_retries(&tempfile, &format!("{}/{}", base, url), help_on_error);
884883
t!(std::fs::rename(&tempfile, dest_path));
885884
}
@@ -971,6 +970,28 @@ impl<'a> Builder<'a> {
971970
t!(fs::remove_dir_all(dst.join(directory_prefix)));
972971
}
973972

973+
/// Returns whether the SHA256 checksum of `path` matches `expected`.
974+
pub(crate) fn verify(&self, path: &Path, expected: &str) -> bool {
975+
use sha2::Digest;
976+
977+
self.verbose(&format!("verifying {}", path.display()));
978+
let mut hasher = sha2::Sha256::new();
979+
// FIXME: this is ok for rustfmt (4.1 MB large at time of writing), but it seems memory-intensive for rustc and larger components.
980+
// Consider using streaming IO instead?
981+
let contents = if self.config.dry_run { vec![] } else { t!(fs::read(path)) };
982+
hasher.update(&contents);
983+
let found = hex::encode(hasher.finalize().as_slice());
984+
let verified = found == expected;
985+
if !verified && !self.config.dry_run {
986+
println!(
987+
"invalid checksum: \n\
988+
found: {found}\n\
989+
expected: {expected}",
990+
);
991+
}
992+
return verified;
993+
}
994+
974995
/// Obtain a compiler at a given stage and for a given host. Explicitly does
975996
/// not take `Compiler` since all `Compiler` instances are meant to be
976997
/// obtained through this function, since it ensures that they are valid

src/bootstrap/config.rs

+57-9
Original file line numberDiff line numberDiff line change
@@ -1486,6 +1486,7 @@ fn maybe_download_rustfmt(builder: &Builder<'_>) -> Option<PathBuf> {
14861486
#[derive(Deserialize)]
14871487
struct Stage0Metadata {
14881488
dist_server: String,
1489+
checksums_sha256: HashMap<String, String>,
14891490
rustfmt: Option<RustfmtMetadata>,
14901491
}
14911492
#[derive(Deserialize)]
@@ -1495,10 +1496,11 @@ fn maybe_download_rustfmt(builder: &Builder<'_>) -> Option<PathBuf> {
14951496
}
14961497

14971498
let stage0_json = builder.read(&builder.src.join("src").join("stage0.json"));
1498-
let metadata = t!(serde_json::from_str::<Stage0Metadata>(&stage0_json));
1499-
let RustfmtMetadata { date, version } = metadata.rustfmt?;
1499+
let Stage0Metadata { dist_server, checksums_sha256, rustfmt } =
1500+
t!(serde_json::from_str::<Stage0Metadata>(&stage0_json));
1501+
let RustfmtMetadata { date, version } = rustfmt?;
15001502
let channel = format!("{version}-{date}");
1501-
let mut dist_server = env::var("RUSTUP_DIST_SERVER").unwrap_or(metadata.dist_server);
1503+
let mut dist_server = env::var("RUSTUP_DIST_SERVER").unwrap_or(dist_server);
15021504
dist_server.push_str("/dist");
15031505

15041506
let host = builder.config.build;
@@ -1510,8 +1512,15 @@ fn maybe_download_rustfmt(builder: &Builder<'_>) -> Option<PathBuf> {
15101512
}
15111513

15121514
let filename = format!("rustfmt-{version}-{build}.tar.xz", build = host.triple);
1513-
download_component(builder, &dist_server, filename, "rustfmt-preview", &date, "stage0");
1514-
assert!(rustfmt_path.exists());
1515+
download_component(
1516+
builder,
1517+
&dist_server,
1518+
filename,
1519+
"rustfmt-preview",
1520+
&date,
1521+
"stage0",
1522+
Some(checksums_sha256),
1523+
);
15151524

15161525
builder.fix_bin_or_dylib(&bin_root.join("bin").join("rustfmt"));
15171526
builder.fix_bin_or_dylib(&bin_root.join("bin").join("cargo-fmt"));
@@ -1564,6 +1573,7 @@ fn download_ci_component(builder: &Builder<'_>, filename: String, prefix: &str,
15641573
prefix,
15651574
commit,
15661575
"ci-rustc",
1576+
None,
15671577
)
15681578
}
15691579

@@ -1574,17 +1584,55 @@ fn download_component(
15741584
prefix: &str,
15751585
key: &str,
15761586
destination: &str,
1587+
checksums: Option<HashMap<String, String>>,
15771588
) {
15781589
let cache_dst = builder.out.join("cache");
15791590
let cache_dir = cache_dst.join(key);
15801591
if !cache_dir.exists() {
15811592
t!(fs::create_dir_all(&cache_dir));
15821593
}
15831594

1595+
let bin_root = builder.out.join(builder.config.build.triple).join(destination);
15841596
let tarball = cache_dir.join(&filename);
1585-
if !tarball.exists() {
1586-
builder.download_component(base_url, &format!("{key}/{filename}"), &tarball, "");
1597+
let url = format!("{key}/{filename}");
1598+
1599+
// For the beta compiler, put special effort into ensuring the checksums are valid.
1600+
// FIXME: maybe we should do this for download-rustc as well? but it would be a pain to update
1601+
// this on each and every nightly ...
1602+
let checksum = if let Some(checksums) = &checksums {
1603+
let error = format!(
1604+
"src/stage0.json doesn't contain a checksum for {url}. \
1605+
Pre-built artifacts might not be available for this \
1606+
target at this time, see https://doc.rust-lang.org/nightly\
1607+
/rustc/platform-support.html for more information."
1608+
);
1609+
// TODO: add an enum { Commit, Published } so we don't have to hardcode `dist` in two places
1610+
let sha256 = checksums.get(&format!("dist/{url}")).expect(&error);
1611+
if tarball.exists() {
1612+
if builder.verify(&tarball, sha256) {
1613+
builder.unpack(&tarball, &bin_root, prefix);
1614+
return;
1615+
} else {
1616+
builder.verbose(&format!(
1617+
"ignoring cached file {} due to failed verification",
1618+
tarball.display()
1619+
));
1620+
builder.remove(&tarball);
1621+
}
1622+
}
1623+
Some(sha256)
1624+
} else if tarball.exists() {
1625+
return;
1626+
} else {
1627+
None
1628+
};
1629+
1630+
builder.download_component(base_url, &url, &tarball, "");
1631+
if let Some(sha256) = checksum {
1632+
if !builder.verify(&tarball, sha256) {
1633+
panic!("failed to verify {}", tarball.display());
1634+
}
15871635
}
1588-
let bin_root = builder.out.join(builder.config.build.triple).join(destination);
1589-
builder.unpack(&tarball, &bin_root, prefix)
1636+
1637+
builder.unpack(&tarball, &bin_root, prefix);
15901638
}

0 commit comments

Comments
 (0)