On 18/09/09 19:32, Luke Kanies wrote:
> On Sep 18, 2009, at 9:37 AM, Markus Roberts wrote:
> 
>>> I think I remember the following cause for the bug:
>>  * 0.24 generates a CA whose CN=$ca_fqdn.
>>  * upgrade to 0.25
>>  * 0.25 client connects to master, ask for "ca" cert
>>  * master send cert whose CN=$ca_fqdn
>>  * Clients wants to locally write it as $ca_fqdn.pem, so
>>    thinks it is a normal cert, not a CA.
>>  * Clients can not authenticate the server because
>>    there is no "ca.pem" file.
>>
>> But if Luke found his patch fixes #2617, it might be that I didn't get
>> exactly the issue and the above is wrong.
>>
>> Another possibility would be that our testing was flawed (I don't  
>> think it was, but it's a logical possibility).  If that's the case,  
>> the most obvious difference would be that we were testing on a  
>> single machine--this could conceivably lead to the client seeing  
>> some file that was "on the master" if they shared a confdir, or some  
>> other such mechanism.
>>
>> Probably the easiest way to rule out all these possibilities in one  
>> go is to try it in an environment that was known to fail for you  
>> with 25.0; if it still fails, we haven't fixed the problem, while if  
>> it no longer exhibits the symptoms the chances are good that the  
>> ticket is fixed.
> 
> Ok, I looked into this more closely.  The following code fixes it for  
> me:

Were you able to reproduce the issue I was seeing?

> diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/ 
> rest.rb
> index e1ee89f..ec3f70f 100644
> --- a/lib/puppet/indirector/rest.rb
> +++ b/lib/puppet/indirector/rest.rb
> @@ -66,7 +66,11 @@ class Puppet::Indirector::REST <  
> Puppet::Indirector::Terminus
>       end
> 
>       def find(request)
> -        deserialize network(request).get(indirection2uri(request),  
> headers)
> +        return nil unless result =  
> deserialize(network(request).get(indirection2uri(request), headers))
> +        unless result.name == request.key
> +            result.name = request.key
> +        end
> +        result
>       end
> 
>       def search(request)

Ouch, that's a central change for sure :-)

> In other words, when we find a cert via REST, we're converting it to  
> the PEM-formatted cert, with just the cert name, which may or may not  
> be equal to the name we asked for.  This basically just makes sure  
> that we return an instance whose name matches the requested name,  
> regardless of what's used internally.

Indeed that should fix the issue I saw.

> I think this is actually the right behaviour, because it allows some  
> separation between internals and the envelope (which separation is the  
> problem here).  

Yes, I do think this is right.

> The only question is whether this is the right  
> behaviour for everything, and it's a potentially large change in  
> behaviour, at least in terms of the number of classes it could affect.

I'm afraid the change is a little bit too central, and it is difficult 
to evaluate the side effects.

Wouldn't it be better to just make this change in the 
certifiicate_authority/rest.rb file?
-- 
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