On 18/09/09 20:28, Luke Kanies wrote: > 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?
No, I was seeing exactly #2617 and #2619. I meant in fact: were you able to get locally a scenario matching the one I outlined in my previous e-mail. I guess the answer is yes, based on your last patch. >>> 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... Ah, ok. The above patch is in indirector/rest so for every indirection, hence my question. > 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? Sounds the best to me. It leaves us a whole major release cycle to detect misbehaviour and side effects and at the same time fixes the issue for 0.25.x. -- 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 -~----------~----~----~----~------~----~------~--~---
