Updating knife ssl fetch to correctly store certificates when it does not contain a common name. Stores the certificate under the URI host instead of the common name in that case2919/ssl_fetch_no_cn

Signed-off-by: tyler-ball <tyleraball@gmail.com>

3 files changed, 83 insertions, 5 deletions

diff --git a/lib/chef/knife/ssl_fetch.rb b/lib/chef/knife/ssl_fetch.rb
index 5af1a905d5..238796c804 100644
--- a/lib/chef/knife/ssl_fetch.rb
+++ b/lib/chef/knife/ssl_fetch.rb
@@ -89,8 +89,11 @@ class Chef
 
       def cn_of(certificate)
         subject = certificate.subject
-        cn_field_tuple = subject.to_a.find { |field| field[0] == "CN" }
-        cn_field_tuple[1]
+        if cn_field_tuple = subject.to_a.find { |field| field[0] == "CN" }
+          cn_field_tuple[1]
+        else
+          nil
+        end
       end
 
       # Convert the CN of a certificate into something that will work well as a
@@ -117,9 +120,10 @@ class Chef
       def write_cert(cert)
         FileUtils.mkdir_p(trusted_certs_dir)
         cn = cn_of(cert)
-        filename = File.join(trusted_certs_dir, "#{normalize_cn(cn)}.crt")
-        ui.msg("Adding certificate for #{cn} in #{filename}")
-        File.open(filename, File::CREAT | File::TRUNC | File::RDWR, 0644) do |f|
+        filename = cn.nil? ? "#{host}_#{Time.new.to_i}" : normalize_cn(cn)
+        full_path = File.join(trusted_certs_dir, "#{filename}.crt")
+        ui.msg("Adding certificate for #{filename} in #{full_path}")
+        File.open(full_path, File::CREAT | File::TRUNC | File::RDWR, 0644) do |f|
           f.print(cert.to_s)
         end
       end
diff --git a/spec/data/trusted_certs/example_no_cn.crt b/spec/data/trusted_certs/example_no_cn.crt
new file mode 100644
index 0000000000..6b42b40d99
--- /dev/null
+++ b/spec/data/trusted_certs/example_no_cn.crt
@@ -0,0 +1,36 @@
+-----BEGIN CERTIFICATE-----
+MIIGPzCCBCegAwIBAgIJAKwtLqBeqNzfMA0GCSqGSIb3DQEBBQUAMHIxCzAJBgNV
+BAYTAlVTMQswCQYDVQQIEwJXQTEQMA4GA1UEBxMHU2VhdHRsZTEQMA4GA1UEChMH
+WW91Q29ycDETMBEGA1UECxMKT3BlcmF0aW9uczEdMBsGCSqGSIb3DQEJARYObWVA
+ZXhhbXBsZS5jb20wHhcNMTYxMDMxMTkxMzQ2WhcNMjYxMDI5MTkxMzQ2WjByMQsw
+CQYDVQQGEwJVUzELMAkGA1UECBMCV0ExEDAOBgNVBAcTB1NlYXR0bGUxEDAOBgNV
+BAoTB1lvdUNvcnAxEzARBgNVBAsTCk9wZXJhdGlvbnMxHTAbBgkqhkiG9w0BCQEW
+Dm1lQGV4YW1wbGUuY29tMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEA
+s1OiWnMV3shxVccqzenDBww5rSou9Ab/VqujKisJ54dXyHukYMxh9MJwlRDsy0FB
+uKRAyNfhM43hSMYhtF7NS//D1lI/LDvIQkBaH8R834bvK102Avmsx7zKPOo/CUkd
+g7uuL2eRzRszEuAREH1E7/PpTj11CjirG9i7FlbKj7vDA1Nqvtb0kHdiQuH2Cojy
+Uf1uVFyE5UliFXtePDrxpOAfJUbcSdOLsK8olKHGCb0cfN/tCfbyEY8rHGsAUK2A
+afuHRTR7pRQwfqJ5EK3DBbbFz+GSi+9zWFOudfqTsczS/HtpMbF8HBwqBAa+mpU/
+UjmhpTYQ+rgVtWtEcttboeK6jvFBFLyQ6VRcrDi/8lmAnm1Q+RZk5g3GwZMhIMNU
+5XQZf6jsUsIFBuOaRyLn9dXkgyO7gOy8n8Yw+YdIFt29kaqZ6pu9kpS0jquxzSKj
+MVS4OrThLwgazfQu/BlOvJpQfcNPI/VP9c41yHKpeoIh6oxNDc/212/wwgwPgD83
+8YXddupaSuE++h9Z10CCZgwux8deTlMjzETIMiIo8R3KV0pJgZ11akeJ8USr+QQ2
++fO/GdpNUa5nNTgF3t4zTF3DPToqj6KDgxLhUdXopF1hLYgwr8FKOtn9KXP+I0hz
+hWzZoX9gwFLEPrUy265Gpw8TVTmNuSiiZtgJDSDKTBcCAwEAAaOB1zCB1DAdBgNV
+HQ4EFgQUr5Y6dxhyVxfhwFsEKLDIXxQ2zBswgaQGA1UdIwSBnDCBmYAUr5Y6dxhy
+VxfhwFsEKLDIXxQ2zBuhdqR0MHIxCzAJBgNVBAYTAlVTMQswCQYDVQQIEwJXQTEQ
+MA4GA1UEBxMHU2VhdHRsZTEQMA4GA1UEChMHWW91Q29ycDETMBEGA1UECxMKT3Bl
+cmF0aW9uczEdMBsGCSqGSIb3DQEJARYObWVAZXhhbXBsZS5jb22CCQCsLS6gXqjc
+3zAMBgNVHRMEBTADAQH/MA0GCSqGSIb3DQEBBQUAA4ICAQBYXgqXAnocH6i8o47c
+BZPMGO9y4LCB4YNIrZPKRNFvRl2aolA5KiBxr6WJp1iczxVA4lCmXU1LGfvRPHec
+nHtVax3+Q1JCZhBSv/txQTjgn72qoJyCsPmjyWifbE1jFdRj0g74/Eu/0ku3L0vV
+jTlqzJXQIzRkQm+Y5OrZo92tXaOgO+C0qdd6gaEaIIya6bzrBpW95NtVymhXi2Qf
+7G7Z/yw8XhoQiDJaPHF6XavC3dYvi51cehnPR4E6Jok23kbJEe3Qw5Yh747JjSsS
+Sz07CKqTFcFjHI2f1sFdDjw34lj5mtOf3pEpRGGmvzkF8zm/sVQQ2rCKnqEe7zPy
+Bg9guzVpORG+g76hGFZcYnz78LLNrIYcuYoLcbbZh404wjmifVKO5OC1dRgmJTuc
+VnJe92568Y9cUAjrLztxp5gwXgYUllsXweJ2UGiHxSBqUfCCGG5vK5sYs52HR6wJ
+wRSvwk/VHifYPxJ54RRB51ebYjmD1j41tRseHdFq21qpXSvr9DFLUJBvdN9zA/6t
+xCBlXAdYxD0n0/bruUYNoXBeMhLp+WKSAQvTlVIyqoNQCo1OBBzBVNg9otl3jw5d
+1QOhodRqmS5UQAJptuXtk8WN8OZqMCCeogIfdpa5tJG+/fxFML9EvqedS4c05Wf/
+oYdVLVWSjyoA2l4Xb4LdexAgCg==
+-----END CERTIFICATE-----
diff --git a/spec/unit/knife/ssl_fetch_spec.rb b/spec/unit/knife/ssl_fetch_spec.rb
index 8bb4810b88..bc49c40241 100644
--- a/spec/unit/knife/ssl_fetch_spec.rb
+++ b/spec/unit/knife/ssl_fetch_spec.rb
@@ -108,6 +108,24 @@ E
 
   end
 
