Update Chef Client to retrieve client.pem from Windows CertStore

Signed-off-by: John McCrae <john.mccrae@progress.com>

10 files changed, 78 insertions, 86 deletions

diff --git a/Gemfile.lock b/Gemfile.lock
index aa1fb19707..14db94e9c9 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -71,6 +71,7 @@ PATH
       aws-sdk-s3 (~> 1.91)
       aws-sdk-secretsmanager (~> 1.46)
       chef-config (= 18.0.9)
+      chef-powershell (~> 1.0.12)
       chef-utils (= 18.0.9)
       chef-vault
       chef-zero (>= 14.0.11)
@@ -164,7 +165,7 @@ GEM
       debug_inspector (>= 0.0.1)
     builder (3.2.4)
     byebug (11.1.3)
-    chef-powershell (1.0.7)
+    chef-powershell (1.0.11)
       ffi (~> 1.15)
       ffi-yajl (~> 2.4)
     chef-telemetry (1.1.1)
diff --git a/chef-universal-mingw32.gemspec b/chef-universal-mingw32.gemspec
index 798ba3a45b..7b5d82f936 100644
--- a/chef-universal-mingw32.gemspec
+++ b/chef-universal-mingw32.gemspec
@@ -15,7 +15,7 @@ gemspec.add_dependency "wmi-lite", "~> 1.0"
 gemspec.add_dependency "win32-taskscheduler", "~> 2.0"
 gemspec.add_dependency "iso8601", ">= 0.12.1", "< 0.14" # validate 0.14 when it comes out
 gemspec.add_dependency "win32-certstore", "~> 0.6.2" # 0.5+ required for specifying user vs. system store
-gemspec.add_dependency "chef-powershell", "~> 1.0.7" # The guts of the powershell_exec code have been moved to its own gem, chef-powershell. It's part of the chef-powershell-shim repo.
+gemspec.add_dependency "chef-powershell", "~> 1.0.11" # The guts of the powershell_exec code have been moved to its own gem, chef-powershell. It's part of the chef-powershell-shim repo.
 gemspec.extensions << "ext/win32-eventlog/Rakefile"
 gemspec.files += Dir.glob("{distro,ext}/**/*")
 
diff --git a/lib/chef/client.rb b/lib/chef/client.rb
index 006591ce1c..5ec15fb582 100644
--- a/lib/chef/client.rb
+++ b/lib/chef/client.rb
@@ -638,6 +638,9 @@ class Chef
     #
     def register(client_name = node_name, config = Chef::Config)
       if Chef::HTTP::Authenticator.detect_certificate_key(client_name)
+        if File.exists?(config[:client_key])
+          logger.warn("WARNING - Client key #{client_name} is present on disk, ignoring that in favor of key stored in CertStore")
+        end
         events.skipping_registration(client_name, config)
         logger.trace("Client key #{client_name} is present in certificate repository - skipping registration")
         config[:client_key] = "Cert:\\LocalMachine\\My\\chef-#{client_name}"
diff --git a/lib/chef/http/authenticator.rb b/lib/chef/http/authenticator.rb
index 60718ee995..58c964ef6a 100644
--- a/lib/chef/http/authenticator.rb
+++ b/lib/chef/http/authenticator.rb
@@ -26,6 +26,8 @@ class Chef
   class HTTP
     class Authenticator
       DEFAULT_SERVER_API_VERSION = "2".freeze
+      # cspell:disable-next-line
+      SOME_CHARS = "~!@#%^&*_-+=`|\\(){}[<]:;'>,.?/0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz".each_char.to_a
 
       extend Chef::Mixin::PowershellExec
 
@@ -129,7 +131,7 @@ class Chef
 
         if key_file == nil? && raw_key == nil?
           puts "\nNo key detected\n"
-        elsif results != false
+        elsif !!results
           # results variable can be 1 of 2 values - "False" or the contents of a key.
           @raw_key = results
         elsif !!key_file
