Support for certificate revocation and checking connections on the server against the CRL

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@1475 980ebf18-57e1-0310-9a29-db15c13687c0

4 files changed, 236 insertions, 58 deletions

diff --git a/lib/puppet/server.rb b/lib/puppet/server.rb
index 107bf83a2..4a949b712 100644
--- a/lib/puppet/server.rb
+++ b/lib/puppet/server.rb
@@ -44,6 +44,22 @@ module Puppet
 
                 @authconfig
             end
+            
+            # Read the CA cert and CRL and populate an OpenSSL::X509::Store
+            # with them, with flags appropriate for checking client 
+            # certificates for revocation
+            def x509store
+                unless File.exist?(Puppet[:cacrl])
+                    raise Puppet::Error, "Could not find CRL"
+                end
+                crl = OpenSSL::X509::CRL.new(File.read(Puppet[:cacrl]))
+                store = OpenSSL::X509::Store.new
+                store.purpose = OpenSSL::X509::PURPOSE_ANY
+                store.flags = OpenSSL::X509::V_FLAG_CRL_CHECK_ALL|OpenSSL::X509::V_FLAG_CRL_CHECK
+                store.add_cert(@cacert)
+                store.add_crl(crl)
+                return store
+            end
 
             def initialize(hash = {})
                 Puppet.info "Starting server for Puppet version %s" % Puppet.version
@@ -97,6 +113,7 @@ module Puppet
                     end
                 end
 
+                hash[:SSLCertificateStore] = x509store
                 hash[:SSLCertificate] = @cert
                 hash[:SSLPrivateKey] = @key
                 hash[:SSLStartImmediately] = true
diff --git a/lib/puppet/sslcertificates/ca.rb b/lib/puppet/sslcertificates/ca.rb
index 72b073378..89b7b183c 100644
--- a/lib/puppet/sslcertificates/ca.rb
+++ b/lib/puppet/sslcertificates/ca.rb
@@ -1,6 +1,6 @@
 class Puppet::SSLCertificates::CA
     Certificate = Puppet::SSLCertificates::Certificate
