Merge pull request #13552 from chef/jfm/chef18_cert_retrieval2

Correcting cert retrieval issues for multiple user scenarios

3 files changed, 112 insertions, 29 deletions

diff --git a/spec/integration/client/client_spec.rb b/spec/integration/client/client_spec.rb
index 1bd84fa940..38d414701a 100644
--- a/spec/integration/client/client_spec.rb
+++ b/spec/integration/client/client_spec.rb
@@ -35,14 +35,14 @@ describe "chef-client" do
     @server = @api = nil
   end
 
-  def install_certificate_in_store(client_name)
+  def install_certificate_in_store(client_name, store_location)
     if ChefUtils.windows?
       powershell_exec! <<~EOH
         if (-not (($PSVersionTable.PSVersion.Major -ge 5) -and ($PSVersionTable.PSVersion.Build -ge 22000)) ) {
-          New-SelfSignedCertificate -CertStoreLocation Cert:\\LocalMachine\\My -DnsName "#{client_name}"
+          New-SelfSignedCertificate -CertStoreLocation Cert:\\#{store_location}\\My -DnsName "#{client_name}"
         }
         else {
-          New-SelfSignedCertificate -CertStoreLocation Cert:\\LocalMachine\\My -Subject "#{client_name}" -FriendlyName "#{client_name}" -KeyExportPolicy Exportable
+          New-SelfSignedCertificate -CertStoreLocation Cert:\\#{store_location}\\My -Subject "#{client_name}" -FriendlyName "#{client_name}" -KeyExportPolicy Exportable
         }
       EOH
     end
@@ -50,14 +50,6 @@ describe "chef-client" do
 
   def create_registry_key
     ::Chef::HTTP::Authenticator.get_cert_password
-    # @win32registry = Chef::Win32::Registry.new
-    # path = "HKEY_LOCAL_MACHINE\\Software\\Progress\\Authentication"
-    # unless @win32registry.key_exists?(path)
-    #   @win32registry.create_key(path, true)
-    # end
-    # password = SOME_CHARS.sample(1 + rand(SOME_CHARS.count)).join[0...14]
-    # values = { name: "PfxPass", type: :string, data: password }
-    # @win32registry.set_value(path, values)
   end
 
   def remove_certificate_from_store
@@ -111,6 +103,9 @@ describe "chef-client" do
       tempfile.close
       @path = tempfile.path
       Chef::Config.validation_key = @path
+      if ChefUtils.windows?
+        create_registry_key
+      end
 
       file "config/client.rb", <<~EOM
        local_mode true
@@ -201,17 +196,27 @@ describe "chef-client" do
 
     if ChefUtils.windows?
       context "and the private key is in the Windows CertStore" do
-        before do
-          install_certificate_in_store(client_name)
+
+        it "should verify that the cert is loaded in the \\LocalMachine\\My store" do
+          Chef::Config[:auth_key_registry_type] = "machine"
+          install_certificate_in_store(client_name, "LocalMachine")
           create_registry_key
+          expect(Chef::HTTP::Authenticator.check_certstore_for_key(hostname)).to eq(true)
         end
 
-        after do
+        it "should verify that the export password for the pfx is loaded in the Registry" do
+          expect(verify_export_password_exists.result).to eq(true)
+        end
+
+        it "should verify that a private key is returned to me" do
+          expect(Chef::HTTP::Authenticator.retrieve_certificate_key(client_name)).not_to be nil
           remove_certificate_from_store
-          remove_registry_key
         end
 
-        it "should verify that the cert is loaded in the LocalMachine\\My" do
+        it "should verify that the cert is loaded in the \\CurrentUser\\My store" do
+          Chef::Config[:auth_key_registry_type] = "user"
+          install_certificate_in_store(client_name, "CurrentUser")
+          create_registry_key
           expect(Chef::HTTP::Authenticator.check_certstore_for_key(hostname)).to eq(true)
         end
 
@@ -221,6 +226,7 @@ describe "chef-client" do
 
         it "should verify that a private key is returned to me" do
           expect(Chef::HTTP::Authenticator.retrieve_certificate_key(client_name)).not_to be nil
+          remove_certificate_from_store
         end
       end
     end
