diff options
author | John McCrae <jmccrae@chef.io> | 2021-04-20 17:06:10 -0700 |
---|---|---|
committer | John McCrae <jmccrae@chef.io> | 2021-04-21 10:13:35 -0700 |
commit | 2297ea3e906c80fae670c4c92562b64044868c38 (patch) | |
tree | 4390333cc12aba788188d38560e8d58ea2eaf525 | |
parent | 0a82c9bae78392a38862a27b1de8f54d0ed3e044 (diff) | |
download | chef-2297ea3e906c80fae670c4c92562b64044868c38.tar.gz |
windows_certificate: properly add/remove pfx and private keys, changed output for all fetch commands to require an output path instead of dumping to screen, added the ability to create a certificate from a URL
Signed-off-by: John McCrae <jmccrae@chef.io>
-rw-r--r-- | .gitattributes | 21 | ||||
-rw-r--r-- | Gemfile.lock | 6 | ||||
-rw-r--r-- | chef-universal-mingw32.gemspec | 2 | ||||
-rw-r--r-- | lib/chef/resource/windows_certificate.rb | 266 | ||||
-rw-r--r-- | omnibus/Gemfile.lock | 44 | ||||
-rw-r--r-- | spec/functional/resource/windows_certificate_spec.rb | 127 | ||||
-rw-r--r-- | tasks/rspec.rb | 6 |
7 files changed, 340 insertions, 132 deletions
diff --git a/.gitattributes b/.gitattributes index 311062a0b3..c56b2e6f1d 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,4 +1,17 @@ -# git config merge.ignore.name 'ignore changes merge driver' -# git config merge.ignore.driver 'touch %A' -lib/chef/version.rb merge=ignore -*.reg text eol=crlf
\ No newline at end of file +# Set the default behavior, in case people don't have core.autocrlf set. +* text=auto + +# Explicitly declare text files you want to always be normalized and converted +# to native line endings on checkout. +*.rb text + +# Declare files that will always have CRLF/LF line endings on checkout. +*.sln text eol=crlf +*.rb text eol=lf +Gemfile eol=lf +*.erb eol=lf +Rakefile eol=lf + +# Denote all files that are truly binary and should not be modified. +*.png binary +*.jpg binary
\ No newline at end of file diff --git a/Gemfile.lock b/Gemfile.lock index 3f54e8866d..72adb599da 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -109,7 +109,7 @@ PATH tty-table (~> 0.11) uuidtools (>= 2.1.5, < 3.0) win32-api (~> 1.5.3) - win32-certstore (~> 0.5.0) + win32-certstore (~> 0.6.2) win32-event (~> 0.6.1) win32-eventlog (= 0.6.3) win32-mmap (~> 0.4.1) @@ -378,7 +378,7 @@ GEM hashdiff (>= 0.4.0, < 2.0.0) webrick (1.7.0) win32-api (1.5.3-universal-mingw32) - win32-certstore (0.5.3) + win32-certstore (0.6.2) ffi mixlib-shellout win32-event (0.6.3) @@ -448,4 +448,4 @@ DEPENDENCIES webmock BUNDLED WITH - 2.2.4 + 2.1.4 diff --git a/chef-universal-mingw32.gemspec b/chef-universal-mingw32.gemspec index 018d45d27b..ac70e7c4c3 100644 --- a/chef-universal-mingw32.gemspec +++ b/chef-universal-mingw32.gemspec @@ -14,7 +14,7 @@ gemspec.add_dependency "win32-service", ">= 2.1.5", "< 3.0" 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.5.0" # 0.5+ required for specifying user vs. system store +gemspec.add_dependency "win32-certstore", "~> 0.6.2" # 0.6.2+ required for specifying user vs. system store gemspec.extensions << "ext/win32-eventlog/Rakefile" gemspec.files += Dir.glob("{distro,ext}/**/*") diff --git a/lib/chef/resource/windows_certificate.rb b/lib/chef/resource/windows_certificate.rb index 6d0e080b3f..047d8b969f 100644 --- a/lib/chef/resource/windows_certificate.rb +++ b/lib/chef/resource/windows_certificate.rb @@ -19,6 +19,7 @@ require_relative "../util/path_helper" require_relative "../resource" +require_relative "../exceptions" module Win32 autoload :Certstore, "win32-certstore" if Chef::Platform.windows? end @@ -62,11 +63,11 @@ class Chef DOC property :source, String, - description: "The source file (for create and acl_add), thumbprint (for delete and acl_add) or subject (for delete) if it differs from the resource block's name.", + description: "The source file (for `create` and `acl_add`), thumbprint (for `delete`, `export`, and `acl_add`), or subject (for `delete` or `export`) if it differs from the resource block's name.", name_property: true property :pfx_password, String, - description: "The password to access the source if it is a pfx file." + description: "The password to access the object with if it is a PFX file." property :private_key_acl, Array, description: "An array of 'domain\\account' entries to be granted read-only access to the certificate's private key. Not idempotent." @@ -79,8 +80,7 @@ class Chef description: "Use the `CurrentUser` store instead of the default `LocalMachine` store. Note: Prior to #{ChefUtils::Dist::Infra::CLIENT}. 16.10 this property was ignored.", default: false - property :cert_path, String, - description: "The path to the certificate." + deprecated_property_alias :cert_path, :output_path, "The cert_path property was renamed output_path in the 17.0 release of #{ChefUtils::Dist::Infra::CLIENT}. Please update your cookbooks to use the new property name." # lazy used to set default value of sensitive to true if password is set property :sensitive, [TrueClass, FalseClass], @@ -92,16 +92,19 @@ class Chef default: false, introduced: "16.8" - action :create, description: "Creates or updates a certificate" do - # Extension of the certificate - ext = ::File.extname(new_resource.source) + property :output_path, String, + description: "A path on the node where a certificate object (PFX, PEM, CER, KEY, etc) can be exported to.", + introduced: "17.0" + + action :create, description: "Creates or updates a certificate." do + ext = get_file_extension(new_resource.source) # PFX certificates contains private keys and we import them with some other approach import_certificates(fetch_cert_object(ext), (ext == ".pfx")) end # acl_add is a modify-if-exists operation : not idempotent - action :acl_add, description: "Adds read-only entries to a certificate's private key ACL" do + action :acl_add, description: "Adds read-only entries to a certificate's private key ACL." do if ::File.exist?(new_resource.source) hash = "$cert.GetCertHashString()" @@ -124,8 +127,9 @@ class Chef end end - action :delete, description: "Deletes a certificate" do + action :delete, description: "Deletes a certificate." do cert_obj = fetch_cert + if cert_obj converge_by("Deleting certificate #{new_resource.source} from Store #{new_resource.store_name}") do delete_cert @@ -135,10 +139,21 @@ class Chef end end - action :fetch, description: "Fetches a certificate" do - cert_obj = fetch_cert + action :fetch, description: "Fetches a certificate." do + unless new_resource.output_path + raise Chef::Exceptions::ResourceNotFound, "You must include an output_path parameter when calling the fetch action" + end + + if ::File.extname(new_resource.output_path) == ".pfx" + powershell_exec!(pfx_ps_cmd(resolve_thumbprint(new_resource.source), store_location: ps_cert_location, store_name: new_resource.store_name, output_path: new_resource.output_path, password: new_resource.pfx_password )) + else + cert_obj = fetch_cert + end + if cert_obj - show_or_store_cert(cert_obj) + converge_by("Fetching certificate #{new_resource.source} from Store \\#{ps_cert_location}\\#{new_resource.store_name}") do + export_cert(cert_obj, output_path: new_resource.output_path, store_name: new_resource.store_name , store_location: ps_cert_location, pfx_password: new_resource.pfx_password) + end else Chef::Log.debug("Certificate not found") end @@ -153,6 +168,7 @@ class Chef end action_class do + @local_pfx_path = "" CERT_SYSTEM_STORE_LOCAL_MACHINE = 0x00020000 CERT_SYSTEM_STORE_CURRENT_USER = 0x00010000 @@ -162,10 +178,10 @@ class Chef store.add(cert_obj) end - def add_pfx_cert + def add_pfx_cert(path) exportable = new_resource.exportable ? 1 : 0 store = ::Win32::Certstore.open(new_resource.store_name, store_location: native_cert_location) - store.add_pfx(new_resource.source, new_resource.pfx_password, exportable) + store.add_pfx(path, new_resource.pfx_password, exportable) end def delete_cert @@ -175,12 +191,71 @@ class Chef def fetch_cert store = ::Win32::Certstore.open(new_resource.store_name, store_location: native_cert_location) - store.get(resolve_thumbprint(new_resource.source)) + if new_resource.output_path && ::File.extname(new_resource.output_path) == ".key" + fetch_key + + else + store.get(resolve_thumbprint(new_resource.source), store_name: new_resource.store_name, store_location: native_cert_location) + end + end + + def fetch_key + require "openssl" unless defined?(OpenSSL) + file_name = ::File.basename(new_resource.output_path, ::File.extname(new_resource.output_path)) + directory = ::File.dirname(new_resource.output_path) + pfx_file = file_name + ".pfx" + new_pfx_output_path = ::File.join(Chef::FileCache.create_cache_path("pfx_files"), pfx_file) + powershell_exec(pfx_ps_cmd(resolve_thumbprint(new_resource.source), store_location: ps_cert_location, store_name: new_resource.store_name, output_path: new_pfx_output_path, password: new_resource.pfx_password )) + pkcs12 = OpenSSL::PKCS12.new(::File.binread(new_pfx_output_path), new_resource.pfx_password) + f = ::File.open(new_resource.output_path, "w") + f.write(pkcs12.key.to_s) + f.flush + f.close + end + + def get_file_extension(file_name) + type = url_or_file?(file_name) + + if type == "file" + ::File.extname(file_name) + elsif type == "url" + require "open-uri" unless defined?(OpenURI) + uri = URI.parse(file_name) + output_file = ::File.basename(uri.path) + ::File.extname(output_file) + end + end + + def get_file_name(path_name) + type = url_or_file?(path_name) + + if type == "file" + ::File.extname(path_name) + elsif type == "url" + require "open-uri" unless defined?(OpenURI) + uri = URI.parse(path_name) + ::File.basename(uri.path) + end + end + + # did I get passed a file, a url, or a mistake? + def url_or_file?(source) + require "uri" unless defined?(URI) + + uri = URI.parse(source) + + if source == uri.is_a?(URI::HTTP) || uri.is_a?(URI::HTTPS) + "url" + elsif ::File.file?(source) + "file" + else + raise Chef::Exceptions::FileNotFound, "Invalid Source File" + end end # Thumbprints should be exactly 40 Hex characters def valid_thumbprint?(string) - string.scan(/\H/).empty? && string.length == 40 + string.match?(/[0-9A-Fa-f]/) && string.length == 40 end def get_thumbprint(store_name, location, source) @@ -205,45 +280,11 @@ class Chef def verify_cert(thumbprint = new_resource.source) store = ::Win32::Certstore.open(new_resource.store_name, store_location: native_cert_location) - store.valid?(resolve_thumbprint(thumbprint)) - end - - def show_or_store_cert(cert_obj) - if new_resource.cert_path - export_cert(cert_obj, new_resource.cert_path) - if ::File.size(new_resource.cert_path) > 0 - Chef::Log.info("Certificate export in #{new_resource.cert_path}") - else - ::File.delete(new_resource.cert_path) - end - else - Chef::Log.info(cert_obj.display) - end - end - - def export_cert(cert_obj, cert_path) - out_file = ::File.new(cert_path, "w+") - case ::File.extname(cert_path) - when ".pem" - out_file.puts(cert_obj.to_pem) - when ".der" - out_file.puts(cert_obj.to_der) - when ".cer" - cert_out = shell_out("openssl x509 -text -inform DER -in #{cert_obj.to_pem} -outform CER").stdout - out_file.puts(cert_out) - when ".crt" - cert_out = shell_out("openssl x509 -text -inform DER -in #{cert_obj.to_pem} -outform CRT").stdout - out_file.puts(cert_out) - when ".pfx" - cert_out = shell_out("openssl pkcs12 -export -nokeys -in #{cert_obj.to_pem} -outform PFX").stdout - out_file.puts(cert_out) - when ".p7b" - cert_out = shell_out("openssl pkcs7 -export -nokeys -in #{cert_obj.to_pem} -outform P7B").stdout - out_file.puts(cert_out) + if new_resource.pfx_password.nil? + store.valid?(resolve_thumbprint(thumbprint), store_location: native_cert_location, store_name: new_resource.store_name ) else - Chef::Log.info("Supported certificate format .pem, .der, .cer, .crt, .pfx and .p7b") + store.valid?(resolve_thumbprint(thumbprint), store_location: native_cert_location, store_name: new_resource.store_name) end - out_file.close end # this array structure is solving 2 problems. The first is that we need to have support for both the CurrentUser AND LocalMachine stores @@ -252,6 +293,14 @@ class Chef new_resource.user_store ? "CurrentUser" : "LocalMachine" end + def pfx_ps_cmd(thumbprint, store_location: "LocalMachine", store_name: "My", output_path:, password: ) + <<-CMD + $my_pwd = ConvertTo-SecureString -String "#{password}" -Force -AsPlainText + $cert = Get-ChildItem -path cert:\\#{store_location}\\#{store_name} -Recurse | Where { $_.Thumbprint -eq "#{thumbprint.upcase}" } + Export-PfxCertificate -Cert $cert -FilePath "#{output_path}" -Password $my_pwd + CMD + end + def native_cert_location new_resource.user_store ? CERT_SYSTEM_STORE_CURRENT_USER : CERT_SYSTEM_STORE_LOCAL_MACHINE end @@ -331,7 +380,44 @@ class Chef # @raise [OpenSSL::PKCS12::PKCS12Error] When incorrect password is provided for PFX certificate # def fetch_cert_object(ext) - contents = ::File.binread(new_resource.source) + type = url_or_file?(new_resource.source) + + if type == "file" + begin + ::File.exist?(new_resource.source) + contents = ::File.binread(new_resource.source) + rescue => exception + message = "Unable to load the certificate object from the specified local path : #{new_resource.source}\n" + message << exception.message + raise Chef::Exceptions::FileNotFound, message + end + elsif type == "url" + require "uri" unless defined?(URI) + uri = URI(new_resource.source) + state = uri.is_a?(URI::HTTP) && !uri.host.nil? ? true : false + if state + begin + output_file_name = get_file_name(new_resource.source) + unless Dir.exist?(Chef::Config[:file_cache_path]) + Dir.mkdir(Chef::Config[:file_cache_path]) + end + local_path = ::File.join(Chef::Config[:file_cache_path], output_file_name) + @local_pfx_path = local_path + ::File.open(local_path, "wb") do |file| + file.write URI.open(new_resource.source).read + end + rescue => exception + message = "Not Able to Download Certificate Object at the URL specified : #{new_resource.source}\n" + message << exception.message + raise Chef::Exceptions::FileNotFound, message + end + + contents = ::File.binread(local_path) + + else + raise Chef::Exceptions::InvalidRemoteFileURI, "Not Able to Download Certificate Object at the URL specified : #{new_resource.source}" + end + end case ext when ".pfx" @@ -348,24 +434,76 @@ class Chef end end + def export_cert(cert_obj, output_path:, store_name:, store_location:, pfx_password:) + # Delete the cert if it exists. This is non-destructive in that it only removes the file and not the entire path. + # We want to ensure we're not randomly loading an old stinky cert. + if ::File.exists?(output_path) + ::File.delete(output_path) + end + + unless ::File.directory?(::File.dirname(output_path)) + FileUtils.mkdir_p(::File.dirname(output_path)) + end + + out_file = ::File.new(output_path, "w+") + + case ::File.extname(output_path) + when ".pem" + out_file.puts(cert_obj) + when ".der" + out_file.puts(cert_obj.to_der) + when ".cer" + cert_out = shell_out("openssl x509 -text -inform DER -in #{cert_obj.to_pem} -outform CER").stdout + out_file.puts(cert_out) + when ".crt" + cert_out = shell_out("openssl x509 -text -inform DER -in #{cert_obj} -outform CRT").stdout + out_file.puts(cert_out) + when ".pfx" + pfx_ps_cmd(resolve_thumbprint(new_resource.source), store_location: store_location, store_name: store_name, output_path: output_path, password: pfx_password ) + when ".p7b" + cert_out = shell_out("openssl pkcs7 -export -nokeys -in #{cert_obj.to_pem} -outform P7B").stdout + out_file.puts(cert_out) + when ".key" + out_file.puts(cert_obj) + else + Chef::Log.info("Supported certificate format .pem, .der, .cer, .crt, and .p7b") + end + + out_file.close + 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) + def import_certificates(cert_objs, is_pfx, store_name: new_resource.store_name, store_location: native_cert_location) [cert_objs].flatten.each do |cert_obj| - thumbprint = OpenSSL::Digest.new("SHA1", 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") + # thumbprint = OpenSSL::Digest.new("SHA1", cert_obj.to_der).to_s + # pkcs = OpenSSL::PKCS12.new(cert_obj, new_resource.pfx_password) + # cert = OpenSSL::X509::Certificate.new(pkcs.certificate.to_pem) + thumbprint = OpenSSL::Digest.new("SHA1", cert_obj.to_der).to_s + if is_pfx + if verify_cert(thumbprint) == true + Chef::Log.debug("Certificate is already present") + else + type = url_or_file?(new_resource.source) + if type == "file" + converge_by("Creating a PFX #{new_resource.source} for Store #{new_resource.store_name}") do + add_pfx_cert(new_resource.source) + end + elsif type == "url" + converge_by("Creating a PFX #{@local_pfx_path} for Store #{new_resource.store_name}") do + add_pfx_cert(@local_pfx_path) + end + end + end else - converge_by("Adding certificate #{new_resource.source} into #{ps_cert_location} Store #{new_resource.store_name}") do - if is_pfx - add_pfx_cert - else + if verify_cert(thumbprint) == true + Chef::Log.debug("Certificate is already present") + else + converge_by("Creating a certificate #{new_resource.source} for Store #{new_resource.store_name}") do add_cert(cert_obj) end end diff --git a/omnibus/Gemfile.lock b/omnibus/Gemfile.lock index f446e8445d..6faf46f680 100644 --- a/omnibus/Gemfile.lock +++ b/omnibus/Gemfile.lock @@ -1,9 +1,9 @@ GIT remote: https://github.com/chef/omnibus - revision: dd5789621fa32da5846b9cb92ea902015265734c + revision: 79c80e0414e17069e6cbe121a55fc36e867d7a04 branch: master specs: - omnibus (8.1.3) + omnibus (8.1.6) aws-sdk-s3 (~> 1) chef-cleanroom (~> 1.0) chef-utils (>= 15.4) @@ -18,7 +18,7 @@ GIT GIT remote: https://github.com/chef/omnibus-software - revision: 0dcaeb1940283bcb98c00dcfe9bb2821726ffa0a + revision: 3ac1dbed61173f0919b9f8215bc1af00d7a6c27b branch: master specs: omnibus-software (4.0.0) @@ -32,8 +32,8 @@ GEM artifactory (3.0.15) awesome_print (1.9.2) aws-eventstream (1.1.1) - aws-partitions (1.442.0) - aws-sdk-core (3.113.1) + aws-partitions (1.446.0) + aws-sdk-core (3.114.0) aws-eventstream (~> 1, >= 1.0.2) aws-partitions (~> 1, >= 1.239.0) aws-sigv4 (~> 1.1) @@ -41,7 +41,7 @@ GEM aws-sdk-kms (1.43.0) aws-sdk-core (~> 3, >= 3.112.0) aws-sigv4 (~> 1.1) - aws-sdk-s3 (1.93.0) + aws-sdk-s3 (1.93.1) aws-sdk-core (~> 3, >= 3.112.0) aws-sdk-kms (~> 1) aws-sigv4 (~> 1.1) @@ -64,12 +64,12 @@ GEM solve (~> 4.0) thor (>= 0.20) builder (3.2.4) - chef (16.11.7) + chef (16.13.16) addressable bcrypt_pbkdf (~> 1.1) bundler (>= 1.10) - chef-config (= 16.11.7) - chef-utils (= 16.11.7) + chef-config (= 16.13.16) + chef-utils (= 16.13.16) chef-vault chef-zero (>= 14.0.11) diff-lcs (>= 1.2.4, < 1.4.0) @@ -101,12 +101,12 @@ GEM tty-screen (~> 0.6) tty-table (~> 0.11) uuidtools (>= 2.1.5, < 3.0) - chef (16.11.7-universal-mingw32) + chef (16.13.16-universal-mingw32) addressable bcrypt_pbkdf (~> 1.1) bundler (>= 1.10) - chef-config (= 16.11.7) - chef-utils (= 16.11.7) + chef-config (= 16.13.16) + chef-utils (= 16.13.16) chef-vault chef-zero (>= 14.0.11) diff-lcs (>= 1.2.4, < 1.4.0) @@ -150,9 +150,9 @@ GEM win32-taskscheduler (~> 2.0) wmi-lite (~> 1.0) chef-cleanroom (1.0.2) - chef-config (16.11.7) + chef-config (16.13.16) addressable - chef-utils (= 16.11.7) + chef-utils (= 16.13.16) fuzzyurl mixlib-config (>= 2.2.12, < 4.0) mixlib-shellout (>= 2.0, < 4.0) @@ -160,7 +160,7 @@ GEM chef-telemetry (1.0.29) chef-config concurrent-ruby (~> 1.0) - chef-utils (16.11.7) + chef-utils (16.13.16) chef-vault (4.1.0) chef-zero (15.0.4) ffi-yajl (~> 2.2) @@ -177,10 +177,10 @@ GEM ed25519 (1.2.4) erubi (1.10.0) erubis (2.7.0) - faraday (1.3.0) + faraday (1.3.1) faraday-net_http (~> 1.0) multipart-post (>= 1.2, < 3) - ruby2_keywords + ruby2_keywords (>= 0.0.4) faraday-net_http (1.0.1) faraday_middleware (1.0.0) faraday (~> 1.0) @@ -202,7 +202,7 @@ GEM highline (2.0.3) httpclient (2.8.3) iniparse (1.5.0) - inspec-core (4.29.3) + inspec-core (4.32.0) addressable (~> 2.4) chef-telemetry (~> 1.0) faraday (>= 0.9.0, < 1.4) @@ -286,7 +286,7 @@ GEM octokit (4.20.0) faraday (>= 0.9) sawyer (~> 0.8.0, >= 0.5.3) - ohai (16.12.3) + ohai (16.13.0) chef-config (>= 12.8, < 17) chef-utils (>= 16.0, < 17) ffi (~> 1.9) @@ -311,7 +311,7 @@ GEM zhexdump (>= 0.0.2) plist (3.6.0) proxifier (1.0.3) - pry (0.14.0) + pry (0.14.1) coderay (~> 1.1) method_source (~> 1.0) public_suffix (4.0.6) @@ -371,7 +371,7 @@ GEM toml-rb (2.0.1) citrus (~> 3.0, > 3.0) tomlrb (1.3.0) - train-core (3.6.0) + train-core (3.6.2) addressable (~> 2.5) ffi (!= 1.13.0) json (>= 1.8, < 3.0) @@ -388,7 +388,7 @@ GEM tty-cursor (~> 0.7) tty-color (0.6.0) tty-cursor (0.7.1) - tty-prompt (0.23.0) + tty-prompt (0.23.1) pastel (~> 0.8) tty-reader (~> 0.8) tty-reader (0.9.0) diff --git a/spec/functional/resource/windows_certificate_spec.rb b/spec/functional/resource/windows_certificate_spec.rb index b5d0484e0c..df2d1cbec8 100644 --- a/spec/functional/resource/windows_certificate_spec.rb +++ b/spec/functional/resource/windows_certificate_spec.rb @@ -22,26 +22,44 @@ require "chef/resource/windows_certificate" describe Chef::Resource::WindowsCertificate, :windows_only do include Chef::Mixin::PowershellExec - def create_store + def create_store(store_location: "LocalMachine", store_name: store) powershell_exec <<~EOC - New-Item -Path Cert:\\LocalMachine\\#{store} + New-Item -Path Cert:\\#{store_location}\\#{store_name} EOC end - def delete_store + def delete_store(store_location: "LocalMachine", store_name: store) powershell_exec <<~EOC - Remove-Item -Path Cert:\\LocalMachine\\#{store} -Recurse + Remove-Item -Path Cert:\\#{store_location}\\#{store_name} -Recurse EOC end - def certificate_count + def certificate_count(store_location: "LocalMachine", store_name: store) powershell_exec(<<~EOC).result.to_i - (Get-ChildItem -Force -Path Cert:\\LocalMachine\\#{store} | measure).Count + (Get-ChildItem -Force -Path Cert:\\#{store_location}\\#{store_name} | measure).Count + EOC + end + + def list_certificates(store_location: "LocalMachine", store_name: store) + powershell_exec(<<~EOC) + Get-ChildItem -Force -Path Cert:\\#{store_location}\\#{store_name} -Recurse + EOC + end + + def refresh_certstore(store_location: "LocalMachine") + powershell_exec(<<~EOC) + Get-ChildItem -Force -Path Cert:\\#{store_location} -Recurse EOC end let(:password) { "P@ssw0rd!" } let(:store) { "Chef-Functional-Test" } + let(:store_name) { "MY" } + let(:store_location) { "LocalMachine" } + let(:download_cert_url) { "https://testingchef.blob.core.windows.net/files/test.cer" } + let(:cert_output_path) { ::File.join(Chef::Config[:file_cache_path], "output.cer") } + let(:pfx_output_path) { ::File.join(Chef::Config[:file_cache_path], "output.pfx") } + let(:key_output_path) { ::File.join(Chef::Config[:file_cache_path], "output.key") } let(:cer_path) { File.join(CHEF_SPEC_DATA, "windows_certificates", "test.cer") } let(:base64_path) { File.join(CHEF_SPEC_DATA, "windows_certificates", "base64_test.cer") } let(:pem_path) { File.join(CHEF_SPEC_DATA, "windows_certificates", "test.pem") } @@ -68,12 +86,16 @@ describe Chef::Resource::WindowsCertificate, :windows_only do .and_return(true) create_store + end after { delete_store } describe "action: create" do it "starts with no certificates" do + delete_store + create_store + foo = list_certificates expect(certificate_count).to eq(0) end @@ -103,6 +125,14 @@ describe Chef::Resource::WindowsCertificate, :windows_only do expect(resource).to be_updated_by_last_action end + it "can add a certificate from a valid url" do + resource.source = download_cert_url + resource.run_action(:create) + + expect(certificate_count).to eq(1) + expect(resource).to be_updated_by_last_action + end + it "can add a base64 encoded certificate idempotently" do resource.source = base64_path resource.run_action(:create) @@ -157,9 +187,13 @@ describe Chef::Resource::WindowsCertificate, :windows_only do expect { resource.run_action(:create) }.to raise_error(OpenSSL::PKCS12::PKCS12Error) end + after { delete_store } end describe "action: verify" do + before do + create_store + end it "fails with no certificates in the store" do expect(Chef::Log).to receive(:info).with("Certificate not found") @@ -230,13 +264,13 @@ describe Chef::Resource::WindowsCertificate, :windows_only do end describe "action: fetch" do - it "does nothing with no certificates in the store" do - expect(Chef::Log).not_to receive(:info) - - resource.source = tests_thumbprint - resource.run_action(:fetch) - - expect(resource).not_to be_updated_by_last_action + context "with no certificate in the store" do + it "throws an error with no certificates in the store" do + expect(Chef::Log).not_to receive(:info) + resource.source = others_thumbprint + resource.output_path = cert_output_path + expect { resource.run_action :fetch }.to raise_error(ArgumentError) + end end context "with a certificate in the store" do @@ -247,18 +281,10 @@ describe Chef::Resource::WindowsCertificate, :windows_only do it "succeeds with a valid thumbprint" do resource.source = tests_thumbprint - - Dir.mktmpdir do |dir| - path = File.join(dir, "test.pem") - expect(Chef::Log).to receive(:info).with("Certificate export in #{path}") - - resource.cert_path = path - resource.run_action(:fetch) - - expect(File.exist?(path)).to be_truthy - end - - expect(resource).not_to be_updated_by_last_action + local_output_path = ::File.join(Chef::Config[:file_cache_path], "test.pem") + resource.output_path = local_output_path + resource.run_action(:fetch) + expect(File.exist?(local_output_path)).to be_truthy end it "fails with an invalid thumbprint" do @@ -269,23 +295,54 @@ describe Chef::Resource::WindowsCertificate, :windows_only do Dir.mktmpdir do |dir| path = File.join(dir, "test.pem") - resource.cert_path = path - resource.run_action(:fetch) - - expect(File.exist?(path)).to be_falsy + resource.output_path = path + expect { resource.run_action :fetch }.to raise_error(ArgumentError) end - expect(resource).not_to be_updated_by_last_action end end + + context "with a pfx/pkcs12 object in the store" do + before do + create_store + refresh_certstore + resource.source = pfx_path + resource.pfx_password = password + resource.exportable = true + resource.run_action(:create) + end + + it "exports a PFX file with a valid thumbprint" do + resource.source = tests_thumbprint + resource.pfx_password = password + resource.output_path = pfx_output_path + resource.run_action(:fetch) + expect(File.exist?(pfx_output_path)).to be_truthy + end + + it "exports a key file with a valid thumbprint" do + resource.source = tests_thumbprint + resource.pfx_password = password + resource.output_path = key_output_path + resource.run_action(:fetch) + expect(File.exist?(key_output_path)).to be_truthy + end + + it "throws an exception when output_path is not specified" do + resource.source = tests_thumbprint + resource.pfx_password = password + expect { resource.run_action :fetch }.to raise_error(::Chef::Exceptions::ResourceNotFound) + end + + after { delete_store } + + end end describe "action: delete" do - it "does nothing when attempting to delete a certificate that doesn't exist" do - expect(Chef::Log).to receive(:debug).with("Certificate not found") - + it "throws an argument error when attempting to delete a certificate that doesn't exist" do resource.source = tests_thumbprint - resource.run_action(:delete) + expect { resource.run_action :delete }.to raise_error(ArgumentError) end it "deletes an existing certificate while leaving other certificates alone" do @@ -303,7 +360,7 @@ describe Chef::Resource::WindowsCertificate, :windows_only do expect(certificate_count).to eq(1) expect(resource).to be_updated_by_last_action - resource.run_action(:delete) + expect { resource.run_action :delete }.to raise_error(ArgumentError) expect(certificate_count).to eq(1) expect(resource).not_to be_updated_by_last_action diff --git a/tasks/rspec.rb b/tasks/rspec.rb index 050c40be5c..084b39df94 100644 --- a/tasks/rspec.rb +++ b/tasks/rspec.rb @@ -43,7 +43,7 @@ begin desc "Run all chef specs in spec directory" RSpec::Core::RakeTask.new(:spec) do |t| t.verbose = false - t.rspec_opts = %w{--profile} + t.rspec_opts = %w{--profile --format doc} t.pattern = FileList["spec/**/*_spec.rb"].reject do |path| path =~ /knife.*/ end @@ -78,8 +78,8 @@ begin %i{unit functional integration stress}.each do |sub| desc "Run the chef specs under spec/#{sub}" RSpec::Core::RakeTask.new(sub) do |t| - t.verbose = false - t.rspec_opts = %w{--profile} + t.verbose = true + t.rspec_opts = %w{--profile --format doc} t.pattern = FileList["spec/#{sub}/**/*_spec.rb"].reject do |path| path =~ /knife.*/ end |