'insecure directories' warning

'insecure directories' warning

[Adam Spiers]

A month ago, Bart wrote:
> The other problem is that when I answer "y" to the question "there are
> insecure directories -- proceed?" I expected that to mean that it
> would USE those directories, not ignore them.  If it's going to skip
> those directories anyway, why bother to ask the question?  Giving me a
> choice between no directories and half of them is no choice at all; it
> just leaves me with a partly-working completion system, which is
> baffling.

Nothing seems to have been done about this.  Also, it would be nice if
the user gets told which directories are insecure, otherwise he
doesn't stand much of a chance of fixing them.

Maybe I should file a SF bug report?

Re: 'insecure directories' warning

[Bart Schaefer]

On Jul 25,  4:17pm, Adam Spiers wrote:
} Subject: 'insecure directories' warning
}
} A month ago, Bart wrote:
} > The other problem is that when I answer "y" to the question "there are
} > insecure directories -- proceed?" I expected that to mean that it
} > would USE those directories, not ignore them.
} 
} Nothing seems to have been done about this.

Yes, it has.  The patch was 12033 (the ChangeLog entry is rather brief).

} Also, it would be nice if the user gets told which directories are
} insecure, otherwise he doesn't stand much of a chance of fixing them.

Hrm.  Maybe a separate function to check for and report this?  I don't
know if we want the compinit output to get any more verbose than it is.
On the other hand, it's only a one-line change, so maybe some of you can
try this out and see what you think (the output is a couple of screenfuls
long for me because of RedHat's special groups trick):

Index: Completion/Core/compinit
===================================================================
@@ -371,6 +371,7 @@
 
     if [[ -n "$_i_q" ]]; then
       if [[ "$_i_fail" = ask ]]; then
+	print -l - $_i_wdirs $_i_wfiles
         if ! read -q "?There are insecure $_i_q, continue [ny]? "; then
           unfunction compinit compdef
           unset _comp_dumpfile _comp_secure compprefuncs comppostfuncs \


-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: 'insecure directories' warning

[Adam Spiers]

Bart Schaefer (schaefer@candle.brasslantern.com) wrote:
> On Jul 25,  4:17pm, Adam Spiers wrote:
> } Subject: 'insecure directories' warning
> }
> } A month ago, Bart wrote:
> } > The other problem is that when I answer "y" to the question "there are
> } > insecure directories -- proceed?" I expected that to mean that it
> } > would USE those directories, not ignore them.
> } 
> } Nothing seems to have been done about this.
> 
> Yes, it has.  The patch was 12033 (the ChangeLog entry is rather brief).

Ah right.  Sorry 'bout that :-)

