Import PFX certificates with their private keys

- Using `add_pfx` of Win32::Certstore to import a PFX certificate with its thumbprint
- Using correct version of `win32-certstore` to support these changes
- Added Test cases
- Minor cleanup and opimization

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

4 files changed, 90 insertions, 19 deletions

diff --git a/Gemfile.lock b/Gemfile.lock
index ae1df9e411..05eb8ec1b9 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -63,7 +63,7 @@ PATH
       syslog-logger (~> 1.6)
       uuidtools (~> 2.1.5)
       win32-api (~> 1.5.3)
-      win32-certstore (>= 0.1.8)
+      win32-certstore (~> 0.2.4)
       win32-dir (~> 0.5.0)
       win32-event (~> 0.6.1)
       win32-eventlog (= 0.6.3)
diff --git a/chef-universal-mingw32.gemspec b/chef-universal-mingw32.gemspec
index 7e1a9d1551..9bbcbc635d 100644
--- a/chef-universal-mingw32.gemspec
+++ b/chef-universal-mingw32.gemspec
@@ -16,7 +16,7 @@ gemspec.add_dependency "windows-api", "~> 0.4.4"
 gemspec.add_dependency "wmi-lite", "~> 1.0"
 gemspec.add_dependency "win32-taskscheduler", "~> 2.0"
 gemspec.add_dependency "iso8601", "~> 0.12.1"
-gemspec.add_dependency "win32-certstore", ">= 0.1.8"
+gemspec.add_dependency "win32-certstore", "~> 0.2.4"
 gemspec.extensions << "ext/win32-eventlog/Rakefile"
 gemspec.files += Dir.glob("{distro,ext}/**/*")
 
diff --git a/lib/chef/resource/windows_certificate.rb b/lib/chef/resource/windows_certificate.rb
index 8b6a52711a..0dc8ee31b2 100644
--- a/lib/chef/resource/windows_certificate.rb
+++ b/lib/chef/resource/windows_certificate.rb
@@ -60,6 +60,10 @@ class Chef
       action :create do
         description "Creates or updates a certificate."
 
+        # 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
         thumbprint = OpenSSL::Digest::SHA1.new(cert_obj.to_der).to_s # Fetch its thumbprint
 
@@ -69,7 +73,11 @@ class Chef
           Chef::Log.debug("Certificate is already present")
         else
           converge_by("Adding certificate #{new_resource.source} into Store #{new_resource.store_name}") do
-            add_cert(cert_obj)
+            if ext == ".pfx"
+              add_pfx_cert
+            else
+              add_cert(cert_obj)
+            end
           end
         end
       end
@@ -139,6 +147,11 @@ class Chef
           store.add(cert_obj)
         end
 
+        def add_pfx_cert
+          store = ::Win32::Certstore.open(new_resource.store_name)
+          store.add_pfx(new_resource.source, new_resource.pfx_password)
+        end
+
         def delete_cert
           store = ::Win32::Certstore.open(new_resource.store_name)
           store.delete(new_resource.source)
@@ -260,28 +273,25 @@ class Chef
           set_acl_script
         end
 
-        # Returns the certificate string of the given
-        # input certificate in PEM format
-        def raw_source
-          ext = ::File.extname(new_resource.source)
-          convert_pem(ext, new_resource.source)
-        end
-
         # Uses powershell command to convert crt/der/cer/pfx & p7b certificates
         # In PEM format and returns its certificate content
-        def convert_pem(ext, source)
+        def convert_pem(ext)
           out = case ext
-                when ".crt", ".der"
-                  powershell_out("openssl x509 -text -inform DER -in #{source} -outform PEM").stdout
-                when ".cer"
-                  powershell_out("openssl x509 -text -inform DER -in #{source} -outform PEM").stdout
+                when ".crt", ".cer", ".der"
+                  powershell_out("openssl x509 -text -inform DER -in #{new_resource.source} -outform PEM")
                 when ".pfx"
