[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.

https://github.com/rubygems/rubygems/commit/c38f502b73

3 files changed, 32 insertions, 10 deletions

diff --git a/lib/rubygems/remote_fetcher.rb b/lib/rubygems/remote_fetcher.rb
index 4b5c74e0ea..52a8bc243e 100644
--- a/lib/rubygems/remote_fetcher.rb
+++ b/lib/rubygems/remote_fetcher.rb
@@ -267,7 +267,7 @@ class Gem::RemoteFetcher
 
   def fetch_s3(uri, mtime = nil, head = false)
     begin
-      public_uri = s3_uri_signer(uri).sign
+      public_uri = s3_uri_signer(uri, head).sign
     rescue Gem::S3URISigner::ConfigurationError, Gem::S3URISigner::InstanceProfileError => e
       raise FetchError.new(e.message, "s3://#{uri.host}")
     end
@@ -275,8 +275,8 @@ class Gem::RemoteFetcher
   end
 
   # we have our own signing code here to avoid a dependency on the aws-sdk gem
-  def s3_uri_signer(uri)
-    Gem::S3URISigner.new(uri)
+  def s3_uri_signer(uri, head)
+    Gem::S3URISigner.new(uri, head)
   end
 
   ##
diff --git a/lib/rubygems/s3_uri_signer.rb b/lib/rubygems/s3_uri_signer.rb
index 236819abb8..8dbf236442 100644
--- a/lib/rubygems/s3_uri_signer.rb
+++ b/lib/rubygems/s3_uri_signer.rb
@@ -27,9 +27,11 @@ class Gem::S3URISigner
   end
 
   attr_accessor :uri
+  attr_accessor :head
 
-  def initialize(uri)
+  def initialize(uri, head)
     @uri = uri
+    @head = head
   end
 
   ##
@@ -73,7 +75,7 @@ class Gem::S3URISigner
 
   def generate_canonical_request(canonical_host, query_params)
     [
-      "GET",
+      head ? "HEAD" : "GET",
       uri.path,
       query_params,
       "host:#{canonical_host}",
diff --git a/test/rubygems/test_gem_remote_fetcher_s3.rb b/test/rubygems/test_gem_remote_fetcher_s3.rb
index 91571525c7..8dcf3469f1 100644
--- a/test/rubygems/test_gem_remote_fetcher_s3.rb
+++ b/test/rubygems/test_gem_remote_fetcher_s3.rb
@@ -18,7 +18,7 @@ class TestGemRemoteFetcherS3 < Gem::TestCase
     @a1.loaded_from = File.join(@gemhome, "specifications", @a1.full_name)
   end
 
-  def assert_fetch_s3(url, signature, token=nil, region="us-east-1", instance_profile_json=nil)
+  def assert_fetch_s3(url, signature, token=nil, region="us-east-1", instance_profile_json=nil, head=false)
     fetcher = Gem::RemoteFetcher.new nil
     @fetcher = fetcher
     $fetched_uri = nil
@@ -33,9 +33,9 @@ class TestGemRemoteFetcherS3 < Gem::TestCase
       res
     end
 
-    def fetcher.s3_uri_signer(uri)
+    def fetcher.s3_uri_signer(uri, head)
       require "json"
-      s3_uri_signer = Gem::S3URISigner.new(uri)
+      s3_uri_signer = Gem::S3URISigner.new(uri, head)
       def s3_uri_signer.ec2_metadata_credentials_json
         JSON.parse($instance_profile)
       end
@@ -45,10 +45,14 @@ class TestGemRemoteFetcherS3 < Gem::TestCase
       s3_uri_signer
     end
 
-    data = fetcher.fetch_s3 Gem::URI.parse(url)
+    res = fetcher.fetch_s3 Gem::URI.parse(url), nil, head
 
     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
-    assert_equal "success", data
+    if !head
+      assert_equal "success", res
+    else
+      assert_equal 200, res.code
+    end
   ensure
     $fetched_uri = nil
   end
@@ -65,6 +69,22 @@ class TestGemRemoteFetcherS3 < Gem::TestCase
     Gem.configuration[:s3_source] = nil
   end
 
+  def test_fetch_s3_head_request
+    Gem.configuration[:s3_source] = {
+      "my-bucket" => { id: "testuser", secret: "testpass" },
+    }
+    url = "s3://my-bucket/gems/specs.4.8.gz"
+    Time.stub :now, Time.at(1_561_353_581) do
+      token = nil
+      region = "us-east-1"
+      instance_profile_json = nil
+      head = true
+      assert_fetch_s3 url, "a3c6cf9a2db62e85f4e57f8fc8ac8b5ff5c1fdd4aeef55935d05e05174d9c885", token, region, instance_profile_json, head
+    end
+  ensure
+    Gem.configuration[:s3_source] = nil
+  end
+
   def test_fetch_s3_config_creds_with_region
     Gem.configuration[:s3_source] = {
       "my-bucket" => { id: "testuser", secret: "testpass", region: "us-west-2" },