The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

Bug #2620

Regex problem in puppetmaster auth.conf

Added by Pete Emerson over 4 years ago. Updated almost 4 years ago.

Status:ClosedStart date:09/09/2009
Priority:NormalDue date:
Assignee:Markus Roberts% Done:

0%

Category:-
Target version:0.25.1
Affected Puppet version:0.25.0 Branch:
Keywords:

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com

This issue is currently not available for export. If you are experiencing the issue described below, please file a new ticket in JIRA. Once a new ticket has been created, please add a link to it that points back to this Redmine ticket.


Description

I started with exactly the auth.conf from here:

http://github.com/reductivelabs/puppet/blob/c2e26b9bb28ebcb8e07822015f99bd6a971b51c8/conf/auth.conf

When I run the puppetmasterd in —no-daemon —debug —trace mode, I see this when the client connects:

info: access[/catalog/([/]+)$]: allowing ‘method’ find info: access[/catalog/([/]+)$]: allowing $1 access info: access[/certificate_revocation_list/ca]: allowing ‘method’ find info: access[/certificate_revocation_list/ca]: allowing * access info: access[/report]: allowing ‘method’ save info: access[/report]: allowing * access info: access[/file]: allowing * access info: access[/certificate/ca]: adding authentication no info: access[/certificate/ca]: allowing ‘method’ find info: access[/certificate/ca]: allowing * access info: access[/certificate/]: adding authentication no info: access[/certificate/]: allowing ‘method’ find info: access[/certificate/]: allowing * access info: access[/certificate_request]: adding authentication no info: access[/certificate_request]: allowing ‘method’ find info: access[/certificate_request]: allowing ‘method’ save info: access[/certificate_request]: allowing * access info: access[/]: adding authentication any info: access[/catalog/([/]+)$]: defaulting to no access for 01.admin.mgmt.nym1 warning: Denying access: Forbidden request: 01.admin.mgmt.nym1(8.12.235.107) access to /catalog/01.admin.mgmt.nym1 [find] authenticated at line 52 /usr/lib/ruby/site_ruby/1.8/puppet/network/rights.rb:79:in fail_on_deny' /usr/lib/ruby/site_ruby/1.8/puppet/network/rest_authconfig.rb:36:inallowed?‘ /usr/lib/ruby/site_ruby/1.8/puppet/network/rest_authorization.rb:21:in check_authorization' /usr/lib/ruby/site_ruby/1.8/puppet/network/http/handler.rb:66:inprocess’ /usr/lib/ruby/site_ruby/1.8/puppet/network/http/webrick/rest.rb:23:in service' /usr/lib/ruby/1.8/webrick/httpserver.rb:104:inservice' /usr/lib/ruby/1.8/webrick/httpserver.rb:65:in run' /usr/lib/ruby/1.8/webrick/server.rb:173:instart_thread' /usr/lib/ruby/1.8/webrick/server.rb:162:in start' /usr/lib/ruby/1.8/webrick/server.rb:162:instart_thread' /usr/lib/ruby/1.8/webrick/server.rb:95:in start' /usr/lib/ruby/1.8/webrick/server.rb:92:ineach' /usr/lib/ruby/1.8/webrick/server.rb:92:in start' /usr/lib/ruby/1.8/webrick/server.rb:23:instart' /usr/lib/ruby/1.8/webrick/server.rb:82:in start' /usr/lib/ruby/site_ruby/1.8/puppet/network/http/webrick.rb:40:inlisten' /usr/lib/ruby/site_ruby/1.8/puppet/network/http/webrick.rb:40:in initialize' /usr/lib/ruby/site_ruby/1.8/puppet/network/http/webrick.rb:40:innew' /usr/lib/ruby/site_ruby/1.8/puppet/network/http/webrick.rb:40:in listen' /usr/lib/ruby/1.8/thread.rb:135:insynchronize' /usr/lib/ruby/site_ruby/1.8/puppet/network/http/webrick.rb:37:in listen' /usr/lib/ruby/site_ruby/1.8/puppet/network/server.rb:131:inlisten' /usr/lib/ruby/site_ruby/1.8/puppet/network/server.rb:146:in start' /usr/lib/ruby/site_ruby/1.8/puppet/daemon.rb:128:instart' /usr/lib/ruby/site_ruby/1.8/puppet/application/puppetmasterd.rb:122:in main' /usr/lib/ruby/site_ruby/1.8/puppet/application.rb:226:insend' /usr/lib/ruby/site_ruby/1.8/puppet/application.rb:226:in run_command' /usr/lib/ruby/site_ruby/1.8/puppet/application.rb:217:inrun' /usr/lib/ruby/site_ruby/1.8/puppet/application.rb:306:in exit_on_fail' /usr/lib/ruby/site_ruby/1.8/puppet/application.rb:217:inrun' /usr/sbin/puppetmasterd:66 err: Forbidden request: 01.admin.mgmt.nym1(8.12.235.107) access to /catalog/01.admin.mgmt.nym1 [find] authenticated at line 52

