Refactor X509 certificate validation and debugging.

2 files changed, 72 insertions, 45 deletions

diff --git a/lib/chef/knife/ssl_check.rb b/lib/chef/knife/ssl_check.rb
index f7edd6b019..ac37280211 100644
--- a/lib/chef/knife/ssl_check.rb
+++ b/lib/chef/knife/ssl_check.rb
@@ -108,50 +108,18 @@ class Chef
 
       def verify_X509
         cert_debug_msg = ""
-        if configuration.trusted_certs_dir && Dir.exist?(configuration.trusted_certs_dir)
-          # Check each trusted certificate to ensure that it meets X509 standard.
-          Dir.glob(File.join(configuration.trusted_certs_dir, "*.{crt,pem}")).each do |cert_name|
-            store = OpenSSL::X509::Store.new
-            cert = OpenSSL::X509::Certificate.new(IO.read(File.expand_path(cert)))
-            begin
-              store.add(cert)
-              # test if the store can verify the cert we just added
-              unless store.verify(cert) # true if verified, false if not
-                cert_debug_msg << "#{File.expand_path(cert_name)}: #{store.error_string}\n"
-              end
-            rescue OpenSSL::X509::StoreError
-              cert_debug_msg << "#{File.expand_path(cert_name)}: #{store.error_string}\n"
-            end
+        trusted_certificates.each do |cert_name|
+          message = check_X509_certificate(cert_name)
+          unless message.nil?
+            cert_debug_msg << File.expand_path(cert_name) + ": " + message + "\n"
           end
         end
 
         unless cert_debug_msg.empty?
-          ui.msg("\n#{ui.color("Configuration Info:", :bold)}\n\n")
-          debug_ssl_settings
-          debug_chef_ssl_config
-
-          ui.warn(<<-BAD_CERTS
-There are invalid certificates in your trusted_certs_dir.
-Although Chef may be configured to trust these certificates,
-OpenSSL may not use the following certificates when verifying SSL connections:
-
-#{cert_debug_msg}
-
-#{ui.color("TO FIX THESE WARNINGS:", :bold)}
-
-We are working on documentation for generating self-signed certificates with Subject
-Alternative Names (SANs). For now, please refer to the discussion on GitHub Issue 1700
-
-     https://github.com/opscode/chef/issues/1700
-
-and direct any further questions to github.com/opscode/chef/issues or the chef
-mailing lists.
-
-BAD_CERTS
-          # @TODO: ^ needs URL
-          return false
+          debug_invalid_X509(cert_debug_msg)
         end
-        return true
+
+        true # Maybe the bad certs won't hurt...
       end
 
       def verify_cert
@@ -175,6 +143,42 @@ BAD_CERTS
         false
       end
 
+      def debug_invalid_X509(cert_debug_msg)
+        ui.msg("\n#{ui.color("Configuration Info:", :bold)}\n\n")
+        debug_ssl_settings
+        debug_chef_ssl_config
+
+        ui.warn(<<-BAD_CERTS)
+There are invalid certificates in your trusted_certs_dir.
+OpenSSL may not use the following certificates when verifying SSL connections:
+
+#{cert_debug_msg}
+
+#{ui.color("TO FIX THESE WARNINGS:", :bold)}
+
+We are working on documentation for resolving common issues uncovered here.
+
+* If the certificate is generated by the server, you may try redownloading the
+server's certificate. By default, the certificate is stored in the following
+location on the host where your chef-server runs:
+
+  /var/opt/chef-server/nginx/ca/SERVER_HOSTNAME.crt
+
+Copy that file to your trusted_certs_dir (currently: #{configuration.trusted_certs_dir})
+using SSH/SCP or some other secure method, then re-run this command to confirm
+that the server's certificate is now trusted.
+
+* If generating self-signed certificates with Subject Alternative Names, check out this
+post on Stack Overflow on how to generate these using OpenSSL:
+
+  http://stackoverflow.com/a/21494483
+
+IMPORTANT: You do not need to change the value of 'keyUseage' under '[ v3_ca ]'.
+
+BAD_CERTS
+        # @TODO: ^ needs URL once documentation is posted.
+      end
+
       def debug_invalid_cert
         noverify_socket.connect
         issuer_info = noverify_socket.peer_cert.issuer
@@ -252,6 +256,29 @@ ADVICE
         end
       end
 
+      private
+      def trusted_certificates
+        if configuration.trusted_certs_dir && Dir.exist?(configuration.trusted_certs_dir)
+          Dir.glob(File.join(configuration.trusted_certs_dir, "*.{crt,pem}"))
+        else
+          []
+        end
+      end
+
+      def check_X509_certificate(cert_file)
+        store = OpenSSL::X509::Store.new
+        cert = OpenSSL::X509::Certificate.new(IO.read(File.expand_path(cert_file)))
+        begin
+          store.add_cert(cert)
+          # test if the store can verify the cert we just added
+          unless store.verify(cert) # true if verified, false if not
+            return store.error_string
+          end
+        rescue OpenSSL::X509::StoreError => e
+          return e.message
+        end
+        return nil
+      end
     end
   end
 end
diff --git a/spec/unit/knife/ssl_check_spec.rb b/spec/unit/knife/ssl_check_spec.rb
index 6896a6b80f..bb803ce2ca 100644
--- a/spec/unit/knife/ssl_check_spec.rb
+++ b/spec/unit/knife/ssl_check_spec.rb
@@ -99,17 +99,17 @@ E
     let(:name_args) { %w{https://foo.example.com:8443} }
 
     let(:trusted_certs_dir) { File.join(CHEF_SPEC_DATA, "trusted_certs") }
-    let(:trusted_certs) { [File.join(trusted_certs_dir, "example.crt")] }
+    let(:trusted_cert_file) { File.join(trusted_certs_dir, "example.crt") }
 
     let(:store) { OpenSSL::X509::Store.new }
-    let(:certificate) { OpenSSL::X509::Certificate.new(IO.read(trusted_certs[0])) }
+    let(:certificate) { OpenSSL::X509::Certificate.new(IO.read(trusted_cert_file)) }
 
     before do
       Chef::Config[:trusted_certs_dir] = trusted_certs_dir
-      Dir.stub(:glob).with(File.join(trusted_certs_dir, "*.{crt,pem}")).and_return(trusted_certs)
-      store.stub(:add).with(certificate)
+      ssl_check.stub(:trusted_certificates).and_return([trusted_cert_file])
+      store.stub(:add_cert).with(certificate)
       OpenSSL::X509::Store.stub(:new).and_return(store)
-      OpenSSL::X509::Certificate.stub(:new).with(IO.read(trusted_certs[0])).and_return(certificate)
+      OpenSSL::X509::Certificate.stub(:new).with(IO.read(trusted_cert_file)).and_return(certificate)
       ssl_check.stub(:verify_cert).and_return(true)
       ssl_check.stub(:verify_cert_host).and_return(true)
     end
@@ -132,7 +132,7 @@ E
       end
 
       it "generates a warning message with invalid certificate file names" do
-        expect(ssl_check.ui).to receive(:warn).with(/#{trusted_certs[0]}: unable to get local issuer certificate/)
+        expect(ssl_check.ui).to receive(:warn).with(/#{trusted_cert_file}: unable to get local issuer certificate/)
         ssl_check.run
       end
     end