-    attr_accessor :keyfile, :file, :config, :dir, :cert
+    attr_accessor :keyfile, :file, :config, :dir, :cert, :crl
 
     Puppet.setdefaults(:ca,
         :cadir => {  :default => "$ssldir/ca",
@@ -26,6 +26,12 @@ class Puppet::SSLCertificates::CA
             :group => "$group",
             :desc => "The CA public key."
         },
+        :cacrl => { :default => "$cadir/ca_crl.pem",
+            :owner => "$user",
+            :group => "$group",
+            :mode => 0664,
+            :desc => "The certificate revocation list (CRL) for the CA."
+        },
         :caprivatedir => { :default => "$cadir/private",
             :owner => "$user",
             :group => "$group",
@@ -130,6 +136,7 @@ class Puppet::SSLCertificates::CA
         end
 
         self.getcert
+        init_crl
         unless FileTest.exists?(@config[:serial])
             Puppet.config.write(:serial) do |f|
                 f << "%04X" % 1
@@ -223,7 +230,6 @@ class Puppet::SSLCertificates::CA
         Puppet.config.write(:cacert) do |f|
             f.puts @cert.to_pem
         end
-        @key = cert.key
         return cert
     end
 
@@ -278,34 +284,12 @@ class Puppet::SSLCertificates::CA
             raise Puppet::Error, "CSR sign verification failed"
         end
 
-        # i should probably check key length...
-
-        # read the ca cert in
-        cacert = OpenSSL::X509::Certificate.new(
-            File.read(@config[:cacert])
-        )
-
-        cakey = nil
-        if @config[:password]
-            cakey = OpenSSL::PKey::RSA.new(
-                File.read(@config[:cakey]), @config[:password]
-            )
-        else
-            cakey = OpenSSL::PKey::RSA.new(
-                File.read(@config[:cakey])
-            )
-        end
-
-        unless cacert.check_private_key(cakey)
-            raise Puppet::Error, "CA Certificate is invalid"
-        end
-
         serial = File.read(@config[:serial]).chomp.hex
         newcert = Puppet::SSLCertificates.mkcert(
             :type => :server,
             :name => csr.subject,
             :days => @config[:ca_days],
-            :issuer => cacert,
+            :issuer => @cert,
             :serial => serial,
             :publickey => csr.public_key
         )
@@ -315,11 +299,11 @@ class Puppet::SSLCertificates::CA
             f << "%04X" % (serial + 1)
         end
 
-        newcert.sign(cakey, OpenSSL::Digest::SHA1.new)
+        sign_with_key(newcert)
 
         self.storeclientcert(newcert)
 
-        return [newcert, cacert]
+        return [newcert, @cert]
     end
 
     # Store the client's CSR for later signing.  This is called from
@@ -338,6 +322,20 @@ class Puppet::SSLCertificates::CA
         end
     end
 
+    # Revoke the certificate with serial number SERIAL issued by this
+    # CA. The REASON must be one of the OpenSSL::OCSP::REVOKED_* reasons
+    def revoke(serial, reason = OpenSSL::OCSP::REVOKED_STATUS_KEYCOMPROMISE)
+        time = Time.now
+        revoked = OpenSSL::X509::Revoked.new
+        revoked.serial = serial
+        revoked.time = time
+        enum = OpenSSL::ASN1::Enumerated(reason)
+        ext = OpenSSL::X509::Extension.new("CRLReason", enum)
+        revoked.add_extension(ext)
+        @crl.add_revoked(revoked)
+        store_crl
+    end
+
     # Store the certificate that we generate.
     def storeclientcert(cert)
         host = thing2name(cert)
@@ -352,6 +350,61 @@ class Puppet::SSLCertificates::CA
             f.print cert.to_pem
         end
     end
+
+    private
+    def init_crl
+        if FileTest.exists?(@config[:cacrl])
+            @crl = OpenSSL::X509::CRL.new(
+                File.read(@config[:cacrl])
+            )
+        else
+            # Create new CRL
+            @crl = OpenSSL::X509::CRL.new
+            @crl.issuer = @cert.subject
+            @crl.version = 1
+            store_crl
+            @crl
+        end
+    end
+        
+    def store_crl
+        # Increment the crlNumber
+        e = @crl.extensions.find { |e| e.oid == 'crlNumber' }
+        ext = @crl.extensions.reject { |e| e.oid == 'crlNumber' }
+        crlNum = OpenSSL::ASN1::Integer(e ? e.value.to_i + 1 : 0)
+        ext << OpenSSL::X509::Extension.new("crlNumber", crlNum)
+        @crl.extensions = ext
+
+        # Set last/next update
+        now = Time.now
+        @crl.last_update = now
+        # Keep CRL valid for 5 years
+        @crl.next_update = now + 5 * 365*24*60*60
+
+        sign_with_key(@crl)
+        Puppet.config.write(:cacrl) do |f|
+            f.puts @crl.to_pem
+        end
+    end
+
+    def sign_with_key(signable, digest = OpenSSL::Digest::SHA1.new)
+        cakey = nil
+        if @config[:password]
+            cakey = OpenSSL::PKey::RSA.new(
+                File.read(@config[:cakey]), @config[:password]
+            )
+        else
+            cakey = OpenSSL::PKey::RSA.new(
+                File.read(@config[:cakey])
+            )
+        end
+
+        unless @cert.check_private_key(cakey)
+            raise Puppet::Error, "CA Certificate is invalid"
+        end
+
+        signable.sign(cakey, digest)
+    end
 end
 
 # $Id$
diff --git a/test/certmgr/certmgr.rb b/test/certmgr/certmgr.rb
index 8c88fe4d6..66376fcea 100755
--- a/test/certmgr/certmgr.rb
+++ b/test/certmgr/certmgr.rb
@@ -261,4 +261,59 @@ class TestCertMgr < Test::Unit::TestCase
         }
         assert_nil(cert)
     end
+
+    def test_crl
+        ca = mkCA()
+        h1 = mkSignedCert(ca, "host1.example.com")
+        h2 = mkSignedCert(ca, "host2.example.com")
+        
+        assert(ca.cert.verify(ca.cert.public_key))
+        assert(h1.verify(ca.cert.public_key))
+        assert(h2.verify(ca.cert.public_key))
+
+        crl = ca.crl
+        assert_not_nil(crl)
+        
+        store = mkStore(ca)
+        assert( store.verify(ca.cert))
+        assert( store.verify(h1, [ca.cert]))
+        assert( store.verify(h2, [ca.cert]))
+
+        ca.revoke(h1.serial)
+
+        # Recreate the CA from disk
+        ca = mkCA()
+        store = mkStore(ca)
+        assert( store.verify(ca.cert))
+        assert(!store.verify(h1, [ca.cert]))
+        assert( store.verify(h2, [ca.cert]))
+        
+        ca.revoke(h2.serial)
+        assert_equal(1, ca.crl.extensions.size)
+
+        File::open("/tmp/crl.pem", "w") { |f| f.write(ca.crl.to_pem) }
+        # Recreate the CA from disk
+        ca = mkCA()
+        store = mkStore(ca)
+        assert( store.verify(ca.cert))
+        assert(!store.verify(h1, [ca.cert]))
+        assert(!store.verify(h2, [ca.cert]))
+    end
+
+    def mkSignedCert(ca, host)
+        cert = mkcert(host)
+        assert_nothing_raised {
+            signedcert, cacert = ca.sign(cert.mkcsr)
+            return signedcert
+        }
+    end
+
+    def mkStore(ca)
+        store = OpenSSL::X509::Store.new
+        store.purpose = OpenSSL::X509::PURPOSE_SSL_CLIENT
+        store.flags = OpenSSL::X509::V_FLAG_CRL_CHECK
+        store.add_cert(ca.cert)
+        store.add_crl(ca.crl)
+        store
+    end
 end
diff --git a/test/server/server.rb b/test/server/server.rb
index ea74a0a1c..e279d8440 100644
--- a/test/server/server.rb
+++ b/test/server/server.rb
@@ -24,44 +24,16 @@ class TestServer < Test::Unit::TestCase
     # we have to use fork here, because we apparently can't use threads
     # to talk to other threads
     def test_connect_with_fork
-        server = nil
         Puppet[:autosign] = true
-
-        # create a server just serving status
-        assert_nothing_raised() {
-            server = Puppet::Server.new(
-                :Port => @@port,
-                :Handlers => {
-                    :CA => {}, # so that certs autogenerate
-                    :Status => nil
-                }
-            )
-
-        }
-
-        # and fork
-        serverpid = fork {
-            assert_nothing_raised() {
-                trap(:INT) { server.shutdown }
-                server.start
-            }
-        }
-        @@tmppids << serverpid
+        serverpid, server = mk_status_server
 
         # create a status client, and verify it can talk
-        client = nil
-        assert_nothing_raised() {
-            client = Puppet::Client::StatusClient.new(
-                :Server => "localhost",
-                :Port => @@port
-            )
-        }
-        retval = nil
+        client = mk_status_client
 
+        retval = nil
         assert_nothing_raised() {
             retval = client.status
         }
-
         assert_equal(1, retval)
     end
 
@@ -145,4 +117,85 @@ class TestServer < Test::Unit::TestCase
         }
         assert(FileTest.exists?(server.pidfile), "PID file was not created")
     end
