Crates.io uses deprecated AWS signature v2 for S3 upload

[cswindle]

Currently crates.io is using signature v2 in the authorisation header for uploading crates to S3, however this is deprecated:

https://docs.aws.amazon.com/general/latest/gr/signature-version-2.html

This causes an issue while trying to create a private crates.io which is using an S3 bucket on one of the new regions or a private S3 which only support v4 (eg minio).

Is there a reason that crates.io can't be switched over to use signature v4?

[jtgeibel]

Thanks a lot for pointing this out! I'm definitely in support of dropping our internal implementation (under src/s3) in place of something from the ecosystem. However, I haven't evaluated any of the alternatives yet. Anything that you are familiar with, would recommend?

[cswindle]

The only crate that I found that looked like it would be good to use is rusoto, the others I looked at did not have the following:

• working tests
• clean compile
• CI

On that basis I tested to make sure that I was able to use rusoto to talk to minio and it worked fine, so I know that signature v4 must work. Based on the fact that the crate has integration tests to validate that S3 works as well I have no doubt it will also work correctly with S3.

Below is the example that I used to ensure that I could upload to a minio server:

extern crate rusoto_core;
extern crate rusoto_s3;

use rusoto_s3::{S3Client, PutObjectRequest, DeleteObjectRequest, StreamingBody};
use rusoto_core::Region;
use rusoto_s3::S3;

fn main() {

    let region = Region::Custom {
        name: "us-east-1".to_owned(),
        endpoint: "<local server>".to_owned(),
    };

    let s3client = S3Client::new(region);

    let req = PutObjectRequest {
        bucket: "crates-test".to_string(),
        key: "test/test.txt".to_string(),
        body: Some("test".as_bytes().to_vec().into()),
        ..Default::default()
    };

    s3client.put_object(req).sync().unwrap();
}

I think that it will be pretty easy to replace the s3 module with this crate and I am happy to do this and send a PR (assuming you are happy with the choice of crate).

[sgrif]

I'm not sure this is actually worth spending any time addressing, since the plan is to have cargo perform the S3 upload rather than crates.io in the future.

[jtgeibel]

since the plan is to have cargo perform the S3 upload rather than crates.io in the future.

I do have a few concerns about this. We currently do some verification of the uploaded tarball. In particular we ensure that the tar file cannot exploit bugs in old versions of cargo to write files to improper places. It's unclear to me how long we want to ensure these old clients are protected and its possible that there will be future vulnerabilities which we want to mitigate in a similar way.

We also enforce upload size limits, which we can increase above the default limit on a per-crate basis. Taking a quick look at the S3 POST Policy documentation, I believe we can encode content length, content type, and other constraints in the signed policy that we would hand to cargo authorizing the upload. So it looks like we could still implement this per-crate size override. At the same time, I think that as long as we choose the right dependency to rely on, switching to v4 signatures now may make the migration to a direct-upload scheme easier later.

[sgrif]

We can do all of those things async, after the upload is completed, before we update the index

[jtgeibel]

That's a great point on making the checks async. Although, if we can find an S3 dependency that supports both workflows now it seems that would ease the migration since the server will need to support both proxied (for old clients) and direct-upload workflows in the future.

[cswindle]

I'm not sure this is actually worth spending any time addressing, since the plan is to have cargo perform the S3 upload rather than crates.io in the future.

I am doing this partly so that we can setup an alternate registry within the company I work for. I have not seen any RFCs relating to switching to use cargo to perform the upload, has there been any thoughts on implications on alternative registries (see rust-lang/rust#44931)?

[jtgeibel]

[...] relating to switching to use cargo to perform the upload, has there been any thoughts on implications on alternative registries (see rust-lang/rust#44931)?

Maybe we could add a flag to the config.json file in the index, letting cargo know that this particular registry supports the new upload API. For alternative registries without this flag, cargo would use the v1 API.

[cswindle]

@withoutboats already proposed (and implemented) adding a versioned API in config.json (see rust-lang/cargo#4747), that PR got dropped, but it might be worth re-opening and sending in.

[withoutboats]

I'm not sure this is actually worth spending any time addressing, since the plan is to have cargo perform the S3 upload rather than crates.io in the future.

I'm not sure where this plan has been discussed but I also have concerns about how it would impact custom registries. In general, we need some notion of versioning these APIs that the registries can communicate before we can make breaking changes to the API between cargo and crates.io.

I have a more recent PR for this rust-lang/cargo#5915, but its CI is currently failing because I haven't found the time to deal with understanding the necessary parts of cargo's test suite set up.

[Turbo87]

we have transitioned the codebase to use https://crates.io/crates/object_store, which AFAIK does not use these outdated signatures :)