From 0afdf28c1ff0522b534cdc325cfc84bda97436a0 Mon Sep 17 00:00:00 2001 From: Nimesh-Msys Date: Tue, 12 Feb 2019 15:57:15 +0530 Subject: Windows Certificate: Add support to import Base 64 encoded CER certificates - Till now, cer certificates were only considered to be in binary format (DER) - They can also be base-64 encoded(PEM) - We should only append "inform DER" only if it is a binary certificate, otherwise, default ("inform PEM") would support base64 encoded certificates. - Added test caes - Ensured Chef style Signed-off-by: Nimesh-Msys --- lib/chef/resource/windows_certificate.rb | 9 ++++++++- spec/data/windows_certificates/base64_test.cer | 22 ++++++++++++++++++++++ .../resource/windows_certificate_spec.rb | 16 ++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 spec/data/windows_certificates/base64_test.cer diff --git a/lib/chef/resource/windows_certificate.rb b/lib/chef/resource/windows_certificate.rb index a4225583e8..252fa24fcd 100644 --- a/lib/chef/resource/windows_certificate.rb +++ b/lib/chef/resource/windows_certificate.rb @@ -277,7 +277,9 @@ class Chef def convert_pem(ext) out = case ext when ".crt", ".cer", ".der" - powershell_out("openssl x509 -text -inform DER -in #{new_resource.source} -outform PEM") + command = "openssl x509 -text -in #{new_resource.source} -outform PEM" + command += " -inform DER" if binary_cert? + powershell_out(command) when ".pfx" powershell_out("openssl pkcs12 -in #{new_resource.source} -nodes -passin pass:'#{new_resource.pfx_password}'") when ".p7b" @@ -299,6 +301,11 @@ class Chef end_cert = "-----END CERTIFICATE-----" begin_cert + out[/#{begin_cert}(.*?)#{end_cert}/m, 1] + end_cert end + + # Checks if the certificate is binary encoded or not + def binary_cert? + powershell_out("file -b --mime-encoding #{new_resource.source}").stdout.strip == "binary" + end end end diff --git a/spec/data/windows_certificates/base64_test.cer b/spec/data/windows_certificates/base64_test.cer new file mode 100644 index 0000000000..0d90bf81e3 --- /dev/null +++ b/spec/data/windows_certificates/base64_test.cer @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDjjCCAnagAwIBAgIQNH6iXZnEKbFOEQ7D9f9iCTANBgkqhkiG9w0BAQsFADBK +MSMwIQYDVQQDDBpBIEJhc2U2NCBEdW1teSBDZXJ0aWZpY2F0ZTEjMCEGCSqGSIb3 +DQEJARYUdGVzdGJ5cnNwZWNAY2hlZi5jb20wHhcNMTkwMjEyMDk1ODM2WhcNMjAw +MjEyMTAxODM2WjBKMSMwIQYDVQQDDBpBIEJhc2U2NCBEdW1teSBDZXJ0aWZpY2F0 +ZTEjMCEGCSqGSIb3DQEJARYUdGVzdGJ5cnNwZWNAY2hlZi5jb20wggEiMA0GCSqG +SIb3DQEBAQUAA4IBDwAwggEKAoIBAQDSy2Qlf2k1X3y/YgEjnvD0K8NeKgXKKi62 +RHRMTJ2+6KSg+I1MqHZC+BVrfzehuJVby5kM7tGLF8FvM3q7X/5oSPg8pvLZzIV0 +pBrpVPCTYw8fnlmFKBt/+m2XOqsWyL59yP+p66SHAKmoLYTGu8dkGvgJn3dwKNen +VFmwadteVfKs2wFW/ZwUxH4aLloCa8KSyqstIXrYQmdqqFOSuEgkynalD19dozSv +QtkQ9FZPuFGDwNpdO7OrcjE1lTUlzuth7CqV/pj4GYJhK/PPtO8Ing/BtwZm5XB8 +2yvvLVnL7Y/hikg2ENKA9fOYk52zR/kkd7d8qoJva7WlYEXTZvpdAgMBAAGjcDBu +MA4GA1UdDwEB/wQEAwIFoDAdBgNVHSUEFjAUBggrBgEFBQcDAgYIKwYBBQUHAwEw +HgYDVR0RBBcwFYITd3d3LnRlc3RieXJzcGVjLmNvbTAdBgNVHQ4EFgQUuL1l247K +h+cVH9LehmQgXuV8F6MwDQYJKoZIhvcNAQELBQADggEBAMTJW5tSZ/g2AP45EUwj +PLDnDLY4YnsJDQ7Jo58EAY6givUc+ZnKRWxYAYNBOKcqDM5E4pXi3Fa1lKYR1vMu +5AThPaDXhv18ljGAs21MYt9hl7PqdzbfX4ejF+jCD4UrE8bGtxuDc1WQ2HbeJtdj +0j7BPPNXfcvPAIyX3BEOQFUPgvVAqzWMQLpdUKg+sNUJZijqKQv11xVALGHtxqGB +1MFrdl6D/idODfhcdo2n1tBMyOGhHwEOBLqB1PTH72g5J4BVx4iwH/gh8PRmMy0P +eJkNspgOBGPOhNpe7bhmK45MBuJpmjyl/CYCqtQvaEdpbuRQIgc2e+YRMfR71qYp +Em8= +-----END CERTIFICATE----- diff --git a/spec/functional/resource/windows_certificate_spec.rb b/spec/functional/resource/windows_certificate_spec.rb index f60b63ade9..a9ed99d318 100644 --- a/spec/functional/resource/windows_certificate_spec.rb +++ b/spec/functional/resource/windows_certificate_spec.rb @@ -60,6 +60,7 @@ describe Chef::Resource::WindowsCertificate, :windows_only, :appveyor_only do 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(:base64_path) { File.join(certificate_path, "base64_test.cer") } let(:pem_path) { File.join(certificate_path, "test.pem") } let(:pfx_path) { File.join(certificate_path, "test.pfx") } let(:out_path) { File.join(certificate_path, "testout.pem") } @@ -174,6 +175,21 @@ describe Chef::Resource::WindowsCertificate, :windows_only, :appveyor_only do end end + context "Adds Base64 Encoded CER" do + before do + win_certificate.source = base64_path + win_certificate.run_action(:create) + end + it "Imports certificate into store" do + expect(no_of_certificates).to eq(1) + end + it "Idempotent: Does not converge while adding again" do + win_certificate.run_action(:create) + expect(no_of_certificates).to eq(1) + expect(win_certificate).not_to be_updated_by_last_action + end + end + context "Adds PEM" do before do win_certificate.source = pem_path -- cgit v1.2.1 From 3031e99e96e9f9e046599d6e60e44cf5b1b1e2e1 Mon Sep 17 00:00:00 2001 From: Nimesh-Msys Date: Wed, 13 Feb 2019 17:55:05 +0530 Subject: Minor fixes as per the review comments. - Checking file formats by uisng ruby itself, instead of shelling it out. Signed-off-by: Nimesh-Msys --- lib/chef/resource/windows_certificate.rb | 37 +++++++++++++++++--------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/lib/chef/resource/windows_certificate.rb b/lib/chef/resource/windows_certificate.rb index 252fa24fcd..c5db262d15 100644 --- a/lib/chef/resource/windows_certificate.rb +++ b/lib/chef/resource/windows_certificate.rb @@ -21,6 +21,7 @@ require "chef/util/path_helper" require "chef/resource" require "win32-certstore" if Chef::Platform.windows? require "openssl" +require "open3" class Chef class Resource @@ -275,19 +276,19 @@ class Chef # Uses powershell command to convert crt/der/cer/pfx & p7b certificates # In PEM format and returns its certificate content def convert_pem(ext) - out = case ext - when ".crt", ".cer", ".der" - command = "openssl x509 -text -in #{new_resource.source} -outform PEM" - command += " -inform DER" if binary_cert? - powershell_out(command) - when ".pfx" - powershell_out("openssl pkcs12 -in #{new_resource.source} -nodes -passin pass:'#{new_resource.pfx_password}'") - when ".p7b" - powershell_out("openssl pkcs7 -print_certs -in #{new_resource.source} -outform PEM") - else - powershell_out("openssl x509 -text -inform #{ext.delete(".")} -in #{new_resource.source} -outform PEM") - end - + command = case ext + when ".crt", ".cer", ".der" + cmd = "openssl x509 -text -in #{new_resource.source} -outform PEM" + pem_cert? ? cmd : cmd + " -inform DER" + when ".pfx" + "openssl pkcs12 -in #{new_resource.source} -nodes -passin pass:'#{new_resource.pfx_password}'" + when ".p7b" + "openssl pkcs7 -print_certs -in #{new_resource.source} -outform PEM" + else + "openssl x509 -text -inform #{ext.delete('.')} -in #{new_resource.source} -outform PEM" + end + + out = powershell_out(command) if out.exitstatus == 0 format_raw_out(out.stdout) else @@ -302,12 +303,14 @@ class Chef begin_cert + out[/#{begin_cert}(.*?)#{end_cert}/m, 1] + end_cert end - # Checks if the certificate is binary encoded or not - def binary_cert? - powershell_out("file -b --mime-encoding #{new_resource.source}").stdout.strip == "binary" + # Checks if the given certificate is a PEM certificate or not + def pem_cert? + details, status = Open3.capture2e("file", new_resource.source) + return false unless status.success? + + details.rpartition(":").last.strip == "PEM certificate" end end - end end end -- cgit v1.2.1 From ccd9b7c5ca3487880b4bd2aee8581e6fda53d192 Mon Sep 17 00:00:00 2001 From: Nimesh-Msys Date: Fri, 15 Feb 2019 21:21:49 +0530 Subject: Fetching OpenSSL::X509::Certificate object without shell interactions. - Previously, we were building a new OpenSSL::X509::Certificate object with PEM string contents. - To get this content, we were using openssl utility and were converting all certificate types into PEM - And were extracting the PEM contents out of resulting data. - It could directly be done via correct usage of applicable classes. - Reverted using Open3. It uses shell hence did not solved the purpose. Signed-off-by: Nimesh-Msys --- lib/chef/resource/windows_certificate.rb | 71 +++++++++++----------- .../resource/windows_certificate_spec.rb | 2 +- 2 files changed, 35 insertions(+), 38 deletions(-) diff --git a/lib/chef/resource/windows_certificate.rb b/lib/chef/resource/windows_certificate.rb index c5db262d15..ebc846644b 100644 --- a/lib/chef/resource/windows_certificate.rb +++ b/lib/chef/resource/windows_certificate.rb @@ -21,7 +21,6 @@ require "chef/util/path_helper" require "chef/resource" require "win32-certstore" if Chef::Platform.windows? require "openssl" -require "open3" class Chef class Resource @@ -53,7 +52,7 @@ class Chef description: "" # lazy used to set default value of sensitive to true if password is set - property :sensitive, [ TrueClass, FalseClass ], + property :sensitive, [TrueClass, FalseClass], description: "Ensure that sensitive resource data is not logged by the chef-client.", default: lazy { |r| r.pfx_password ? true : false }, skip_docs: true @@ -62,9 +61,7 @@ class Chef # Extension of the certificate ext = ::File.extname(new_resource.source) - raw_source = convert_pem(ext) - - cert_obj = OpenSSL::X509::Certificate.new(raw_source) # A certificate object in memory + cert_obj = fetch_cert_object(ext) # Fetch OpenSSL::X509::Certificate object thumbprint = OpenSSL::Digest::SHA1.new(cert_obj.to_der).to_s # Fetch its thumbprint # Need to check if return value is Boolean:true @@ -248,6 +245,7 @@ class Chef def acl_script(hash) return "" if new_resource.private_key_acl.nil? || new_resource.private_key_acl.empty? + # this PS came from http://blogs.technet.com/b/operationsguy/archive/2010/11/29/provide-access-to-private-keys-commandline-vs-powershell.aspx # and from https://msdn.microsoft.com/en-us/library/windows/desktop/bb204778(v=vs.85).aspx set_acl_script = <<-EOH @@ -273,42 +271,41 @@ class Chef set_acl_script end - # Uses powershell command to convert crt/der/cer/pfx & p7b certificates - # In PEM format and returns its certificate content - def convert_pem(ext) - command = case ext - when ".crt", ".cer", ".der" - cmd = "openssl x509 -text -in #{new_resource.source} -outform PEM" - pem_cert? ? cmd : cmd + " -inform DER" - when ".pfx" - "openssl pkcs12 -in #{new_resource.source} -nodes -passin pass:'#{new_resource.pfx_password}'" - when ".p7b" - "openssl pkcs7 -print_certs -in #{new_resource.source} -outform PEM" - else - "openssl x509 -text -inform #{ext.delete('.')} -in #{new_resource.source} -outform PEM" - end - - out = powershell_out(command) - if out.exitstatus == 0 - format_raw_out(out.stdout) + # Method returns an OpenSSL::X509::Certificate object + # + # Based on its extension, the certificate contents are used to initialize + # PKCS12 (PFX), PKCS7 (P7B) objects which contains OpenSSL::X509::Certificate. + # + # @note Other then PEM, all the certificates are usually in binary format, and hence + # their contents are loaded by using File.binread + # + # @param ext [String] Extension of the certificate + # + # @return [OpenSSL::X509::Certificate] Object containing certificate's attributes + # + # @raise [OpenSSL::PKCS12::PKCS12Error] When incorrect password is provided for PFX certificate + # + def fetch_cert_object(ext) + contents = if binary_cert? + ::File.binread(new_resource.source) + else + ::File.read(new_resource.source) + end + + case ext + when ".pfx" + OpenSSL::PKCS12.new(contents, new_resource.pfx_password).certificate + when ".p7b" + OpenSSL::PKCS7.new(contents).certificates.first else - raise out.stderr + OpenSSL::X509::Certificate.new(contents) end end - # Returns the certificate content - def format_raw_out(out) - begin_cert = "-----BEGIN CERTIFICATE-----" - end_cert = "-----END CERTIFICATE-----" - begin_cert + out[/#{begin_cert}(.*?)#{end_cert}/m, 1] + end_cert - end - - # Checks if the given certificate is a PEM certificate or not - def pem_cert? - details, status = Open3.capture2e("file", new_resource.source) - return false unless status.success? - - details.rpartition(":").last.strip == "PEM certificate" + # @return [Boolean] Whether the certificate file is binary encoded or not + # + def binary_cert? + powershell_out!("file -b --mime-encoding #{new_resource.source}").stdout.strip == "binary" end end end diff --git a/spec/functional/resource/windows_certificate_spec.rb b/spec/functional/resource/windows_certificate_spec.rb index a9ed99d318..eac7ee31b2 100644 --- a/spec/functional/resource/windows_certificate_spec.rb +++ b/spec/functional/resource/windows_certificate_spec.rb @@ -228,7 +228,7 @@ describe Chef::Resource::WindowsCertificate, :windows_only, :appveyor_only do win_certificate.pfx_password = "Invalid password" end it "Raises an error" do - expect { win_certificate.run_action(:create) }.to raise_error(RuntimeError) + expect { win_certificate.run_action(:create) }.to raise_error(OpenSSL::PKCS12::PKCS12Error) end end end -- cgit v1.2.1