[PATCH/puppet 1/1] Fixed #1806 - supplementary groups are not reset

[James Turnbull]

Patch thanks to Till Maas

Signed-off-by: James Turnbull <ja...@lovedthanlost.net>
---
lib/puppet/util.rb | 5 +++--
lib/puppet/util/suidmanager.rb | 12 +++++++++++-
2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index e1e6992..28a2599 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -55,10 +55,11 @@ module Util
end
unless Puppet::Util::SUIDManager.uid == user
begin
+ Puppet::Util::SUIDManager.initgroups(user)
Puppet::Util::SUIDManager.uid = user
Puppet::Util::SUIDManager.euid = user
- rescue
- $stderr.puts "could not change to user %s" % user
+ rescue => detail
+ $stderr.puts "Could not change to user %s: %s" % [user, detail]
exit(74)
end
end
diff --git a/lib/puppet/util/suidmanager.rb b/lib/puppet/util/suidmanager.rb
index c5df0d1..a0a9178 100644
--- a/lib/puppet/util/suidmanager.rb
+++ b/lib/puppet/util/suidmanager.rb
@@ -7,7 +7,7 @@ module Puppet::Util::SUIDManager
extend Forwardable

to_delegate_to_process = [ :euid=, :euid, :egid=, :egid,
- :uid=, :uid, :gid=, :gid ]
+ :uid=, :uid, :gid=, :gid, :groups=, :groups ]

to_delegate_to_process.each do |method|
def_delegator Process, method
@@ -26,13 +26,16 @@ module Puppet::Util::SUIDManager
# We set both because some programs like to drop privs, i.e. bash.
old_uid, old_gid = self.uid, self.gid
old_euid, old_egid = self.euid, self.egid
+ old_groups = self.groups
begin
self.egid = convert_xid :gid, new_gid if new_gid
+ self.initgroups(convert_xid(:uid, new_uid)) if new_uid
self.euid = convert_xid :uid, new_uid if new_uid

yield
ensure
self.euid, self.egid = old_euid, old_egid
+ self.groups = old_groups
end
end
module_function :asuser
@@ -49,6 +52,13 @@ module Puppet::Util::SUIDManager
end
module_function :convert_xid

+ # Initialize supplementary groups
+ def initgroups(user)
+ require 'etc'
+ Process.initgroups(Etc.getpwuid(user).name, Process.gid)
+ end
+
+ module_function :initgroups

def run_and_capture(command, new_uid=nil, new_gid=nil)
output = Puppet::Util.execute(command, :failonfail => false, :uid => new_uid, :gid => new_gid)
--
1.6.0.6

[James Turnbull]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

James Turnbull wrote:
> Patch thanks to Till Maas
>
> Signed-off-by: James Turnbull <ja...@lovedthanlost.net>
> ---
> lib/puppet/util.rb | 5 +++--
> lib/puppet/util/suidmanager.rb | 12 +++++++++++-
> 2 files changed, 14 insertions(+), 3 deletions(-)

Hi - this never got reviewed - comments, etc?

Regards

James Turnbull

- --
Author of:
* Pro Linux System Administration (http://tinyurl.com/linuxadmin)
* Pulling Strings with Puppet (http://tinyurl.com/pupbook)
* Pro Nagios 2.0 (http://tinyurl.com/pronagios)
* Hardening Linux (http://tinyurl.com/hardeninglinux)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEVAwUBSwTFMSFa/lDkFHAyAQIpdwf+KCOLrAWB/6mTD3JqYOFDgr3xaDPBeV1t
3Lzlc7+UKl5dRVot/uVMJA3D1e5d9xiJCuUO9iry6xWUrEti/yAN+hV2uw4V1bf6
UQc2vEO6ArmzXnfHvsKVcwBKGrwios4dYb8Mqut1/fXtk/6/BQrfI2K0DdTjClOF
Q2GA+ylTvhJxI14BALF1r1lNvBStN2rLaSR8BJ7sSJHMa9TKJaZWX0ILSgQh1x8T
z9jswCV/5pRFATZhL205czoyoBivZZ861u9STs8DlswKamYwXhdWMPILLfhB/x6o
OdIkN+hbdJpHGLA5eIkHGM3oBYAPpKYwT3y307bUrOFHFA80J/+P/g==
=sh+c
-----END PGP SIGNATURE-----

[Luke Kanies]

Seems correct, but this whole problem space is messy enough that real-
world testing matters most, I think.

> --~--~---------~--~----~------------~-------~--~----~
> You received this message because you are subscribed to the Google
> Groups "Puppet Developers" group.
> To post to this group, send email to puppe...@googlegroups.com
> To unsubscribe from this group, send email to puppet-dev+...@googlegroups.com
> For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en
> -~----------~----~----~----~------~----~------~--~---
>


--
To have a right to do a thing is not at all the same as to be right
in doing it. -- G. K. Chesterton
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com

[Peter Meier]

> Seems correct, but this whole problem space is messy enough that real-
> world testing matters most, I think.

I tested it and so far it seems to work fine.

cheers pete

[Todd Zullinger]

Luke Kanies wrote:
> Seems correct, but this whole problem space is messy enough that
> real- world testing matters most, I think.

We've had Till's patch in the Fedora and EPEL builds For a few months
now and haven't gotten any bug reports (for what that's worth).

The one caveat might be that we're shipping 0.24.8 still, and I think
there are minor differences to get the patch to apply to 0.25.x, with
the small possibility of introducing a bug there).

I'm happy to add this patch to the unofficial Fedora/EPEL builds I
have and let folks give it some beating.

--
Todd OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Mollison's Bureaucracy Hypothesis:
If an idea can survive a bureaucratic review and be implemented it
wasn't worth doing.

[Luke Kanies]

On Nov 19, 2009, at 10:17 AM, Todd Zullinger wrote:

> Luke Kanies wrote:
>> Seems correct, but this whole problem space is messy enough that
>> real- world testing matters most, I think.
>
> We've had Till's patch in the Fedora and EPEL builds For a few months
> now and haven't gotten any bug reports (for what that's worth).
>
> The one caveat might be that we're shipping 0.24.8 still, and I think
> there are minor differences to get the patch to apply to 0.25.x, with
> the small possibility of introducing a bug there).
>
> I'm happy to add this patch to the unofficial Fedora/EPEL builds I
> have and let folks give it some beating.

With multiple reports of it working, I'm satisfied.

--
People are more violently opposed to fur than leather because it is
safer to harrass rich women than motorcycle gangs.