diff --git a/spec/unit/client_spec.rb b/spec/unit/client_spec.rb
index 7f02eacb39..ae11fd9793 100644
--- a/spec/unit/client_spec.rb
+++ b/spec/unit/client_spec.rb
@@ -310,25 +310,49 @@ describe Chef::Client, :windows_only do
   end
 
   context "when the client intially boots the first time" do
-    it "verfies that a certificate was correctly created and exists in the Cert Store" do
+    it "verfies that a certificate was correctly created and exists in the LocalMachine Cert Store" do
+      Chef::Config[:node_name] = "test"
       new_pfx = my_client.generate_pfx_package(cert_name, end_date)
       my_client.import_pfx_to_store(new_pfx)
       expect(my_client.check_certstore_for_key(cert_name)).not_to be false
+      delete_certificate(cert_name)
     end
 
     it "correctly returns a new Publc Key" do
       new_pfx = my_client.generate_pfx_package(cert_name, end_date)
       cert_object = new_pfx.certificate.public_key.to_pem
       expect(cert_object.to_s).to match(/PUBLIC KEY/)
+      delete_certificate(cert_name)
+    end
+
+  end
+
+  context "when the client intially boots the first time and auth_key_registry_type is set to 'user' " do
+    it "verfies that a certificate was correctly created and exists in the CurrentUser Cert Store" do
+      Chef::Config[:node_name] = "test"
+      Chef::Config[:auth_key_registry_type] = "user"
+      new_pfx = my_client.generate_pfx_package(cert_name, end_date)
+      my_client.import_pfx_to_store(new_pfx)
+      expect(my_client.check_certstore_for_key(cert_name)).not_to be false
+      delete_certificate(cert_name)
+    end
+
+    it "correctly returns a new Publc Key" do
+      Chef::Config[:auth_key_registry_type] = "user"
+      new_pfx = my_client.generate_pfx_package(cert_name, end_date)
+      cert_object = new_pfx.certificate.public_key.to_pem
+      expect(cert_object.to_s).to match(/PUBLIC KEY/)
+      delete_certificate(cert_name)
     end
 
   end
 
   def delete_certificate(cert_name)
+    Chef::Config[:auth_key_registry_type] == "user" ? store = "CurrentUser" : store = "LocalMachine"
     require "chef/mixin/powershell_exec"
     extend Chef::Mixin::PowershellExec
     powershell_code = <<~CODE
-      Get-ChildItem -path cert:\\LocalMachine\\My -Recurse -Force  | Where-Object { $_.Subject -Match "#{cert_name}" } | Remove-item
+      Get-ChildItem -path cert:\\#{store}\\My -Recurse -Force  | Where-Object { $_.Subject -Match "#{cert_name}" } | Remove-item
     CODE
     powershell_exec!(powershell_code)
   end
diff --git a/spec/unit/http/authenticator_spec.rb b/spec/unit/http/authenticator_spec.rb
index bbd952b436..a088a1f4f9 100644
--- a/spec/unit/http/authenticator_spec.rb
+++ b/spec/unit/http/authenticator_spec.rb
@@ -18,6 +18,9 @@
 
 require "spec_helper"
 require "chef/http/authenticator"
+require "chef/mixin/powershell_exec"
+
+require_relative "../../../lib/chef/win32/registry"
 
 describe Chef::HTTP::Authenticator, :windows_only do
   let(:class_instance) { Chef::HTTP::Authenticator.new(client_name: "test") }
@@ -28,7 +31,7 @@ describe Chef::HTTP::Authenticator, :windows_only do
   let(:node_name) { "test" }
   let(:passwrd) { "some_insecure_password" }
 
-  before do
+  before(:each) do
     Chef::Config[:node_name] = node_name
     cert_name = "chef-#{node_name}"
     d = Time.now
@@ -36,6 +39,7 @@ describe Chef::HTTP::Authenticator, :windows_only do
     end_date = end_date.utc.iso8601
 
     my_client = Chef::Client.new
+    class_instance.get_cert_password
     pfx = my_client.generate_pfx_package(cert_name, end_date)
     my_client.import_pfx_to_store(pfx)
   end
@@ -47,10 +51,21 @@ describe Chef::HTTP::Authenticator, :windows_only do
     delete_certificate(cert_name)
   end
 
