From: Herbert Bärschneider Subject: Re: updating adduser logging To: Stuart Henderson Cc: Omar Polo , "tech@openbsd.org" Date: Sun, 27 Oct 2024 17:03:09 +0000 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 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.