(#550) intermediate certificate chains

For bonus effort, this will also test SANs by moving the validation
logic to openssl, instead of trying to parse the CN out of the subject.

Contains ruby 2.4/2.5 compat for OpenSSL::SSL::Context depreciating the
use of `extra_cert_chain` - the ruby shipped with puppet 5.X is behind
a revision.

Ruby's OpenSSL doesn't make this easy, which I suspect is more to do
with OpenSSL than Ruby.  There are alternatives which reduce some of
this, like R509 but haven't investigated them yet.

* Add intermediate certificate processing - hacky :(
* Add intermediate chain certs
* Add spec tests to validate intermediate chain handling
* Add something to detect `/usr/bin/false` vs `/bin/false` because no one can agree where it goes.

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,19 @@ 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)
 
-        return false unless incoming
+        return false unless incoming.any?
 
         begin
           ca = OpenSSL::X509::Store.new.add_file(ca_path)
@@ -349,27 +350,42 @@ 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.first, incoming)
+          if log
+            Log.warn("Failed to verify certificate %s against CA %s in %s" % [
+              incoming.first.subject.to_s,
+              incoming.first.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
+        if log
+          Log.debug("Verified certificate %s against CA %s" % [
+            incoming.first.subject.to_s,
+            incoming.first.issuer.to_s
+          ])
+        end
 
-        raise("Could not parse certificate with subject %s as it has no CN part" % [incoming.subject.to_s]) if cn_parts.empty?
+        unless OpenSSL::SSL.verify_certificate_identity(incoming.first, name)
+          raise("Could not parse certificate with subject %s as it has no CN part, or name %s invalid" % [
+            incoming.first.subject.to_s,
+            name
+          ])
+        end
 
-        cn_parts[1]
+        name
       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)
+        pubcert.scan(/(-----BEGIN CERTIFICATE-----.+?-----END CERTIFICATE-----)/m).map {|cstr| OpenSSL::X509::Certificate.new(cstr[0])}
       rescue OpenSSL::X509::CertificateError
         Log.warn("Received certificate is not a valid x509 certificate: %s: %s" % [$!.class, $!.to_s]) if log
         nil
@@ -390,7 +406,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 +656,24 @@ 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))
+
+        cert = OpenSSL::X509::Certificate.new(File.read(client_public_cert))
+        key = OpenSSL::PKey::RSA.new(File.read(client_private_key))
+
+        extra_chain_cert = File.read(client_public_cert).lines.reject { |line| line[0] == "#" }
+                               .join("")
+                               .scan(/(-----BEGIN CERTIFICATE-----.+?-----END CERTIFICATE-----)/m)
+                               .drop(1)
+                               .map {|cstr| OpenSSL::X509::Certificate.new(cstr[0])}
+
+        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