Index | Thread | Search

From:
Herbert Bärschneider <herbert.baerschneider@protonmail.com>
Subject:
Re: updating adduser logging
To:
Stuart Henderson <stu@spacehopper.org>
Cc:
Omar Polo <op@omarpolo.com>, "tech@openbsd.org" <tech@openbsd.org>
Date:
Sun, 27 Oct 2024 17:03:09 +0000

Download raw body.

Thread
Hi,

I think duplicate logging is not necessary.
Someone relying on this logging is a valid concern, but I see harmonizing the logging into syslog as more important. 
Also, I suspect anyone thorough enough to monitor the logfile of adduser would also monitor syslog in general (as it covers way more information), effectively retaining the information - just in a different logfile.

Kind Regards

On Thursday, October 24th, 2024 at 12:20 PM, Stuart Henderson <stu@spacehopper.org> wrote:

> On 2024/10/23 19:42, Herbert Bärschneider wrote:
> 
> > Hi,
> > 
> > following a unified diff
> > 
> > diff --git adduser.8 adduser.8
> > index 639f7458b2b..84a90d7e9e7 100644
> > --- adduser.8
> > +++ adduser.8
> > @@ -352,9 +352,6 @@ message file for
> > backup of original message file
> > .It Pa /etc/skel
> > skeletal login directory
> > -.It Pa /var/log/adduser
> > -log file for
> > -.Nm
> 
> 
> Someone might be relying on the existing logging.
> 
> Seems to me that, if this is changed at all (I have no opinion on that),
> it should be an addition not a replacement.
> 
> > .El
> > .Sh EXAMPLES
> > Start
> > diff --git adduser.perl adduser.perl
> > index 19e07a04014..3d176188cf6 100644
> > --- adduser.perl
> > +++ adduser.perl
> > @@ -30,6 +30,7 @@
> > 
> > use IPC::Open2;
> > use Fcntl qw(:DEFAULT :flock);
> > +use Sys::Syslog;
> > 
> > ################
> > # main
> > @@ -50,6 +51,8 @@ if (!$check_only && $#batch < 0) {
> > &hints;
> > }
> > 
> > +openlog("adduser", "nofatal,pid", "LOG_USER"); # setup syslog connection
> > +
> > # check
> > $changes = 0;
> > &variable_check; # check for valid variables
> > @@ -81,7 +84,6 @@ sub variables {
> > $message_file = "/etc/adduser.message";
> > $config = "/etc/adduser.conf"; # config file for adduser
> > $config_read = 1; # read config file
> > - $logfile = "/var/log/adduser"; # logfile
> > $home = "/home"; # default HOME
> > $etc_shells = "/etc/shells";
> > $etc_passwd = "/etc/master.passwd";
> > @@ -811,7 +813,7 @@ sub new_users {
> > &new_users_pwdmkdb("$new_entry");
> > &new_users_group_update;
> > &new_users_passwd_update; print "Added user `$name''\\n"; - &adduser_log("$name:*:$u_id:$g_id($group_login):$fullname"); + syslog("LOG_INFO", "new user added: name=$name ($fullname), uid=$u_id, gid=$g_id, group_login=$group_login, home=$home/$name, sh=$sh"); &home_create($name, $group_login); &new_users_sendmessage; } else { @@ -858,7 +860,7 @@ sub batch { &new_users_group_update; &new_users_passwd_update; print "Added user` $name''\n";
> > &sendmessage($name, @message_buffer) if $send_message ne "no";
> > - &adduser_log("$name:*:$u_id:$g_id($group_login):$fullname");
> > + syslog("LOG_INFO", "new user added: name=$name ($fullname), uid=$u_id, gid=$g_id, group_login=$group_login, home=$home/$name, sh=$sh");
> > &home_create($name, $group_login);
> > }
> > 
> > @@ -1092,25 +1094,6 @@ sub create_conf {
> > &config_write(1);
> > }
> > 
> > -# log for new user in /var/log/adduser
> > -sub adduser_log {
> > - local($string) = @;
> > - local($e);
> > -
> > - return 1 if $logfile eq "no";
> > -
> > - local($sec, $min, $hour, $mday, $mon, $year) = localtime;
> > - $year += 1900;
> > - $mon++;
> > -
> > - foreach $e ('sec', 'min', 'hour', 'mday', 'mon') {
> > - # '7' -> '07'
> > - eval "\$$e = 0 . \$$e" if (eval "\$$e" < 10);
> > - }
> > -
> > - &append_file($logfile, "$year/$mon/$mday $hour:$min:$sec $string");
> > -}
> > -
> > # create HOME directory, copy dotfiles from $dotdir to $HOME
> > sub home_create {
> > local($name, $group) = @;
> > @@ -1589,9 +1572,6 @@ message_file = "$message_file"
> > # config file for adduser ("/etc/adduser.conf")
> > config = "$config"
> > 
> > -# logfile ("/var/log/adduser" or "no")
> > -logfile = "$logfile"
> > -
> > # default HOME directory ("/home")
> > home = "$home"
> > 
> > @@ -1651,4 +1631,5 @@ END {
> > close PTMP;
> > unlink($etc_ptmp) || warn "Error: unable to remove $etc_ptmp: $!\nPlease verify that $etc_ptmp no longer exists!\n";
> > }
> > + closelog();
> > }
> > diff --git rmuser.perl rmuser.perl
> > index bda29b86c19..22f1cb63613 100644
> > --- rmuser.perl
> > +++ rmuser.perl
> > @@ -36,6 +36,7 @@
> > # $From: rmuser.perl,v 1.2 1996/12/07 21:25:12 ache Exp $
> > 
> > use Fcntl qw(:DEFAULT :flock);
> > +use Sys::Syslog;
> > 
> > $ENV{"PATH"} = "/bin:/sbin:/usr/bin:/usr/sbin";
> > umask(022);
> > @@ -97,6 +98,7 @@ if ($< != 0) {
> > }
> > 
> > &open_files;
> > +openlog("rmuser", "nofatal,pid", "LOG_USER"); # setup syslog connection
> > 
> > if ($#ARGV == 0) {
> > # Username was given as a parameter
> > @@ -187,6 +189,7 @@ if (-e "$crontab_dir/$login_name") {
> > # Copy master password file to new file less removed user's entry
> > 
> > &update_passwd_file;
> > +syslog("LOG_INFO", "user removed: name=$login_name");
> > 
> > #
> > # Remove the user from all groups in /etc/group
> > @@ -215,6 +218,7 @@ if (-e "$mail_dir/$login_name" || -l "$mail_dir/$login_name") {
> > #
> > # All done!
> > 
> > +closelog();
> > exit 0;
> > 
> > sub get_login_name {
> > 
> > On Tuesday, October 22nd, 2024 at 7:55 PM, Omar Polo op@omarpolo.com wrote:
> > 
> > > Hello,
> > > 
> > > On 2024/10/22 17:11:04 +0000, Herbert Bärschneider herbert.baerschneider@protonmail.com wrote:
> > > 
> > > > PS: I couldn't figure out how to make these patches with CVS, so
> > > > plain diff it is
> > > 
> > > I have no opinion on the contents of the diff, but please send at least
> > > a unified diff (using diff(1) -u flag).
> > > 
> > > For how to generate diffs with CVS, please see this FAQ
> > > https://www.openbsd.org/faq/faq5.html#Diff, but notice that assuming
> > > 
> > > ~/.cvsrc is the default one, just running `cvs diff' in a CVS checkout
> > > will produce a unified diff that you can then attach to a mail.