From 9f8c667ae2b0ccdd9fa0a77a7d215aaf2df98c5f Mon Sep 17 00:00:00 2001 From: John McCrae Date: Sun, 5 Feb 2023 08:20:57 +0600 Subject: Correcting cert retrieval issues for multiple user scenarios Signed-off-by: John McCrae --- spec/integration/client/client_spec.rb | 38 +++++++++-------- spec/unit/client_spec.rb | 28 ++++++++++++- spec/unit/http/authenticator_spec.rb | 75 +++++++++++++++++++++++++++++----- 3 files changed, 112 insertions(+), 29 deletions(-) (limited to 'spec') 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 -- cgit v1.2.1