Logcheck devel - Dec 2005 - Bug#344553: logcheck: Fails silently to read config file

Package: logcheck
Version: 1.2.42
Severity: minor
Tags: patch

Logcheck does not report any error if the config file is not readable
or does not exists.  This may easily happen, as logcheck is run as
logcheck user and while one is testing a new configuration on live
system with running configuration intact.

Following fragment may help:

 # Now source the config file - before things that should not be changed
 if [ -r $CONFFILE ]; then
-       . $CONFFILE
+       . $CONFFILE
+else
+    error "Config file $CONFFILE not exists or readable"
 fi


-- System Information:
Debian Release: 3.1
  APT prefers stable
  APT policy: (900, 'stable'), (400, 'testing'), (300,
'unstable')
Architecture: i386 (i686)
Shell:  /bin/sh linked to /bin/bash
Kernel: Linux 2.6.15-rc4
Locale: LANG=fi_FI at euro, LC_CTYPE=fi_FI at euro (charmap=ISO-8859-15)

Versions of packages logcheck depends on:
ii  adduser          3.80                    Add and remove users and groups
ii  cron             3.0pl1-92               management of regular
background p
ii  debconf [debconf 1.4.62                  Debian configuration
management sy
ii  debianutils      2.15.1                  Miscellaneous utilities
specific t
ii  grep             2.5.1.ds2-4             GNU grep, egrep and fgrep
ii  lockfile-progs   0.1.10                  Programs for locking and
unlocking
ii  logcheck-databas 1.2.42                  database of system log
rules for t
ii  logtail          1.2.42                  Print log file lines that
have not
ii  mailx            1:8.1.2-0.20050715cvs-1 A simple mail user agent
ii  sendmail-bin [ma 8.13.4-3                powerful, efficient, and
scalable
ii  sysklogd [system 1.4.1-17                System Logging Daemon

logcheck recommends no packages.

-- debconf information excluded

On Fri, Dec 23, 2005 at 08:21:53PM +0200, Markus Peuhkuri
wrote:
> Package: logcheck
> Version: 1.2.42
> Severity: minor
> Tags: patch
> 
> Logcheck does not report any error if the config file is not readable
> or does not exists.  This may easily happen, as logcheck is run as
> logcheck user and while one is testing a new configuration on live
> system with running configuration intact.
> 
> Following fragment may help:
> 
>  # Now source the config file - before things that should not be changed
>  if [ -r $CONFFILE ]; then
> -       . $CONFFILE
> +       . $CONFFILE
> +else
> +    error "Config file $CONFFILE not exists or readable"
>  fi
The patch is greatly appreciated.  Thanks, Markus. Your change will be in the
next release.

-- 
Todd Troxell
http://rapidpacket.com/~xtat

Todd Troxell wrote:
> I see your point.  The config is not really essential.
> 
> What do you think about this:
> 
> if [ -f $CONFFILE -a -r $CONFFILE]; then
The problem is still that if CONFFILE is somehow mistyped, it still
fails silently even if the fragment fixes the error I had with
permissions.  For my view the correct operation would be following:

1) if CONFFILE (from command line) is set, use it.  If it does not
exists or is unreadable, issue an error

2) if CONFFILE is not set, but the default CONFFILE exists AND is
readable use it

3) if default CONFFILE exists, but is unreadable, provide an error

4) if default CONFFILE does not exists, use defauls


To have 1), command line argument processing should be modified as below

    case "$opt" in
        c)
            debug "Setting CONFFILE to $OPTARG"
            CONFFILE="$OPTARG"
	    if [ ! -r $CONFFILE ]; then	
		  error "Config file $CONFFILE unreadable or does not exists"
	    fi
          ;;

For 2) and 4), the existing condition is ok, but needs additional
condition for 3).

if [ -r $CONFFILE ]; then
  . $CONFFILE
elif [ -f $CONFFILE ]; then  # this provides 3)
  error "Config file $CONFFILE unreadable"
fi



(sorry, if line wrap is problem).

-- 
Markus Peuhkuri | http://www.iki.fi/puhuri/

On Mon, Jan 02, 2006 at 02:03:19PM +0200, Markus Peuhkuri
wrote:
> Maximilian Attems wrote:
> 
> >
> >no the debian packaging takes care of that,
> >no need to issue an error in that case.
> >if you change the permissions of the CONFFILE you are on your own.
> >  
> >
> In NO CASE should program functionality depend on some package
> management system.  The program should be able to work as well without
> or with random package management system.
> 
> Considering that debian userid management is something that is very
> fragile, I would prefer that the script also checks possible problems
> with configuration.  Even more important this is when it is about
> logcheck, whose sole purpose is to detect unintended events in systems.
first calm down your words. :)
getting enerved is not a good way to push something.

second you give _no_ argument why CONFFILE is so important.
logcheck works fine without it.

third the nacked change introduces potential break-ups on current
working setups. we wont change semantics for $random_reasons.
we check about real reasons like not readable log files.
thus are worth to alert the admin.

fourth why is the debian userid managment fragile?
works very nicely for me on lots of boxes.

fifth why did you change the ownerships of CONFFILE?
there might be many cool reasons to think about,
none was named.

-- 
maks

Maximilian Attems wrote:
>second you give _no_ argument why CONFFILE is so important.
>logcheck works fine without it.
>
>  
>
If config file is defined on command line argument, it should be read in
and an error given if it not readable.  If the config file exists, it
should be read.
>third the nacked change introduces potential break-ups on current
>working setups. we wont change semantics for $random_reasons.
>  
>
The case that gets broken is that if the /etc/logcheck/logcheck.conf is
not readable by logcheck user.  I do not know, if there is any setup
like that, but lets say it is a quite interesting setup.  I would value
clear error messages or at least warnings over that.
>we check about real reasons like not readable log files.
>thus are worth to alert the admin.
>  
>
I think that existing config file that is unreadable is something
abnormal,  but YMMV.
>fourth why is the debian userid managment fragile?
>works very nicely for me on lots of boxes.
>  
>
Maybe I just cannot do it, but as I had recently to do system reinstall
because of disk crash.  I recovered config files from backups but those
ended up with wrong ownerships and I had to fix them by hand.  The
system UIDs were different on different installations: the other was
installed, packages add, upgraded, and packages add while the later had
about all packages installed at once.
>fifth why did you change the ownerships of CONFFILE?
>there might be many cool reasons to think about,
>none was named.
>  
>
The problem was that I wanted to experiment with new config file.  It
was owned by my $LUSER UID, and then I ran "sudo -u logcheck logcheck -c
config -t ".  Unfortunatly, the config file was mode 600, and logcheck
did not provide any error, just used default settings and I was totaly
lost with that wondering why my changes were not visible.

One may change ownership of configuration file unintentionaly (pick you
$EDITOR right)
>first calm down your words.  :) 
>getting enerved is not a good way to push something.
>  
>
It was no intended such, more like emphasis what I value in building
robust systems (would *no* *case* been better?).  It is good that
package management makes sure that everything is ok, but each input must
be validated and checked for.