Must disable /usr/libexec/security on backup disks

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

Must disable /usr/libexec/security on backup disks

Rupert Gallagher
Since /usr/libexec/security runs blindly on every attached storage media, it also runs on mounted tape and backup data volumes. This is stupid.
Reply | Threaded
Open this post in threaded view
|

Re: Must disable /usr/libexec/security on backup disks

Theo de Raadt-2
Rupert Gallagher <[hidden email]> wrote:

> This is stupid.

Your tone is the real stupid.


Reply | Threaded
Open this post in threaded view
|

Re: Must disable /usr/libexec/security on backup disks

Todd C. Miller-3
In reply to this post by Rupert Gallagher
On Sun, 13 Sep 2020 09:17:02 -0000, Rupert Gallagher wrote:

> Since /usr/libexec/security runs blindly on every attached storage media, it
> also runs on mounted tape and backup data volumes.

It might be best to only check file systems listed in /etc/fstab
that don't have noauto in the options field.

 - todd

Reply | Threaded
Open this post in threaded view
|

Re: Must disable /usr/libexec/security on backup disks

Roderick
In reply to this post by Theo de Raadt-2

On Sun, 13 Sep 2020, Theo de Raadt wrote:

> Rupert Gallagher <[hidden email]> wrote:
>
>> This is stupid.
>
> Your tone is the real stupid.

Well, at least it is not diabolic like the infame tritone.

Rod.

Reply | Threaded
Open this post in threaded view
|

Re: Must disable /usr/libexec/security on backup disks

Ingo Schwarze
In reply to this post by Todd C. Miller-3
Hi Todd,

Todd C. Miller wrote on Sun, Sep 13, 2020 at 03:13:04PM -0600:
> On Sun, 13 Sep 2020 09:17:02 -0000, Rupert Gallagher wrote:

>> Since /usr/libexec/security runs blindly on every attached storage
>> media, it also runs on mounted tape and backup data volumes.

> It might be best to only check file systems listed in /etc/fstab
> that don't have noauto in the options field.

I'm not convinced about that.  Filesystems that are not automatically
mounted can serve a wide range of purposes.  Some may still be
mounted often, maybe even most of the time, depending on what they
are used for.  Some such file systems may permit SUID and/or device
files, so not checking them may be a dubious idea.

I don't think the OP raised an actual problem.  There are already
two solutions for it.  First, a backup file system should usually
be mounted, populated, and unmounted quickly rather than remaining
mounted all the time, to minimize the risk of damage to the backup.
Of course you do *not* run the backup at the same time as daily(8),
or even if you run the backup from daily.local(8), then you don't
run it in parallel to security(8), so there usually isn't any problem
in the first place.

Even if, for some weird reason, you want to keep the backup mounted
all the time, there is still no problem.  On some such machines,
checking it regularly for dangerous files might even be useful.
In cases where that is not useful, and more so if it causes problems
of some kind, just use SUIDSKIP as documented in security(8).
Only a human can decide which file systems should usefully be
checked, i don't think there is a reasonable way to guess from
fstab(5) or in some other automated way.

To summarize, i don't see why we should change the code.

Yours,
  Ingo

Reply | Threaded
Open this post in threaded view
|

Re: Must disable /usr/libexec/security on backup disks

Theo de Raadt-2
Ingo Schwarze <[hidden email]> wrote:

> are used for.  Some such file systems may permit SUID and/or device
> files, so not checking them may be a dubious idea.

The script could identify mountpoints with safer mount options and
reduce scanning on them.

That will also encourage admins to use restrictive mount options when
possible.

OTOH, Issues complained about a decade late... are often overblown.

Reply | Threaded
Open this post in threaded view
|

Re: Must disable /usr/libexec/security on backup disks

Ingo Schwarze
Hi Theo,

Theo de Raadt wrote on Mon, Sep 14, 2020 at 04:06:08AM -0600:
> Ingo Schwarze <[hidden email]> wrote:

