[Adduser-devel] Ditch full paths in favor of locally set PATH?

Stephen Gran sgran at debian.org
Wed Apr 26 19:42:24 UTC 2006


This one time, at band camp, Marc Haber said:
> Hi,
> 
> in reply to #357978, I intend to drop the explicitly listed paths from
> the adduser code. Instead, ENV{"PATH"} will be reset to
> /sbin:/usr/sbin:/usr/local/sbin to allow local diversions while still
> preferring the system binaries.
> 
> Any comments?

I think if the sysadmin really wants to override one of the important
system utilities, it should be allowed.  Indeed, I do this at one site
at work to make passwd talk to LDAP.

This patch looks for the right executable in $PATH, instead of
hardcoding, which feels correct to me.

This patch also does a couple of other things at the same time, but I
can split that out if you prefer.  

It adds vim modelines (yay!)
It replaces system(chmod|chown) with the perl builtins
It changes a few systemcall to &systemcall

For the last one, I'd like to discuss it at some point.  So far, I
have been using the &subroutine (new style) calling convention, since
that seems like what you have been using up to this point.  It has
some advantages, but it also has some disadvantages.  It overrides
prototyping, for one thing, which I generally like to use in my perl.
Using prototyping means, of course, that you have to be very careful
about the order of your subroutine declarations when using strict,
or else you get unnecessary runtime warnings.

But that's a talk for another time, and a lot of code cleanup either
way.  For now, I'm just trying to make it consistent.

I'll try to get the patch right this time :)

steve at spartacus:~/NMU/adduser$ svn up
At revision 562.
steve at spartacus:~/NMU/adduser$ svn diff
Index: deluser
===================================================================
--- deluser     (revision 562)
+++ deluser     (working copy)
@@ -28,8 +28,6 @@
 # See the usage subroutine for explanation about how the program can be called
 ####################

-$ENV{"PATH"} = "/sbin:/bin:/usr/sbin:/usr/bin";
-
 use warnings;
 use strict;
 use Getopt::Long;
@@ -289,13 +287,17 @@
          print "backup_name = $backup_name";
          print $filesfile join("\n", at files);
          $filesfile->close();
-         systemcall("/bin/tar", "-cf", $backup_name, "--files-from", $filesfilename);
-         systemcall("chmod","600", $backup_name);
-         systemcall("chown","root:root", $backup_name);
+         my $tar = &which('tar');
+         &systemcall($tar, "-cf", $backup_name, "--files-from", $filesfilename);
+         chmod 600, $backup_name;
+         my $rootid = 0;
+         chown $rootid, $rootid, $backup_name;
          unlink($filesfilename);
-         if(-e "/usr/bin/bzip2") {
+         my $bzip2 = &which('bzip2', 1);
+         my $gzip = &which('gzip'), 1);
+         if($bzip) {
              systemcall("/usr/bin/bzip2", $backup_name);
-         } elsif(-e "/bin/gzip") {
+         } elsif($gzip) {
              systemcall("/bin/gzip", "--best", $backup_name);
          }
       }
@@ -311,12 +313,14 @@

     if (system("crontab -l $user >/dev/null 2>&1") == 0) {
       # crontab -l returns 1 if there is no crontab
-      systemcall("/usr/bin/crontab -r $user");
+      my $crontab = &which('crontab');
+      &systemcall($crontab, "-r", $user);
       s_print (gtx("Removing crontab\n"));
     }

     s_printf (gtx("Removing user `%s'...\n"),$user);
-    systemcall("/usr/sbin/userdel", $user);
+    my $userdel = &which('userdel');
+    &systemcall($userdel, $user);
     &invalidate_nscd();

     systemcall('/usr/local/sbin/deluser.local', $user, $pw_uid,
@@ -355,7 +359,8 @@
     endpwent;

     s_printf (gtx("Removing group `%s'...\n"),$group);
-    systemcall("/usr/sbin/groupdel",$group);
+    my $groupdel = &which('groupdel');
+    &systemcall($groupdel,$group);
     &invalidate_nscd();
     s_print (gtx("done.\n"));
     exit 0;
@@ -391,7 +396,8 @@

     s_printf (gtx("Removing user `%s' from group `%s'...\n"),$user,$group);
     #systemcall("usermod","-G", join(",", at groups), $user );
-    systemcall('/usr/bin/gpasswd','-M', join(',', at members), $group);
+    my $gpasswd = &which('gpasswd');
+    &systemcall($gpasswd,'-M', join(',', at members), $group);
     &invalidate_nscd();
     s_print (gtx("done.\n"));
 }
@@ -470,4 +476,4 @@
     return(defined getgrnam($exist_group));
 }

-
+# vim:set ai et sts=4 sw=4 tw=0:
Index: adduser
===================================================================
--- adduser     (revision 562)
+++ adduser     (working copy)
@@ -269,7 +269,8 @@

     printf (gtx("Adding group `%s' (%s)...\n"),$new_name,$new_gid) if $verbose;
     &invalidate_nscd("group");
