Is 1777 mode for $rundir really needed?
Todd Zullinger
1777 mode for $rundir is needed. As far as I can tell from reading
the code and commit messages, it's used to avoid requiring a puppet
user and group, as well as for the tests?
In the Fedora/EPEL packages, I think we'd be better off avoiding
setting 1777, since we know that we'll have a puppet user/group and
keeping the permissions as tight as we can helps everyone sleep a
little better.
But I'm not really sure I understand all of the reasons and use cases
where 1777 makes sense, so I'd like to learn more before submitting
any real patches. I was thinking of something like this perhaps
(which is based on the 0.24.x branch, just for testing package
patches):
diff --git c/lib/puppet/defaults.rb w/lib/puppet/defaults.rb
index e36dd70..2f6c1ff 100644
--- c/lib/puppet/defaults.rb
+++ w/lib/puppet/defaults.rb
@@ -39,14 +39,25 @@ module Puppet
logopts = ["$vardir/log", "The Puppet log directory."]
end
setdefaults(:main, :logdir => logopts)
-
+
# This name hackery is necessary so that the rundir is set reasonably during
# unit tests.
- if Process.uid == 0 and %w{puppetd puppetmasterd}.include?(self.name)
- rundir = "/var/run/puppet"
+ if Process.uid == 0 and %w{puppetd puppetmasterd}.include?(File.basename($0))
+ runopts = {
+ :default => "/var/run/puppet",
+ :mode => 0755,
+ :owner => "$user",
+ :group => "$group",
+ :desc => "Where Puppet PID files are kept."
+ }
else
- rundir = "$vardir/run"
+ runopts = {
+ :default => "$vardir/run",
+ :mode => 01777,
+ :desc => "Where Puppet PID files are kept."
+ }
end
+ setdefaults(:main, :rundir => runopts)
self.setdefaults(:main,
:trace => [false, "Whether to print stack traces on some errors"],
@@ -66,11 +77,6 @@ module Puppet
:owner => "root",
:desc => "Where SSL certificates are kept."
},
- :rundir => {
- :default => rundir,
- :mode => 01777,
- :desc => "Where Puppet PID files are kept."
- },
:genconfig => [false,
"Whether to just print a configuration to stdout and exit. Only makes
sense when used interactively. Takes into account arguments specified
I ran the spec task with this and didn't notice any failures related
to the changes, but then, I did it on a system where the puppet user
exists (though I ran the spec task as a normal user).
I probably just lack a clue, but I couldn't figure out when self.name
would ever match puppetd or puppetmasterd in the if Process.uid == 0
test. That's why I used the ugly File.basename($0) part.
If anyone can help clue me in on what is wrong with this idea and/or
the specific diff above, I'd appreciate it.
¹ https://bugzilla.redhat.com/show_bug.cgi?id=495096
(puppet SPEC file defines improper modes for some directories)
--
Todd OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Whenever you find yourself on the side of the majority, it is time to
pause and reflect.
-- Mark Twain
Luke Kanies
> Looking over Red Hat bug #495096¹, I'm wondering in what cases the
> 1777 mode for $rundir is needed. As far as I can tell from reading
> the code and commit messages, it's used to avoid requiring a puppet
> user and group, as well as for the tests?
>
> In the Fedora/EPEL packages, I think we'd be better off avoiding
> setting 1777, since we know that we'll have a puppet user/group and
> keeping the permissions as tight as we can helps everyone sleep a
> little better.
>
> But I'm not really sure I understand all of the reasons and use cases
> where 1777 makes sense, so I'd like to learn more before submitting
> any real patches. I was thinking of something like this perhaps
> (which is based on the 0.24.x branch, just for testing package
> patches):
Essentially, yes, it's used to make it so we don't require the
'puppet' user and group on the client.
I know that RHEL et al include them in the package, but e.g., gems
don't have them. I've accidentally done things that require them, and
it's always filed very quickly as a bug, so it's clear that people at
least aren't expecting it.
We could see about changing the requirements so that even the client
need a puppet user and group.
At this point, I'd only consider this if it had a special case for
those platforms or situations where you could guarantee that the user/
group were there. Either that, or have this patch applied by those
packaging systems. Long term, yeah, it makes more sense to see about
making the user/group required.
--
Measure with a micrometer. Mark with chalk. Cut with an axe.
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com
Todd Zullinger
> Essentially, yes, it's used to make it so we don't require the
> 'puppet' user and group on the client.
>
> I know that RHEL et al include them in the package, but e.g., gems
> don't have them. I've accidentally done things that require them,
> and it's always filed very quickly as a bug, so it's clear that
> people at least aren't expecting it.
>
> We could see about changing the requirements so that even the client
> need a puppet user and group.
Alternately, would it be possible and not too much overhead to check
for the existence of $user and $group in defaults.rb and only set 1777
if not present?
(Not that I'm in the camp who would oppose just requiring $user and
$group to exist.)
> At this point, I'd only consider this if it had a special case for
> those platforms or situations where you could guarantee that the
> user/ group were there. Either that, or have this patch applied by
> those packaging systems. Long term, yeah, it makes more sense to
> see about making the user/group required.
It's not a big deal if the Fedora/EPEL packages patch this out, if
that ends up being the best thing to do for now.
Thanks for the explanation. :)
--
Todd OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Never do today that which will become someone else's responsibility
tomorrow.
Luke Kanies
> Luke Kanies wrote:
>> Essentially, yes, it's used to make it so we don't require the
>> 'puppet' user and group on the client.
>>
>> I know that RHEL et al include them in the package, but e.g., gems
>> don't have them. I've accidentally done things that require them,
>> and it's always filed very quickly as a bug, so it's clear that
>> people at least aren't expecting it.
>>
>> We could see about changing the requirements so that even the client
>> need a puppet user and group.
>
> Alternately, would it be possible and not too much overhead to check
> for the existence of $user and $group in defaults.rb and only set 1777
> if not present?
They're always in defaults.rb, they're just not always present on the
machine.
It wouldn't be all that much overhead to check if they're present on
the machine, but it could possibly get a bit self-referential, since
it'd make the most sense to just use a 'user' and 'group' instance. I
expect Settings could relatively easily be extended to tell you
whether the user and group exist, since it knows how to create them
and everything.
>
> (Not that I'm in the camp who would oppose just requiring $user and
> $group to exist.)
>
>> At this point, I'd only consider this if it had a special case for
>> those platforms or situations where you could guarantee that the
>> user/ group were there. Either that, or have this patch applied by
>> those packaging systems. Long term, yeah, it makes more sense to
>> see about making the user/group required.
>
> It's not a big deal if the Fedora/EPEL packages patch this out, if
> that ends up being the best thing to do for now.
>
> Thanks for the explanation. :)
No problem.
--
SCSI is *not* magic. There are fundamental technical reasons
why it is necessary to sacrifice a young goat to your SCSI chain
now and then.
Todd Zullinger
Signed-off-by: Todd Zullinger <t...@pobox.com>
---
Luke Kanies wrote:
>
> On May 31, 2009, at 6:42 PM, Todd Zullinger wrote:
>> Alternately, would it be possible and not too much overhead to check
>> for the existence of $user and $group in defaults.rb and only set 1777
>> if not present?
>
> They're always in defaults.rb, they're just not always present on the
> machine.
Yeah, that's what I meant. ;)
> It wouldn't be all that much overhead to check if they're present on
> the machine, but it could possibly get a bit self-referential, since
> it'd make the most sense to just use a 'user' and 'group' instance. I
> expect Settings could relatively easily be extended to tell you
> whether the user and group exist, since it knows how to create them
> and everything.
Doing that properly is likely not within my Ruby limited skill set.
But perhaps setting a more restrictive mode when running popped or
puppetmasterd as root would be a reasonable step for now? After
looking at the code a bit more, I think 'self.name' should be 'name'
in the line:
if Process.uid == 0 and %w{puppetd puppetmasterd}.include?(self.name)
Otherwise, I've never seen self.name contain anything but Puppet. If
I'm not just missing the point entirely, would making that change and then
setting the rundir mode appropriately in that if/else work well for
the unit tests and folks without a puppet user/group?
lib/puppet/defaults.rb | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index a2ccd8a..9dabbde 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -42,10 +42,12 @@ module Puppet
# This name hackery is necessary so that the rundir is set reasonably during
# unit tests.
- if Process.uid == 0 and %w{puppetd puppetmasterd}.include?(self.name)
+ if Process.uid == 0 and %w{puppetd puppetmasterd}.include?(name)
rundir = "/var/run/puppet"
+ rundirmode = 0755
else
rundir = "$vardir/run"
+ rundirmode = 01777
end
self.setdefaults(:main,
@@ -62,7 +64,7 @@ module Puppet
},
:rundir => {
:default => rundir,
- :mode => 01777,
+ :mode => rundirmode,
:desc => "Where Puppet PID files are kept."
},
:genconfig => [false,
--
1.6.3.2
--
Todd OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Deliberation, n.:
The act of examining one's bread to determine which side it is
buttered on.
-- Ambrose Bierce, "The Devil's Dictionary"
Luke Kanies
>
> Subject: [PATCH/puppet] Avoid world-writable $rundir when possible
>
> Signed-off-by: Todd Zullinger <t...@pobox.com>
> ---
>
> Luke Kanies wrote:
>>
>> On May 31, 2009, at 6:42 PM, Todd Zullinger wrote:
>>> Alternately, would it be possible and not too much overhead to check
>>> for the existence of $user and $group in defaults.rb and only set
>>> 1777
>>> if not present?
>>
>> They're always in defaults.rb, they're just not always present on the
>> machine.
>
> Yeah, that's what I meant. ;)
>
>> It wouldn't be all that much overhead to check if they're present on
>> the machine, but it could possibly get a bit self-referential, since
>> it'd make the most sense to just use a 'user' and 'group'
>> instance. I
>> expect Settings could relatively easily be extended to tell you
>> whether the user and group exist, since it knows how to create them
>> and everything.
>
> Doing that properly is likely not within my Ruby limited skill set.
> But perhaps setting a more restrictive mode when running popped or
> puppetmasterd as root would be a reasonable step for now?
root) warring with puppetd (which always does).
The only real way to do it is to check to see if the puppet user
exists, and behave differently then.
I'd settle for a shell-out hack or something, I guess, but this is
inherently messy and self-referential, like I said.
> After
> looking at the code a bit more, I think 'self.name' should be 'name'
> in the line:
>
> if Process.uid == 0 and %w{puppetd puppetmasterd}.include?(self.name)
>
> Otherwise, I've never seen self.name contain anything but Puppet. If
> I'm not just missing the point entirely, would making that change
> and then
> setting the rundir mode appropriately in that if/else work well for
> the unit tests and folks without a puppet user/group?
same host. If you resolve that, then essentially everything else goes
away.
I went to a restaurant that serves "breakfast at anytime". So I
ordered French Toast during the Renaissance. -- Stephen Wright
Jeroen van Meeuwen (GMail)
> On Jun 6, 2009, at 4:40 PM, Todd Zullinger wrote:
>
In the case of these downstream packages, setting a mode of 0755,
ownership puppet:puppet on the RPM level would suffice for all of our
branches. It enables the puppet user (puppetmasterd) to write (and the
puppet user is available on the system due to packaging), as well as the
root user (puppetd).
I'm very much interested to continue down the road of checking for a
puppet user/group to exist on the machine on which puppet is deployed,
but I'm afraid I'm too unfamiliar with Luke's vision on how things are
supposed to work to come up with a proper patch.
Am I understanding correctly that;
- We can make $vardir/run mode 0755 if the puppet user exists,
- We can make $vardir/run mode 0775 if (only) the puppet group exists,
- puppetd (running as user root) can always write to $vardir/puppet and
so does not require any additional permissions, and
- puppetmasterd requires 01777 when no puppet user or group exists.
Is this the logic we're after? Thanks in advance,
Kind regards,
Jeroen van Meeuwen
-kanarip
Luke Kanies
>
> On 06/10/2009 09:56 PM, Luke Kanies wrote:
>> On Jun 6, 2009, at 4:40 PM, Todd Zullinger wrote:
>>
>
> In the case of these downstream packages, setting a mode of 0755,
> ownership puppet:puppet on the RPM level would suffice for all of our
> branches. It enables the puppet user (puppetmasterd) to write (and the
> puppet user is available on the system due to packaging), as well as
> the
> root user (puppetd).
>
> I'm very much interested to continue down the road of checking for a
> puppet user/group to exist on the machine on which puppet is deployed,
> but I'm afraid I'm too unfamiliar with Luke's vision on how things are
> supposed to work to come up with a proper patch.
It's not so much that I have a vision as that there are just practical
considerations.
>
> Am I understanding correctly that;
>
> - We can make $vardir/run mode 0755 if the puppet user exists,
> - We can make $vardir/run mode 0775 if (only) the puppet group exists,
> - puppetd (running as user root) can always write to $vardir/puppet
> and
> so does not require any additional permissions, and
> - puppetmasterd requires 01777 when no puppet user or group exists.
>
> Is this the logic we're after? Thanks in advance,
Essentially yes, although puppetmasterd essentially requires read/
write access, not 1777 specifically.
--
Silence is a text easy to misread.
-- A. A. Attanasio, 'The Eagle and the Sword'