>> are used for.  Some such file systems may permit SUID and/or device
>> files, so not checking them may be a dubious idea.

> The script could identify mountpoints with safer mount options and
> reduce scanning on them.
>
> That will also encourage admins to use restrictive mount options when
> possible.

I think that is an interesting idea.  That would be the patch below.
Given that the function find_special_files() looks for SUID, SGID,
and device files, i suggest this logic: skip a mount point if any
of the following is true:

 - it does not have the "local" mount option
 - or it has both the "nodev" and the "nosuid" mount options

I don't think explicitly matching the parentheses is needed.
The code below is simpler and possibly even more robust.


There is one minor downside.  Some people will once get mails similar
to the following:

  Setuid deletions:
  -r-sr-xr-x 2 root ... Mar 29 15:58:55 2020 /co/destdir/base/sbin/ping
  -r-sr-xr-x 2 root ... Mar 29 15:58:55 2020 /co/destdir/base/sbin/ping6
  -r-sr-x--- 1 root ... Mar 29 15:58:56 2020 /co/destdir/base/sbin/shutdown
  ...

  Device deletions:
  crw------- 1 ... 79, 0 ... /usr/obj/distrib/amd64/ramdiskA/mr.fs.d/dev/bio
  crw------- 1 ... 23, 0 ... /usr/obj/distrib/amd64/ramdiskA/mr.fs.d/dev/bpf
  ...

Nothing changed on disk, but security(8) now skips some file systems.
Then again, i don't think a one-time mail is a serious problem.


I suspect the "$type" test is obsolete and can be deleted because
i don't think any of the file system types afs, nnpfs, and procfs
are supported nowadays, but since that is unrelated, i'm not proposing
to change that in the same diff.  If people agree that should be
deleted, i'll send a separate diff.


> OTOH, Issues complained about a decade late... are often overblown.

Sure, but when somebody has a smart idea (like the one you just brought
forward), there is nothing wrong with polishing small turds, too.

Opinions, concerns, tests, OKs?
  Ingo


