From: Ingo Schwarze Subject: Re: Suppressing anoncvs/gotd security(8) warnings To: Stefan Sperling Cc: Jon Higgs , Daniel Jakots , tech@openbsd.org Date: Sat, 29 Mar 2025 14:14:53 +0100 [ I moved this from misc@ to tech@ because there is now a patch, and people should be able to see security(8) patches before commit. ] The issue at hand is that libexec/security contains a somewhat ugly pedestrian exception allowing an anoncvs account without a password but there is no easy, non-obfuscated way to do the same for gotd(1). Even though the "game of trees" project is not yet in base, i consider it important enough that throwing wrenches into it feels at least unfortunate. Stefan Sperling wrote on Fri, Mar 28, 2025 at 03:54:05PM +0100: > On Thu, Mar 27, 2025 at 12:14:39PM +0100, Ingo Schwarze wrote: >> I think the proper order of actions is: >> >> 1. stsp@ deciding on the recommendation >> and documenting and testing it. >> 2. Either of us drafting a patch to security >> and all three of us testing it. >> 3. stsp@ committing the got(1) patch(es), either of us committing >> the security(8) patch. >> 4. stsp@ making a got(1) release and updating the port. >> 5. You running the updated port in production. >> >> Thoughts? > I don't see a need for such strong conventions. > > The security script just issues warnings after the fact. If a mistake is > made during setup, such that unauthorized shell access is possible without > a password, then by the time the security script gets to run and sends > mail and the admin gets to read it and starts to act upon it, it is already > too late to do anything about the problem. The system should be assumed > compromised and will need to be decommissioned. Arguably, yes. The point of security(8) is to make it more likely for some mistakes to be found, and that always happens after the fact. Decding *how* the admin should fix the problem is out of the scope of security(8). In extreme cases, reinstalling from scratch may indeed be the safest option. > So why can the username and shell not be a simple configurable knob, like > SKIPSUID already is, to suppress this warning for a specific account the > admin is already very much aware of? I think knobs ought to be added sparingly, we don't want proliferation to get out of hand. Then again, here we already have somewhat ugly code that could be made more systematic, and there is no good reason to pamper anoncvs but punish gotd. > As it is right now, with no way to disable this report short of editing > the script, I just keep deleting all daily security mail from my gotd > servers unread, assuming I know what it is warning me about yet again. If a gap in the feature set is annoying developers so much that they can no longer profit from the whole set of features containing the gap, i think that's an argument to do something about it. What do you think about the following patch? Yours, Ingo Index: etc/daily =================================================================== RCS file: /cvs/src/etc/daily,v diff -u -r1.100 daily --- etc/daily 4 Jul 2024 05:06:58 -0000 1.100 +++ etc/daily 29 Mar 2025 13:06:27 -0000 @@ -173,7 +173,7 @@ install -o 0 -g 0 -m 600 -b /dev/null $MAINOUT start_part "Running security(8):" -export SUIDSKIP +export PASSWDSKIP SUIDSKIP /usr/libexec/security end_part rm -f $PARTOUT Index: libexec/security/security =================================================================== RCS file: /cvs/src/libexec/security/security,v diff -u -r1.47 security --- libexec/security/security 9 Mar 2025 20:10:17 -0000 1.47 +++ libexec/security/security 29 Mar 2025 13:06:28 -0000 @@ -75,7 +75,9 @@ my $filename = '/etc/master.passwd'; $check_title = "Checking the $filename file:"; nag !(open my $fh, '<', $filename), "open: $filename: $!" and return; - my (%logins, %uids); + my (%logins, %uids, %skip); + %skip = map { $_ => 1 } split ' ', $ENV{PASSWDSKIP} + if $ENV{PASSWDSKIP}; while (my $line = <$fh>) { chomp $line; nag $line !~ /\S/, @@ -96,8 +98,7 @@ } nag length $name > 31, "Login $name has more than 31 characters."; - nag $pwd eq '' && !($name eq 'anoncvs' && - $shell =~ /\/anoncvssh$/), + nag $pwd eq '' && !$skip{"$name:$shell"}, "Login $name has no password."; if ($pwd ne '' && $pwd ne 'skey' && Index: share/man/man8/security.8 =================================================================== RCS file: /cvs/src/share/man/man8/security.8,v diff -u -r1.27 security.8 --- share/man/man8/security.8 24 Dec 2024 17:08:50 -0000 1.27 +++ share/man/man8/security.8 29 Mar 2025 13:06:28 -0000 @@ -113,7 +113,18 @@ The following variables can be set in .Pa /etc/daily.local : .Pp -.Bl -tag -width "SUIDSKIP" -compact +.Bl -tag -width "PASSWDSKIP" -compact +.It Ev PASSWDSKIP +A whitespace-separated list of +.Ar name : Ns Ar shell +pairs allowed to have empty passwords. +For example, a machine running both CVS and gotd for anonymous access +might set: +.Bd -literal -offset indent +PASSWDSKIP="anoncvs:/usr/local/bin/anoncvssh + anonymous:/usr/local/bin/gotsh" +.Ed +.Pp .It Ev SUIDSKIP A whitespace-separated list of absolute paths to be skipped in setuid/setgid file checks and in device special file checks.