Does this solve the problem that a puppetmaster cannot run puppetd on it
self when using CRL with apache?
and maybe on a similar matter, does it make sense to revoke the certificate
each time you do a puppetca --clean ?

thanks,
Ohad

On Sat, Jul 4, 2009 at 5:23 AM, Brice Figureau <
[email protected]> wrote:

>
> Hi,
>
> I'm depiling some of the easy tickets in my redmine page. This one was
> really
> easier than I first thought when it was assigned to me.
> So I think it can make it to 0.25, it's low risk and fixes a ssl bug.
>
> Available in my github repository, branch tickets/master/2082
>
> Brice
>
> Original commit msg:
> This patch does two things:
>  * it enhance puppetca to list revoked certificates (prefixed by -)
>  * it fixes the ca crl verification which was broken
>
> Signed-off-by: Brice Figureau <[email protected]>
> ---
>  lib/puppet/ssl/certificate_authority.rb           |    1 +
>  lib/puppet/ssl/certificate_authority/interface.rb |   10 +++++++++-
>  sbin/puppetca                                     |    3 ++-
>  spec/integration/ssl/certificate_authority.rb     |    8 +++-----
>  spec/unit/ssl/certificate_authority.rb            |    8 +++++++-
>  spec/unit/ssl/certificate_authority/interface.rb  |    6 +++++-
>  6 files changed, 27 insertions(+), 9 deletions(-)
>
> diff --git a/lib/puppet/ssl/certificate_authority.rb
> b/lib/puppet/ssl/certificate_authority.rb
> index 4a7d461..06f1d00 100644
> --- a/lib/puppet/ssl/certificate_authority.rb
> +++ b/lib/puppet/ssl/certificate_authority.rb
> @@ -276,6 +276,7 @@ class Puppet::SSL::CertificateAuthority
>         store.add_file Puppet[:cacert]
>         store.add_crl crl.content if self.crl
>         store.purpose = OpenSSL::X509::PURPOSE_SSL_CLIENT
> +        store.flags =
> OpenSSL::X509::V_FLAG_CRL_CHECK_ALL|OpenSSL::X509::V_FLAG_CRL_CHECK
>
>         unless store.verify(cert.content)
>             raise "Certificate for %s failed verification" % name
> diff --git a/lib/puppet/ssl/certificate_authority/interface.rb
> b/lib/puppet/ssl/certificate_authority/interface.rb
> index e455295..bf41d94 100644
> --- a/lib/puppet/ssl/certificate_authority/interface.rb
> +++ b/lib/puppet/ssl/certificate_authority/interface.rb
> @@ -60,8 +60,16 @@ class Puppet::SSL::CertificateAuthority::Interface
>         end
>
>         hosts.uniq.sort.each do |host|
> -            if signed.include?(host)
> +            revoked = false
> +            begin
> +                ca.verify(host) unless requests.include?(host)
> +            rescue
> +                revoked = true
> +            end
> +            if not revoked and signed.include?(host)
>                 puts "+ " + host
> +            elsif revoked
> +                puts "- " + host
>             else
>                 puts host
>             end
> diff --git a/sbin/puppetca b/sbin/puppetca
> index 572f72c..51826d9 100755
> --- a/sbin/puppetca
> +++ b/sbin/puppetca
> @@ -55,7 +55,8 @@
>  #
>  # list::
>  #   List outstanding certificate requests.  If '--all' is specified,
> -#   signed certificates are also listed, prefixed by '+'.
> +#   signed certificates are also listed, prefixed by '+', and revoked
> +#   certificates are prefixed by '-'.
>  #
>  # print::
>  #   Print the full-text version of a host's certificate.
> diff --git a/spec/integration/ssl/certificate_authority.rb
> b/spec/integration/ssl/certificate_authority.rb
> index 5f963f7..553c9b3 100755
> --- a/spec/integration/ssl/certificate_authority.rb
> +++ b/spec/integration/ssl/certificate_authority.rb
> @@ -50,13 +50,11 @@ describe Puppet::SSL::CertificateAuthority do
>     end
>
>     it "should be able to revoke a host certificate" do
> -        pending("This test doesn't actually work yet") do
> -            @ca.generate("newhost")
> +        @ca.generate("newhost")
>
> -            @ca.revoke("newhost")
> +        @ca.revoke("newhost")
>
> -            lambda { @ca.verify("newhost") }.should raise_error
> -        end
> +        lambda { @ca.verify("newhost") }.should raise_error
>     end
>
>     it "should have a CRL" do
> diff --git a/spec/unit/ssl/certificate_authority.rb
> b/spec/unit/ssl/certificate_authority.rb
> index 4c2466d..8011430 100755
> --- a/spec/unit/ssl/certificate_authority.rb
> +++ b/spec/unit/ssl/certificate_authority.rb
> @@ -585,7 +585,7 @@ describe Puppet::SSL::CertificateAuthority do
>
>         describe "and verifying certificates" do
>             before do
> -                @store = stub 'store', :verify => true, :add_file => nil,
> :purpose= => nil, :add_crl => true
> +                @store = stub 'store', :verify => true, :add_file => nil,
> :purpose= => nil, :add_crl => true, :flags= => nil
>
>                 OpenSSL::X509::Store.stubs(:new).returns @store
>
> @@ -631,6 +631,12 @@ describe Puppet::SSL::CertificateAuthority do
>                 @ca.verify("me")
>             end
>
> +            it "should set the store flags to check the crl" do
> +                @store.expects(:flags=).with
> OpenSSL::X509::V_FLAG_CRL_CHECK_ALL|OpenSSL::X509::V_FLAG_CRL_CHECK
> +
> +                @ca.verify("me")
> +            end
> +
>             it "should use the store to verify the certificate" do
>                 @cert.expects(:content).returns "mycert"
>
> diff --git a/spec/unit/ssl/certificate_authority/interface.rb
> b/spec/unit/ssl/certificate_authority/interface.rb
> index 784c6cf..c615544 100755
> --- a/spec/unit/ssl/certificate_authority/interface.rb
> +++ b/spec/unit/ssl/certificate_authority/interface.rb
> @@ -176,6 +176,7 @@ describe Puppet::SSL::CertificateAuthority::Interface
> do
>             describe "and an empty array was provided" do
>                 it "should print a string containing all certificate
> requests" do
>                     @ca.expects(:waiting?).returns %w{host1 host2}
> +                    @ca.stubs(:verify)
>
>                     @applier = @class.new(:list, [])
>
> @@ -189,12 +190,14 @@ describe Puppet::SSL::CertificateAuthority::Interface
> do
>                 it "should print a string containing all certificate
> requests and certificates" do
>                     @ca.expects(:waiting?).returns %w{host1 host2}
>                     @ca.expects(:list).returns %w{host3 host4}
> +                    @ca.stubs(:verify)
> +                    @ca.expects(:verify).with("host3").raises
>
>                     @applier = @class.new(:list, :all)
>
>                     @applier.expects(:puts).with "host1"
>                     @applier.expects(:puts).with "host2"
> -                    @applier.expects(:puts).with "+ host3"
> +                    @applier.expects(:puts).with "- host3"
>                     @applier.expects(:puts).with "+ host4"
>
>                     @applier.apply(@ca)
> @@ -205,6 +208,7 @@ describe Puppet::SSL::CertificateAuthority::Interface
> do
>                 it "should print a string of all named hosts that have a
> waiting request" do
>                     @ca.expects(:waiting?).returns %w{host1 host2}
>                     @ca.expects(:list).returns %w{host3 host4}
> +                    @ca.stubs(:verify)
>
>                     @applier = @class.new(:list, %w{host1 host2 host3
> host4})
>
> --
> 1.6.0.2
>
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to 
[email protected]
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to