From d2be12ef6171f75a074aca8caaeaf834e1f2aac8 Mon Sep 17 00:00:00 2001 From: drbrain Date: Fri, 8 Feb 2013 02:58:19 +0000 Subject: [PATCH] * lib/rubygems/security/policy.rb: Raise proper exceptions when verifying unsigned gems (instead of crashing). * test/rubygems/test_gem_security_policy.rb: Tests for the above. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@39153 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 6 ++ lib/rubygems/security/policy.rb | 34 ++++++++++-- test/rubygems/test_gem_security_policy.rb | 67 +++++++++++++++++++++++ 3 files changed, 101 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index d058c99cfd..ab4f13be96 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +Fri Feb 8 11:53:33 2013 Eric Hodel + + * lib/rubygems/security/policy.rb: Raise proper exceptions when + verifying unsigned gems (instead of crashing). + * test/rubygems/test_gem_security_policy.rb: Tests for the above. + Fri Feb 8 10:44:44 2013 Eric Hodel * test/rubygems/test_gem_dependency_installer.rb: Improve coverage of diff --git a/lib/rubygems/security/policy.rb b/lib/rubygems/security/policy.rb index d1539e4985..f3e4568117 100644 --- a/lib/rubygems/security/policy.rb +++ b/lib/rubygems/security/policy.rb @@ -49,13 +49,18 @@ class Gem::Security::Policy # and is valid for the given +time+. def check_chain chain, time - chain.each_cons 2 do |issuer, cert| - check_cert cert, issuer, time - end + raise Gem::Security::Exception, 'missing signing chain' unless chain + raise Gem::Security::Exception, 'empty signing chain' if chain.empty? - true - rescue Gem::Security::Exception => e - raise Gem::Security::Exception, "invalid signing chain: #{e.message}" + begin + chain.each_cons 2 do |issuer, cert| + check_cert cert, issuer, time + end + + true + rescue Gem::Security::Exception => e + raise Gem::Security::Exception, "invalid signing chain: #{e.message}" + end end ## @@ -74,6 +79,9 @@ class Gem::Security::Policy # If the +issuer+ is +nil+ no verification is performed. def check_cert signer, issuer, time + raise Gem::Security::Exception, 'missing signing certificate' unless + signer + message = "certificate #{signer.subject}" if not_before = signer.not_before and not_before > time then @@ -97,6 +105,12 @@ class Gem::Security::Policy # Ensures the public key of +key+ matches the public key in +signer+ def check_key signer, key + unless signer and key then + return true unless @only_signed + + raise Gem::Security::Exception, 'missing key or signature' + end + raise Gem::Security::Exception, "certificate #{signer.subject} does not match the signing key" unless signer.public_key.to_pem == key.public_key.to_pem @@ -109,8 +123,12 @@ class Gem::Security::Policy # +time+. def check_root chain, time + raise Gem::Security::Exception, 'missing signing chain' unless chain + root = chain.first + raise Gem::Security::Exception, 'missing root certificate' unless root + raise Gem::Security::Exception, "root certificate #{root.subject} is not self-signed " + "(issuer #{root.issuer})" if @@ -124,8 +142,12 @@ class Gem::Security::Policy # the digests of the two certificates match according to +digester+ def check_trust chain, digester, trust_dir + raise Gem::Security::Exception, 'missing signing chain' unless chain + root = chain.first + raise Gem::Security::Exception, 'missing root certificate' unless root + path = Gem::Security.trust_dir.cert_path root unless File.exist? path then diff --git a/test/rubygems/test_gem_security_policy.rb b/test/rubygems/test_gem_security_policy.rb index 568bf69d08..1ce93fbd95 100644 --- a/test/rubygems/test_gem_security_policy.rb +++ b/test/rubygems/test_gem_security_policy.rb @@ -34,6 +34,7 @@ class TestGemSecurityPolicy < Gem::TestCase @no = Gem::Security::NoSecurity @almost_no = Gem::Security::AlmostNoSecurity @low = Gem::Security::LowSecurity + @medium = Gem::Security::MediumSecurity @high = Gem::Security::HighSecurity @chain = Gem::Security::Policy.new( @@ -85,6 +86,14 @@ class TestGemSecurityPolicy < Gem::TestCase assert @chain.check_chain chain, Time.now end + def test_check_chain_empty_chain + e = assert_raises Gem::Security::Exception do + @chain.check_chain [], Time.now + end + + assert_equal 'empty signing chain', e.message + end + def test_check_chain_invalid chain = [PUBLIC_CERT, CHILD_CERT, INVALIDCHILD_CERT] @@ -97,6 +106,14 @@ class TestGemSecurityPolicy < Gem::TestCase "was not issued by #{CHILD_CERT.subject}", e.message end + def test_check_chain_no_chain + e = assert_raises Gem::Security::Exception do + @chain.check_chain nil, Time.now + end + + assert_equal 'missing signing chain', e.message + end + def test_check_cert assert @low.check_cert(PUBLIC_CERT, nil, Time.now) end @@ -135,10 +152,28 @@ class TestGemSecurityPolicy < Gem::TestCase assert @low.check_cert(CHILD_CERT, PUBLIC_CERT, Time.now) end + def test_check_cert_no_signer + e = assert_raises Gem::Security::Exception do + @high.check_cert(nil, nil, Time.now) + end + + assert_equal 'missing signing certificate', e.message + end + def test_check_key assert @almost_no.check_key(PUBLIC_CERT, PRIVATE_KEY) end + def test_check_key_no_signer + assert @almost_no.check_key(nil, nil) + + e = assert_raises Gem::Security::Exception do + @high.check_key(nil, nil) + end + + assert_equal 'missing key or signature', e.message + end + def test_check_key_wrong_key e = assert_raises Gem::Security::Exception do @almost_no.check_key(PUBLIC_CERT, ALTERNATE_KEY) @@ -154,6 +189,14 @@ class TestGemSecurityPolicy < Gem::TestCase assert @chain.check_root chain, Time.now end + def test_check_root_empty_chain + e = assert_raises Gem::Security::Exception do + @chain.check_root [], Time.now + end + + assert_equal 'missing root certificate', e.message + end + def test_check_root_invalid_signer chain = [INVALID_SIGNER_CERT] @@ -178,6 +221,14 @@ class TestGemSecurityPolicy < Gem::TestCase e.message end + def test_check_root_no_chain + e = assert_raises Gem::Security::Exception do + @chain.check_root nil, Time.now + end + + assert_equal 'missing signing chain', e.message + end + def test_check_trust Gem::Security.trust_dir.trust_cert PUBLIC_CERT @@ -190,6 +241,14 @@ class TestGemSecurityPolicy < Gem::TestCase assert @high.check_trust [PUBLIC_CERT, CHILD_CERT], @sha1, @trust_dir end + def test_check_trust_empty_chain + e = assert_raises Gem::Security::Exception do + @chain.check_trust [], @sha1, @trust_dir + end + + assert_equal 'missing root certificate', e.message + end + def test_check_trust_mismatch Gem::Security.trust_dir.trust_cert PUBLIC_CERT @@ -201,6 +260,14 @@ class TestGemSecurityPolicy < Gem::TestCase "does not match signing root certificate checksum", e.message end + def test_check_trust_no_chain + e = assert_raises Gem::Security::Exception do + @chain.check_trust nil, @sha1, @trust_dir + end + + assert_equal 'missing signing chain', e.message + end + def test_check_trust_no_trust e = assert_raises Gem::Security::Exception do @high.check_trust [PUBLIC_CERT], @sha1, @trust_dir