drbrain	2013-02-08 11:58:19 +0900 (Fri, 08 Feb 2013)

  New Revision: 39153

  http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=39153

  Log:
    * 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.

  Modified files:
    trunk/ChangeLog
    trunk/lib/rubygems/security/policy.rb
    trunk/test/rubygems/test_gem_security_policy.rb

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 39152)
+++ ChangeLog	(revision 39153)
@@ -1,3 +1,9 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1
+Fri Feb  8 11:53:33 2013  Eric Hodel  <drbrain@s...>
+
+	* 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  <drbrain@s...>
 
 	* test/rubygems/test_gem_dependency_installer.rb:  Improve coverage of
Index: lib/rubygems/security/policy.rb
===================================================================
--- lib/rubygems/security/policy.rb	(revision 39152)
+++ lib/rubygems/security/policy.rb	(revision 39153)
@@ -49,13 +49,18 @@ class Gem::Security::Policy https://github.com/ruby/ruby/blob/trunk/lib/rubygems/security/policy.rb#L49
   # 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 https://github.com/ruby/ruby/blob/trunk/lib/rubygems/security/policy.rb#L79
   # 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 https://github.com/ruby/ruby/blob/trunk/lib/rubygems/security/policy.rb#L105
   # 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 https://github.com/ruby/ruby/blob/trunk/lib/rubygems/security/policy.rb#L123
   # +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 https://github.com/ruby/ruby/blob/trunk/lib/rubygems/security/policy.rb#L142
   # 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
Index: test/rubygems/test_gem_security_policy.rb
===================================================================
--- test/rubygems/test_gem_security_policy.rb	(revision 39152)
+++ test/rubygems/test_gem_security_policy.rb	(revision 39153)
@@ -34,6 +34,7 @@ class TestGemSecurityPolicy < Gem::TestC https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_security_policy.rb#L34
     @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::TestC https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_security_policy.rb#L86
     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::TestC https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_security_policy.rb#L106
                  "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::TestC https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_security_policy.rb#L152
     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::TestC https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_security_policy.rb#L189
     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::TestC https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_security_policy.rb#L221
                  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::TestC https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_security_policy.rb#L241
     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::TestC https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_security_policy.rb#L260
                  "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

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/