On 5/07/09 3:04, Luke Kanies wrote:
> On Jul 3, 2009, at 2:23 PM, Brice Figureau 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.
> 
> Nice.
> 
>> 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
> 
> If you actually understand the CRL stuff and could document it  
> somewhere, that would be just awesome.  I have vague memories of late  
> nights creating and testing and reading up on CRLs, and I came out  
> with what I thought was functional code but no clear idea why it had  
> to be built that way.  Like most non-core parts of SSL, not many  
> people seem to use it and even fewer people talk about it.

It happens I coded a cryptographic library in Java 10 years ago for a 
company I worked for, so I still have some cryptographic knowledge (and 
tons of books).

I also fixed several JRuby-OpenSSL (the java version of MRI OpenSSL), so 
I now know a little bit more about the ruby openssl extension.
My understanding about CRL is that you don't really need a cert store.
You can do: crl.verify(cert) and it's the same way as the with the 
store. But if you use a cert store you need to tell openssl to actually 
check against the CRL which is not the default.

I can maybe document something, the only thing is that I'm not sure what 
is of interest or not. What would you expect this document to have?

>>         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
> 
> Mightn't there be other reasons for the cert not verifying?  Does the  
> exception provide the reason it didn't verify?  If so, it'd be great  
> to have that reason in parens after the hostname.  E.g.,

I re-read the openssl code and the verify method returns only true or 
false but there is an @error and @error_str accessor to get the OpenSSL 
error code.
I'll do some tests to see how we can enhance the model.

> - host.domain.com (Certificate is revoked)
> - other.domain.com (Certificate is expired)
[snipped]

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

Reply via email to