On Jul 5, 2009, at 3:35 AM, Brice Figureau wrote:
>
> 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?
Essentially, what the CRL is, what you'd expect it to look like, who
needs to have a copy of it, and why you'd use it.
I expect the main confusion is around who needs to have a copy of it
and what the heck it is.
I don't think this is critical, but... the CRL has been far more work
over the long haul than it was to implement in the first place, and it
seems like maybe knowing more about them might have helped there. Of
course, 0.25 will help quite a bit because it can do CRL distribution.
>
>>> 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.
Ah, the code made me think it threw an exception if it failed. If
it's not too much, it would definitely be great if we got a useful
error.
--
One of the Ten Commandments for Technicians:
(7) Work thou not on energized equipment, for if thou dost, thy
fellow workers will surely buy beers for thy widow and
console her in other ways.
---------------------------------------------------------------------
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
-~----------~----~----~----~------~----~------~--~---