> } Also, it would be nice if the user gets told which directories are
> } insecure, otherwise he doesn't stand much of a chance of fixing them.
> 
> Hrm.  Maybe a separate function to check for and report this?  I don't
> know if we want the compinit output to get any more verbose than it is.
> On the other hand, it's only a one-line change, so maybe some of you can
> try this out and see what you think (the output is a couple of screenfuls
> long for me because of RedHat's special groups trick):

I'd say it's better than nothing, but yes, it is a bit too verbose if
the problem affects too many functions.  A separate function would be
good; then it could output stuff like:

/usr/local/share/zsh/3.1.9-dev-3/functions/X contains functions not
owned by you or by root; the first such function is _x_arguments.
Continue anyway [ny]? 

I'd also suggest replacing 'continue' with something which makes it
more obvious what the two choices are.  At the moment if you're
invoking compinit from .zshrc, it sounds a bit like you want to exit
the shell if you hit 'n'.

Re: 'insecure directories' warning

[Sven Wischnowsky]

Adam Spiers wrote:

> ...
> 
> > } Also, it would be nice if the user gets told which directories are
> > } insecure, otherwise he doesn't stand much of a chance of fixing them.
> > 
> > Hrm.  Maybe a separate function to check for and report this?  I don't
> > know if we want the compinit output to get any more verbose than it is.
> > On the other hand, it's only a one-line change, so maybe some of you can
> > try this out and see what you think (the output is a couple of screenfuls
> > long for me because of RedHat's special groups trick):
> 
> I'd say it's better than nothing, but yes, it is a bit too verbose if
> the problem affects too many functions.  A separate function would be
> good; then it could output stuff like:
> 
> /usr/local/share/zsh/3.1.9-dev-3/functions/X contains functions not
> owned by you or by root; the first such function is _x_arguments.
> Continue anyway [ny]? 

A separate function would be easy to add, of course. Alternatively we
could give compinit a `verbose' option. Or a `test' option that
doesn't try to load the completion system but performs the checks. Or
both.

> I'd also suggest replacing 'continue' with something which makes it
> more obvious what the two choices are.  At the moment if you're
> invoking compinit from .zshrc, it sounds a bit like you want to exit
> the shell if you hit 'n'.

You can just go ahead and improve that...

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

PATCH: Re: 'insecure directories' warning

[Bart Schaefer]

On Jul 25,  8:27pm, Adam Spiers wrote:
}
} > } Also, it would be nice if the user gets told which directories are
} > } insecure, otherwise he doesn't stand much of a chance of fixing them.
} > 
} > Hrm.  Maybe a separate function to check for and report this?  I don't
} > know if we want the compinit output to get any more verbose than it is.
} 
} I'd say it's better than nothing, but yes, it is a bit too verbose if
} the problem affects too many functions.  A separate function would be
} good; then it could output stuff like:
} 
} /usr/local/share/zsh/3.1.9-dev-3/functions/X contains functions not
} owned by you or by root; the first such function is _x_arguments.

