* 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

4 files changed, 49 insertions, 1 deletions

diff --git a/ChangeLog b/ChangeLog
index 17deaf86c9..df215c00dd 100644
--- a/ChangeLog
+++ b/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
diff --git a/ext/openssl/ossl_x509store.c b/ext/openssl/ossl_x509store.c
index 6f391c137b..cec9dbbb44 100644
--- a/ext/openssl/ossl_x509store.c
+++ b/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;
 }
diff --git a/test/openssl/test_x509store.rb b/test/openssl/test_x509store.rb
index 9964cc8fc4..0ae94e2c64 100644
--- a/test/openssl/test_x509store.rb
+++ b/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 = [
diff --git a/version.h b/version.h
index beca3fa556..59a09ac637 100644
--- a/version.h
+++ b/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