Merge pull request #2444 from matiaskorhonen/feature/gem-signature-ve…

…rification

Gem signature verification

app/models/pusher.rb

@@ -20,6 +20,9 @@ def authorize
   end
 
   def validate
+    signature_missing = "There was a problem saving your gem: \nYou have added cert_chain in gemspec but signature was empty"
+
+    return notify(signature_missing, 403) unless validate_signature_exists?
     (rubygem.valid? && version.valid?) || notify("There was a problem saving your gem: #{rubygem.all_errors(version)}", 403)
   end
 
@@ -43,7 +46,9 @@ def save
   end
 
   def pull_spec
-    @spec = Gem::Package.new(body).spec
+    package = Gem::Package.new(body, gem_security_policy)
+    @spec = package.spec
+    @files = package.files
   rescue StandardError => e
     notify <<-MSG.strip_heredoc, 422
       RubyGems.org cannot process this gem.
@@ -80,7 +85,8 @@ def find
                                      platform: spec.original_platform.to_s,
                                      size: size,
                                      sha256: sha256,
-                                     pusher: user
+                                     pusher: user,
+                                     cert_chain: spec.cert_chain
 
     true
   end
@@ -148,4 +154,24 @@ def notify_unauthorized
       notify("You do not have permission to push to this gem. Ask an owner to add you with: gem owner #{rubygem.name} --add #{user.email}", 403)
     end
   end
+
+  def gem_security_policy
+    @gem_security_policy ||= begin
+      # Verify that the gem signatures match the certificate chain (if present)
+      policy = Gem::Security::LowSecurity.dup
+      # Silence warnings from the verification
+      stream = StringIO.new
+      policy.ui = Gem::StreamUI.new(stream, stream, stream, false)
+      policy
+    end
+  end
+
+  def validate_signature_exists?
+    return true if @spec.cert_chain.empty?
+
+    signatures = @files.select { |file| file[/\.sig$/] }
+
+    expected_signatures = %w[metadata.gz.sig data.tar.gz.sig checksums.yaml.gz.sig]
+    expected_signatures.difference(signatures).empty?
+  end
 end

app/models/version.rb

@@ -17,13 +17,14 @@ class Version < ApplicationRecord
 
   serialize :licenses
   serialize :requirements
+  serialize :cert_chain, CertificateChainSerializer
 
   validates :number, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }, format: { with: /\A#{Gem::Version::VERSION_PATTERN}\z/o }
   validates :platform, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }, format: { with: Rubygem::NAME_PATTERN }
   validates :full_name, presence: true, uniqueness: { case_sensitive: false }
   validates :rubygem, presence: true
   validates :required_rubygems_version, :licenses, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }, allow_blank: true
-  validates :description, :summary, :authors, :requirements, length: { minimum: 0, maximum: MAX_TEXT_FIELD_LENGTH }, allow_blank: true
+  validates :description, :summary, :authors, :requirements, :cert_chain, length: { minimum: 0, maximum: MAX_TEXT_FIELD_LENGTH }, allow_blank: true
 
   validate :unique_canonical_number, on: :create
   validate :platform_and_number_are_unique, on: :create

db/migrate/20200708064226_add_cert_chain_to_versions.rb

@@ -0,0 +1,5 @@
+class AddCertChainToVersions < ActiveRecord::Migration[6.0]
+  def change
+    add_column :versions, :cert_chain, :text
+  end
+end

db/schema.rb

@@ -218,6 +218,7 @@
     t.string "yanked_info_checksum"
     t.bigint "pusher_id"
     t.string "canonical_number"
+    t.text "cert_chain"
     t.index "lower((full_name)::text)", name: "index_versions_on_lower_full_name"
     t.index ["built_at"], name: "index_versions_on_built_at"
     t.index ["created_at"], name: "index_versions_on_created_at"

lib/certificate_chain_serializer.rb

@@ -0,0 +1,18 @@
+class CertificateChainSerializer
+  PATTERN = /-----BEGIN CERTIFICATE-----(?:.|\n)+?-----END CERTIFICATE-----/.freeze
+
+  def self.load(chain)
+    return [] unless chain
+    chain.scan(PATTERN).map do |cert|
+      OpenSSL::X509::Certificate.new(cert)
+    end
+  end
+
+  def self.dump(chain)
+    return if chain.blank?
+    normalised = chain.map do |cert|
+      cert.respond_to?(:to_pem) ? cert : OpenSSL::X509::Certificate.new(cert)
+    end
+    normalised.map(&:to_pem).join
+  end
+end

test/certs/chain.pem

