Index | Thread | Search

From:
Ingo Schwarze <schwarze@usta.de>
Subject:
Re: security(8): Enhance check_mailboxes to skip directories and quota files
To:
tech@openbsd.org
Cc:
Robert <rmz@hostexpert.pl>
Date:
Thu, 9 Jan 2025 21:46:40 +0100

Download raw body.

Thread
Hi,

this patch makes sense to me: it is very simple and straightforward
and i can see no downside.

Any OKs for the version appended below?
  Ingo


Robert wrote on Fri, Jan 03, 2025 at 03:21:21PM +0100:

> I propose a simple patch to `/usr/libexec/security` to enhance the 
> check_mailboxes function by adding the ability to skip directories and 
> specific quota files (quota.user and quota.group). This change improves 
> the robustness of the function when handling various mail storage 
> configurations.
> 
> Motivation
> 
> 1. Directories in `/var/mail`:
>        - It's not uncommon to encounter directories in `/var/mail`. 
>          These could be:
>           - System-generated directories like `lost+found`.
>           - Chroot directories, e.g., `/var`, containing isolated 
>             environments.
>           - Virtual mailbox structures, such as `Maildir`, also
>             supported by OpenSMTPD for delivering mail.
> 
>        These directories should be ignored, as they do not represent 
>        individual mailboxes and should not interfere with the script's
>        checks.
> 
> 2. Quota files:
>        - Files such as `quota.user` and `quota.group` may be found in 
>          the same directory and are not directly related to user
>          mailboxes.  Including them in the checks could lead to
>          unnecessary warnings or errors.

While /var/mail is not usually a separate filesystem, such a
configuration wouldn't be nonsensical either because mail servers
are often dedicated machines and having a separate file system
to isolate the main user data of a machine isn't a bad idea.

> Proposed patch attached.

One slight change with respect to the original patch:

Since check_mailboxes() does a stat(3p) anyway, move the directory
check after the stat(3p) and use S_ISDIR(3p) rather than doing
a second stat with -d(3p).


Index: security
===================================================================
RCS file: /cvs/src/libexec/security/security,v
diff -u -p -r1.44 security
--- security	24 Dec 2024 17:08:50 -0000	1.44
+++ security	9 Jan 2025 20:29:04 -0000
@@ -456,11 +456,14 @@ sub check_mailboxes {
 	foreach my $name (readdir $dh) {
 		next if $name =~ /^\.\.?$/;
 		next if $name =~ /.\.lock$/;
+		next if $name eq 'quota.user';
+		next if $name eq 'quota.group';
 		my ($mode, $fuid, $fgid) = (stat "$dir/$name")[2,4,5];
 		unless (defined $mode) {
 			nag !$!{ENOENT}, "stat: $dir/$name: $!";
 			next;
 		}
+		next if S_ISDIR($mode);
 		my $fname = (getpwuid $fuid)[0] // $fuid;
 		my $gname = (getgrgid $fgid)[0] // $fgid;
 		nag $fname ne $name,