Skip to content

Commit 5d880b7

Browse files
rye-stripehsbt
authored andcommitted
[rubygems/rubygems] Correctly sign S3 HEAD requests
We sometimes send HEAD requests. The s3_uri_signer.rb code allways assumed GETs. This lead to consistently getting 403 responses back from S3. Recently, S3 attempted to change the behaviour of how 403s are handled when TCP connections are reused, which escalated this bug from "just noise" to "breaks gem installs". They've reverted that behaviour, so the severity of this problem is back to "just noise". Either way, it's a bug in rubygems and warrants a fix it. ruby/rubygems@c38f502b73
1 parent af6012b commit 5d880b7

3 files changed

Lines changed: 32 additions & 10 deletions

File tree

lib/rubygems/remote_fetcher.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,16 +267,16 @@ def fetch_path(uri, mtime = nil, head = false)
267267

268268
def fetch_s3(uri, mtime = nil, head = false)
269269
begin
270-
public_uri = s3_uri_signer(uri).sign
270+
public_uri = s3_uri_signer(uri, head).sign
271271
rescue Gem::S3URISigner::ConfigurationError, Gem::S3URISigner::InstanceProfileError => e
272272
raise FetchError.new(e.message, "s3://#{uri.host}")
273273
end
274274
fetch_https public_uri, mtime, head
275275
end
276276

277277
# we have our own signing code here to avoid a dependency on the aws-sdk gem
278-
def s3_uri_signer(uri)
279-
Gem::S3URISigner.new(uri)
278+
def s3_uri_signer(uri, head)
279+
Gem::S3URISigner.new(uri, head)
280280
end
281281

282282
##

lib/rubygems/s3_uri_signer.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,11 @@ def to_s # :nodoc:
2727
end
2828

2929
attr_accessor :uri
30+
attr_accessor :head
3031

31-
def initialize(uri)
32+
def initialize(uri, head)
3233
@uri = uri
34+
@head = head
3335
end
3436

3537
##
@@ -73,7 +75,7 @@ def generate_canonical_query_params(s3_config, date_time, credential_info, expir
7375

7476
def generate_canonical_request(canonical_host, query_params)
7577
[
76-
"GET",
78+
head ? "HEAD" : "GET",
7779
uri.path,
7880
query_params,
7981
"host:#{canonical_host}",

test/rubygems/test_gem_remote_fetcher_s3.rb

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ def setup
1818
@a1.loaded_from = File.join(@gemhome, "specifications", @a1.full_name)
1919
end
2020

21-
def assert_fetch_s3(url, signature, token=nil, region="us-east-1", instance_profile_json=nil)
21+
def assert_fetch_s3(url, signature, token=nil, region="us-east-1", instance_profile_json=nil, head=false)
2222
fetcher = Gem::RemoteFetcher.new nil
2323
@fetcher = fetcher
2424
$fetched_uri = nil
@@ -33,9 +33,9 @@ def res.body
3333
res
3434
end
3535

36-
def fetcher.s3_uri_signer(uri)
36+
def fetcher.s3_uri_signer(uri, head)
3737
require "json"
38-
s3_uri_signer = Gem::S3URISigner.new(uri)
38+
s3_uri_signer = Gem::S3URISigner.new(uri, head)
3939
def s3_uri_signer.ec2_metadata_credentials_json
4040
JSON.parse($instance_profile)
4141
end
@@ -45,10 +45,14 @@ def s3_uri_signer.ec2_metadata_credentials_json
4545
s3_uri_signer
4646
end
4747

48-
data = fetcher.fetch_s3 Gem::URI.parse(url)
48+
res = fetcher.fetch_s3 Gem::URI.parse(url), nil, head
4949

5050
assert_equal "https://my-bucket.s3.#{region}.amazonaws.com/gems/specs.4.8.gz?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=testuser%2F20190624%2F#{region}%2Fs3%2Faws4_request&X-Amz-Date=20190624T051941Z&X-Amz-Expires=86400#{token ? "&X-Amz-Security-Token=" + token : ""}&X-Amz-SignedHeaders=host&X-Amz-Signature=#{signature}", $fetched_uri.to_s
51-
assert_equal "success", data
51+
if !head
52+
assert_equal "success", res
53+
else
54+
assert_equal 200, res.code
55+
end
5256
ensure
5357
$fetched_uri = nil
5458
end
@@ -65,6 +69,22 @@ def test_fetch_s3_config_creds
6569
Gem.configuration[:s3_source] = nil
6670
end
6771

72+
def test_fetch_s3_head_request
73+
Gem.configuration[:s3_source] = {
74+
"my-bucket" => { id: "testuser", secret: "testpass" },
75+
}
76+
url = "s3://my-bucket/gems/specs.4.8.gz"
77+
Time.stub :now, Time.at(1_561_353_581) do
78+
token = nil
79+
region = "us-east-1"
80+
instance_profile_json = nil
81+
head = true
82+
assert_fetch_s3 url, "a3c6cf9a2db62e85f4e57f8fc8ac8b5ff5c1fdd4aeef55935d05e05174d9c885", token, region, instance_profile_json, head
83+
end
84+
ensure
85+
Gem.configuration[:s3_source] = nil
86+
end
87+
6888
def test_fetch_s3_config_creds_with_region
6989
Gem.configuration[:s3_source] = {
7090
"my-bucket" => { id: "testuser", secret: "testpass", region: "us-west-2" },

0 commit comments

Comments
 (0)