+
+
+    # Test that a client whose cert has been revoked really can't connect
+    def test_certificate_revocation
+        Puppet[:autosign] = true
+
+        serverpid, server = mk_status_server
+
+        client = mk_status_client
+
+        status = nil
+        assert_nothing_raised() {
+            status = client.status
+        }
+        assert_equal(1, status)
+        client.shutdown
+
+        # Revoke the client's cert
+        ca = Puppet::SSLCertificates::CA.new()
+        fqdn = client.fqdn
+        ca.revoke(ca.getclientcert(fqdn)[0].serial)
+
+        # Restart the server
+        @@port += 1
+        Puppet[:autosign] = false
+        kill_and_wait(serverpid, server.pidfile)
+        serverpid, server = mk_status_server
+
+        client = mk_status_client
+        # This time the client should be denied
+        assert_raise(Puppet::NetworkClientError) {
+            client.status
+        }
+    end
+    
+    def mk_status_client
+        client = nil
+        # Otherwise, the client initalization will trip over itself
+        # since elements created in the last run are still around
+        Puppet::Type::allclear
+
+        assert_nothing_raised() {
+            client = Puppet::Client::StatusClient.new(
+                :Server => "localhost",
+                :Port => @@port
+            )
+        }
+        client
+    end
+
+    def mk_status_server
+        server = nil
+        assert_nothing_raised() {
+            server = Puppet::Server.new(
+                :Port => @@port,
+                :Handlers => {
+                    :CA => {}, # so that certs autogenerate
+                    :Status => nil
+                }
+            )
+
+        }
+        pid = fork {
+            assert_nothing_raised() {
+                trap(:INT) { server.shutdown }
+                server.start
+            }
+        }
+        @@tmppids << pid
+        [pid, server]
+    end
+
+    def kill_and_wait(pid, file)
+        %x{kill -INT #{pid} 2>/dev/null}
+        count = 0
+        while count < 30 && File::exist?(file)
+            count += 1
+            sleep(1)
+        end
+        assert(count < 30, "Killing server #{pid} failed")
+    end
 end