@@ -0,0 +1,56 @@
+-----BEGIN CERTIFICATE-----
+MIIFKTCCBBGgAwIBAgISBFspP+tJfRaC6xprreB4Rp9KMA0GCSqGSIb3DQEBCwUA
+MDIxCzAJBgNVBAYTAlVTMRYwFAYDVQQKEw1MZXQncyBFbmNyeXB0MQswCQYDVQQD
+EwJSMzAeFw0yMTA0MTcwMjQzMTlaFw0yMTA3MTYwMjQzMTlaMBwxGjAYBgNVBAMT
+EXd3dy5jb2Rlb3Rha3UuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKC
+AQEAx6h5vNPfkkrtYWxn1PWDDLRAwrGmZbkYPttjHBRSwTcd7rsIX4PcSzw9fWxm
+K4vIkAYoKAElIvsSE3xRUjyzMrACfdhK5J8rG25fq94iVyoYaNBQV0WMJkO6X47s
+hGeIKkK91ohR5b2tMw3/z9zELP0TVo2TPG7rYsBZm34myldqDA8yVEBEOa+Qdpda
+9xewPhkkdpAU55qgWTrD21m7vGq9WpsBz4wNKnwVsaugtkRH82VPIfaL4ZI9kox6
+QoPWe/tHUBdlDkuT7ud77eLAWnC/5Clg28/9GU/Z8Nj8SrrKuXL6WUXmxxaAhWUR
+Qx4VblZeuIpwd0nHyP0hz4CWKQIDAQABo4ICTTCCAkkwDgYDVR0PAQH/BAQDAgWg
+MB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAMBgNVHRMBAf8EAjAAMB0G
+A1UdDgQWBBTKiSGZuLFSIG2JPbFSZa9TxMu5WTAfBgNVHSMEGDAWgBQULrMXt1hW
+y65QCUDmH6+dixTCxjBVBggrBgEFBQcBAQRJMEcwIQYIKwYBBQUHMAGGFWh0dHA6
+Ly9yMy5vLmxlbmNyLm9yZzAiBggrBgEFBQcwAoYWaHR0cDovL3IzLmkubGVuY3Iu
+b3JnLzAcBgNVHREEFTATghF3d3cuY29kZW90YWt1LmNvbTBMBgNVHSAERTBDMAgG
+BmeBDAECATA3BgsrBgEEAYLfEwEBATAoMCYGCCsGAQUFBwIBFhpodHRwOi8vY3Bz
+LmxldHNlbmNyeXB0Lm9yZzCCAQUGCisGAQQB1nkCBAIEgfYEgfMA8QB3AJQgvB6O
+1Y1siHMfgosiLA3R2k1ebE+UPWHbTi9YTaLCAAABeN3s/lgAAAQDAEgwRgIhAKFY
+Q+vBe3zyeBazxp8kVN7oLvcQ6Y9PPz199tVhYnEbAiEAhU/xdbQaY/6b93h+7NTF
+sPG7X4lq/3UoNgoXcAVGZgoAdgD2XJQv0XcwIhRUGAgwlFaO400TGTO/3wwvIAvM
+TvFk4wAAAXjd7P5OAAAEAwBHMEUCIQDWd79+jWaGuf3acm5/yV95jL2KvzeGFfdU
+HZlKIeWFmAIgDSZ6ug7AyhYNKjzFV4ZSICln+L4yI92EpOa+8gDG6/0wDQYJKoZI
+hvcNAQELBQADggEBAHIhMYm06lLFmJL+cfIg5fFEmFNdHmmZn88Hypv4/MtmqTKv
+5asF/z3TvhW4hX2+TY+NdcqGT7cZFo/ZF/tS6oBXPgmBYM1dEfp2FAdnGNOySC5Y
+7RC4Uk9TUpP2g101YBmj6dQKQluAwIQk+gO4MSlHE0J0U/lMpjvrLWcuHbV4/xWJ
+IdM+iPq8GeYt5epYmNc7XeRIgv7V3RxDQdBv2OVM5mtPVerdiO0ISrdbe5mvz2+Z
+rhSg+EJNHlmMwcq5HqtMwS8M8Ax+vLmWCOkPWXhyV8wQaQcFjZJfpIGUvCnMTqsh
+kSIYXq2CbSDUUFRFssNN6EdVms0KnmW3BUu0xAk=
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIIEZTCCA02gAwIBAgIQQAF1BIMUpMghjISpDBbN3zANBgkqhkiG9w0BAQsFADA/
+MSQwIgYDVQQKExtEaWdpdGFsIFNpZ25hdHVyZSBUcnVzdCBDby4xFzAVBgNVBAMT
+DkRTVCBSb290IENBIFgzMB4XDTIwMTAwNzE5MjE0MFoXDTIxMDkyOTE5MjE0MFow
+MjELMAkGA1UEBhMCVVMxFjAUBgNVBAoTDUxldCdzIEVuY3J5cHQxCzAJBgNVBAMT
+AlIzMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAuwIVKMz2oJTTDxLs
+jVWSw/iC8ZmmekKIp10mqrUrucVMsa+Oa/l1yKPXD0eUFFU1V4yeqKI5GfWCPEKp
+Tm71O8Mu243AsFzzWTjn7c9p8FoLG77AlCQlh/o3cbMT5xys4Zvv2+Q7RVJFlqnB
+U840yFLuta7tj95gcOKlVKu2bQ6XpUA0ayvTvGbrZjR8+muLj1cpmfgwF126cm/7
+gcWt0oZYPRfH5wm78Sv3htzB2nFd1EbjzK0lwYi8YGd1ZrPxGPeiXOZT/zqItkel
+/xMY6pgJdz+dU/nPAeX1pnAXFK9jpP+Zs5Od3FOnBv5IhR2haa4ldbsTzFID9e1R
+oYvbFQIDAQABo4IBaDCCAWQwEgYDVR0TAQH/BAgwBgEB/wIBADAOBgNVHQ8BAf8E
+BAMCAYYwSwYIKwYBBQUHAQEEPzA9MDsGCCsGAQUFBzAChi9odHRwOi8vYXBwcy5p
+ZGVudHJ1c3QuY29tL3Jvb3RzL2RzdHJvb3RjYXgzLnA3YzAfBgNVHSMEGDAWgBTE
+p7Gkeyxx+tvhS5B1/8QVYIWJEDBUBgNVHSAETTBLMAgGBmeBDAECATA/BgsrBgEE
+AYLfEwEBATAwMC4GCCsGAQUFBwIBFiJodHRwOi8vY3BzLnJvb3QteDEubGV0c2Vu
+Y3J5cHQub3JnMDwGA1UdHwQ1MDMwMaAvoC2GK2h0dHA6Ly9jcmwuaWRlbnRydXN0
+LmNvbS9EU1RST09UQ0FYM0NSTC5jcmwwHQYDVR0OBBYEFBQusxe3WFbLrlAJQOYf
+r52LFMLGMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjANBgkqhkiG9w0B
+AQsFAAOCAQEA2UzgyfWEiDcx27sT4rP8i2tiEmxYt0l+PAK3qB8oYevO4C5z70kH
+ejWEHx2taPDY/laBL21/WKZuNTYQHHPD5b1tXgHXbnL7KqC401dk5VvCadTQsvd8
+S8MXjohyc9z9/G2948kLjmE6Flh9dDYrVYA9x2O+hEPGOaEOa1eePynBgPayvUfL
+qjBstzLhWVQLGAkXXmNs+5ZnPBxzDJOLxhF2JIbeQAcH5H0tZrUlo5ZYyOqA7s9p
+O5b85o3AM/OJ+CktFBQtfvBhcJVd9wvlwPsk+uyOy2HI7mNxKKgsBTt375teA2Tw
+UdHkhVNcsAKX1H7GNNLOEADksd86wuoXvg==
+-----END CERTIFICATE-----