@@ -162,9 +164,15 @@ class Chef
           raise Chef::Exceptions::Win32RegKeyMissing
         end
 
-        if present.map { |h| h[:name] }[0] == "PfxPass"
-          present.map { |h| h[:data] }[0].to_s
+        present.each do |secret|
+          if secret[:name] == "PfxPass"
+            return secret[:data]
+          end
         end
+
+        # if we make it this far, that means there is no valid password in the Registry. Fail out to correct that.
+        raise Chef::Exceptions::Win32RegKeyMissing
+
       rescue Chef::Exceptions::Win32RegKeyMissing
         # if we don't have a password, log that and generate one
         Chef::Log.warn "Authentication Hive and value not present in registry, creating it now"
@@ -172,12 +180,10 @@ class Chef
         unless @win32registry.key_exists?(new_path)
           @win32registry.create_key(new_path, true)
         end
-        # cspell:disable-next-line
-        some_chars = "~!@#%^&*_-+=`|\\(){}[<]:;'>,.?/0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz".each_char.to_a
-        password = some_chars.sample(1 + rand(some_chars.count)).join[0...14]
+        password = SOME_CHARS.sample(1 + rand(SOME_CHARS.count)).join[0...14]
         values = { name: "PfxPass", type: :string, data: password }
         @win32registry.set_value(new_path, values)
-        password
+        return password
       end
 
       def self.retrieve_certificate_key(client_name)
@@ -187,33 +193,31 @@ class Chef
 
           password = get_cert_password
 
-          unless password
-            return false
+          return false unless password
+
+          if check_certstore_for_key(client_name)
+            powershell_code = <<~CODE
+                Try {
+                  $my_pwd = ConvertTo-SecureString -String "#{password}" -Force -AsPlainText;
+                  $cert = Get-ChildItem -path cert:\\LocalMachine\\My -Recurse | Where-Object { $_.Subject -match "#{client_name}" } -ErrorAction Stop;
+                  $tempfile = [System.IO.Path]::GetTempPath() + "export_pfx.pfx";
+                  Export-PfxCertificate -Cert $cert -Password $my_pwd -FilePath $tempfile;
+                }
+                Catch {
+                  return $false
+                }
+            CODE
+            my_result = powershell_exec!(powershell_code).result
+
+            if !!my_result
+              pkcs = OpenSSL::PKCS12.new(File.binread(my_result["PSPath"].split("::")[1]), password)
+              ::File.delete(my_result["PSPath"].split("::")[1])
+              return OpenSSL::PKey::RSA.new(pkcs.key.to_pem)
+            end
           end
-
-          powershell_code = <<~CODE
-              Try {
-                $my_pwd = ConvertTo-SecureString -String "#{password}" -Force -AsPlainText;
-                $cert = Get-ChildItem -path cert:\\LocalMachine\\My -Recurse | Where-Object { $_.Subject -match "#{client_name}" } -ErrorAction Stop;
-                $tempfile = [System.IO.Path]::GetTempPath() + "export_pfx.pfx";
-                Export-PfxCertificate -Cert $cert -Password $my_pwd -FilePath $tempfile;
-              }
-              Catch {
-                return $false
-              }
-          CODE
-          my_result = powershell_exec!(powershell_code).result
-
-          if my_result != false
-            pkcs = OpenSSL::PKCS12.new(File.binread(my_result["PSPath"].split("::")[1]), password)
-            ::File.delete(my_result["PSPath"].split("::")[1])
-            OpenSSL::PKey::RSA.new(pkcs.key.to_pem)
-          else
-            false
-          end
-        else # doing nothing for the Mac and Linux clients for now
-          false
         end
+
+        return false
       end
 
       def authentication_headers(method, url, json_body = nil, headers = nil)
