Hi,
Doug Moss wrote on Mon, May 04, 2020 at 03:01:43PM +0000:
> For OpenBSD 6.6, amd64
> in the daily script to check security:
> /usr/libexec/security
>
> at line 248 for checking if the umask is set:
> my @list = qw(/etc/profile /root/.profile);
>
> shouldn't that be instead:
> my @list = qw(/.profile /root/.profile);
>
> I think /etc/profile does not exist, and /.profile is the default one.
So, here is a diff to also check /.profile.
Some remarks:
1. It is both more robust and simpler to also check existence
and non-emptiness of startup files in check_umask(), such
that check_sh() need not check that at multiple places.
2. I'm not trying to sneak unrelated changes into the diff:
the change in check_root_path() is related. With the diff,
we are testing more files. If we don't say which file an
issue is located in, we may confuse the sysadmin.
3. The PATH and ENV checks in check_sh() have to be done in just
the same way as they are now, for each of the files in turn.
But the umask check is different. It is not sufficient that
the umask is set in /root/.profile *or* in /.profile because
sometimes one gets executed, sometimes the other, so it should
be set in both, or in /etc/profile.
4. The order of the conditions in
nag !(check_umask($filename) || $etcumask)
may seem surprising at first. But it is intentional that
check_umask() is not short-circuited but always called, even
if we already know that $etcumask is set, because in addition
to catching situations where the umask is not set at all,
we also want to catch situations where it is set to dubious
values, even when it is set at multiple places.
5. The function check_csh() suffers from essentially the same issue
as check_sh(): it fails to consider /.cshrc. But the situation
over there is even worse: right now, check_csh() considers the
umask set when it is set in just one of the four files. If
that is merely one of the "login" files, shells that are not
login shells still won't have a umask. The solution may be to
check the value of the umask in all of the five files where it
is set, but to require that it be set at least at these places:
(/root/.cshrc AND /.cshrc) OR /etc/csh.cshrc, removing the
"login" files from the "is it set at all" check.
But this diff is already large enough. Before deciding whether
to tackle check_csh(), too, i'd first like some feedback on the
diff shown below.
I would be more or less comfortable with committing the diff shown
below after unlock, but i worry a bit that committing it now may
not be good timing.
What do you think: Do we want this at all? Do we want it now?
And should i prepare another diff for check_csh()?
Yours,
Ingo
Index: security
===================================================================
RCS file: /cvs/src/libexec/security/security,v
retrieving revision 1.38
diff -U4 -p -r1.38 security
--- security 27 Dec 2016 09:17:52 -0000 1.38
+++ security 5 May 2020 11:14:32 -0000
@@ -185,8 +185,9 @@ sub check_group {
}
sub check_umask {
my ($filename) = @_;
+ -s $filename || return 0;
nag !(open my $fh, '<', $filename), "open: $filename: $!" and return;
my $umaskset;
while (<$fh>) {
next unless /^\s*umask\s+([0-7]+)/;
@@ -209,9 +210,11 @@ sub check_root_path {
nag !(defined $path && $path =~ s/^PATH=[:\s]*//),
"Failed to find PATH in $filename."
and return;
foreach my $dir (split /[:\s]+/, $path) {
- nag $dir eq '.', "The root path includes ." and next;
+ nag $dir eq '.',
+ "The root path in $filename includes ."
+ and next;
next unless -d $dir;
my $mode = (stat(_))[2];
nag $mode & S_IWGRP,
"Root path directory $dir is group writable.";
@@ -244,34 +247,35 @@ sub check_csh {
"\nRoot csh startup files do not set the umask.";
}
sub check_sh {
- my @list = qw(/etc/profile /root/.profile);
- $check_title = "Checking root sh paths, umask values:\n@list";
+ my @list = qw(/etc/profile /root/.profile /.profile);
+ $check_title = "Checking root sh paths:\n@list";
my @env_path;
- my $umaskset = 0;
foreach my $filename (@list) {
next unless -s $filename;
- $umaskset ||= check_umask($filename);
-
nag !(open my $fh, '-|', qw(/bin/sh -c),
". $filename; echo ENV=\$ENV; echo PATH=\$PATH"),
"cannot spawn /bin/sh: $!"
and next;
my @output = <$fh>;
close_or_nag $fh, "sh $filename" or next;
chomp @output;
check_root_path pop @output, $filename;
-
my $env = pop @output;
nag !(defined $env && $env =~ /^ENV=\s*(\S*)/),
"Failed to find ENV in $filename."
and next;
push @env_path, $1 if $1 ne '';
}
- nag !$umaskset,
- "\nRoot sh startup files do not set the umask.";
+ $check_title = "Checking root sh umask values:\n@list";
+ my $etcfile = shift @list;
+ my $etcumask = check_umask($etcfile);
+ foreach my $filename (@list) {
+ nag !(check_umask($filename) || $etcumask),
+ "Neither $etcfile nor $filename sets the umask.";
+ }
return @env_path;
}
sub check_ksh {
