On Sep 18, 2009, at 11:09 AM, Brice Figureau wrote: > > 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?
Was that separate from the main issue? > >> 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? It's actually in certificate/rest.rb, but... I'm of two minds. On the one hand, that does keep the change to a minimum, but on the other hand, it opens us up to exactly this kind of bug in the future. Maybe two patches? One for 0.25.x with the minimal change, and one for master that fixes it for everyone? -- Sometimes I think we're alone. Sometimes I think we're not. In either case, the thought is staggering. --R. Buckminster Fuller --------------------------------------------------------------------- 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 -~----------~----~----~----~------~----~------~--~---