Lines 51 through 54 of the auth.conf:

allow nodes to retrieve their own catalog (ie their configuration)

path ~ ^/catalog/([/]+)$ method find allow $1

When I change ‘allow $1’ to ‘allow *’, the client is able to connect and it successfully ran my manifest.

If I change my allow line to ‘allow fakesstringhere’, I see this:

info: access[/catalog/([/]+)$]: allowing fakestringhere access

When I change it back to ‘allow $1’:

info: access[/catalog/([/]+)$]: allowing $1 access

It seems like the regex capture of ([/]+) isn’t being stored in $1, and $1 is being used literally instead of substituting in the value from the regex?

In case versions are interesting, I’m using CentOS 5 with the rpms found at http://tmz.fedorapeople.org/repo/puppet/epel/5/x86_64/

puppet-0.25.0-0.4.el5.noarch puppet-server-0.25.0-0.4.el5.noarch ruby-1.8.5-5.el5_3.7.x86_64 ruby-augeas-0.3.0-1.el5.x86_64 ruby-devel-1.8.5-5.el5_3.7.x86_64 rubygems-1.3.1-1.el5.noarch ruby-irb-1.8.5-5.el5_3.7.x86_64 ruby-libs-1.8.5-5.el5_3.7.x86_64 ruby-rdoc-1.8.5-5.el5_3.7.x86_64 ruby-shadow-1.4.1-7.el5.x86_64

ruby gem info (although passenger is out of the mix): fastthread (1.0.7) passenger (2.2.2) rack (1.0.0) rake (0.8.7)


Related issues

Related to Puppet - Bug #2531: opaque strings don't match for catalog retrieval via REST... Closed 08/13/2009
Related to Puppet - Bug #2570: namespaceauth.conf rejects domains that begin with digits Closed 08/25/2009

History

#1 Updated by Markus Roberts over 4 years ago

  • Status changed from Unreviewed to Accepted
  • Target version set to 0.25.1

#2 Updated by Markus Roberts over 4 years ago

  • Assignee set to Markus Roberts

This appears to be very closely related to 2531.

Specifically, I believe the problem is caused by the periods in the domain name. This I suspect results in a comparison between an array of domain name segments (in reverse order, so top-level first) with an array consisting of a single string (the expansion of $1) with internal periods.

On the previous ticket we decided on making the minimal fix that would handle the specific case in question rather than a larger refactoring to handle the general problem. For the point release (0.25.1) policy dictates another minimalistic solution, but I’ll reiterate my support for a full refactoring.

#3 Updated by Pete Emerson over 4 years ago

In an attempt to see if #2531 (which is marked closed) solves this problem, I did:

git clone git://reductivelabs.com/puppet

Applied this patch as talked about in http://projects.reductivelabs.com/issues/2531#note-11 and committed by James Turnbull:

http://projects.reductivelabs.com/projects/puppet/repository/revisions/a35e9bf918db0f6fca45d8b0b002a372cff4f982/diff/lib/puppet/network/authstore.rb

and ran install.rb after wiping out my old installation.

I got the same results: info: access[/catalog/([/]+)$]: allowing $1 access

Pete

#4 Updated by Markus Roberts over 4 years ago

Yeah, that’s what I would expect (though the wording of the message, “allowing $1 access”, isn’t a good diagnostic as that message wasn’t adjusted by the patch).

That patch was intentionally minimalistic, and only dealt with a specific use case. I’m working up a similarly targeted patch for you to try, but the real solution will be to refactor that code.

Should have a sketch of a solution up as a for-testing-only branch in a few minutes.

I’ve temporarily removed the testing branch. It broke other things and was inadequate as a solution to the present ticket. I’ll post another stab when I have one.

#5 Updated by Markus Roberts over 4 years ago

