Merge pull request #8242 from MsysTechnologiesllc/MSYS-937_win_certificate_import_nested

windows_certificate: Import nested certificates while importing P7B certs.

3 files changed, 111 insertions, 18 deletions

diff --git a/lib/chef/resource/windows_certificate.rb b/lib/chef/resource/windows_certificate.rb
index ebc846644b..e7261adfba 100644
--- a/lib/chef/resource/windows_certificate.rb
+++ b/lib/chef/resource/windows_certificate.rb
@@ -61,22 +61,9 @@ class Chef
 
         # Extension of the certificate
         ext = ::File.extname(new_resource.source)
-        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
-        # If not then the given certificate should be added in certstore
-        if verify_cert(thumbprint) == true
-          Chef::Log.debug("Certificate is already present")
-        else
-          converge_by("Adding certificate #{new_resource.source} into Store #{new_resource.store_name}") do
-            if ext == ".pfx"
-              add_pfx_cert
-            else
-              add_cert(cert_obj)
-            end
-          end
-        end
+        # PFX certificates contains private keys and we import them with some other aproach
+        import_certificates(fetch_cert_object(ext), (ext == ".pfx"))
       end
 
       # acl_add is a modify-if-exists operation : not idempotent
@@ -271,7 +258,7 @@ class Chef
           set_acl_script
         end
 
-        # Method returns an OpenSSL::X509::Certificate object
+        # Method returns an OpenSSL::X509::Certificate object. Might also return multiple certificates if present in certificate path
         #
         # Based on its extension, the certificate contents are used to initialize
         # PKCS12 (PFX), PKCS7 (P7B) objects which contains OpenSSL::X509::Certificate.
@@ -294,9 +281,14 @@ class Chef
 
           case ext
           when ".pfx"
-            OpenSSL::PKCS12.new(contents, new_resource.pfx_password).certificate
+            pfx = OpenSSL::PKCS12.new(contents, new_resource.pfx_password)
+            if pfx.ca_certs.nil?
+              pfx.certificate
+            else
+              [pfx.certificate] + pfx.ca_certs
+            end
           when ".p7b"
-            OpenSSL::PKCS7.new(contents).certificates.first
+            OpenSSL::PKCS7.new(contents).certificates
           else
             OpenSSL::X509::Certificate.new(contents)
           end
@@ -307,6 +299,32 @@ class Chef
         def binary_cert?
           powershell_out!("file -b --mime-encoding #{new_resource.source}").stdout.strip == "binary"
         end
+
+        # Imports the certificate object into cert store
+        #
+        # @param cert_objs [OpenSSL::X509::Certificate] Object containing certificate's attributes
+        #
+        # @param is_pfx [Boolean] true if we want to import a PFX certificate
+        #
+        def import_certificates(cert_objs, is_pfx)
+          [cert_objs].flatten.each do |cert_obj|
+            thumbprint = OpenSSL::Digest::SHA1.new(cert_obj.to_der).to_s # Fetch its thumbprint
+
+            # Need to check if return value is Boolean:true
+            # If not then the given certificate should be added in certstore
+            if verify_cert(thumbprint) == true
+              Chef::Log.debug("Certificate is already present")
+            else
+              converge_by("Adding certificate #{new_resource.source} into Store #{new_resource.store_name}") do
+                if is_pfx
+                  add_pfx_cert
+                else
+                  add_cert(cert_obj)
+                end
+              end
+            end
+          end
+        end
       end
     end
   end
diff --git a/spec/data/windows_certificates/test.p7b b/spec/data/windows_certificates/test.p7b
new file mode 100644
index 0000000000..cf8cfae58c
--- /dev/null
+++ b/spec/data/windows_certificates/test.p7b
diff --git a/spec/functional/resource/windows_certificate_spec.rb b/spec/functional/resource/windows_certificate_spec.rb
index eac7ee31b2..79b3cd890b 100644
--- a/spec/functional/resource/windows_certificate_spec.rb
+++ b/spec/functional/resource/windows_certificate_spec.rb
@@ -62,11 +62,14 @@ describe Chef::Resource::WindowsCertificate, :windows_only, :appveyor_only do
   let(:cer_path) { File.join(certificate_path, "test.cer") }
   let(:base64_path) { File.join(certificate_path, "base64_test.cer") }
   let(:pem_path) { File.join(certificate_path, "test.pem") }
+  let(:p7b_path) { File.join(certificate_path, "test.p7b") }
   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") }
   let(:others_thumbprint) { "AD393859B2D2D4161D224F16CBD3D16555753A20" }
+  let(:p7b_thumbprint) { "50954A52DDFA2043F36EA9026FDD95EC252048D0" }
+  let(:p7b_nested_thumbprint) { "4A3333FC4E1274995AF5A95810881C86F2DF7FBD" }
 
   before do
     opts = { store_name: store }
@@ -205,6 +208,23 @@ describe Chef::Resource::WindowsCertificate, :windows_only, :appveyor_only do
       end
     end
 
+    context "Adds P7B" do
+      before do
+        win_certificate.source = p7b_path
+        win_certificate.run_action(:create)
+      end
+      it "Imports certificate into store" do
+        expect(no_of_certificates).not_to eq(0)
+      end
+      it "Idempotent: Does not converge while adding again" do
+        win_certificate.run_action(:create)
+        expect(win_certificate).not_to be_updated_by_last_action
+      end
+      it "Nested certificates are also imported" do
+        expect(no_of_certificates).to eq(2)
+      end
+    end
+
     context "Adds PFX" do
       context "With valid password" do
         before do
@@ -289,6 +309,61 @@ describe Chef::Resource::WindowsCertificate, :windows_only, :appveyor_only do
         end
       end
     end
+
+    context "When multiple certificates are present" do
+      before do
+        win_certificate.source = p7b_path
+        win_certificate.run_action(:create)
+      end
+
+      context "With main certificate's thumbprint" do
+        before do
+          win_certificate.source = p7b_thumbprint
+          win_certificate.run_action(:verify)
+        end
+        it "Initial check if certificate is present" do
+          expect(no_of_certificates).to eq(2)
+        end
+        it "Displays correct message" do
+          expect(stdout.string.strip).to eq("Certificate is valid")
+        end
+        it "Does not converge while verifying" do
+          expect(win_certificate).not_to be_updated_by_last_action
+        end
+      end
+
+      context "With nested certificate's thumbprint" do
+        before do
+          win_certificate.source = p7b_nested_thumbprint
+          win_certificate.run_action(:verify)
+        end
+        it "Initial check if certificate is present" do
+          expect(no_of_certificates).to eq(2)
+        end
+        it "Displays correct message" do
+          expect(stdout.string.strip).to eq("Certificate is valid")
+        end
+        it "Does not converge while verifying" do
+          expect(win_certificate).not_to be_updated_by_last_action
+        end
+      end
+
+      context "For an invalid thumbprint" do
+        before do
+          win_certificate.source = others_thumbprint
+          win_certificate.run_action(:verify)
+        end
+        it "Initial check if certificate is present" do
+          expect(no_of_certificates).to eq(2)
+        end
+        it "Displays correct message" do
+          expect(stdout.string.strip).to eq("Certificate not found")
+        end
+        it "Does not converge while verifying" do
+          expect(win_certificate).not_to be_updated_by_last_action
+        end
+      end
+    end
   end
 
   describe "action: fetch" do