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 -~----------~----~----~----~------~----~------~--~---
