On 5/07/09 17:09, Luke Kanies wrote: > On Jul 5, 2009, at 7:19 AM, Brice Figureau wrote: > >> 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 | 11 ++++++++++- >> lib/puppet/ssl/certificate_authority/interface.rb | 10 +++++++++- >> sbin/puppetca | 4 +++- >> 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, 37 insertions(+), 10 deletions(-) >> >> diff --git a/lib/puppet/ssl/certificate_authority.rb b/lib/puppet/ >> ssl/certificate_authority.rb >> index 4a7d461..10d13c2 100644 >> --- a/lib/puppet/ssl/certificate_authority.rb >> +++ b/lib/puppet/ssl/certificate_authority.rb >> @@ -17,6 +17,14 @@ class Puppet::SSL::CertificateAuthority >> >> require 'puppet/ssl/certificate_authority/interface' >> >> + class CertificateVerificationError < RuntimeError >> + attr_accessor :error_code >> + >> + def initialize(code) >> + @error_code = code >> + end >> + end >> + >> class << self >> include Puppet::Util::Cacher >> >> @@ -276,9 +284,10 @@ 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 >> + raise CertificateVerificationError.new(store.error), >> store.error_string >> end >> end > > Very nice. > >> diff --git a/lib/puppet/ssl/certificate_authority/interface.rb b/lib/ >> puppet/ssl/certificate_authority/interface.rb >> index e455295..689ab10 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 >> Puppet::SSL::CertificateAuthority::CertificateVerificationError => >> details >> + revoked = details.to_s >> + end >> + if not revoked and signed.include?(host) >> puts "+ " + host >> + elsif revoked >> + puts "- " + host + " (" + revoked + ")" > > The code looks fine, but the use of 'revoked' here again implies that > the only failure you'd get would be a cert revocation, but (e.g.) I > see time-based cert invalidity all the time.
You mean the variable name? Because its content can be anything from "certificate revoked" to "certificate is not yet valid" or "certificate has expired". OpenSSL defines about hundreds error strings :-) If the variable name really worries you, I'll change it to "failure" or "invalid", or whatever you prefer. For such a small change I won't post the patch again, or shall I? -- Brice Figureau My Blog: http://www.masterzen.fr/ --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
