diff --git a/ext/openssl/ossl_x509store.c b/ext/openssl/ossl_x509store.c index 61543d44f..f7c73d01e 100644 --- a/ext/openssl/ossl_x509store.c +++ b/ext/openssl/ossl_x509store.c @@ -301,17 +301,15 @@ ossl_x509store_add_file(VALUE self, VALUE file) { X509_STORE *store; X509_LOOKUP *lookup; - char *path = NULL; + const char *path; - if(file != Qnil){ - path = StringValueCStr(file); - } GetX509Store(self, store); + path = StringValueCStr(file); lookup = X509_STORE_add_lookup(store, X509_LOOKUP_file()); - if(lookup == NULL) ossl_raise(eX509StoreError, NULL); - if(X509_LOOKUP_load_file(lookup, path, X509_FILETYPE_PEM) != 1){ - ossl_raise(eX509StoreError, NULL); - } + if (!lookup) + ossl_raise(eX509StoreError, "X509_STORE_add_lookup"); + if (X509_LOOKUP_load_file(lookup, path, X509_FILETYPE_PEM) != 1) + ossl_raise(eX509StoreError, "X509_LOOKUP_load_file"); #if OPENSSL_VERSION_NUMBER < 0x10101000 || defined(LIBRESSL_VERSION_NUMBER) /* * X509_load_cert_crl_file() which is called from X509_LOOKUP_load_file() @@ -336,17 +334,15 @@ ossl_x509store_add_path(VALUE self, VALUE dir) { X509_STORE *store; X509_LOOKUP *lookup; - char *path = NULL; + const char *path; - if(dir != Qnil){ - path = StringValueCStr(dir); - } GetX509Store(self, store); + path = StringValueCStr(dir); lookup = X509_STORE_add_lookup(store, X509_LOOKUP_hash_dir()); - if(lookup == NULL) ossl_raise(eX509StoreError, NULL); - if(X509_LOOKUP_add_dir(lookup, path, X509_FILETYPE_PEM) != 1){ - ossl_raise(eX509StoreError, NULL); - } + if (!lookup) + ossl_raise(eX509StoreError, "X509_STORE_add_lookup"); + if (X509_LOOKUP_add_dir(lookup, path, X509_FILETYPE_PEM) != 1) + ossl_raise(eX509StoreError, "X509_LOOKUP_add_dir"); return self; } diff --git a/test/openssl/test_x509store.rb b/test/openssl/test_x509store.rb index e9602e343..a84a4ff0a 100644 --- a/test/openssl/test_x509store.rb +++ b/test/openssl/test_x509store.rb @@ -26,15 +26,20 @@ def test_nosegv_on_cleanup ctx.verify end - def test_add_file + def test_add_file_path ca_exts = [ ["basicConstraints", "CA:TRUE", true], ["keyUsage", "cRLSign,keyCertSign", true], ] - cert1 = issue_cert(@ca1, @rsa1024, 1, ca_exts, nil, nil) - cert2 = issue_cert(@ca2, @rsa2048, 1, ca_exts, nil, nil) - tmpfile = Tempfile.open { |f| f << cert1.to_pem << cert2.to_pem; f } + cert1_subj = OpenSSL::X509::Name.parse_rfc2253("CN=Cert 1") + cert1_key = Fixtures.pkey("rsa-1") + cert1 = issue_cert(cert1_subj, cert1_key, 1, ca_exts, nil, nil) + cert2_subj = OpenSSL::X509::Name.parse_rfc2253("CN=Cert 2") + cert2_key = Fixtures.pkey("rsa-2") + cert2 = issue_cert(cert2_subj, cert2_key, 1, ca_exts, nil, nil) + # X509::Store#add_file reads concatenated PEM file + 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) @@ -42,9 +47,23 @@ def test_add_file assert_equal true, store.verify(cert1) assert_equal true, store.verify(cert2) + # X509::Store#add_path + Dir.mktmpdir do |dir| + hash1 = "%08x.%d" % [cert1_subj.hash, 0] + File.write(File.join(dir, hash1), cert1.to_pem) + store = OpenSSL::X509::Store.new + store.add_path(dir) + + assert_equal true, store.verify(cert1) + assert_equal false, store.verify(cert2) + end + # OpenSSL < 1.1.1 leaks an error on a duplicate certificate assert_nothing_raised { store.add_file(tmpfile.path) } assert_equal [], OpenSSL.errors + + # Non-String is given + assert_raise(TypeError) { store.add_file(nil) } ensure tmpfile and tmpfile.close! end