On Jul 5, 2009, at 8:49 AM, Brice Figureau wrote: > > 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?
yeah, I mean the variable name, and no, it's not worth posting a new patch for that. Thanks. -- It is a very sad thing that nowadays there is so little useless information. -- Oscar Wilde --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
