* Luke Kanies <[email protected]> [090802 17:12]: > > I'm a bit lost on which patches are actually going to be merged; I > thought this was separate threads or something.
Sorry for that; while preparing this patch I discovered that the trivial fix which already got reviewed never got merged. James suggested I resend both. > +1, with one comment below. > > On Aug 2, 2009, at 6:55 AM, Christian Hofstaedtler wrote: > > > > > From: Christian Hofstaedtler <[email protected]> > > > > Fix #2386, by checking either Request.env or ENV for the SSL > > environment > > variables. This is necessary as Passenger 2.2.3 changed the location > > of > > these vars, even though the Rack spec says nothing about ENV or these > > variables. > > --- > > ext/rack/README | 11 +++++++---- > > lib/puppet/network/http/rack/httphandler.rb | 22 ++++++++++++++++++ > > ++++ > > lib/puppet/network/http/rack/rest.rb | 8 ++++---- > > lib/puppet/network/http/rack/xmlrpc.rb | 8 ++++---- > > 4 files changed, 37 insertions(+), 12 deletions(-) > > > > diff --git a/ext/rack/README b/ext/rack/README > > index 63b8fde..3bdcca5 100644 > > --- a/ext/rack/README > > +++ b/ext/rack/README > > @@ -39,11 +39,11 @@ rackup is part of the rack gem. Make sure it's > > in your path. > > Apache with Passenger (aka mod_rails) > > ------------------------------------- > > > > -Make sure puppetmasterd ran at least once, so the SSL certificates > > +Make sure puppetmasterd ran at least once, so the CA & SSL > > certificates > > got set up. > > > > Requirements: > > - Passenger version 2.2.2 or newer [1] > > + Passenger version 2.2.2 or newer*** > > Rack version 1.0.0 > > Apache 2.x > > SSL Module loaded > > @@ -65,6 +65,9 @@ instead an implicit setuid will be done, to the > > user whom owns > > config.ru. Therefore, config.ru shall be owned by the puppet user. > > > > > > -[1] http://www.modrails.com/install.html > > - > > +*** Important note about Passenger versions: > > + 2.2.2 is known to work. > > + 2.2.3-2.2.4 are known to *NOT* work. > > + 2.2.5 (when it is released) is expected to work properly again. > > + Passenger installation doc: http://www.modrails.com/install.html > > > > diff --git a/lib/puppet/network/http/rack/httphandler.rb b/lib/ > > puppet/network/http/rack/httphandler.rb > > index e142068..96cd09a 100644 > > --- a/lib/puppet/network/http/rack/httphandler.rb > > +++ b/lib/puppet/network/http/rack/httphandler.rb > > @@ -12,5 +12,27 @@ class Puppet::Network::HTTP::RackHttpHandler > > raise NotImplementedError, "Your RackHttpHandler subclass is > > supposed to override service(request)" > > end > > > > + protected > > + > > + # Older Passenger versions passed all Environment vars in > > app(env), > > + # but since 2.2.3 they are really in ENV. > > + # Mongrel, etc. may also still use request.env. > > + def ssl_client_header(request) > > + _env_or_request_env(Puppet[:ssl_client_header], request) > > + end > > + > > + def ssl_client_verify_header(request) > > + _env_or_request_env(Puppet[:ssl_client_verify_header], > > request) > > + end > > + > > + def _env_or_request_env(var, request) > > This is counter to existing style - I think we have almost none, if > any, methods starting with '_'. What's the point of doing so? Should > it mean something specific to me? I've fixed this, by dropping the starting '_'; I guess this is (bad) habit carried over from some Python work I was doing recently. I've also dropped 'protected' and the debug messages below in my updated branch. > > > > + if ENV.include?(var) > > + Puppet.debug "rack: using var '%s' from ENV, value: > > '%s'" % [var, ENV[var]] > > + ENV[var] > > + else > > + Puppet.debug "rack: using var '%s' from request.env, > > value: '%s'" % [var, request.env[var]] > > + request.env[var] > > + end > > + end > > end > > > > diff --git a/lib/puppet/network/http/rack/rest.rb b/lib/puppet/ > > network/http/rack/rest.rb > > index 1047512..bdca651 100644 > > --- a/lib/puppet/network/http/rack/rest.rb > > +++ b/lib/puppet/network/http/rack/rest.rb > > @@ -63,11 +63,11 @@ class Puppet::Network::HTTP::RackREST < > > Puppet::Network::HTTP::RackHttpHandler > > result[:ip] = request.ip > > > > # if we find SSL info in the headers, use them to get a > > hostname. > > - # try this with :ssl_client_header, which defaults should > > work for > > - # Apache with StdEnvVars. > > - if dn = request.env[Puppet[:ssl_client_header]] and > > dn_matchdata = dn.match(/^.*?CN\s*=\s*(.*)/) > > + # try this with :ssl_client_header. > > + # For Apache you need special configuration, see ext/rack/ > > README. > > + if dn = ssl_client_header(request) and dn_matchdata = > > dn.match(/^.*?CN\s*=\s*(.*)/) > > result[:node] = dn_matchdata[1].to_str > > - result[:authenticated] = > > (request.env[Puppet[:ssl_client_verify_header]] == 'SUCCESS') > > + result[:authenticated] = > > (ssl_client_verify_header(request) == 'SUCCESS') > > else > > result[:node] = resolve_node(result) > > result[:authenticated] = false > > diff --git a/lib/puppet/network/http/rack/xmlrpc.rb b/lib/puppet/ > > network/http/rack/xmlrpc.rb > > index 4fc9e82..9d0f486 100644 > > --- a/lib/puppet/network/http/rack/xmlrpc.rb > > +++ b/lib/puppet/network/http/rack/xmlrpc.rb > > @@ -43,11 +43,11 @@ class Puppet::Network::HTTP::RackXMLRPC < > > Puppet::Network::HTTP::RackHttpHandler > > ip = request.ip > > > > # if we find SSL info in the headers, use them to get a > > hostname. > > - # try this with :ssl_client_header, which defaults should > > work for > > - # Apache with StdEnvVars. > > - if dn = request.env[Puppet[:ssl_client_header]] and > > dn_matchdata = dn.match(/^.*?CN\s*=\s*(.*)/) > > + # try this with :ssl_client_header. > > + # For Apache you need special configuration, see ext/rack/ > > README. > > + if dn = ssl_client_header(request) and dn_matchdata = > > dn.match(/^.*?CN\s*=\s*(.*)/) > > node = dn_matchdata[1].to_str > > - authenticated = > > (request.env[Puppet[:ssl_client_verify_header]] == 'SUCCESS') > > + authenticated = (ssl_client_verify_header(request) == > > 'SUCCESS') > > else > > begin > > node = Resolv.getname(ip) > > -- > > 1.5.6.5 > > > > > > > > > > -- > Men never do evil so completely and cheerfully as when they do it from a > religious conviction. --Blaise Pascal > --------------------------------------------------------------------- > Luke Kanies | http://reductivelabs.com | http://madstop.com > > > > -- christian hofstaedtler --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
