compaudit slow with many groups

compaudit slow with many groups

[Danek Duvall]

At work, we have 1168 groups defined in NIS, and the entire database
dumped via "getent group" is over 100kB.  This makes compaudit very slow
on startup if the groups are not all cached locally, and makes it a drag
logging in to a machine with zsh as my shell.

The code to blame is on line 85:

    local GROUP GROUPMEM _i_pw _i_gid _i_ulwdirs
    while IFS=: read GROUP _i_pw _i_gid GROUPMEM; do
      if (( UID == EUID )); then
        [[ $GROUP == $LOGNAME ]] && break
      else
        (( _i_gid == EGID )) && break       # Somewhat arbitrary
      fi
    done <<(getent group)

which seems somewhat inefficient.  Would something along the lines of

    local GROUP GROUPMEM _i_pw _i_gid _i_ulwdirs
    if (( UID == EUID )); then
    	getent group $UID | IFS=: read GROUP _i_pw _i_gid GROUPMEM
	[[ $GROUP == $LOGNAME ]] || unset GROUP GROUPMEM
    else
    	getent group $EGID | IFS=: read GROUP _i_pw _i_gid GROUPMEM
	(( _i_gid == EGID )) || unset GROUP GROUPMEM
    fi

work?  I'm not positive this is the right set of tests, but if not,
there ought to be some similar way of reducing the overhead of the
function.  Is there any system where getent can't take two arguments?

Or perhaps it ought to be conditional on some style being set or unset.

Thanks,
Danek

Re: compaudit slow with many groups

[Danek Duvall]

On Fri, Mar 19, 2004 at 01:57:24PM -0800, Danek Duvall wrote:

>     local GROUP GROUPMEM _i_pw _i_gid _i_ulwdirs
>     if (( UID == EUID )); then
>     	getent group $UID | IFS=: read GROUP _i_pw _i_gid GROUPMEM
> 	[[ $GROUP == $LOGNAME ]] || unset GROUP GROUPMEM
>     else
>     	getent group $EGID | IFS=: read GROUP _i_pw _i_gid GROUPMEM
> 	(( _i_gid == EGID )) || unset GROUP GROUPMEM
>     fi

Hm.  That's pretty obviously broken.  But I think this is right:

    if ((UID == EUID )); then
      getent group $LOGNAME | IFS=: read GROUP _i_pw _i_gid GROUPMEM
    else
      getent group $EGID | IFS=: read GROUP _i_pw _i_gid GROUPMEM
    fi

Much quicker.  :)

Thanks,
Danek

Re: compaudit slow with many groups

[Peter Stephenson]

Danek Duvall wrote:
> Hm.  That's pretty obviously broken.  But I think this is right:
> 
>     if ((UID == EUID )); then
>       getent group $LOGNAME | IFS=: read GROUP _i_pw _i_gid GROUPMEM
>     else
>       getent group $EGID | IFS=: read GROUP _i_pw _i_gid GROUPMEM
>     fi

I've committed this and the fix for the typo Danek spotted.... slug it
out if you think it should be done another way.

-- 
Peter Stephenson <pws@pwstephenson.fsnet.co.uk>
Work: pws@csr.com
Web: http://www.pwstephenson.fsnet.co.uk

Re: compaudit slow with many groups

[Oliver Kiddle]

On 19 Mar, Danek Duvall wrote:

> work?  I'm not positive this is the right set of tests, but if not,
> there ought to be some similar way of reducing the overhead of the
> function.  Is there any system where getent can't take two arguments?

At the top of compaudit, it is doing:
  [[ -x /usr/bin/getent ]] || getent() { cat /etc/$1 }
so that function replacement won't handle two argumemts. The patch
below allows it to. I'm fairly sure using the two arguments is
otherwise portable. Clearly this function could still be better (NIS
etc is not handled).

I've never been entirely happy with the compaudit stuff. It takes time
to run, most people who know about it add -u to compinit and it isn't
as if the shell refuses to run world writable programs found in the
path. If a security check is necessary, would it not be better done
from C code when the, function/whatever is actually loaded. It is
equally applicable to stuff picked up from $path and $module_path as
$fpath.

Also, would it not be better if compinit also listed the insecure files
or directed users to run compaudit directly. Otherwise it isn't exactly
obvious which files are insecure.

Oliver

Index: Completion/compaudit
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/compaudit,v
retrieving revision 1.7
diff -u -r1.7 compaudit
--- Completion/compaudit	22 Mar 2004 19:59:26 -0000	1.7
+++ Completion/compaudit	30 Mar 2004 14:12:18 -0000
@@ -15,7 +15,13 @@
 emulate -L zsh
 setopt extendedglob
 
-[[ -x /usr/bin/getent ]] || getent() { cat /etc/$1 }
+[[ -x /usr/bin/getent ]] || getent() {
+  if [[ $2 = <-> ]]; then
+    grep ":$2:[^:]*$" /etc/$1
+  else
+    grep "^$2:" /etc/$1
+  fi
+}
 
 # The positional parameters are the directories to check, else fpath.
 if (( $# )); then