-                  powershell_out("openssl pkcs12 -in #{source} -nodes -passin pass:'#{new_resource.pfx_password}'").stdout
+                  powershell_out("openssl pkcs12 -in #{new_resource.source} -nodes -passin pass:'#{new_resource.pfx_password}'")
                 when ".p7b"
-                  powershell_out("openssl pkcs7 -print_certs -in #{source} -outform PEM").stdout
+                  powershell_out("openssl pkcs7 -print_certs -in #{new_resource.source} -outform PEM")
+                else
+                  powershell_out("openssl x509 -text -inform #{ext.delete(".")} -in #{new_resource.source} -outform PEM")
                 end
-          out = ::File.read(source) if out.nil? || out.empty?
-          format_raw_out(out)
+
+          if out.exitstatus == 0
+            format_raw_out(out.stdout)
+          else
+            raise out.stderr
+          end
         end
 
         # Returns the certificate content
diff --git a/spec/functional/resource/windows_certificate_spec.rb b/spec/functional/resource/windows_certificate_spec.rb
index 188a0dc28d..f60b63ade9 100644
--- a/spec/functional/resource/windows_certificate_spec.rb
+++ b/spec/functional/resource/windows_certificate_spec.rb
@@ -61,6 +61,7 @@ describe Chef::Resource::WindowsCertificate, :windows_only, :appveyor_only do
   let(:certificate_path) { File.expand_path(File.join(CHEF_SPEC_DATA, "windows_certificates")) }
   let(:cer_path) { File.join(certificate_path, "test.cer") }
   let(:pem_path) { File.join(certificate_path, "test.pem") }
+  let(:pfx_path) { File.join(certificate_path, "test.pfx") }
   let(:out_path) { File.join(certificate_path, "testout.pem") }
   let(:tests_thumbprint) { "3180B3E3217862600BD7B2D28067B03D41576A4F" }
   let(:other_cer_path) { File.join(certificate_path, "othertest.cer") }
@@ -157,6 +158,66 @@ describe Chef::Resource::WindowsCertificate, :windows_only, :appveyor_only do
     end
   end
 
+  describe "Works for various formats" do
+    context "Adds CER" do
+      before do
+        win_certificate.source = cer_path
+        win_certificate.run_action(:create)
+      end
+      it "Imports certificate into store" do
+        expect(no_of_certificates).to eq(1)
+      end
+      it "Idempotent: Does not converge while adding again" do
+        win_certificate.run_action(:create)
+        expect(no_of_certificates).to eq(1)
+        expect(win_certificate).not_to be_updated_by_last_action
+      end
+    end
+
+    context "Adds PEM" do
+      before do
+        win_certificate.source = pem_path
+        win_certificate.run_action(:create)
+      end
+      it "Imports certificate into store" do
+        expect(no_of_certificates).to eq(1)
+      end
+      it "Idempotent: Does not converge while adding again" do
+        win_certificate.run_action(:create)
+        expect(no_of_certificates).to eq(1)
+        expect(win_certificate).not_to be_updated_by_last_action
+      end
+    end
+
+    context "Adds PFX" do
+      context "With valid password" do
+        before do
+          win_certificate.source = pfx_path
+          win_certificate.pfx_password = password
+          win_certificate.run_action(:create)
+        end
+        it "Imports certificate into store" do
+          expect(no_of_certificates).to eq(1)
+        end
+        it "Idempotent: Does not converge while adding again" do
+          win_certificate.run_action(:create)
+          expect(no_of_certificates).to eq(1)
+          expect(win_certificate).not_to be_updated_by_last_action
+        end
+      end
+
+      context "With Invalid password" do
+        before do
+          win_certificate.source = pfx_path
+          win_certificate.pfx_password = "Invalid password"
+        end
+        it "Raises an error" do
+          expect { win_certificate.run_action(:create) }.to raise_error(RuntimeError)
+        end
+      end
+    end
+  end
+
   describe "action: verify" do
     context "When a certificate is not present" do
       before do