Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add timestamp validation to tarball files #3859

Closed
sylvestre opened this issue Aug 22, 2021 · 7 comments
Closed

Add timestamp validation to tarball files #3859

sylvestre opened this issue Aug 22, 2021 · 7 comments
Labels
A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works

Comments

@sylvestre
Copy link

Describe the bug
If I have my history correct, it is quite unlikely that we have any Rust file written during the 1970 years, we should have files with timestamp from these years.

To Reproduce

$ wget  https://crates.io/api/v1/crates/grep-cli/0.1.6/download 
$ tar zxvf download
$  ls -al grep-cli-0.1.6/Cargo.toml
-rw-r--r-- 1 sylvestre sylvestre 1404  1 janv.  1970 grep-cli-0.1.6/Cargo.toml

Expected behavior

crates.io should probably reject incorrect/invalid timestamps
(it is causing pain the Debian packaging side)

It was probably caused by:
rust-lang/cargo#9512

@sylvestre sylvestre added the C-bug 🐞 Category: unintended, undesired behavior label Aug 22, 2021
@Turbo87 Turbo87 added A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works and removed C-bug 🐞 Category: unintended, undesired behavior labels Aug 22, 2021
@Turbo87
Copy link
Member

Turbo87 commented Aug 22, 2021

crates.io should probably reject incorrect/invalid timestamps

I'm not sure how we'd be able to tell if it's incorrect or not. 1970 sure sounds wrong, but where is the threshold?

@sylvestre
Copy link
Author

2000 would be probably good enough.

@Turbo87
Copy link
Member

Turbo87 commented Aug 22, 2021

maybe, but I'm not sure what the difference is between 1970 being wrong and 2000 being wrong 🤔

@sylvestre
Copy link
Author

Avoid some Debian errors like:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=976369
(I know it isn't the best argument ever :)

jamessan added a commit to jamessan/tar-rs that referenced this issue Aug 22, 2021
As described in rust-lang/crates.io#3859, the current arbitrary
timestamp of 123456789 leads to Debian packages being auto-rejected due
to the old timestamp.  This happens for any timestamps before 1975.

Instead of 123456789, use another, more recent arbitrary timestamp --
the timestamp of the first commit for what would become Rust.

graydon/rust-prehistory@b0fd440
alexcrichton pushed a commit to alexcrichton/tar-rs that referenced this issue Aug 23, 2021
As described in rust-lang/crates.io#3859, the current arbitrary
timestamp of 123456789 leads to Debian packages being auto-rejected due
to the old timestamp.  This happens for any timestamps before 1975.

Instead of 123456789, use another, more recent arbitrary timestamp --
the timestamp of the first commit for what would become Rust.

graydon/rust-prehistory@b0fd440
@Turbo87 Turbo87 changed the title Unexpected timestamps (1970) in the crates.io tarballs Add timestamp validation to tarball files Aug 30, 2021
bors added a commit to rust-lang/cargo that referenced this issue Jun 1, 2022
Enforce to use tar v0.4.38

The latest version of `tar` crate includes alexcrichton/tar-rs#262, which uses a bit recent timestamp when archiving.
However, [it seems rust-lang/rust uses v0.4.37](https://github.com/rust-lang/rust/blob/e0944922007e1bb4fe59809293acf4364410cccc/Cargo.lock#L5124-L5132). This PR enforces to use v0.4.38 and it would _fix_ rust-lang/crates.io#3859 eventually.
@JohnTitor
Copy link
Member

I've made Cargo use tar v0.4.38 which includes alexcrichton/tar-rs#262, but some files still have 1970-01-01 including Cargo.toml:

[~/ws/semverver-0.1.51]$ ls -al
total 104
drwxr-xr-x  5 koybi koybi  4096 Jun 23 20:28 .
drwxr-xr-x 19 koybi koybi  4096 Jun 23 20:28 ..
-rw-r--r--  1 koybi koybi   200 Jul 24  2006 build.rs
-rw-r--r--  1 koybi koybi 33025 Jan  1  1970 Cargo.lock
-rw-r--r--  1 koybi koybi  1571 Jan  1  1970 Cargo.toml
-rw-r--r--  1 koybi koybi  1131 Jul 24  2006 Cargo.toml.orig
-rw-r--r--  1 koybi koybi    94 Jan  1  1970 .cargo_vcs_info.json
-rw-r--r--  1 koybi koybi  1842 Jul 24  2006 CONTRIBUTING.md
-rw-r--r--  1 koybi koybi    67 Jul 24  2006 .gitattributes
-rw-r--r--  1 koybi koybi    41 Jul 24  2006 .gitignore
-rw-r--r--  1 koybi koybi  1540 Jul 24  2006 LICENSE
-rw-r--r--  1 koybi koybi 10167 Jul 24  2006 README.md
-rw-r--r--  1 koybi koybi   141 Jul 24  2006 rust-toolchain
drwxr-xr-x  2 koybi koybi  4096 Jun 23 20:28 scripts
drwxr-xr-x  3 koybi koybi  4096 Jun 23 20:28 src
drwxr-xr-x  5 koybi koybi  4096 Jun 23 20:28 tests

Maybe some files' mtimes are set to 1 by https://github.com/alexcrichton/tar-rs/blob/f4f439ca0cd3a984d2a66fb8e42f6e2307876afd/src/entry.rs#L475?

@JohnTitor
Copy link
Member

While I think it's reasonable to prefer a newer timestamp, I'm not sure if we should validate and reject it on crates.io. If I understand correctly, it would reject many old Cargo versions, no?

@Turbo87
Copy link
Member

Turbo87 commented Jun 24, 2022

yeah, I tend to agree. we shouldn't reject these files, but we should ideally fix this on the cargo side (or whereever the issue exists)

@Turbo87 Turbo87 closed this as not planned Won't fix, can't repro, duplicate, stale Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works
Projects
None yet
Development

No branches or pull requests

3 participants