There’s an added complexity due to the initial prefix (01.) being taken as the start of a numeric IP address, so the approach I was considering won’t suffice. I do have a good reproduction case now though, and will post a full patch in the next day or so.

#6 Updated by Brice Figureau over 4 years ago

Markus Roberts wrote:

This appears to be very closely related to 2531.

Specifically, I believe the problem is caused by the periods in the domain name. This I suspect results in a comparison between an array of domain name segments (in reverse order, so top-level first) with an array consisting of a single string (the expansion of $1) with internal periods.

I suspect you are right, but I’m wondering why I didn’t catch this issue when I coded this. The solution IMHO would be to munge_name the pattern after interpolation.

On the previous ticket we decided on making the minimal fix that would handle the specific case in question rather than a larger refactoring to handle the general problem. For the point release (0.25.1) policy dictates another minimalistic solution, but I’ll reiterate my support for a full refactoring.

It’s up to you or Luke, since you’re taking care of this ticket.

#7 Updated by Markus Roberts over 4 years ago

Brice —

Yeah, that was pretty much my initial thought, but munge-after-interpolate and variants didn’t pan out.

The specific case we didn’t catch was where the interpolation contained dots, began and ended with digits, yet was a host name and not an ip address. It’s getting tangled with the numeric ip handling logic somewhere along the way.

I’m testing various ideas, should have a fix out this morning.

— Markus

#8 Updated by Markus Roberts over 4 years ago

  • Status changed from Accepted to Ready For Checkin

Got it, along with a handful of other edge cases. Branch up at http://github.com/MarkusQ/puppet/tree/ticket/master/2620

#9 Updated by Pete Emerson over 4 years ago

I grabbed the branch by doing this:

git clone git://github.com/MarkusQ/puppet.git puppet

and I got the same results, so I think I didn’t grab the right branch?

Pete

#10 Updated by Markus Roberts over 4 years ago

You then need to do a “git checkout ticket/master/2620” in that directory to get 0.25 + the patch; what you got was just the root of my tree, which is (IIRC) 0.25 RC1.

#11 Updated by Pete Emerson over 4 years ago

Gah, forgive my SVN ways, I’m clearly doing something wrong:

[~]# git clone git://github.com/MarkusQ/puppet.git puppet Initialized empty Git repository in /root/puppet/.git/ remote: Counting objects: 51418, done. remote: Compressing objects: 100% (14191/14191), done. remote: Total 51418 (delta 36850), reused 50769 (delta 36287) Receiving objects: 100% (51418/51418), 9.41 MiB | 2676 KiB/s, done. Resolving deltas: 100% (36850/36850), done. [~]# cd puppet [puppet]# git checkout ticket/master/2620 error: pathspec ‘ticket/master/2620’ did not match any file(s) known to git.

#12 Updated by Andrew Shafer over 4 years ago

Pete,

You have to make the local branch first.

try: git checkout -b ticket/master/2620 origin/master/ticket/2620

#13 Updated by Andrew Shafer over 4 years ago

FYI, that will make the new branch and checkout in one command

The format is git checkout -b

In this case you are going to be branching from a remote branch and the default name for a remote that you cloned is ‘origin’

#14 Updated by Pete Emerson over 4 years ago

Nut. I still need hand holding.

[root ~]# git clone git://github.com/MarkusQ/puppet.git puppet
Initialized empty Git repository in /root/puppet/.git/
remote: Counting objects: 51418, done.
remote: Compressing objects: 100% (14191/14191), done.
remote: Total 51418 (delta 36850), reused 50769 (delta 36287)
Receiving objects: 100% (51418/51418), 9.41 MiB | 2658 KiB/s, done.
Resolving deltas: 100% (36850/36850), done.
[root ~]# cd puppet
[root puppet]# ls
autotest  CHANGELOG  COPYING   ext         lib      man       README           README.rst  spec
bin       conf       examples  install.rb  LICENSE  Rakefile  README.queueing  sbin        test
[root puppet]# git checkout -b ticket/master/2620 origin/master/ticket/2620
fatal: git checkout: updating paths is incompatible with switching branches/forcing
Did you intend to checkout 'origin/master/ticket/2620' which can not be resolved as commit?

#15 Updated by James Turnbull over 4 years ago

  • Status changed from Ready For Checkin to Closed

Pushed in commit:7404e31d1ec418e9fdc276e0e619c045567cc00c in branch 0.25.x

Also available in: Atom PDF