Issue #3556 has been updated by Markus Roberts.
It appears the problem is in puppet/parser/resource.rb, in override_parameter.rb, specifically at the end (line numbers from 0.25.x): <pre> 355 # Accept a parameter from an override. 356 def override_parameter(param) : : 380 # Merge with previous value, if the parameter was generated with the +> syntax. 381 # It's important that we use the new param instance here, not the old one, 382 # so that the source is registered correctly for later overrides. 383 param.value = [current.value, param.value].flatten if param.add 384 385 set_parameter(param) 386 end </pre> Note that on line 383 we're *modifying* the value of the parameter (name, soruce, value) tupple so that when we apply the override to multiple objects they accumulate the results. The code in question came in with this commit, which also removed a test I don't understand. Commit: commit:"1bf3999ec08f41e35036b303914987e2c0174922" Author: Luke Kanies <[email protected]> Date: Mon Nov 19 16:30:20 2007 -0600 <pre> Fixing a failing test from my fix for #446 -- I had changed the behaviour of Resource#override_parameter unintentionally. I've corrected the comments so it's clear why the original behaviour was there. diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb index ea0f93f..2f7fff1 100644 --- a/lib/puppet/parser/resource.rb +++ b/lib/puppet/parser/resource.rb @@ -403,12 +403,11 @@ class Puppet::Parser::Resource # If we've gotten this far, we're allowed to override. # Merge with previous value, if the parameter was generated with the +> syntax. - if param.add - current.value = [current.value, param.value].flatten - else - # Just replace the existing parameter with this new one. - @params[param.name] = param - end + # It's important that we use the new param instance here, not the old one, + # so that the source is registered correctly for later overrides. + param.value = [current.value, param.value].flatten if param.add + + @params[param.name] = param end # Verify that all passed parameters are valid. This throws an error if </pre> The proper fix is probably to make a new parameter object either here (which would be easy) or in the code that loops through multiple resources (which would be moe efficient). I'm not clear headed enough to code up a fix tonight (cold/allergies); hopefully these notes will be useful if anyone wants to take a stab at it before I am. ---------------------------------------- Bug #3556: plusignment on multiple User's groups merges ALL their groups (security issue IMHO) http://projects.puppetlabs.com/issues/3556 Author: Anthony Caetano Status: Investigating Priority: Urgent Assigned to: Category: group Target version: Affected version: 0.25.4 Keywords: plusignment, security, group, user Branch: A simplified example: I have two virtual users, a sysadmin with wheel and sudo to root access and a pleb user. On a node I want to give both access to the additional group "wwwadm". The sysadmin has groups "wheel, users", the pleb only has "users". If I do this: User["sysadmin", "pleb"] { groups +> "wwwadm" } realize User["sysadmin", "pleb"] Then user pleb gets the "wheel" group! He then belongs to "users, wheel, wwwadm". This is unexpected (for me at least) and has created a bit of a mess as "tidying up" manifests to be more concise has granted a bunch of users in the environment very elevated privileges :-/ I would expect: User["sysadmin", "pleb"] { groups +> "wwwadm" } to be identical to: User["sysadmin"] { groups +> "wwwadm" } User["pleb"] { groups +> "wwwadm" } It isn't. The first form is positively dangerous at present. In the /var/lib/puppet/client_yaml/catalog/$fqdn.yaml for the user I can see that groups are duplicated numerous times in the above example the "users" group appears twice. Example for c&p replication: * The virtual users class class user::virtual { @user { 'sysadmin': groups => ['wheel','users'], ensure => 'present', comment => 'Test User 1', } @user { 'pleb': groups => ['users'], ensure => 'present', comment => 'Test User 2', } } **** First case (the problem) node testing { class user::testing inherits user::virtual { User["sysadmin", "pleb"] { groups +> net } realize User["sysadmin", "pleb"] } include user::testing } .yaml snippet (notice wheel is there and users is duplicated): parameters: :ensure: present :groups: - users - wheel - users - net :comment: Test User 2 :managehome: true reference: !ruby/object:Puppet::Resource::Reference builtin_type: title: pleb type: User * Second case (correct) node testing { class user::testing inherits user::virtual { User["sysadmin"] { groups +> net } User["pleb"] { groups +> net } realize User["sysadmin", "pleb"] } include user::testing } .yaml snippet (as expected): parameters: :ensure: present :groups: - users - net :comment: Test User 2 :managehome: true reference: !ruby/object:Puppet::Resource::Reference builtin_type: title: pleb type: User -- You have received this notification because you have either subscribed to it, or are involved in it. To change your notification preferences, please click here: http://projects.puppetlabs.com/my/account -- You received this message because you are subscribed to the Google Groups "Puppet Bugs" 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-bugs?hl=en.