Index: security
===================================================================
RCS file: /cvs/src/libexec/security/security,v
retrieving revision 1.38
diff -u -p -r1.38 security
--- security 27 Dec 2016 09:17:52 -0000 1.38
+++ security 14 Sep 2020 11:13:47 -0000
@@ -540,9 +540,10 @@ sub find_special_files {
     "cannot spawn mount: $!"
     and return;
  while (<$fh>) {
- my ($path, $type) = /\son\s+(.*?)\s+type\s+(\w+)/;
+ my ($path, $type, $opt) = /\son\s+(.*?)\s+type\s+(\w+)(.*)/;
  $skip{$path} = 1 if $path &&
-    ($type =~ /^(?:a|nnp|proc)fs$/ || !/\(.*local.*\)/);
+    ($type =~ /^(?:a|nnp|proc)fs$/ || $opt !~ /local/ ||
+     ($opt =~ /nodev/ && $opt =~ /nosuid/));
  }
  close_or_nag $fh, "mount" or return;
 

Reply | Threaded
Open this post in threaded view
|

Re: Must disable /usr/libexec/security on backup disks

Brian Brombacher


> On Sep 14, 2020, at 7:43 AM, Ingo Schwarze <[hidden email]> wrote:
>
> Hi Theo,
>
> Theo de Raadt wrote on Mon, Sep 14, 2020 at 04:06:08AM -0600:
>> Ingo Schwarze <[hidden email]> wrote:
>
>>> are used for.  Some such file systems may permit SUID and/or device
>>> files, so not checking them may be a dubious idea.
>
>> The script could identify mountpoints with safer mount options and
>> reduce scanning on them.
>>
>> That will also encourage admins to use restrictive mount options when
>> possible.
>
> I think that is an interesting idea.  That would be the patch below.
> Given that the function find_special_files() looks for SUID, SGID,
> and device files, i suggest this logic: skip a mount point if any
> of the following is true:
>
> - it does not have the "local" mount option
> - or it has both the "nodev" and the "nosuid" mount options
>
> I don't think explicitly matching the parentheses is needed.
> The code below is simpler and possibly even more robust.
>
>
> There is one minor downside.  Some people will once get mails similar
> to the following:
>
>  Setuid deletions:
>  -r-sr-xr-x 2 root ... Mar 29 15:58:55 2020 /co/destdir/base/sbin/ping
>  -r-sr-xr-x 2 root ... Mar 29 15:58:55 2020 /co/destdir/base/sbin/ping6
>  -r-sr-x--- 1 root ... Mar 29 15:58:56 2020 /co/destdir/base/sbin/shutdown
>  ...
>
>  Device deletions:
>  crw------- 1 ... 79, 0 ... /usr/obj/distrib/amd64/ramdiskA/mr.fs.d/dev/bio
>  crw------- 1 ... 23, 0 ... /usr/obj/distrib/amd64/ramdiskA/mr.fs.d/dev/bpf
>  ...
>
> Nothing changed on disk, but security(8) now skips some file systems.
> Then again, i don't think a one-time mail is a serious problem.
>
>
> I suspect the "$type" test is obsolete and can be deleted because
> i don't think any of the file system types afs, nnpfs, and procfs
> are supported nowadays, but since that is unrelated, i'm not proposing
> to change that in the same diff.  If people agree that should be
> deleted, i'll send a separate diff.
>
>
>> OTOH, Issues complained about a decade late... are often overblown.
>
> Sure, but when somebody has a smart idea (like the one you just brought
> forward), there is nothing wrong with polishing small turds, too.
>
> Opinions, concerns, tests, OKs?

Love the idea; however, the only drawback is if some Bad Person is twiddling around and leaves a suid or dev around on a file system that is nosuid or nodev, you lose visibility.

Then again, they own the box... so it’s not really helpful catching the real Predators.

Maybe an option to always scan regardless of fs options?

>  Ingo
>
>
> Index: security
> ===================================================================
> RCS file: /cvs/src/libexec/security/security,v
> retrieving revision 1.38
> diff -u -p -r1.38 security
> --- security    27 Dec 2016 09:17:52 -0000    1.38
> +++ security    14 Sep 2020 11:13:47 -0000
> @@ -540,9 +540,10 @@ sub find_special_files {
>        "cannot spawn mount: $!"
>        and return;
>    while (<$fh>) {
> -        my ($path, $type) = /\son\s+(.*?)\s+type\s+(\w+)/;
> +        my ($path, $type, $opt) = /\son\s+(.*?)\s+type\s+(\w+)(.*)/;
>        $skip{$path} = 1 if $path &&
> -            ($type =~ /^(?:a|nnp|proc)fs$/ || !/\(.*local.*\)/);
> +            ($type =~ /^(?:a|nnp|proc)fs$/ || $opt !~ /local/ ||
> +             ($opt =~ /nodev/ && $opt =~ /nosuid/));
>    }
>    close_or_nag $fh, "mount" or return;
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Must disable /usr/libexec/security on backup disks

Ingo Schwarze
Hi Brian,

Brian Brombacher wrote on Mon, Sep 14, 2020 at 07:55:11AM -0400:

> Love the idea; however, the only drawback is if some Bad Person
> is twiddling around and leaves a suid or dev around on a file system
> that is nosuid or nodev, you lose visibility.

Doesn't look like a problem to me; that such bits and files are
ignored on file systems with these mount options is the whole point
of these options.  So AFAICT, such files are not special in such
places and hence visibility is not really useful.

> Maybe an option to always scan regardless of fs options?

I dislike options unless there is a really strong need for them.
Why would you want to be notified about SUID files on a nosuid
file system?  What would you want to do about them, and why?

Yours,
  Ingo

Reply | Threaded
Open this post in threaded view
|

Re: Must disable /usr/libexec/security on backup disks

Brian Brombacher


> On Sep 14, 2020, at 8:11 AM, Ingo Schwarze <[hidden email]> wrote:
>
> Hi Brian,
>
> Brian Brombacher wrote on Mon, Sep 14, 2020 at 07:55:11AM -0400:
>
>> Love the idea; however, the only drawback is if some Bad Person
>> is twiddling around and leaves a suid or dev around on a file system
>> that is nosuid or nodev, you lose visibility.
>
> Doesn't look like a problem to me; that such bits and files are
> ignored on file systems with these mount options is the whole point
> of these options.  So AFAICT, such files are not special in such
> places and hence visibility is not really useful.
>
>> Maybe an option to always scan regardless of fs options?
>
> I dislike options unless there is a really strong need for them.
> Why would you want to be notified about SUID files on a nosuid
> file system?  What would you want to do about them, and why?
>

I guess I was looking at it from the perspective of defense against attackers.  If some lazy hacker left a file laying around, or they exploited something and were able to create such files but couldn’t take advantage, the visibility would be helpful.

It’s early and my coffee probably hasn’t kicked in ;)

> Yours,
>  Ingo
>

Reply | Threaded
Open this post in threaded view
|

Re: Must disable /usr/libexec/security on backup disks

Todd C. Miller-3
In reply to this post by Ingo Schwarze
On Mon, 14 Sep 2020 13:40:03 +0200, Ingo Schwarze wrote:

> I think that is an interesting idea.  That would be the patch below.
> Given that the function find_special_files() looks for SUID, SGID,
> and device files, i suggest this logic: skip a mount point if any
> of the following is true:
>
>  - it does not have the "local" mount option
>  - or it has both the "nodev" and the "nosuid" mount options
>
> I don't think explicitly matching the parentheses is needed.
> The code below is simpler and possibly even more robust.

I like it.  The other idea I had was to simply declare that mounts
under a certain directory (such as /mnt) would not be checked, but
I think this is a more elegant approach.

 - todd

Reply | Threaded
Open this post in threaded view
|

Re: Must disable /usr/libexec/security on backup disks

Theo de Raadt-2
In reply to this post by Ingo Schwarze
Ingo Schwarze <[hidden email]> wrote:

> Hi Brian,
>
> Brian Brombacher wrote on Mon, Sep 14, 2020 at 07:55:11AM -0400:
>
> > Love the idea; however, the only drawback is if some Bad Person
> > is twiddling around and leaves a suid or dev around on a file system
> > that is nosuid or nodev, you lose visibility.
>
> Doesn't look like a problem to me; that such bits and files are
> ignored on file systems with these mount options is the whole point
> of these options.  So AFAICT, such files are not special in such
> places and hence visibility is not really useful.
>
> > Maybe an option to always scan regardless of fs options?
>
> I dislike options unless there is a really strong need for them.
> Why would you want to be notified about SUID files on a nosuid
> file system?  What would you want to do about them, and why?

I am happy enough with the diff, and also dislike having a flag.

Can we get it commited and revisit the situation in 10 years?

Reply | Threaded
Open this post in threaded view
|

Re: Must disable /usr/libexec/security on backup disks

Ingo Schwarze
Hi Theo,

Theo de Raadt wrote on Mon, Sep 14, 2020 at 07:27:23AM -0600:

> I am happy enough with the diff, and also dislike having a flag.
> Can we get it commited

Done.

> and revisit the situation in 10 years?

I'm sorry, i cannot promise to keep my TODO list in order for ten
years, it often takes less than a month to forget items that should
be on it.

That said, i certainly agree that re-auditing code that hasn't been
looked at for a decade is almost always useful.  Any code.  Provided
that people manage to find sufficient time for reading code.

Yours,
  Ingo