test/integration/push_test.rb

@@ -96,6 +96,16 @@ class PushTest < ActionDispatch::IntegrationTest
     assert_match(/A yanked version pushed by a previous owner of this gem already exists \(sandworm-1.0.0\)/, response.body)
   end
 
+  test "publishing a gem with ceritifcate but not signatures" do
+    build_gem "sandworm", "2.0.0" do |gemspec|
+      gemspec.cert_chain = [File.read(File.expand_path("../certs/chain.pem", __dir__))]
+    end
+
+    push_gem "sandworm-2.0.0.gem"
+    assert_response :forbidden
+    assert_match(/You have added cert_chain in gemspec but signature was empty/, response.body)
+  end
+
   def push_gem(path)
     post api_v1_rubygems_path,
       env: { "RAW_POST_DATA" => File.read(path) },

test/unit/certificate_chain_serializer_test.rb

@@ -0,0 +1,61 @@
+require "test_helper"
+
+class CertificateChainSerializerTest < ActiveSupport::TestCase
+  context ".load" do
+    setup do
+      @cert_chain = File.read(File.expand_path("../certs/chain.pem", __dir__))
+    end
+
+    should "return an empty array when no certificates are present" do
+      assert_equal [], CertificateChainSerializer.load("")
+      assert_equal [], CertificateChainSerializer.load(nil)
+    end
+
+    should "return an array of certificates when certificates are present" do
+      certs = CertificateChainSerializer.load(@cert_chain)
+      assert_equal 2, certs.size
+      assert_equal "379469669351564281569116418161349711273802", certs[0].serial.to_s
+      assert_equal "85078157426496920958827089468591623647", certs[1].serial.to_s
+    end
+  end
+
+  context ".dump" do
+    setup do
+      @certs = Array.new(2) do
+        key = OpenSSL::PKey::RSA.new(1024)
+        public_key = key.public_key
+
+        subject = "/C=FI/O=Test/OU=Test/CN=Test"
+
+        cert = OpenSSL::X509::Certificate.new
+        cert.subject = cert.issuer = OpenSSL::X509::Name.parse(subject)
+        cert.not_before = Time.current
+        cert.not_after = 1.year.from_now
+        cert.public_key = public_key
+        cert.serial = 0x0
+        cert.version = 2
+        cert.sign(key, OpenSSL::Digest.new("SHA256"))
+        cert
+      end
+    end
+
+    should "return an nil when no certificates are present" do
+      assert_nil CertificateChainSerializer.dump([])
+      assert_nil CertificateChainSerializer.dump(nil)
+    end
+
+    should "return a certificate chain string when certificates are present" do
+      assert_equal @certs.map(&:to_pem).join, CertificateChainSerializer.dump(@certs)
+    end
+
+    should "return a certificate chain when the chain certificates are in PEMs" do
+      pems = @certs.map(&:to_pem)
+      assert_equal pems.join, CertificateChainSerializer.dump(pems)
+    end
+
+    should "strip out excessive newlines from the certificate PEMs" do
+      pems = @certs.map { |cert| "#{cert.to_pem}\n\n\n" }
+      assert_equal @certs.map(&:to_pem).join, CertificateChainSerializer.dump(pems)
+    end
+  end
+end