+  describe "#cn_of" do
+    let(:certificate) { double("Certificate", subject: subject) }
+
+    describe "when the certificate has a common name" do
+      let(:subject) { [["CN", "common name"]] }
+      it "returns the common name" do
+        expect(ssl_fetch.cn_of(certificate)).to eq("common name")
+      end
+    end
+
+    describe "when the certificate does not have a common name" do
+      let(:subject) { [] }
+      it "returns nil" do
+        expect(ssl_fetch.cn_of(certificate)).to eq(nil)
+      end
+    end
+  end
+
   describe "fetching the remote cert chain" do
 
     let(:name_args) { %w{https://foo.example.com:8443} }
@@ -180,5 +198,25 @@ ERROR_TEXT
 
     end
 
+    describe "when the certificate does not have a CN" do
+      let(:self_signed_crt_path) { File.join(CHEF_SPEC_DATA, "trusted_certs", "example_no_cn.crt") }
+      let(:self_signed_crt) { OpenSSL::X509::Certificate.new(File.read(self_signed_crt_path)) }
+
+      before do
+        expect(ssl_fetch).to receive(:proxified_socket).with("foo.example.com", 8443).and_return(tcp_socket)
+        expect(OpenSSL::SSL::SSLSocket).to receive(:new).with(tcp_socket, ssl_fetch.noverify_peer_ssl_context).and_return(ssl_socket)
+        expect(ssl_socket).to receive(:connect)
+        expect(ssl_socket).to receive(:peer_cert_chain).and_return([self_signed_crt])
+        expect(Time).to receive(:new).and_return(1)
+      end
+
+      it "fetches the certificate and writes it to a file in the trusted_certs_dir" do
+        run
+        stored_cert_path = File.join(trusted_certs_dir, "foo.example.com_1.crt")
+        expect(File).to exist(stored_cert_path)
+        expect(File.read(stored_cert_path)).to eq(File.read(self_signed_crt_path))
+      end
+    end
+
   end
 end