The patch below extracts the secure-directories test from compinit and
puts it in its own function `compaudit'.  It doesn't yet do anything
nice about reducing the amount of output, but it does have some code to
handle the RedHat group-writable directory foo.

Things about this patch that might be controversial:

* I've made compaudit a separate file, autoloaded from compinit.  That
  doesn't seem too far afield as it's intended to be runnable separately
  and compinit already does something like that with compdump.  However,
  it does mean that the _compdir-searching code has also been removed
  from compinit, so maybe we want to fold it back into compinit.

* The RedHat stuff reads /etc/group directly (no NIS or YP support) and
  possibly should have a style or something to turn it off for non-RHL
  systems.

* The use of _compdir has changed slightly -- it's now ignored if it's
  set to empty, but it gets set to a likely value from $fpath if it is
  not set at all, so that we know where to go crawling when the reason
  for the search is that the number of functions found is too few.

On Jul 26,  9:14am, Sven Wischnowsky wrote:
}
} Adam Spiers wrote:
} > I'd also suggest replacing 'continue' with something which makes it
} > more obvious what the two choices are.
} 
} You can just go ahead and improve that...

Oh, good.  I did, a little.

Index: Completion/Core/compaudit
========================================================================
@@ -0,0 +1,130 @@
+# So that this file can also be read with `.' or `source' ...
+compaudit() {                           # Define and then call
+
+# Audit the fpath to assure that it contains all the directories needed by
+# the completion system, and that those directories are at least unlikely
+# to contain dangerous files.  This is far from perfect, as the modes or
+# ownership of files or directories might change between the time of the
+# audit and the time the function is executed.
+
+# This function is designed to be called from compinit, which assumes that
+# it is in the same directory, i.e., it can be autoloaded from the initial
+# fpath as compinit was.  Most local parameter names in this function must
+# therefore be the same as those used in compinit.
+
+emulate -L zsh
+setopt extendedglob
+
+# The positional parameters are the directories to check, else fpath.
+if (( $# )); then
+  local _compdir=''
+elif (( $#fpath == 0 )); then
+  print 'compaudit: No directories in $fpath, cannot continue' 1>&2
+  return 1
+else
+  set -- $fpath
+fi
+
+# _i_check is defined by compinit; used here as a test for whether this
+# function is running standalone or was called by compinit.  If called
+# by compinit, we use parameters that are defined in compinit's scope,
+# otherwise we make them local here.
+(( $+_i_check )) || {
+  local _i_q _i_line _i_file _i_fail=verbose
+  local -a _i_files _i_addfiles _i_wdirs _i_wfiles
+  local -a -U +h fpath
+}
+
+fpath=( $* )
+
+# _compdir may be defined by the user; see the compinit documentation.
+# If it isn't defined, we want it to point somewhere sensible, but the
+# user is allowed to set it to empty to bypass the check below.
+(( $+_compdir )) || {
+  local _compdir=${fpath[(r)*/$ZSH_VERSION/*]}
+  [[ -z $_compdir ]] && _compdir=$fpath[1]
+  [[ -d $_compdir/../Core ]] && _compdir=${_compdir:h}
+}
+
+_i_wdirs=()
+_i_wfiles=()
+
+_i_files=( ${^~fpath:/.}/^([^_]*|*~|*.zwc)(N) )
+if [[ -n $_compdir ]]; then
+  if [[ $#_i_files -lt 20 || $_compdir = */Core || -d $_compdir/Core ]]; then
+    # Too few files: we need some more directories, or we need to check
+    # that all directories (not just Core) are present.
+    _i_addfiles=()
+    if [[ $_compdir = */Core ]]; then
+      # Add all the Completion subdirectories
+      _i_addfiles=(${_compdir:h}/*(/))
+    elif [[ -d $_compdir/Core ]]; then
+      # Likewise
+      _i_addfiles=(${_compdir}/*(/))
+    fi
+    for _i_line in {1..$#i_addfiles}; do
+      _i_file=${_i_addfiles[$_i_line]}
+      [[ -d $_i_file && -z ${fpath[(r)$_i_file]} ]] ||
+        _i_addfiles[$_i_line]=
+    done
+    fpath=($fpath $_i_addfiles)
+    _i_files=( ${^~fpath:/.}/^([^_]*|*~|*.zwc)(N) )
+  fi
+fi
+
+[[ $_i_fail == use ]] && return 0
+
+# RedHat Linux "per-user groups" check.  This is tricky, because it's very
+# difficult to tell whether the sysadmin has put someone else into your
+# "private" group (e.g., via the default group field in /etc/passwd, or
+# by NFS group sharing with an untrustworthy machine).  So we must assume
+# that this has not happened, and pick the best group.
+
+local GROUP GROUPMEM _i_pw _i_gid
+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 < /etc/group
+
+# We search for:
+# - world/group-writable directories in fpath not owned by root and the user
+# - parent-directories of directories in fpath that are world/group-writable
+#   and not owned by root and the user (that would allow someone to put a
+#   digest file for one of the directories into the parent directory)
+# - digest files for one of the directories in fpath not owned by root and
+#   the user
+# - and for files in directories from fpath not owned by root and the user
+#   (including zwc files)
+
+if [[ $GROUP == $LOGNAME && ( -z $GROUPMEM || $GROUPMEM == $LOGNAME ) ]]; then
+  _i_wdirs=( ${^fpath}(Nf:g+w:^g:${GROUP}:,f:o+w:,^u0u${EUID})
+             ${^fpath}/..(Nf:g+w:^g:${GROUP}:,f:o+w:,^u0u${EUID}) )
+else
+  _i_wdirs=( ${^fpath}(Nf:g+w:,f:o+w:,^u0u${EUID})
+             ${^fpath}/..(Nf:g+w:,f:o+w:,^u0u${EUID}) )
+fi
+_i_wdirs=( $_i_wdirs ${^fpath}.zwc^([^_]*|*~)(N^u0u${EUID}) )
+_i_wfiles=( ${^fpath}/^([^_]*|*~)(N^u0u${EUID}) )
+
+case "${#_i_wdirs}:${#_i_wfiles}" in
+(0:0) _i_q= ;;
+(0:*) _i_q=files ;;
+(*:0) _i_q=directories ;;
+(*:*) _i_q='directories and files' ;;
+esac
+
+if [[ -n "$_i_q" ]]; then
+  [[ $_i_fail == verbose ]] && {
+    print There are insecure ${_i_q}: 1>&2
+    print -l - $_i_wdirs $_i_wfiles
+  }
+  return 1
+fi
+return 0
+
+}                                       # Define and then call
+
+compaudit "$@"
Index: Completion/Core/compinit
========================================================================
@@ -1,10 +1,11 @@
 # Initialisation for new style completion. This mainly contains some helper
 # functions and aliases. Everything else is split into different files that
-# will automatically be made autoloaded (see the end of this file).
-# The names of the files that will be considered for autoloading have to
-# start with an underscores (like `_setopt').
-# The first line of these files will be read and has to say what should be
-# done with its contents:
+# will automatically be made autoloaded (see the end of this file).  The
+# names of the files that will be considered for autoloading are those that
+# begin with an underscores (like `_setopt').
+#
+# The first line of each of these files is read and must indicate what
+# should be done with its contents:
 #
 #   `#compdef <names ...>'
 #     If the first line looks like this, the file is autoloaded as a
@@ -57,6 +58,13 @@
 # the end).  This takes the dumpfile as an argument.  -d (with the
 # default dumpfile) is now the default; to turn off dumping use -D.
 
+# The -C flag bypasses both the check for rebuilding the dump file and the
+# usual call to compaudit; the -i flag causes insecure directories found by
+# compaudit to be ignored, and the -u flag causes all directories found by
+# compaudit to be used (without security checking).  Otherwise the user is
+# queried for whether to use or ignore the insecure directories (which
+# means compinit should not be called from non-interactive shells).
+
 emulate -L zsh
 setopt extendedglob
 
@@ -321,57 +329,13 @@
 _i_wdirs=()
 _i_wfiles=()
 
+autoload -U compaudit
 if [[ -n "$_i_check" ]]; then
-  _i_files=( ${^~fpath:/.}/^([^_]*|*~|*.zwc)(N) )
-  if [[ $#_i_files -lt 20 || $_compdir = */Core || -d $_compdir/Core ]]; then
-    # Too few files:  we need some more directories,
-    # or we need to check that all directories (not just Core) are present.
-    if [[ -n $_compdir ]]; then
-      _i_addfiles=()
-      if [[ $_compdir = */Core ]]; then
-        # Add all the Completion subdirectories
-        _i_addfiles=(${_compdir:h}/*(/))
-      elif [[ -d $_compdir/Core ]]; then
-        # Likewise
-        _i_addfiles=(${_compdir}/*(/))
-      fi
-      for _i_line in {1..$#i_addfiles}; do
-        _i_file=${_i_addfiles[$_i_line]}
-        [[ -d $_i_file && -z ${fpath[(r)$_i_file]} ]] ||
-          _i_addfiles[$_i_line]=
-      done
-      fpath=($fpath $_i_addfiles)
-      _i_files=( ${^~fpath:/.}/^([^_]*|*~|*.zwc)(N) )
-    fi
-  fi
-  if [[ "$_i_fail" != use ]]; then
-    typeset _i_q
-
-    # We search for:
-    # - world/group-writable directories in fpath not owned by root and the user
-    # - parent-directories of directories in fpath that are world/group-writable
-    #   and not owned by root and the user (that would allow someone to put a
-    #   digest file for one of the directories into the parent directory)
-    # - digest files for one of the directories in fpath not owned by root and
-    #   the user
-    # - and for files in directories from fpath not owned by root and the user
-    #   (including zwc files)
-
-    _i_wdirs=( ${^fpath}(Nf:g+w:,f:o+w:,^u0u${EUID})
-               ${^fpath}/..(Nf:g+w:,f:o+w:,^u0u${EUID})
-               ${^fpath}.zwc^([^_]*|*~)(N^u0u${EUID}) )
-    _i_wfiles=( ${^fpath}/^([^_]*|*~)(N^u0u${EUID}) )
-
-    case "${#_i_wdirs}:${#_i_wfiles}" in
-    0:0) _i_q= ;;
-    0:*) _i_q=files ;;
-    *:0) _i_q=directories ;;
-    *:*) _i_q='directories and files' ;;
-    esac
-
+  typeset _i_q
+  if ! eval compaudit; then
     if [[ -n "$_i_q" ]]; then
       if [[ "$_i_fail" = ask ]]; then
-        if ! read -q "?There are insecure $_i_q, continue [ny]? "; then
+        if ! read -q "?There are insecure $_i_q, use them anyway [ny]? "; then
           unfunction compinit compdef
           unset _comp_dumpfile _comp_secure compprefuncs comppostfuncs \
                 _comps _patcomps _postpatcomps _compautos _lastcomp
@@ -461,7 +425,7 @@
   bindkey '^i' complete-word
 fi
 
-unfunction compinit
-autoload -U compinit
+unfunction compinit compaudit
+autoload -U compinit compaudit
 
 return 0
Index: Doc/Zsh/compsys.yo
========================================================================
@@ -48,9 +48,9 @@
 need to restart the shell to see the changes.
 
 To run tt(compinstall) you will need to make sure it is in a directory
-mentioned in your tt($fpath) parameter, which should already be the case if
+mentioned in your tt(fpath) parameter, which should already be the case if
 zsh was properly configured as long as your startup files do not remove the
-appropriate directories from tt($fpath).  Then it must be autoloaded
+appropriate directories from tt(fpath).  Then it must be autoloaded
 (`tt(autoload -U compinstall)' is recommended).  You can abort the
 installation any time you are being prompted for information, and your
 tt(.zshrc) will not be altered at all; changes only take place right at the
@@ -65,7 +65,7 @@
 tt(compinstall) it will be called automatically from your tt(.zshrc).
 
 To initialize the system, the function tt(compinit) should be in a
-directory mentioned in the tt($fpath) variable, and should be autoloaded
+directory mentioned in the tt(fpath) parameter, and should be autoloaded
 (`tt(autoload -U compinit)' is recommended), and then run simply as
 `tt(compinit)'.  This will define a
 few utility functions, arrange for all the necessary shell functions to be
@@ -110,14 +110,25 @@
 not already in the function search path.
 
 For security reasons tt(compinit) also checks if the completion system
-would use files not owned by root or the current user or files in
+would use files not owned by root or by the current user, or files in
 directories that are world- or group-writable or that are not owned by 
-root or the current user.  If such files or directories are found,
-tt(Compinit) will ask if the completion system should really be used.
-To avoid these tests and make all files found be used without asking,
-the option tt(-u) can be given and to make tt(compinit) silently
-ignore all insecure files and directories the options tt(-i) can be
-given.
+root or by the current user.  If such files or directories are found,
+tt(compinit) will ask if the completion system should really be used.  To
+avoid these tests and make all files found be used without asking, use the
+option tt(-u), and to make tt(compinit) silently ignore all insecure files
+and directories use the option tt(-i).  This security check is skipped
+entirely when the tt(-C) option is given.
+
+findex(compaudit)
+The security check can be retried at any time by running the function
+tt(compaudit).  This is the same check used by tt(compinit), but when it
+is executed directly any changes to tt(fpath) are made local to the
+function so they do not persist.  The directories to be checked may be
+passed as arguments; if none are given, tt(compaudit) uses tt(fpath) and
+tt(_compdir) to find completion system directories, adding missing ones
+to tt(fpath) as necessary.  To force a check of exactly the directories
+currently named in tt(fpath), set tt(_compdir) to an empty string before
+calling tt(compaudit) or tt(compinit).
 
 subsect(Autoloaded files)
 cindex(completion system, autoloaded functions)

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net