Merge pull request #552 from vjanelle/support_intermediate_certs_and_san

(#550) intermediate certificate chains

lib/mcollective/security/choria.rb

@@ -392,7 +392,7 @@ def should_cache_certname?(pubcert, callerid)
       # @param pubcert [String] PEM encoded X509 public certificate
       # @return [Hash]
       def client_pubcert_metadata(envelope, pubcert)
-        cert = choria.parse_pubcert(pubcert)
+        cert = choria.parse_pubcert(pubcert).first
 
         {
           "create_time" => current_timestamp,

lib/mcollective/util/choria.rb

@@ -329,18 +329,23 @@ def have_ssl_files?(log=true)
       # Validates a certificate against the CA
       #
       # @param pubcert [String] PEM encoded X509 public certificate
+      # @param name [String] name that should be present in the certificate
       # @param log [Boolean] log warnings when true
       # @return [String,false] when succesful, the certname else false
       # @raise [StandardError] in case OpenSSL fails to open the various certificates
       # @raise [OpenSSL::X509::CertificateError] if the CA is invalid
-      def valid_certificate?(pubcert, log=true)
+      def valid_certificate?(pubcert, name, log=true)
         unless File.readable?(ca_path)
           raise("Cannot find or read the CA in %s, cannot verify public certificate" % ca_path)
         end
 
-        incoming = parse_pubcert(pubcert, log)
+        certs = parse_pubcert(pubcert, log)
 
-        return false unless incoming
+        return false if certs.empty?
+
+        incoming = certs.first
+
+        chain = certs[1..-1]
 
         begin
           ca = OpenSSL::X509::Store.new.add_file(ca_path)
@@ -349,27 +354,59 @@ def valid_certificate?(pubcert, log=true)
           raise
         end
 
-        unless ca.verify(incoming)
-          Log.warn("Failed to verify certificate %s against CA %s in %s" % [incoming.subject.to_s, incoming.issuer.to_s, ca_path]) if log
+        unless ca.verify(incoming, chain)
+          if log
+            Log.warn("Failed to verify certificate %s against CA %s in %s" % [
+              incoming.subject.to_s,
+              incoming.issuer.to_s,
+              ca_path
+            ])
+          end
+
           return false
         end
 
         Log.debug("Verified certificate %s against CA %s" % [incoming.subject.to_s, incoming.issuer.to_s]) if log
 
-        cn_parts = incoming.subject.to_a.select {|c| c[0] == "CN"}.flatten
+        unless OpenSSL::SSL.verify_certificate_identity(incoming, name)
+          raise("Could not parse certificate with subject %s as it has no CN part, or name %s invalid" % [
+            incoming.subject.to_s,
+            name
+          ])
+        end
 
-        raise("Could not parse certificate with subject %s as it has no CN part" % [incoming.subject.to_s]) if cn_parts.empty?
+        name
+      end
 
-        cn_parts[1]
+      # Utility function to split a chained certificate String into an Array
+      #
+      # @param pemdata [String] PEM encoded certificate
+      # @return [Array<String,nil>]
+      def ssl_split_pem(pemdata)
+        # Chained certificates typically have the public certificate, along
+        # with every intermediate certificiate.
+        # OpenSSL will stop at the first certificate when using OpenSSL::X509::Certificate.new,
+        # so we need to separate them into a list
+        pemdata.scan(/-----BEGIN CERTIFICATE-----.+?-----END CERTIFICATE-----/m)
+      end
+
+      # Split a string containing chained certificates into an Array of OpenSSL::X509::Certificate.
+      #
+      # @param pemdata [String]
+      # @return [Array<OpenSSL::X509::Certificate,nil>]
+      def ssl_parse_chain(pemdata)
+        ssl_split_pem(pemdata).map do |cpem|
+          OpenSSL::X509::Certificate.new(cpem)
+        end
       end
 
       # Parses a public cert
       #
       # @param pubcert [String] PEM encoded public certificate
       # @param log [Boolean] log warnings when true
-      # @return [OpenSSL::X509::Certificate,nil]
+      # @return [Array<OpenSSL::X509::Certificate,nil>]
       def parse_pubcert(pubcert, log=true)
-        OpenSSL::X509::Certificate.new(pubcert)
+        ssl_parse_chain(pubcert)
       rescue OpenSSL::X509::CertificateError
         Log.warn("Received certificate is not a valid x509 certificate: %s: %s" % [$!.class, $!.to_s]) if log
         nil
@@ -390,7 +427,7 @@ def check_ssl_setup(log=true)
         embedded_certname = nil
 
         begin
-          embedded_certname = valid_certificate?(File.read(client_public_cert))
+          embedded_certname = valid_certificate?(File.read(client_public_cert), certname)
         rescue
           raise(UserError, "The public certificate was not signed by the configured CA")
         end
@@ -640,8 +677,25 @@ def puppet_setting(setting)
       def ssl_context
         context = OpenSSL::SSL::SSLContext.new
         context.ca_file = ca_path
-        context.cert = OpenSSL::X509::Certificate.new(File.read(client_public_cert))
-        context.key = OpenSSL::PKey::RSA.new(File.read(client_private_key))
+
+        public_cert = File.read(client_public_cert)
+        private_key = File.read(client_private_key)
+
+        cert_chain = ssl_parse_chain(public_cert)
+
+        cert = cert_chain.first
+        key = OpenSSL::PKey::RSA.new(private_key)
+
+        extra_chain_cert = cert_chain[1..-1]
+
+        if OpenSSL::SSL::SSLContext.method_defined?(:add_certificate)
+          context.add_certificate(cert, key, extra_chain_cert)
+        else
+          context.cert = OpenSSL::X509::Certificate.new(File.read(client_public_cert))
+          context.key = OpenSSL::PKey::RSA.new(File.read(client_private_key))
+          context.extra_chain_cert = extra_chain_cert
+        end
+
         context.verify_mode = OpenSSL::SSL::VERIFY_PEER
 
         context

spec/fixtures/intermediate/ca.pem

@@ -0,0 +1,14 @@
+-----BEGIN CERTIFICATE-----
+MIICOjCCAd+gAwIBAgIUSHvvZWWyM18ks49RZkbWkyLslH0wCgYIKoZIzj0EAwIw
+eTELMAkGA1UEBhMCWFgxETAPBgNVBAgTCExvY2FsaXR5MQ0wCwYDVQQHEwRDaXR5
+MQ8wDQYDVQQKEwZDaG9yaWExJTAjBgNVBAsTHFVuaXQgdGVzdGluZyBJbnRlcm1l
+ZGlhdGUgQ0ExEDAOBgNVBAMTB1Jvb3QgQ0EwHhcNMTgxMTEzMDEyMzAwWhcNNDgx
+MTA1MDEyMzAwWjB5MQswCQYDVQQGEwJYWDERMA8GA1UECBMITG9jYWxpdHkxDTAL
+BgNVBAcTBENpdHkxDzANBgNVBAoTBkNob3JpYTElMCMGA1UECxMcVW5pdCB0ZXN0
+aW5nIEludGVybWVkaWF0ZSBDQTEQMA4GA1UEAxMHUm9vdCBDQTBZMBMGByqGSM49
+AgEGCCqGSM49AwEHA0IABKKemAj1QsoT3pXQCYK7DD94vNry5BL9OnCmaojzlBFZ
+0n0vZJi7/GHtr/OVnUXBQOD7XOOWkHCwHDJq2O0+Am6jRTBDMA4GA1UdDwEB/wQE
+AwIBBjASBgNVHRMBAf8ECDAGAQH/AgEBMB0GA1UdDgQWBBQ2M6o4bz7r8MgG9Q0/
+7rN8OgoiETAKBggqhkjOPQQDAgNJADBGAiEA/Yxzoa8YLNzIyWQqHq7tJHnnk3qt
+anWV8i+8LIDItw4CIQC6YnE5cNQSUYXtK9L5A8sB8ZcBdO0LIu/zlrbBHQo53A==
+-----END CERTIFICATE-----

spec/fixtures/intermediate/chain-rip.mcollective.pem

@@ -0,0 +1,34 @@
+-----BEGIN CERTIFICATE-----
+MIIDBzCCAqygAwIBAgIUGCd2Rj5pwjR9bGLD9BS6YpWw7SIwCgYIKoZIzj0EAwIw
+gYExCzAJBgNVBAYTAlhYMREwDwYDVQQIEwhMb2NhbGl0eTENMAsGA1UEBxMEQ2l0
+eTEPMA0GA1UEChMGQ2hvcmlhMSUwIwYDVQQLExxVbml0IHRlc3RpbmcgSW50ZXJt
+ZWRpYXRlIENBMRgwFgYDVQQDEw9JbnRlcm1lZGlhdGUgQ0EwHhcNMTgxMTEzMDEy
+MzAwWhcNMTkxMTEzMDEyMzAwWjAaMRgwFgYDVQQDEw9yaXAubWNvbGxlY3RpdmUw
+ggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDNN5jOHMOMISkSrwFLmxIS
+bKoEuDAwlOSbhFHc89GVbPcHziIJPLqur+YdV0xLZht+ZXvSZDsgo4hjZqXsOCbi
+WZkzN5xfSuZqai8NvfEXOjXYjofCh3/bZ7gVcEkSt3EclxbcZ2d1pcX1sjl8GyH5
+pyHmfOJHrpXdVE10Z4QuA58UKZoQ3i9R7ohCVYoUaAJn6+5015x/mWhzuB8ebdjc
+mqt/aYK/f1apIxIo909nkXz2BS94B/s6zUGG89sA47Pi13CNC5u7cJ7VEfAQYHWP
+F8sXzi6u7+MvDVXRcBPsy1sT89+udANidAtxYhn5On/dDB5qMD77DvJcVN/8Qil5
+AgMBAAGjgZwwgZkwDgYDVR0PAQH/BAQDAgWgMB0GA1UdJQQWMBQGCCsGAQUFBwMB
+BggrBgEFBQcDAjAMBgNVHRMBAf8EAjAAMB0GA1UdDgQWBBTmgrLpFNVMbh1C5UsF
+xVDjd549xzAfBgNVHSMEGDAWgBSlQrdE6JCCk8azRsWXnRuk2ctF+jAaBgNVHREE
+EzARgg9yaXAubWNvbGxlY3RpdmUwCgYIKoZIzj0EAwIDSQAwRgIhAIDvVp0fzmEK
+ULH79CDG3TqcCDiGRPwWMyRUFjazykNuAiEAypPXG9z+/MgGIO2lsYyhQR/Kd+ao
+18XVjuUb3P2egYE=
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIICZDCCAgmgAwIBAgIUMHE90peOTHN6Iv2S2R2astND6lswCgYIKoZIzj0EAwIw
+eTELMAkGA1UEBhMCWFgxETAPBgNVBAgTCExvY2FsaXR5MQ0wCwYDVQQHEwRDaXR5
+MQ8wDQYDVQQKEwZDaG9yaWExJTAjBgNVBAsTHFVuaXQgdGVzdGluZyBJbnRlcm1l
+ZGlhdGUgQ0ExEDAOBgNVBAMTB1Jvb3QgQ0EwHhcNMTgxMTEzMDEyMzAwWhcNNDgx
+MTA1MDEyMzAwWjCBgTELMAkGA1UEBhMCWFgxETAPBgNVBAgTCExvY2FsaXR5MQ0w
+CwYDVQQHEwRDaXR5MQ8wDQYDVQQKEwZDaG9yaWExJTAjBgNVBAsTHFVuaXQgdGVz
+dGluZyBJbnRlcm1lZGlhdGUgQ0ExGDAWBgNVBAMTD0ludGVybWVkaWF0ZSBDQTBZ
+MBMGByqGSM49AgEGCCqGSM49AwEHA0IABNGtHy1coQANdtEj/OK8JjgVxQ+owXlq
+X3PWtohIhx1dlD4MS78sPoEblHcU5NAfSPTN23gPw2kalFjV5NJH3I+jZjBkMA4G
+A1UdDwEB/wQEAwIBpjASBgNVHRMBAf8ECDAGAQH/AgEAMB0GA1UdDgQWBBSlQrdE
+6JCCk8azRsWXnRuk2ctF+jAfBgNVHSMEGDAWgBQ2M6o4bz7r8MgG9Q0/7rN8Ogoi
+ETAKBggqhkjOPQQDAgNJADBGAiEAueRTGMy56l9024iI0tE+huS5E0wEu1ZyQfpI
+AnqVQ70CIQCqVCe23uL3Po9THrXrmpVF7n+CJLQnKdpM3uxxsPWAIg==
+-----END CERTIFICATE-----

spec/fixtures/intermediate/rip.mcollective-key.pem

@@ -0,0 +1,27 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIEpgIBAAKCAQEAzTeYzhzDjCEpEq8BS5sSEmyqBLgwMJTkm4RR3PPRlWz3B84i
+CTy6rq/mHVdMS2YbfmV70mQ7IKOIY2al7Dgm4lmZMzecX0rmamovDb3xFzo12I6H
+wod/22e4FXBJErdxHJcW3GdndaXF9bI5fBsh+ach5nziR66V3VRNdGeELgOfFCma
+EN4vUe6IQlWKFGgCZ+vudNecf5loc7gfHm3Y3Jqrf2mCv39WqSMSKPdPZ5F89gUv
+eAf7Os1BhvPbAOOz4tdwjQubu3Ce1RHwEGB1jxfLF84uru/jLw1V0XAT7MtbE/Pf
+rnQDYnQLcWIZ+Tp/3QweajA++w7yXFTf/EIpeQIDAQABAoIBAQCLcAVhtuWfp0Bz
+M3ob1yf2YOM9BbGosOKMUOIW0YxMjZdkNEmoIR1vaJFgylpKuPxha68wi24phTQ8
+5hhDYvv4vIx94oFbtlbNY4zJN5BDCghgNqhEIzFa8SSBXKQqFX7DwF2GMLR5mcPi
+Z9DrZLw3F7rPE3fk4NlvY8KlH1kOsasMFs4azvgftHRcIAd2OycV1tL7L8V5e9Bn
+peX5CnWmo3rZSZcdZADxiecZlanvb+sstCw6iszaJfnVH+TBThf/06nIH8mHxLUR
+6UdE95Lf3lPd5ZQyukwT94SYO5ckuIk2CLXrmcUUzbbD40tXNIe2/2QgVNutpzPP
+soMPaKcBAoGBAPUnqKlLwRMn0S9s2CdYBq6gWSpTmCusEjoqi4lDwQ55b40udE0H
+XxZb3Myjx8n7ql32tX+M4b13s85Eboh/t/KI+0gM3w2P7W4c9WU1zdnXSVEiMfqP
+h7QLwSMMHtbbcHsYRbDvqDgWycS4Rfm4QCmR9M4fYBPCekbAE/50f4AJAoGBANZL
+pj0MjibUpc2mILoMlfT5VirrtZysA5OtV4JTTt88+j5yyDRy5wNL+1v6gWZ2gTNS
+a+6cwEI3w3UyWEksRArzBmdThNChVGmD3Pm0DMwJBUPoGnYMuFe2W5FXl5YJV8Jo
+Rzgu6d5eAtu56hSI3QAwQpuvcsWH7CYyCMdN29nxAoGBALUgC/iow4mHjYHghQLs
+gmNajQY8pNz/UKgw7s8HhAdRqR1CCSMwIwy96jA3gVC143Vw5T/LsqztV6c54ABx
+fFJw6ladS98VS3JjatrQGbqs2Lpc7VgV20kmthdSySYtErmfgT3skvh9vazeCLUr
+cBxGffwcKjvvH7BOEXeaUukhAoGBAK++fMgmatI3pP6h1sceGUE91sf+ZQPnIkvT
+ZigQkGeOR6A9XCl/biuK/cqyB7tzRoRDfRbEYPwtZVPRBQyFjAv6wO6uVQcQt/yM
+0wXJ/pC6eSH20PSte+UbPb9VuZCnetyJzpaqCsx+BxQSRYGvuKc17PpnCdYroaS1
+dfOVy87RAoGBAIPuUBxWFQQAPxAFZFGyGlPAENm3YkKsqx0v1uWRw6V4LL9siiMr
+O30MFmyKBTf5Js/FcVQbf5qTWapNLZE5ypttO9/Uppb6vK/SoksapNiq+thvcvf4
+OFd679tvOhu8yhK8gRDBN6cPQteloBeLD3WB1EH7jWcru0r+Bvj73G2h
+-----END RSA PRIVATE KEY-----

spec/fixtures/intermediate/rip.mcollective.pem

@@ -0,0 +1,19 @@
+-----BEGIN CERTIFICATE-----
+MIIDBzCCAqygAwIBAgIUGCd2Rj5pwjR9bGLD9BS6YpWw7SIwCgYIKoZIzj0EAwIw
+gYExCzAJBgNVBAYTAlhYMREwDwYDVQQIEwhMb2NhbGl0eTENMAsGA1UEBxMEQ2l0
+eTEPMA0GA1UEChMGQ2hvcmlhMSUwIwYDVQQLExxVbml0IHRlc3RpbmcgSW50ZXJt
+ZWRpYXRlIENBMRgwFgYDVQQDEw9JbnRlcm1lZGlhdGUgQ0EwHhcNMTgxMTEzMDEy
+MzAwWhcNMTkxMTEzMDEyMzAwWjAaMRgwFgYDVQQDEw9yaXAubWNvbGxlY3RpdmUw
+ggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDNN5jOHMOMISkSrwFLmxIS
+bKoEuDAwlOSbhFHc89GVbPcHziIJPLqur+YdV0xLZht+ZXvSZDsgo4hjZqXsOCbi
+WZkzN5xfSuZqai8NvfEXOjXYjofCh3/bZ7gVcEkSt3EclxbcZ2d1pcX1sjl8GyH5
+pyHmfOJHrpXdVE10Z4QuA58UKZoQ3i9R7ohCVYoUaAJn6+5015x/mWhzuB8ebdjc
+mqt/aYK/f1apIxIo909nkXz2BS94B/s6zUGG89sA47Pi13CNC5u7cJ7VEfAQYHWP
+F8sXzi6u7+MvDVXRcBPsy1sT89+udANidAtxYhn5On/dDB5qMD77DvJcVN/8Qil5
+AgMBAAGjgZwwgZkwDgYDVR0PAQH/BAQDAgWgMB0GA1UdJQQWMBQGCCsGAQUFBwMB
+BggrBgEFBQcDAjAMBgNVHRMBAf8EAjAAMB0GA1UdDgQWBBTmgrLpFNVMbh1C5UsF
+xVDjd549xzAfBgNVHSMEGDAWgBSlQrdE6JCCk8azRsWXnRuk2ctF+jAaBgNVHREE
+EzARgg9yaXAubWNvbGxlY3RpdmUwCgYIKoZIzj0EAwIDSQAwRgIhAIDvVp0fzmEK
+ULH79CDG3TqcCDiGRPwWMyRUFjazykNuAiEAypPXG9z+/MgGIO2lsYyhQR/Kd+ao
+18XVjuUb3P2egYE=
+-----END CERTIFICATE-----

spec/unit/mcollective/util/bolt_support_spec.rb

@@ -60,11 +60,16 @@ module Util
         end
 
         it "should support fail_ok" do
-          expect { support.run_task(nil, "shell", "command" => "/bin/false") }.to raise_error(
+          # macs don't have a /bin/false, circleci doesn't have a /usr/bin/false :(
+          # Other OSs are now linking /bin to /usr/bin
+          # Everything is terrible and I'm sorry - vjanelle
+          shell = File.exist?("/bin/false") ? "/bin/false" : "/usr/bin/false"
+
+          expect { support.run_task(nil, "shell", "command" => shell) }.to raise_error(
             "Command failed with code 1"
           )
 
-          result = support.run_task(nil, "shell", "command" => "/bin/false", "fail_ok" => true)
+          result = support.run_task(nil, "shell", "command" => shell, "fail_ok" => true)
 
           expect(result.error_set.first.to_hash).to eq(
             "localhost" => {
@@ -75,7 +80,7 @@ module Util
                 "msg" => "Command failed with code 1",
                 "kind" => "choria.playbook/taskerror",
                 "details" => {
-                  "command" => "/bin/false"
+                  "command" => shell
                 }
               }
             }

spec/unit/mcollective/util/choria_spec.rb

@@ -158,21 +158,33 @@ module Util
           choria.expects(:ca_path).returns("/nonexisting").twice
 
           expect {
-            choria.valid_certificate?("x")
+            choria.valid_certificate?("x", "badhostname")
           }.to raise_error("Cannot find or read the CA in /nonexisting, cannot verify public certificate")
         end
 
         it "should fail for CA missmatches" do
           choria.stubs(:ca_path).returns("spec/fixtures/other_ca.pem")
-          expect(choria.valid_certificate?(File.read("spec/fixtures/rip.mcollective.pem"))).to be_falsey
+          expect(choria.valid_certificate?(File.read("spec/fixtures/rip.mcollective.pem"), "rip.mcollective")).to be_falsey
 
           choria.stubs(:ca_path).returns("spec/fixtures/ca_crt.pem")
-          expect(choria.valid_certificate?(File.read("spec/fixtures/other.mcollective.pem"))).to be_falsey
+          expect(choria.valid_certificate?(File.read("spec/fixtures/other.mcollective.pem"), "other.mcollective")).to be_falsey
         end
 
         it "should pass for valid cert/ca combos" do
           choria.stubs(:ca_path).returns("spec/fixtures/ca_crt.pem")
-          expect(choria.valid_certificate?(File.read("spec/fixtures/rip.mcollective.pem"))).to be_truthy
+          expect(choria.valid_certificate?(File.read("spec/fixtures/rip.mcollective.pem"), "rip.mcollective")).to be_truthy
+        end
+      end
+
+      describe "#valid_intermediate_certificate?" do
+        it "should fail for CA missmatches" do
+          choria.stubs(:ca_path).returns("spec/fixtures/other_ca.pem")
+          expect(choria.valid_certificate?(File.read("spec/fixtures/intermediate/chain-rip.mcollective.pem"), "rip.mcollective")).to be_falsey
+        end
+
+        it "should pass for valid client cert w/intermediate CA/ca combos" do
+          choria.stubs(:ca_path).returns("spec/fixtures/intermediate/ca.pem")
+          expect(choria.valid_certificate?(File.read("spec/fixtures/intermediate/chain-rip.mcollective.pem"), "rip.mcollective")).to be_truthy
         end
       end
 
@@ -206,6 +218,19 @@ module Util
           expect(context.cert.subject.to_s).to eq("/CN=rip.mcollective")
           expect(context.key.to_pem).to eq(File.read("spec/fixtures/rip.mcollective.key"))
         end
+
+        it "should create a valid ssl context with intermediate certs" do
+          choria.stubs(:ca_path).returns("spec/fixtures/intermediate/ca.pem")
+          choria.stubs(:client_public_cert).returns("spec/fixtures/intermediate/rip.mcollective.pem")
+          choria.stubs(:client_private_key).returns("spec/fixtures/intermediate/rip.mcollective-key.pem")
+
+          context = choria.ssl_context
+
+          expect(context.verify_mode).to be(OpenSSL::SSL::VERIFY_PEER)
+          expect(context.ca_file).to eq("spec/fixtures/intermediate/ca.pem")
+          expect(context.cert.subject.to_s).to eq("/CN=rip.mcollective")
+          expect(context.key.to_pem).to eq(File.read("spec/fixtures/intermediate/rip.mcollective-key.pem"))
+        end
       end
 
       describe "#federation_collectives" do
@@ -846,7 +871,7 @@ module Util
         it "should fail if the cert is not signed by the CA" do
           choria.expects(:have_ssl_files?).returns(true)
           pub_cert = File.read(choria.client_public_cert)
-          choria.expects(:valid_certificate?).with(pub_cert).raises("rspec fail")
+          choria.expects(:valid_certificate?).with(pub_cert, choria.certname).raises("rspec fail")
           expect { choria.check_ssl_setup }.to raise_error("The public certificate was not signed by the configured CA")
         end