diff --git a/lib/chef/mixin/powershell_exec.rb b/lib/chef/mixin/powershell_exec.rb
index c76c891054..8b11529957 100644
--- a/lib/chef/mixin/powershell_exec.rb
+++ b/lib/chef/mixin/powershell_exec.rb
@@ -99,7 +99,7 @@ class Chef
   module Mixin
     module PowershellExec
       if ChefUtils.windows?
-        include Chef_PowerShell::ChefPowerShell::PowerShellExec
+        include ChefPowerShell::ChefPowerShellModule::PowerShellExec
       end
     end
   end
diff --git a/omnibus/Gemfile.lock b/omnibus/Gemfile.lock
index 815afcd1ff..fadeac802d 100644
--- a/omnibus/Gemfile.lock
+++ b/omnibus/Gemfile.lock
@@ -283,7 +283,7 @@ GEM
     net-ssh-gateway (2.0.0)
       net-ssh (>= 4.0.0)
     nori (2.6.0)
-    octokit (4.21.0)
+    octokit (4.22.0)
       faraday (>= 0.9)
       sawyer (~> 0.8.0, >= 0.5.3)
     ohai (17.9.0)
diff --git a/spec/functional/resource/dsc_script_spec.rb b/spec/functional/resource/dsc_script_spec.rb
index b83c1302c2..3831486547 100644
--- a/spec/functional/resource/dsc_script_spec.rb
+++ b/spec/functional/resource/dsc_script_spec.rb
@@ -260,11 +260,10 @@ describe Chef::Resource::DscScript, :windows_powershell_dsc_only, :ruby64_only d
       end
 
       it "should raise an exception if the cwd is etc" do
