* ext/openssl/ossl_x509store.c: clear error queue after calling

X509_LOOKUP_load_file()

  X509_LOOKUP_load_file(), which ends up calling
  X509_load_cert_crl_file()
  internally, may leave error entries in the queue even when it returns
  non-zero value (which indicates success).

  This will be fixed by OpenSSL 1.1.1, but can be worked around by
  clearing the error queue ourselves.

  Fixes: [Backport #11033]


git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_3@59235 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

ChangeLog

@@ -1,3 +1,18 @@
+Fri Jun 30 21:40:42 2017  Kazuki Yamaguchi <k@rhe.jp>
+
+	* ext/openssl/ossl_x509store.c: clear error queue after calling
+	  X509_LOOKUP_load_file()
+
+	  X509_LOOKUP_load_file(), which ends up calling
+	  X509_load_cert_crl_file()
+	  internally, may leave error entries in the queue even when it returns
+	  non-zero value (which indicates success).
+
+	  This will be fixed by OpenSSL 1.1.1, but can be worked around by
+	  clearing the error queue ourselves.
+
+	  Fixes: [Backport #11033]
+
 Fri Jun 30 21:35:16 2017  Nobuyoshi Nakada  <nobu@ruby-lang.org>
 
 	* gc.c (heap_page_allocate): expand sorted pages before inserting

ext/openssl/ossl_x509store.c

@@ -249,6 +249,13 @@ ossl_x509store_add_file(VALUE self, VALUE file)
     if(X509_LOOKUP_load_file(lookup, path, X509_FILETYPE_PEM) != 1){
         ossl_raise(eX509StoreError, NULL);
     }
+    /*
+     * X509_load_cert_crl_file() which is called from X509_LOOKUP_load_file()
+     * did not check the return value of X509_STORE_add_{cert,crl}(), leaking
+     * "cert already in hash table" errors on the error queue, if duplicate
+     * certificates are found. This will be fixed by OpenSSL 1.1.1.
+     */
+    ERR_clear_error();
 
     return self;
 }

test/openssl/test_x509store.rb

@@ -36,6 +36,32 @@ class OpenSSL::TestX509Store < Test::Unit::TestCase
     OpenSSL::TestUtils.issue_crl(*args)
   end
 
+  def test_add_file
+    now = Time.at(Time.now.to_i)
+    ca_exts = [
+      ["basicConstraints", "CA:TRUE", true],
+      ["keyUsage", "cRLSign,keyCertSign", true],
+    ]
+    cert1 = issue_cert(@ca1, @rsa1024, 1, now, now+3600, ca_exts,
+                       nil, nil, "sha1")
+    cert2 = issue_cert(@ca2, @rsa2048, 1, now, now+3600, ca_exts,
+                       nil, nil, "sha1")
+    tmpfile = Tempfile.open { |f| f << cert1.to_pem << cert2.to_pem; f }
+
+    store = OpenSSL::X509::Store.new
+    assert_equal false, store.verify(cert1)
+    assert_equal false, store.verify(cert2)
+    store.add_file(tmpfile.path)
+    assert_equal true, store.verify(cert1)
+    assert_equal true, store.verify(cert2)
+
+    # OpenSSL < 1.1.1 leaks an error on a duplicate certificate
+    assert_nothing_raised { store.add_file(tmpfile.path) }
+    assert_equal [], OpenSSL.errors
+  ensure
+    tmpfile and tmpfile.close!
+  end
+
   def test_verify
     now = Time.at(Time.now.to_i)
     ca_exts = [

version.h

@@ -1,6 +1,6 @@
 #define RUBY_VERSION "2.3.5"
 #define RUBY_RELEASE_DATE "2017-06-30"
-#define RUBY_PATCHLEVEL 331
+#define RUBY_PATCHLEVEL 332
 
 #define RUBY_RELEASE_YEAR 2017
 #define RUBY_RELEASE_MONTH 6