test/unit/pusher_test.rb

@@ -123,6 +123,22 @@ class PusherTest < ActiveSupport::TestCase
       assert_equal @cutter.code, 422
     end
 
+    should "not be able to save a gem if it is signed and has been tampered with" do
+      @gem = gem_file("valid_signature_tampered-0.0.1.gem")
+      @cutter = Pusher.new(@user, @gem)
+      @cutter.process
+      assert_includes @cutter.message, %(missing signing certificate)
+      assert_equal @cutter.code, 422
+    end
+
+    should "not be able to save a gem if it is signed with an expired certificate" do
+      @gem = gem_file("expired_signature-0.0.0.gem")
+      @cutter = Pusher.new(@user, @gem)
+      @cutter.process
+      assert_includes @cutter.message, %(not valid after 2021-07-08 08:21:01 UTC)
+      assert_equal @cutter.code, 422
+    end
+
     should "not be able to pull spec with metadata containing bad ruby symbols" do
       ["1.0.0", "2.0.0", "3.0.0", "4.0.0"].each do |version|
         @gem = gem_file("dos-#{version}.gem")
@@ -162,6 +178,7 @@ class PusherTest < ActiveSupport::TestCase
       spec.expects(:name).returns "some name"
       spec.expects(:version).times(2).returns Gem::Version.new("1.3.3.7")
       spec.expects(:original_platform).returns "ruby"
+      spec.expects(:cert_chain).returns nil
       @cutter.stubs(:spec).returns spec
       @cutter.stubs(:size).returns 5
       @cutter.stubs(:body).returns StringIO.new("dummy body")
@@ -194,6 +211,7 @@ class PusherTest < ActiveSupport::TestCase
       spec.stubs(:name).returns @rubygem.name
       spec.stubs(:version).returns Gem::Version.new("1.3.3.7")
       spec.stubs(:original_platform).returns "ruby"
+      spec.stubs(:cert_chain).returns nil
       @cutter.stubs(:spec).returns spec
       @cutter.find
 
@@ -225,6 +243,7 @@ class PusherTest < ActiveSupport::TestCase
       spec.stubs(:name).returns @rubygem.name.upcase
       spec.stubs(:version).returns Gem::Version.new("1.3.3.7")
       spec.stubs(:original_platform).returns "ruby"
+      spec.stubs(:cert_chain).returns nil
       @cutter.stubs(:spec).returns spec
       @cutter.find
 
@@ -409,4 +428,22 @@ class PusherTest < ActiveSupport::TestCase
 
     teardown { RubygemFs.mock! }
   end
+
+  context "the gem has been signed and not tampered with" do
+    setup do
+      @gem = gem_file("valid_signature-0.0.0.gem")
+      @cutter = Pusher.new(@user, @gem)
+      @cutter.process
+    end
+
+    should "extracts the certificate chain to the version" do
+      assert_equal 200, @cutter.code
+      assert_not_nil @cutter.version
+      assert_not_nil @cutter.version.cert_chain
+      assert_equal 1, @cutter.version.cert_chain.size
+      assert_equal "/CN=snakeoil/DC=example/DC=invalid", @cutter.version.cert_chain.first.subject.to_s
+    end
+
+    teardown { RubygemFs.mock! }
+  end
 end