-    &systemcall('/usr/sbin/groupadd', '-g', $new_gid, $new_name);
+    my $addgroup = &which('addgroup');
+    &systemcall($addgroup, '-g', $new_gid, $new_name);
     &invalidate_nscd("group");
     print (gtx("Done.\n")) if $verbose;
     exit 0;
@@ -297,7 +298,8 @@

     printf (gtx("Adding group `%s' (%s)...\n"),$new_name,$new_gid) if $verbose;
     &invalidate_nscd("group");
-    &systemcall('/usr/sbin/groupadd', '-g', $new_gid, $new_name);
+    my $groupadd = &which('groupadd');
+    &systemcall($groupadd, '-g', $new_gid, $new_name);
     &invalidate_nscd("group");
     print (gtx("Done.\n")) if $verbose;
     exit 0;
@@ -325,7 +327,8 @@
     #&systemcall('usermod', '-G',
                #join(",", get_users_groups($existing_user), $existing_group),
                #$existing_user);
-    &systemcall('/usr/bin/gpasswd', '-M',
+    my $passwd = &which('passwd');
+    &systemcall($passwd, '-M',
                join(',', get_group_members($existing_group), $existing_user),
                $existing_group);
     #&systemcall('gpasswd', '-a',$existing_user,$existing_group);
@@ -390,7 +393,8 @@
     if ($make_group_also && !getgrnam($new_name)) {
        printf (gtx("Adding new group `%s' (%s).\n"),$new_name,$new_gid) if $verbose;
        $undogroup = $new_name;
-       &systemcall('/usr/sbin/groupadd', '-g', $new_gid, $new_name);
+       my $groupadd = &which('groupadd');
+       &systemcall($groupadd, '-g', $new_gid, $new_name);
        &invalidate_nscd("group");
     }

@@ -399,18 +403,20 @@
     $home_dir = $special_home || &homedir($new_name, $ingroup_name);
        $shell = $special_shell || '/bin/false';
     $undouser = $new_name;
-    &systemcall('/usr/sbin/useradd', '-d', $home_dir, '-g', $ingroup_name, '-s',
+    my $useradd = &which('useradd');
+    &systemcall($useradd, '-d', $home_dir, '-g', $ingroup_name, '-s',
                $shell, '-u', $new_uid, $new_name);
-    print "/usr/bin/chage -M 99999 $new_name\n" if ($verbose > 1);
+    my $chage = &which('chage');
+    print "$chage -M 99999 $new_name\n" if ($verbose > 1);
     # do _not_ use systemcall() here, since systemcall() dies on
     # non-zero exit code and we need to do special handling here!
-    if (system('/usr/bin/chage', '-M', '99999', $new_name)) {
+    if (&systemcall($chage, '-M', '99999', $new_name)) {
        if( ($?>>8) ne 15 ) {
-           &cleanup("$0: `/usr/bin/chage -M 99999 $new_name' returned error code " . ($?>>8) . ".  Aborting.\n")
+           &cleanup("$0: `$chage -M 99999 $new_name' returned error code " . ($?>>8) . ".  Aborting.\n")
              if ($?>>8);
-           &cleanup("$0: `/usr/bin/chage -M 99999 $new_name' exited from signal " . ($?&255) . ".  Aborting.\n");
+           &cleanup("$0: `$chage -M 99999 $new_name' exited from signal " . ($?&255) . ".  Aborting.\n");
        } else {
-           print (gtx("chage failed with return code 15, shadow not enabled, password aging cannot be set. Continuing.\n"));
+           print (gtx("$chage failed with return code 15, shadow not enabled, password aging cannot be set. Continuing.\n"));
        }
     }
     &invalidate_nscd();
@@ -474,7 +480,8 @@
     if ($make_group_also) {
        printf (gtx("Adding new group `%s' (%s).\n"),$new_name,$new_gid) if $verbose;
        $undogroup = $new_name;
-       &systemcall('/usr/sbin/groupadd', '-g', $new_gid, $new_name);
+       my $groupadd = &which('groupadd');
+       &systemcall($groupadd, '-g', $new_gid, $new_name);
        &invalidate_nscd();
     }

@@ -483,7 +490,8 @@
     $home_dir = $special_home || &homedir($new_name, $ingroup_name);
        $shell = $special_shell || $config{"dshell"};
     $undouser = $new_name;
-    &systemcall('/usr/sbin/useradd', '-d', $home_dir, '-g', $ingroup_name, '-s',
+    my $groupadd = &which('useradd');
+    &systemcall($useradd, '-d', $home_dir, '-g', $ingroup_name, '-s',
                $shell, '-u', $new_uid, $new_name);
     &invalidate_nscd();

@@ -492,7 +500,8 @@
     # useradd without -p has left the account disabled (password string is '!')
     if ($ask_passwd) {
        for (;;) {
-          &systemcall('/usr/bin/passwd', $new_name);
+          my $passwd = &which('passwd');
+          &systemcall($passwd, $new_name);
          my $ok = $? & 128;
          if ($ok != 0) {
            my $noexpr = langinfo(NOEXPR());
@@ -518,7 +527,8 @@
        }
     } else {
        if(!$disabled_login) {
-           &systemcall('/usr/sbin/usermod', '-p', '*', $new_name);
+           my $usermod = &which('usermod');
+           &systemcall($usermod, '-p', '*', $new_name);
        }
     }

@@ -528,7 +538,8 @@
     else {
        my $yesexpr = langinfo(YESEXPR());
        for (;;) {
-           &systemcall('/usr/bin/chfn', $new_name);
+           my $chfn = &which('chfn');
+           &systemcall($chfn, $new_name);
            # Translators: [y/N] has to be replaced by values defined in your
            # locale.  You can see by running "locale yesexpr" which regular
            # expression will be checked to find positive answer.
@@ -555,7 +566,8 @@
             printf gtx("Adding user `%s' to group `%s'...\n"),$new_name,$newgrp
                 if $verbose;
             &invalidate_nscd();
-            &systemcall('/usr/bin/gpasswd', '-M',
+            my $gpasswd = &which('gpasswd');
+            &systemcall($gpasswd, '-M',
                         join(',', get_group_members($newgrp), $new_name),
                         $newgrp);
             &invalidate_nscd();
@@ -565,7 +577,8 @@

     if ($config{"quotauser"}) {
        printf (gtx("Setting quota from `%s'.\n"),$config{quotauser});
-       &systemcall('/usr/sbin/edquota', '-p', $config{quotauser}, $new_name);
+       my $edquota = &which('edquota');
+       &systemcall($edquota, '-p', $config{quotauser}, $new_name);
     }

     &systemcall('/usr/local/sbin/adduser.local', $new_name, $new_uid,
@@ -825,23 +838,24 @@
 }

 sub ch_gecos {
+    my $chfn = &which('chfn');
     my $gecos = shift;
     if($gecos =~ /,/)
       {
          my($gecos_name,$gecos_room,$gecos_work,$gecos_home,$gecos_other)
            = split(/,/,$gecos);

-         &systemcall('/usr/bin/chfn', '-f', $gecos_name, '-r', $gecos_room, $new_name);
-         &systemcall('/usr/bin/chfn','-w',$gecos_work,$new_name)
+         &systemcall($chfn, '-f', $gecos_name, '-r', $gecos_room, $new_name);
+         &systemcall($chfn,'-w',$gecos_work,$new_name)
            if(defined($gecos_work));
-         &systemcall('/usr/bin/chfn','-h',$gecos_home,$new_name)
+         &systemcall($chfn,'-h',$gecos_home,$new_name)
            if(defined($gecos_home));
-         &systemcall('/usr/bin/chfn','-o',$gecos_other,$new_name)
+         &systemcall($chfn,'-o',$gecos_other,$new_name)
            if(defined($gecos_other));
       }
     else
       {
-         &systemcall('/usr/bin/chfn', '-f', $gecos, $new_name);
+         &systemcall($chfn, '-f', $gecos, $new_name);
       }
 }

@@ -953,4 +967,4 @@
 # cperl-indent-level:4
 # End:

-
+# vim:set ai et sts=4 sw=4 tw=0:
Index: AdduserCommon.pm
===================================================================
--- AdduserCommon.pm    (revision 562)
+++ AdduserCommon.pm    (working copy)
@@ -30,12 +30,7 @@
     }

     # Check if we need to invalidate the NSCD cache
-    my $nscd;
-    if(-e "/usr/sbin/nscd") {
-        $nscd = "/usr/sbin/nscd";
-    } elsif(-e "/usr/bin/nscd") {
-        $nscd = "/usr/bin/nscd";
-    }
+    my $nscd = &which('nscd',1);
     # this function replaces startnscd and stopnscd (closes: #54726)
     # We are ignoring any error messages given by nscd here since we
     # cannot expect the nscd maintainer and upstream to document their
@@ -166,6 +161,17 @@
     }
 }

+sub which {
+    my ($progname, $nonfatal) = @_ ;
+    for my $dir (split /:/, $ENV{"PATH"}) {
+        if (-x $dir/$progname ) {
+            return $dir/$progname;
+        }
+    }
+    dief(gtx("No program named $progname in \$PATH\n")) unless ($nonfatal);
+}
+
+
 # preseed the configuration variables
 # then read the config file /etc/adduser and overwrite the data hardcoded here
 sub preseed_config {
@@ -210,3 +216,5 @@
 # Local Variables:
 # mode:cperl
 # End:
+
+# vim:set ai et sts=4 sw=4 tw=0:
-- 
 -----------------------------------------------------------------
|   ,''`.                                            Stephen Gran |
|  : :' :                                        sgran at debian.org |
|  `. `'                        Debian user, admin, and developer |
|    `-                                     http://www.debian.org |
 -----------------------------------------------------------------
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.alioth.debian.org/pipermail/adduser-devel/attachments/20060426/59c41c26/attachment.pgp


More information about the Adduser-devel mailing list