diff --git a/app/models/pusher.rb b/app/models/pusher.rb index cf110720f95..d27e7f36536 100644 --- a/app/models/pusher.rb +++ b/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 diff --git a/app/models/version.rb b/app/models/version.rb index 4e7396338f6..177937598e4 100644 --- a/app/models/version.rb +++ b/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 diff --git a/db/migrate/20200708064226_add_cert_chain_to_versions.rb b/db/migrate/20200708064226_add_cert_chain_to_versions.rb new file mode 100644 index 00000000000..73adf54e621 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index b160d5617ef..11c83a6a84b 100644 --- a/db/schema.rb +++ b/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" diff --git a/lib/certificate_chain_serializer.rb b/lib/certificate_chain_serializer.rb new file mode 100644 index 00000000000..d012f31f24b --- /dev/null +++ b/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 diff --git a/test/certs/chain.pem b/test/certs/chain.pem new file mode 100644 index 00000000000..624785d326a --- /dev/null +++ b/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----- diff --git a/test/gems/expired_signature-0.0.0.gem b/test/gems/expired_signature-0.0.0.gem new file mode 100644 index 00000000000..a9966af069f Binary files /dev/null and b/test/gems/expired_signature-0.0.0.gem differ diff --git a/test/gems/valid_signature-0.0.0.gem b/test/gems/valid_signature-0.0.0.gem new file mode 100644 index 00000000000..b8e6328fa09 Binary files /dev/null and b/test/gems/valid_signature-0.0.0.gem differ diff --git a/test/gems/valid_signature_tampered-0.0.1.gem b/test/gems/valid_signature_tampered-0.0.1.gem new file mode 100644 index 00000000000..72bf882b087 Binary files /dev/null and b/test/gems/valid_signature_tampered-0.0.1.gem differ diff --git a/test/integration/push_test.rb b/test/integration/push_test.rb index 795acc90b7d..625a13bbd0f 100644 --- a/test/integration/push_test.rb +++ b/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) }, diff --git a/test/unit/certificate_chain_serializer_test.rb b/test/unit/certificate_chain_serializer_test.rb new file mode 100644 index 00000000000..722db6da810 --- /dev/null +++ b/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 diff --git a/test/unit/pusher_test.rb b/test/unit/pusher_test.rb index 264c76dd43f..c2d2c69ad89 100644 --- a/test/unit/pusher_test.rb +++ b/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