-        # Chef_PowerShell::PowerShellExceptions::PowerShellCommandFailed
         dsc_test_resource.cwd(dsc_environment_fail_etc_directory)
         expect {
           dsc_test_resource.run_action(:run)
-        }.to raise_error(Chef_PowerShell::PowerShellExceptions::PowerShellCommandFailed, /#{exception_message_signature}/)
+        }.to raise_error(ChefPowerShell::PowerShellExceptions::PowerShellCommandFailed, /#{exception_message_signature}/)
       end
     end
   end
diff --git a/spec/integration/client/client_spec.rb b/spec/integration/client/client_spec.rb
index 27a1d3c0d8..a3d42ac081 100644
--- a/spec/integration/client/client_spec.rb
+++ b/spec/integration/client/client_spec.rb
@@ -6,6 +6,9 @@ require "tmpdir"
 require "chef-utils/dist"
 require "chef/mixin/powershell_exec"
 
+# cspell:disable-next-line
+SOME_CHARS = "~!@#%^&*_-+=`|\\(){}[<]:;'>,.?/0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz".freeze
+
 describe "chef-client" do
 
   def recipes_filename
@@ -41,7 +44,7 @@ describe "chef-client" do
   def create_registry_key
     powershell_exec! <<~EOH
       $pfx_password = New-Object -TypeName PSObject
-      $pfx_password | Add-Member -MemberType ScriptProperty -Name "Password" -Value { ("~!@#$%^&*_-+=`|\\(){}[<]:;'>,.?/0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz".tochararray() | Sort-Object { Get-Random })[0..14] -join '' }
+      $pfx_password | Add-Member -MemberType ScriptProperty -Name "Password" -Value { (#{SOME_CHARS}.tochararray() | Sort-Object { Get-Random })[0..14] -join '' }
       if (-not (Test-Path HKLM:\\SOFTWARE\\Progress)){
         New-Item -Path "HKLM:\\SOFTWARE\\Progress\\Authentication" -Force
         New-ItemProperty  -path "HKLM:\\SOFTWARE\\Progress\\Authentication" -name "PfxPass" -value $pfx_password.Password -PropertyType String
@@ -203,7 +206,6 @@ describe "chef-client" do
         end
 
         it "should verify that the cert is loaded in the LocalMachine\\My" do
-          expect(Chef::HTTP::Authenticator).to receive(:check_certstore_for_key).and_call_original
           expect(Chef::HTTP::Authenticator.check_certstore_for_key(client_name)).to eq(true)
         end
 
@@ -212,7 +214,6 @@ describe "chef-client" do
         end
 
         it "should verify that a private key is returned to me" do
-          expect(Chef::HTTP::Authenticator).to receive(:retrieve_certificate_key).and_call_original
           expect(Chef::HTTP::Authenticator.retrieve_certificate_key(client_name)).not_to be_falsey
         end
       end
diff --git a/spec/unit/mixin/powershell_exec_spec.rb b/spec/unit/mixin/powershell_exec_spec.rb
index 6a0af4a6e5..b7e7f2a160 100644
--- a/spec/unit/mixin/powershell_exec_spec.rb
+++ b/spec/unit/mixin/powershell_exec_spec.rb
@@ -26,7 +26,7 @@ describe Chef::Mixin::PowershellExec, :windows_only do
   describe "#powershell_exec" do
     context "not specifying an interpreter" do
       it "runs a basic command and returns a Chef::PowerShell object" do
-        expect(object.powershell_exec("$PSVersionTable")).to be_kind_of(Chef_PowerShell::PowerShell)
+        expect(object.powershell_exec("$PSVersionTable")).to be_kind_of(ChefPowerShell::PowerShell)
       end
 
       it "uses less than version 6" do
@@ -37,7 +37,7 @@ describe Chef::Mixin::PowershellExec, :windows_only do
 
     context "using pwsh interpreter" do
       it "runs a basic command and returns a Chef::PowerShell object" do
-        expect(object.powershell_exec("$PSVersionTable", :pwsh)).to be_kind_of(Chef_PowerShell::Pwsh)
+        expect(object.powershell_exec("$PSVersionTable", :pwsh)).to be_kind_of(ChefPowerShell::Pwsh)
       end
 
       it "uses greater than version 6" do
@@ -48,7 +48,7 @@ describe Chef::Mixin::PowershellExec, :windows_only do
 
     context "using powershell interpreter" do
       it "runs a basic command and returns a Chef::PowerShell object" do
-        expect(object.powershell_exec("$PSVersionTable", :powershell)).to be_kind_of(Chef_PowerShell::PowerShell)
+        expect(object.powershell_exec("$PSVersionTable", :powershell)).to be_kind_of(ChefPowerShell::PowerShell)
       end
 
       it "uses less than version 6" do
@@ -76,11 +76,11 @@ describe Chef::Mixin::PowershellExec, :windows_only do
 
   describe "#powershell_exec!" do
     it "runs a basic command and returns a Chef::PowerShell object" do
-      expect(object.powershell_exec!("$PSVersionTable")).to be_kind_of(Chef_PowerShell::PowerShell)
+      expect(object.powershell_exec!("$PSVersionTable")).to be_kind_of(ChefPowerShell::PowerShell)
     end
 
     it "raises an error if the command fails" do
-      expect { object.powershell_exec!("this-should-error") }.to raise_error(Chef_PowerShell::PowerShellExceptions::PowerShellCommandFailed)
+      expect { object.powershell_exec!("this-should-error") }.to raise_error(ChefPowerShell::PowerShellExceptions::PowerShellCommandFailed)
     end
 
     it "raises an error if the interpreter is invalid" do
diff --git a/spec/unit/platform/query_helpers_spec.rb b/spec/unit/platform/query_helpers_spec.rb
index b030fcf1b3..3537e2f6b3 100644
--- a/spec/unit/platform/query_helpers_spec.rb
+++ b/spec/unit/platform/query_helpers_spec.rb
@@ -18,52 +18,36 @@
 
 require "spec_helper"
 
-if ChefUtils.windows?
-  describe "Chef::Platform#supports_dsc_invoke_resource?" do
-    it "returns false if powershell is not present" do
-      node = Chef::Node.new
-      expect(Chef::Platform.supports_dsc_invoke_resource?(node)).to be_falsey
-    end
 
-    ["1.0", "2.0", "3.0", "4.0", "5.0.10017.9"].each do |version|
-      it "returns false for PowerShell #{version}" do
-        node = Chef::Node.new
-        node.automatic[:languages][:powershell][:version] = version
-        expect(Chef::Platform.supports_dsc_invoke_resource?(node)).to be_falsey
-      end
-    end
+describe "Chef::Platform#supports_dsc_invoke_resource?" :windows_only do
+  it "returns false if powershell is not present" do
+    node = Chef::Node.new
+    expect(Chef::Platform.supports_dsc_invoke_resource?(node)).to be_falsey
+  end
 
-    it "returns true for Powershell 5.0.10018.0" do
+  ["1.0", "2.0", "3.0", "4.0", "5.0.10017.9"].each do |version|
+    it "returns false for PowerShell #{version}" do
       node = Chef::Node.new
-      node.automatic[:languages][:powershell][:version] = "5.0.10018.0"
-      expect(Chef::Platform.supports_dsc_invoke_resource?(node)).to be_truthy
+      node.automatic[:languages][:powershell][:version] = version
+      expect(Chef::Platform.supports_dsc_invoke_resource?(node)).to be_falsey
     end
   end
 
-  describe "Chef::Platform#dsc_refresh_mode_disabled?" do
-    let(:node) { instance_double("Chef::Node") }
-    let(:powershell) { Class.new { include Chef_PowerShell::ChefPowerShell::PowerShellExec } }
-    subject(:object) { powershell.new }
+  it "returns true for Powershell 5.0.10018.0" do
+    node = Chef::Node.new
+    node.automatic[:languages][:powershell][:version] = "5.0.10018.0"
+    expect(Chef::Platform.supports_dsc_invoke_resource?(node)).to be_truthy
+  end
+end
 
-    it "returns true when RefreshMode is Disabled" do
-      execution = object.powershell_exec("Get-DscLocalConfigurationManager", :powershell, timeout: -1)
-      # expect(object.powershell_exec("Get-DscLocalConfigurationManager", :powershell, timeout: -1)).to be_kind_of(Chef_PowerShell::PowerShell)
-      # expect(Chef_PowerShell::PowerShell).to receive(:new)
-      #   .with("Get-DscLocalConfigurationManager")
-      #   .and_return(powershell)
-      # expect(powershell).to receive(:error!)
-      # expect(powershell).to receive(:result).and_return({ "RefreshMode" => "False" })
-      expect(execution.result["RefreshMode"]).to eq "PUSH"
-      expect(Chef::Platform.dsc_refresh_mode_disabled?(node)).to be false
-    end
+describe "Chef::Platform#dsc_refresh_mode_disabled?" do
+  let(:node) { instance_double("Chef::Node") }
+  let(:powershell) { Class.new { include Chef_PowerShell::ChefPowerShell::PowerShellExec } }
+  subject(:object) { powershell.new }
 
-    # it "returns false when RefreshMode is not Disabled" do
-    #   expect(Chef::PowerShell).to receive(:new)
-    #     .with("Get-DscLocalConfigurationManager")
-    #     .and_return(powershell)
-    #   expect(powershell).to receive(:error!)
-    #   expect(powershell).to receive(:result).and_return({ "RefreshMode" => "LaLaLa" })
-    #   expect(Chef::Platform.dsc_refresh_mode_disabled?(node)).to be true
-    # end
+  it "returns true when RefreshMode is Disabled" do
+    execution = object.powershell_exec("Get-DscLocalConfigurationManager", :powershell, timeout: -1)
+    expect(execution.result["RefreshMode"]).to eq "PUSH"
+    expect(Chef::Platform.dsc_refresh_mode_disabled?(node)).to be false
   end
-end
\ No newline at end of file
+end