Fetching OpenSSL::X509::Certificate object without shell interactions.

- Previously, we were building a new OpenSSL::X509::Certificate object with PEM string contents.
- To get this content, we were using openssl utility and were converting all certificate types into PEM
- And were extracting the PEM contents out of resulting data.
- It could directly be done via correct usage of applicable classes.
- Reverted using Open3. It uses shell hence did not solved the purpose.

Signed-off-by: Nimesh-Msys <nimesh.patni@msystechnologies.com>

2 files changed, 35 insertions, 38 deletions

diff --git a/lib/chef/resource/windows_certificate.rb b/lib/chef/resource/windows_certificate.rb
index ca838fd6dd..9a48eafe01 100644
--- a/lib/chef/resource/windows_certificate.rb
+++ b/lib/chef/resource/windows_certificate.rb
@@ -21,7 +21,6 @@ require "chef/util/path_helper"
 require "chef/resource"
 require "win32-certstore" if Chef::Platform.windows?
 require "openssl"
-require "open3"
 
 class Chef
   class Resource
@@ -54,7 +53,7 @@ class Chef
                description: ""
 
       # lazy used to set default value of sensitive to true if password is set
-      property :sensitive, [ TrueClass, FalseClass ],
+      property :sensitive, [TrueClass, FalseClass],
                description: "Ensure that sensitive resource data is not logged by the chef-client.",
                default: lazy { |r| r.pfx_password ? true : false }, skip_docs: true
 
@@ -63,9 +62,7 @@ class Chef
 
         # Extension of the certificate
         ext = ::File.extname(new_resource.source)
-        raw_source = convert_pem(ext)
-
-        cert_obj = OpenSSL::X509::Certificate.new(raw_source) # A certificate object in memory
+        cert_obj = fetch_cert_object(ext) # Fetch OpenSSL::X509::Certificate object
         thumbprint = OpenSSL::Digest::SHA1.new(cert_obj.to_der).to_s # Fetch its thumbprint
 
         # Need to check if return value is Boolean:true
@@ -249,6 +246,7 @@ class Chef
 
         def acl_script(hash)
           return "" if new_resource.private_key_acl.nil? || new_resource.private_key_acl.empty?
+
           # this PS came from http://blogs.technet.com/b/operationsguy/archive/2010/11/29/provide-access-to-private-keys-commandline-vs-powershell.aspx
           # and from https://msdn.microsoft.com/en-us/library/windows/desktop/bb204778(v=vs.85).aspx
           set_acl_script = <<-EOH
@@ -274,42 +272,41 @@ class Chef
           set_acl_script
         end
 
-        # Uses powershell command to convert crt/der/cer/pfx & p7b certificates
-        # In PEM format and returns its certificate content
-        def convert_pem(ext)
-          command = case ext
-                    when ".crt", ".cer", ".der"
-                      cmd = "openssl x509 -text -in #{new_resource.source} -outform PEM"
-                      pem_cert? ? cmd : cmd + " -inform DER"
-                    when ".pfx"
-                      "openssl pkcs12 -in #{new_resource.source} -nodes -passin pass:'#{new_resource.pfx_password}'"
-                    when ".p7b"
-                      "openssl pkcs7 -print_certs -in #{new_resource.source} -outform PEM"
-                    else
-                      "openssl x509 -text -inform #{ext.delete('.')} -in #{new_resource.source} -outform PEM"
-                    end
-
-          out = powershell_out(command)
-          if out.exitstatus == 0
-            format_raw_out(out.stdout)
+        # Method returns an OpenSSL::X509::Certificate object
+        #
+        # Based on its extension, the certificate contents are used to initialize
+        # PKCS12 (PFX), PKCS7 (P7B) objects which contains OpenSSL::X509::Certificate.
+        #
+        # @note Other then PEM, all the certificates are usually in binary format, and hence
+        #       their contents are loaded by using File.binread
+        #
+        # @param ext [String] Extension of the certificate
+        #
+        # @return [OpenSSL::X509::Certificate] Object containing certificate's attributes
+        #
+        # @raise [OpenSSL::PKCS12::PKCS12Error] When incorrect password is provided for PFX certificate
+        #
+        def fetch_cert_object(ext)
+          contents = if binary_cert?
+                       ::File.binread(new_resource.source)
+                     else
+                       ::File.read(new_resource.source)
+                     end
+
+          case ext
+          when ".pfx"
+            OpenSSL::PKCS12.new(contents, new_resource.pfx_password).certificate
+          when ".p7b"
+            OpenSSL::PKCS7.new(contents).certificates.first
           else
-            raise out.stderr
+            OpenSSL::X509::Certificate.new(contents)
           end
         end
 
-        # Returns the certificate content
-        def format_raw_out(out)
-          begin_cert = "-----BEGIN CERTIFICATE-----"
-          end_cert = "-----END CERTIFICATE-----"
-          begin_cert + out[/#{begin_cert}(.*?)#{end_cert}/m, 1] + end_cert
-        end
-
-        # Checks if the given certificate is a PEM certificate or not
-        def pem_cert?
-          details, status = Open3.capture2e("file", new_resource.source)
-          return false unless status.success?
-
-          details.rpartition(":").last.strip == "PEM certificate"
+        # @return [Boolean] Whether the certificate file is binary encoded or not
+        #
+        def binary_cert?
+          powershell_out!("file -b --mime-encoding #{new_resource.source}").stdout.strip == "binary"
         end
       end
     end
diff --git a/spec/functional/resource/windows_certificate_spec.rb b/spec/functional/resource/windows_certificate_spec.rb
index a9ed99d318..eac7ee31b2 100644
--- a/spec/functional/resource/windows_certificate_spec.rb
+++ b/spec/functional/resource/windows_certificate_spec.rb
@@ -228,7 +228,7 @@ describe Chef::Resource::WindowsCertificate, :windows_only, :appveyor_only do
           win_certificate.pfx_password = "Invalid password"
         end
         it "Raises an error" do
-          expect { win_certificate.run_action(:create) }.to raise_error(RuntimeError)
+          expect { win_certificate.run_action(:create) }.to raise_error(OpenSSL::PKCS12::PKCS12Error)
         end
       end
     end