From c27fd3331989b33b9721444c98e77ba367a65270 Mon Sep 17 00:00:00 2001 From: drbrain Date: Thu, 7 Feb 2013 05:56:53 +0000 Subject: [PATCH] * lib/rubygems/package.rb: Ensure digests are generated for signing. * test/rubygems/test_gem_package.rb: Test for the above. * lib/rubygems/security/policy.rb: Ensure digests are present when verifying a gem and match the number of signatures bidirectionally. * test/rubygems/test_gem_security_policy.rb: Test for the above. * lib/rubygems.rb: Documentation improvements (by zzak) git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@39126 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 11 +++ lib/rubygems.rb | 6 +- lib/rubygems/package.rb | 8 +- lib/rubygems/security/policy.rb | 20 +++- test/rubygems/test_gem_package.rb | 54 ++++++++++- test/rubygems/test_gem_security_policy.rb | 109 +++++++++++++++------- 6 files changed, 165 insertions(+), 43 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6e0ad91906..65b553ae08 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +Thu Feb 7 14:56:15 2013 Eric Hodel + + * lib/rubygems/package.rb: Ensure digests are generated for signing. + * test/rubygems/test_gem_package.rb: Test for the above. + + * lib/rubygems/security/policy.rb: Ensure digests are present when + verifying a gem and match the number of signatures bidirectionally. + * test/rubygems/test_gem_security_policy.rb: Test for the above. + + * lib/rubygems.rb: Documentation improvements (by zzak) + Thu Feb 7 05:52:00 2013 Zachary Scott * doc/pty/README: Remove static documentation file diff --git a/lib/rubygems.rb b/lib/rubygems.rb index 54f3fcca89..039d26c1fe 100644 --- a/lib/rubygems.rb +++ b/lib/rubygems.rb @@ -1,9 +1,9 @@ # -*- ruby -*- -# +#-- # Copyright 2006 by Chad Fowler, Rich Kilmer, Jim Weirich and others. # All rights reserved. # See LICENSE.txt for permissions. -# +#++ require 'rbconfig' @@ -109,6 +109,8 @@ require 'rubygems/errors' # Thanks! # # -The RubyGems Team + + module Gem RUBYGEMS_DIR = File.dirname File.expand_path(__FILE__) diff --git a/lib/rubygems/package.rb b/lib/rubygems/package.rb index c662da2a55..82abcd0c6f 100644 --- a/lib/rubygems/package.rb +++ b/lib/rubygems/package.rb @@ -277,9 +277,13 @@ EOM # the security policy. def digest entry # :nodoc: - return unless @checksums + algorithms = if @checksums then + @checksums.keys + else + [Gem::Security::DIGEST_NAME] + end - @checksums.each_key do |algorithm| + algorithms.each do |algorithm| digester = OpenSSL::Digest.new algorithm digester << entry.read(16384) until entry.eof? diff --git a/lib/rubygems/security/policy.rb b/lib/rubygems/security/policy.rb index c34b7605c3..d1539e4985 100644 --- a/lib/rubygems/security/policy.rb +++ b/lib/rubygems/security/policy.rb @@ -152,8 +152,8 @@ class Gem::Security::Policy end def inspect # :nodoc: - "[Policy: %s - data: %p signer: %p chain: %p root: %p " + - "signed-only: %p trusted-only: %p]" % [ + ("[Policy: %s - data: %p signer: %p chain: %p root: %p " + + "signed-only: %p trusted-only: %p]") % [ @name, @verify_chain, @verify_data, @verify_root, @verify_signer, @only_signed, @only_trusted, ] @@ -177,11 +177,16 @@ class Gem::Security::Policy trust_dir = opt[:trust_dir] time = Time.now - signer_digests = digests.find do |algorithm, file_digests| + _, signer_digests = digests.find do |algorithm, file_digests| file_digests.values.first.name == Gem::Security::DIGEST_NAME end - signer_digests = digests.values.first || {} + if @verify_data then + raise Gem::Security::Exception, 'no digests provided (probable bug)' if + signer_digests.nil? or signer_digests.empty? + else + signer_digests = {} + end signer = chain.last @@ -195,6 +200,13 @@ class Gem::Security::Policy check_trust chain, digester, trust_dir if @only_trusted + signatures.each do |file, _| + digest = signer_digests[file] + + raise Gem::Security::Exception, "missing digest for #{file}" unless + digest + end + signer_digests.each do |file, digest| signature = signatures[file] diff --git a/test/rubygems/test_gem_package.rb b/test/rubygems/test_gem_package.rb index 3051147948..d08f46d7d2 100644 --- a/test/rubygems/test_gem_package.rb +++ b/test/rubygems/test_gem_package.rb @@ -429,10 +429,17 @@ class TestGemPackage < Gem::Package::TarTestCase digest = OpenSSL::Digest::SHA1.new digest << metadata_gz - checksum = "#{digest.name}\t#{digest.hexdigest}\n" - tar.add_file 'metadata.gz.sum', 0444 do |io| - io.write checksum + checksums = { + 'SHA1' => { + 'metadata.gz' => digest.hexdigest, + }, + } + + tar.add_file 'checksums.yaml.gz', 0444 do |io| + Zlib::GzipWriter.wrap io do |gz_io| + gz_io.write YAML.dump checksums + end end tar.add_file 'data.tar.gz', 0444 do |io| @@ -504,6 +511,47 @@ class TestGemPackage < Gem::Package::TarTestCase assert_empty package.instance_variable_get(:@files), '@files must empty' end + def test_verify_security_policy_checksum_missing + @spec.cert_chain = [PUBLIC_CERT.to_pem] + @spec.signing_key = PRIVATE_KEY + + build = Gem::Package.new @gem + build.spec = @spec + build.setup_signer + + FileUtils.mkdir 'lib' + FileUtils.touch 'lib/code.rb' + + open @gem, 'wb' do |gem_io| + Gem::Package::TarWriter.new gem_io do |gem| + build.add_metadata gem + build.add_contents gem + + # write bogus data.tar.gz to foil signature + bogus_data = Gem.gzip 'hello' + gem.add_file_simple 'data.tar.gz', 0444, bogus_data.length do |io| + io.write bogus_data + end + + # pre rubygems 2.0 gems do not add checksums + end + end + + Gem::Security.trust_dir.trust_cert PUBLIC_CERT + + package = Gem::Package.new @gem + package.security_policy = Gem::Security::HighSecurity + + e = assert_raises Gem::Security::Exception do + package.verify + end + + assert_equal 'invalid signature', e.message + + refute package.instance_variable_get(:@spec), '@spec must not be loaded' + assert_empty package.instance_variable_get(:@files), '@files must empty' + end + def test_verify_truncate open 'bad.gem', 'wb' do |io| io.write File.read(@gem, 1024) # don't care about newlines diff --git a/test/rubygems/test_gem_security_policy.rb b/test/rubygems/test_gem_security_policy.rb index 22f5375dbf..568bf69d08 100644 --- a/test/rubygems/test_gem_security_policy.rb +++ b/test/rubygems/test_gem_security_policy.rb @@ -31,6 +31,7 @@ class TestGemSecurityPolicy < Gem::TestCase @sha1 = OpenSSL::Digest::SHA1 @trust_dir = Gem::Security.trust_dir.dir # HACK use the object + @no = Gem::Security::NoSecurity @almost_no = Gem::Security::AlmostNoSecurity @low = Gem::Security::LowSecurity @high = Gem::Security::HighSecurity @@ -220,73 +221,108 @@ class TestGemSecurityPolicy < Gem::TestCase def test_verify Gem::Security.trust_dir.trust_cert PUBLIC_CERT - assert @almost_no.verify [PUBLIC_CERT] + assert @almost_no.verify [PUBLIC_CERT], nil, *dummy_signatures end def test_verify_chain_signatures Gem::Security.trust_dir.trust_cert PUBLIC_CERT - data = digest 'hello' - digest = { 'SHA1' => { 0 => data } } - signature = { 0 => sign(data, PRIVATE_KEY) } - - assert @high.verify [PUBLIC_CERT], nil, digest, signature + assert @high.verify [PUBLIC_CERT], nil, *dummy_signatures end def test_verify_chain_key - assert @almost_no.verify [PUBLIC_CERT], PRIVATE_KEY + @almost_no.verify [PUBLIC_CERT], PRIVATE_KEY, *dummy_signatures + end + + def test_verify_no_digests + Gem::Security.trust_dir.trust_cert PUBLIC_CERT + + _, signatures = dummy_signatures + + e = assert_raises Gem::Security::Exception do + @almost_no.verify [PUBLIC_CERT], nil, {}, signatures + end + + assert_equal 'no digests provided (probable bug)', e.message + end + + def test_verify_no_digests_no_security + Gem::Security.trust_dir.trust_cert PUBLIC_CERT + + _, signatures = dummy_signatures + + e = assert_raises Gem::Security::Exception do + @no.verify [PUBLIC_CERT], nil, {}, signatures + end + + assert_equal 'missing digest for 0', e.message + end + + def test_verify_not_enough_signatures + Gem::Security.trust_dir.trust_cert PUBLIC_CERT + + digests, signatures = dummy_signatures + + data = digest 'goodbye' + + signatures[1] = PRIVATE_KEY.sign @sha1.new, data.digest + + e = assert_raises Gem::Security::Exception do + @almost_no.verify [PUBLIC_CERT], nil, digests, signatures + end + + assert_equal 'missing digest for 1', e.message + end + + def test_verify_wrong_digest_type + Gem::Security.trust_dir.trust_cert PUBLIC_CERT + + sha512 = OpenSSL::Digest::SHA512 + + data = sha512.new + data << 'hello' + + digests = { 'SHA512' => { 0 => data } } + signature = PRIVATE_KEY.sign sha512.new, data.digest + signatures = { 0 => signature } + + e = assert_raises Gem::Security::Exception do + @almost_no.verify [PUBLIC_CERT], nil, digests, signatures + end + + assert_equal 'no digests provided (probable bug)', e.message end def test_verify_signatures_chain - data = digest 'hello' - digest = { 'SHA1' => { 0 => data } } - signature = { 0 => sign(data, CHILD_KEY) } - @spec.cert_chain = [PUBLIC_CERT, CHILD_CERT] - assert @chain.verify_signatures @spec, digest, signature + assert @chain.verify_signatures @spec, *dummy_signatures(CHILD_KEY) end def test_verify_signatures_data - data = digest 'hello' - digest = { 'SHA1' => { 0 => data } } - signature = { 0 => sign(data) } - @spec.cert_chain = [PUBLIC_CERT] - @almost_no.verify_signatures @spec, digest, signature + @almost_no.verify_signatures @spec, *dummy_signatures end def test_verify_signatures_root - data = digest 'hello' - digest = { 'SHA1' => { 0 => data } } - signature = { 0 => sign(data, CHILD_KEY) } - @spec.cert_chain = [PUBLIC_CERT, CHILD_CERT] - assert @root.verify_signatures @spec, digest, signature + assert @root.verify_signatures @spec, *dummy_signatures(CHILD_KEY) end def test_verify_signatures_signer - data = digest 'hello' - digest = { 'SHA1' => { 0 => data } } - signature = { 0 => sign(data) } - @spec.cert_chain = [PUBLIC_CERT] - assert @low.verify_signatures @spec, digest, signature + assert @low.verify_signatures @spec, *dummy_signatures end def test_verify_signatures_trust Gem::Security.trust_dir.trust_cert PUBLIC_CERT - data = digest 'hello' - digest = { 'SHA1' => { 0 => data } } - signature = { 0 => sign(data, PRIVATE_KEY) } - @spec.cert_chain = [PUBLIC_CERT] - assert @high.verify_signatures @spec, digest, signature + assert @high.verify_signatures @spec, *dummy_signatures end def test_verify_signatures @@ -372,5 +408,14 @@ class TestGemSecurityPolicy < Gem::TestCase key.sign @sha1.new, data.digest end + def dummy_signatures key = PRIVATE_KEY + data = digest 'hello' + + digests = { 'SHA1' => { 0 => data } } + signatures = { 0 => sign(data, key) } + + return digests, signatures + end + end