-  context "when retrieving a certificate from the certificate store" do
+  context "when retrieving a certificate from the certificate store it" do
+    it "properly creates the password hive in the registry when it doesn't exist" do
+      delete_registry_hive
+      class_instance.get_cert_password
+      win32registry = Chef::Win32::Registry.new
+      expected_path = "HKEY_LOCAL_MACHINE\\Software\\Progress\\Authentication"
+      path_created = win32registry.key_exists?(expected_path)
+      expect(path_created).to be(true)
+    end
+
     it "retrieves a certificate password from the registry when the hive does not already exist" do
       delete_registry_hive
+      password = class_instance.get_cert_password
       expect { class_instance.get_cert_password }.not_to raise_error
+      expect(password).not_to be(nil)
     end
 
     it "should return a password of at least 14 characters in length" do
@@ -58,7 +73,27 @@ describe Chef::HTTP::Authenticator, :windows_only do
       expect(password.length).to eql(14)
     end
 
-    it "correctly retrieves a valid certificate in pem format from the certstore" do
+    it "will retrieve a password from a partial registry hive and upgrades it while using the old decryptor" do
+      delete_registry_hive
+      load_partial_registry_hive
+      password = class_instance.get_cert_password
+      expect(password).to eql(passwrd)
+    end
+
+    it "verifies that the new password is now using a vector" do
+      win32registry = Chef::Win32::Registry.new
+      path = "HKEY_LOCAL_MACHINE\\Software\\Progress\\Authentication"
+      password_blob = win32registry.get_values(path)
+      if password_blob.nil? || password_blob.empty?
+        raise Chef::Exceptions::Win32RegKeyMissing
+      end
+
+      raw_data = password_blob.map { |x| x[:data] }
+      vector = raw_data[2]
+      expect(vector).not_to be(nil)
+    end
+
+    it "correctly retrieves a valid certificate in pem format from the LocalMachine certstore" do
       require "openssl"
       certificate = class_instance.retrieve_certificate_key(node_name)
       cert_object = OpenSSL::PKey::RSA.new(certificate)
@@ -66,21 +101,39 @@ describe Chef::HTTP::Authenticator, :windows_only do
     end
   end
 
-  def delete_certificate(cert_name)
+  def load_partial_registry_hive
+    extend Chef::Mixin::PowershellExec
+    password = "some_insecure_password"
     powershell_code = <<~CODE
-      Get-ChildItem -path cert:\\LocalMachine\\My -Recurse -Force  | Where-Object { $_.Subject -Match "#{cert_name}" } | Remove-item
+      $encrypted_string = ConvertTo-SecureString "#{password}" -AsPlainText -Force
+      $secure_string = ConvertFrom-SecureString $encrypted_string
+      return $secure_string
     CODE
-    powershell_exec!(powershell_code)
+    encrypted_pass = powershell_exec!(powershell_code).result
+    Chef::Config[:auth_key_registry_type] == "user" ? store = "HKEY_CURRENT_USER" : store = "HKEY_LOCAL_MACHINE"
+    hive_path = "#{store}\\Software\\Progress\\Authentication"
+    win32registry = Chef::Win32::Registry.new
+    unless win32registry.key_exists?(hive_path)
+      win32registry.create_key(hive_path, true)
+    end
+    values = { name: "PfxPass", type: :string, data: encrypted_pass }
+    win32registry.set_value(hive_path, values)
   end
 
   def delete_registry_hive
-    @win32registry = Chef::Win32::Registry.new
-    path = "HKEY_LOCAL_MACHINE\\Software\\Progress\\Authentication"
-    present = @win32registry.get_values(path)
-    unless present.nil? || present.empty?
-      @win32registry.delete_key(path, true)
+    win32registry = Chef::Win32::Registry.new
+    hive_path = "HKEY_LOCAL_MACHINE\\Software\\Progress"
+    if win32registry.key_exists?(hive_path)
+      win32registry.delete_key(hive_path, true)
     end
   end
+
+  def delete_certificate(cert_name)
+    powershell_code = <<~CODE
+      Get-ChildItem -path cert:\\LocalMachine\\My -Recurse -Force  | Where-Object { $_.Subject -Match "#{cert_name}" } | Remove-item
+    CODE
+    powershell_exec!(powershell_code)
+  end
 end
 
 describe Chef::HTTP::Authenticator do