diff options
author | Tim Smith <tsmith@chef.io> | 2019-01-24 13:14:49 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-01-24 13:14:49 -0800 |
commit | f525a3a1e3b12f17c298939029926a73fc562623 (patch) | |
tree | 004947dee775ace20d7cd938546a7c756fdf061a | |
parent | 898f8dc977cfc9a3e80ea8687a0986a0ead53cec (diff) | |
parent | 8924f9a7e9e960af470a1b4bd22583574bd5c757 (diff) | |
download | chef-f525a3a1e3b12f17c298939029926a73fc562623.tar.gz |
Merge pull request #8163 from chef/windows_cert_14
windows_certificate: Ensure all actions are fully idempotent
-rw-r--r-- | lib/chef/resource/windows_certificate.rb | 32 | ||||
-rw-r--r-- | spec/data/windows_certificates/othertest.cer | bin | 0 -> 912 bytes | |||
-rw-r--r-- | spec/data/windows_certificates/test.cer | bin | 0 -> 900 bytes | |||
-rw-r--r-- | spec/data/windows_certificates/test.pem | 21 | ||||
-rw-r--r-- | spec/data/windows_certificates/test.pfx | bin | 0 -> 2701 bytes | |||
-rw-r--r-- | spec/functional/resource/windows_certificate_spec.rb | 340 |
6 files changed, 389 insertions, 4 deletions
diff --git a/lib/chef/resource/windows_certificate.rb b/lib/chef/resource/windows_certificate.rb index cf0b46dd6b..8b6a52711a 100644 --- a/lib/chef/resource/windows_certificate.rb +++ b/lib/chef/resource/windows_certificate.rb @@ -59,7 +59,19 @@ class Chef action :create do description "Creates or updates a certificate." - add_cert(OpenSSL::X509::Certificate.new(raw_source)) + + 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 + + # 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 + add_cert(cert_obj) + end + end end # acl_add is a modify-if-exists operation : not idempotent @@ -95,6 +107,8 @@ class Chef converge_by("Deleting certificate #{new_resource.source} from Store #{new_resource.store_name}") do delete_cert end + else + Chef::Log.debug("Certificate not found") end end @@ -105,7 +119,7 @@ class Chef if cert_obj show_or_store_cert(cert_obj) else - Chef::Log.info("Certificate not found") + Chef::Log.debug("Certificate not found") end end @@ -135,9 +149,14 @@ class Chef store.get(new_resource.source) end - def verify_cert + # Checks whether a certificate with the given thumbprint + # is already present and valid in certificate store + # If the certificate is not present, verify_cert returns a String: "Certificate not found" + # But if it is present but expired, it returns a Boolean: false + # Otherwise, it returns a Boolean: true + def verify_cert(thumbprint = new_resource.source) store = ::Win32::Certstore.open(new_resource.store_name) - store.valid?(new_resource.source) + store.valid?(thumbprint) end def show_or_store_cert(cert_obj) @@ -241,11 +260,15 @@ 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) out = case ext when ".crt", ".der" @@ -261,6 +284,7 @@ class Chef format_raw_out(out) end + # Returns the certificate content def format_raw_out(out) begin_cert = "-----BEGIN CERTIFICATE-----" end_cert = "-----END CERTIFICATE-----" diff --git a/spec/data/windows_certificates/othertest.cer b/spec/data/windows_certificates/othertest.cer Binary files differnew file mode 100644 index 0000000000..353f792252 --- /dev/null +++ b/spec/data/windows_certificates/othertest.cer diff --git a/spec/data/windows_certificates/test.cer b/spec/data/windows_certificates/test.cer Binary files differnew file mode 100644 index 0000000000..0f32d742c1 --- /dev/null +++ b/spec/data/windows_certificates/test.cer diff --git a/spec/data/windows_certificates/test.pem b/spec/data/windows_certificates/test.pem new file mode 100644 index 0000000000..cfbec5d188 --- /dev/null +++ b/spec/data/windows_certificates/test.pem @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDgDCCAmigAwIBAgIQEyXvJXC8z6lBIxwnT7/d5jANBgkqhkiG9w0BAQsFADBD +MRwwGgYDVQQDDBNBIER1bW15IENlcnRpZmljYXRlMSMwIQYJKoZIhvcNAQkBFhR0 +ZXN0Ynlyc3BlY0BjaGVmLmNvbTAeFw0xOTAxMjMxODEzNTBaFw0yMDAxMjMxODMz +NTBaMEMxHDAaBgNVBAMME0EgRHVtbXkgQ2VydGlmaWNhdGUxIzAhBgkqhkiG9w0B +CQEWFHRlc3RieXJzcGVjQGNoZWYuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A +MIIBCgKCAQEA1IPsH+S+HKVsJJDuHsqgSQnWAWp7SsBqwnx/t/NZAM6g41mbwafP +EZixFB5G6VAIiUosHcLhFwz00uPwVZIDND1Ez4TxACraF0iJQpy2kmriDq449ccu +fn/d8k417Vj0Hm7mcNpv6uaQrjYhIYFHXKV5aQS/OROQGvwFuWe56uJI25ua9lWR +8yBR621bgn6oW7elBZ8YDQAH88Y0LNo15FBeL2IDUXHBajEfkIRDE3BH+8zcuK4g +RnRJYBBkzFCXvTXLcRyr1zXaow31TeECrUdPGgBO+nTpLqWYWTylAv36C1nMYBn2 +5ItKAsswVEpQMIeQ5ysfaab0Ei3DRZIEjQIDAQABo3AwbjAOBgNVHQ8BAf8EBAMC +BaAwHQYDVR0lBBYwFAYIKwYBBQUHAwIGCCsGAQUFBwMBMB4GA1UdEQQXMBWCE3d3 +dy50ZXN0Ynlyc3BlYy5jb20wHQYDVR0OBBYEFMeiyQLCtZBHbmVnvCkoDnRkR+tB +MA0GCSqGSIb3DQEBCwUAA4IBAQA1hy2yADJ9ULaQMduBt0PiVKP+UKD87OQj0pJK +vFE7WVSxWaphA4XS15hityJt4eHmGF8R6tNxip7eS2mloGGMguijslqvQLICeeCN +/7Ov9CsJJG3R8xVrbEZkPExUbV8swJX68GoVxPi4nSj2TFhizBScaOKLedzIXtv5 +hGSXpl3RfETckTq1wmIVEQE9CUoWkea74zvGc5wXTi3r2ZZxof6olGELqT8W/jyT +vSzUDIC0iwuSVS0AyonBlAnA34ak3Q6a0RCZGK3l1IYz6Cb1JbHHpuCDZPPHooBi +Hbd+SuvfCH9DLgDFJCAOg+X7WCMQAoy9gCY8Ne5oBTYyjmCz +-----END CERTIFICATE----- diff --git a/spec/data/windows_certificates/test.pfx b/spec/data/windows_certificates/test.pfx Binary files differnew file mode 100644 index 0000000000..c774e12efd --- /dev/null +++ b/spec/data/windows_certificates/test.pfx diff --git a/spec/functional/resource/windows_certificate_spec.rb b/spec/functional/resource/windows_certificate_spec.rb new file mode 100644 index 0000000000..188a0dc28d --- /dev/null +++ b/spec/functional/resource/windows_certificate_spec.rb @@ -0,0 +1,340 @@ +# Author: Nimesh Patni (nimesh.patni@msystechnologies.com) +# Copyright: Copyright 2008-2018, Chef Software, Inc. +# License: Apache License, Version 2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +require "spec_helper" +require "chef/mixin/powershell_out" +require "chef/resource/windows_certificate" + +module WindowsCertificateHelper + include Chef::Mixin::PowershellOut + + def create_store(store) + path = "Cert:\\LocalMachine\\" + store + command = <<~EOC + New-Item -Path #{path} + EOC + powershell_out(command) + end + + def cleanup(store) + path = "Cert:\\LocalMachine\\" + store + command = <<~EOC + Remove-Item -Path #{path} -Recurse + EOC + powershell_out(command) + end + + def no_of_certificates + path = "Cert:\\LocalMachine\\" + store + command = <<~EOC + Write-Host (dir #{path} | measure).Count; + EOC + powershell_out(command).stdout.to_i + end +end + +describe Chef::Resource::WindowsCertificate, :windows_only, :appveyor_only do + include WindowsCertificateHelper + + let(:stdout) { StringIO.new } + let(:username) { "ChefFunctionalTest" } + let(:node) { Chef::Node.new } + let(:events) { Chef::EventDispatch::Dispatcher.new } + let(:run_context) { Chef::RunContext.new(node, {}, events) } + let(:new_resource) { Chef::Resource::WindowsCertificate.new(username, run_context) } + let(:password) { "P@ssw0rd!" } + let(:store) { "Chef-Functional-Test" } + 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(: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" } + + before do + opts = { store_name: store } + key = :store_name + to_be = ["TRUSTEDPUBLISHER", "TrustedPublisher", "CLIENTAUTHISSUER", + "REMOTE DESKTOP", "ROOT", "TRUSTEDDEVICES", "WEBHOSTING", + "CA", "AUTHROOT", "TRUSTEDPEOPLE", "MY", "SMARTCARDROOT", "TRUST", + "DISALLOWED"] + + # Byepassing the validation so that we may create a custom store + allow_any_instance_of(Chef::Mixin::ParamsValidate) + .to receive(:_pv_equal_to) + .with(opts, key, to_be) + .and_return(true) + + # Creating a custom store for the testing + create_store(store) + + allow(Chef::Log).to receive(:info) do |msg| + stdout.puts(msg) + end + end + + after { cleanup(store) } + + subject(:win_certificate) do + new_resource.store_name = store + new_resource + end + + it "Initially there are no certificates" do + expect(no_of_certificates).to eq(0) + end + + describe "action :create" do + before do + win_certificate.source = cer_path + win_certificate.run_action(:create) + end + + context "Adding a certificate" do + it "Imports certificate into store" do + expect(no_of_certificates).to eq(1) + end + + it "Converges while addition" do + expect(win_certificate).to be_updated_by_last_action + end + end + + context "Again adding the same certificate" do + before do + win_certificate.run_action(:create) + end + it "Does not imports certificate into store" do + expect(no_of_certificates).to eq(1) + end + it "Idempotent: Does not converge while addition" do + expect(no_of_certificates).to eq(1) + expect(win_certificate).not_to be_updated_by_last_action + end + end + + context "Again adding the same certificate of other format" do + before do + win_certificate.source = pem_path + win_certificate.run_action(:create) + end + it "Does not imports certificate into store" do + expect(no_of_certificates).to eq(1) + end + it "Idempotent: Does not converge while addition" do + expect(no_of_certificates).to eq(1) + expect(win_certificate).not_to be_updated_by_last_action + end + end + + context "Adding another certificate" do + before do + win_certificate.source = other_cer_path + win_certificate.run_action(:create) + end + it "Imports certificate into store" do + expect(no_of_certificates).to eq(2) + end + it "Converges while addition" do + expect(no_of_certificates).to eq(2) + expect(win_certificate).to be_updated_by_last_action + end + end + end + + describe "action: verify" do + context "When a certificate is not present" do + before do + win_certificate.source = tests_thumbprint + win_certificate.run_action(:verify) + end + it "Initial check if certificate is not present" do + expect(no_of_certificates).to eq(0) + 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 + + context "When a certificate is present" do + before do + win_certificate.source = cer_path + win_certificate.run_action(:create) + end + + context "For a valid thumbprint" do + before do + win_certificate.source = tests_thumbprint + win_certificate.run_action(:verify) + end + it "Initial check if certificate is present" do + expect(no_of_certificates).to eq(1) + 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(1) + 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 + context "When a certificate is not present" do + before do + win_certificate.source = tests_thumbprint + win_certificate.run_action(:fetch) + end + it "Initial check if certificate is not present" do + expect(no_of_certificates).to eq(0) + end + it "Does not show any content" do + expect(stdout.string.strip).to be_empty + end + it "Does not converge while fetching" do + expect(win_certificate).not_to be_updated_by_last_action + end + end + + context "When a certificate is present" do + before do + win_certificate.source = cer_path + win_certificate.run_action(:create) + end + + after do + if File.exists?(out_path) + File.delete(out_path) + end + end + + context "For a valid thumbprint" do + before do + win_certificate.source = tests_thumbprint + win_certificate.cert_path = out_path + win_certificate.run_action(:fetch) + end + it "Initial check if certificate is present" do + expect(no_of_certificates).to eq(1) + end + it "Stores Certificate content at given path" do + expect(File.exists?(out_path)).to be_truthy + end + it "Does not converge while fetching" 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.cert_path = out_path + win_certificate.run_action(:fetch) + end + it "Initial check if certificate is present" do + expect(no_of_certificates).to eq(1) + end + it "Does not show any content" do + expect(stdout.string.strip).to be_empty + end + it "Does not store certificate content at given path" do + expect(File.exists?(out_path)).to be_falsy + end + it "Does not converge while fetching" do + expect(win_certificate).not_to be_updated_by_last_action + end + end + end + end + + describe "action: delete" do + context "When a certificate is not present" do + before do + win_certificate.source = tests_thumbprint + win_certificate.run_action(:delete) + end + it "Initial check if certificate is not present" do + expect(no_of_certificates).to eq(0) + end + it "Does not delete any certificate" do + expect(stdout.string.strip).to be_empty + end + end + + context "When a certificate is present" do + before do + win_certificate.source = cer_path + win_certificate.run_action(:create) + end + before { win_certificate.source = tests_thumbprint } + it "Initial check if certificate is present" do + expect(no_of_certificates).to eq(1) + end + it "Deletes the certificate" do + win_certificate.run_action(:delete) + expect(no_of_certificates).to eq(0) + end + it "Converges while deleting" do + win_certificate.run_action(:delete) + expect(win_certificate).to be_updated_by_last_action + end + it "Idempotent: Does not converge while deleting again" do + win_certificate.run_action(:delete) + win_certificate.run_action(:delete) + expect(no_of_certificates).to eq(0) + expect(win_certificate).not_to be_updated_by_last_action + end + it "Deletes the valid certificate" do + # Add another certificate" + win_certificate.source = other_cer_path + win_certificate.run_action(:create) + expect(no_of_certificates).to eq(2) + + # Delete previously added certificate + win_certificate.source = tests_thumbprint + win_certificate.run_action(:delete) + expect(no_of_certificates).to eq(1) + + # Verify another certificate still exists + win_certificate.source = others_thumbprint + win_certificate.run_action(:verify) + expect(stdout.string.strip).to eq("Certificate is valid") + end + end + end +end |