[Pkg-cron-devel] [SCM] Git repository for pkg-cron branch, sf3, updated. debian/3.0pl1-109-20-gde88d23

[Christian Kastner]

The following commit has been merged in the sf3 branch:
commit 82ba77e954dfb70bc07f6e2be4d723f1d532a721
Author: Christian Kastner <debian at kvr.at>
Date:   Sun Sep 25 00:02:57 2011 +0200

    Add patch extending error messages
    
    Print helpful messages instead of the oblique "WRONG INODE INFO"

diff --git a/debian/patches/features/better-reporting-for-security-checks b/debian/patches/features/better-reporting-for-security-checks
new file mode 100644
index 0000000..fae20ca
--- /dev/null
+++ b/debian/patches/features/better-reporting-for-security-checks
@@ -0,0 +1,91 @@
+From: Christian Kastner <debian at kvr.at>
+Date: Fri, 24 Sep 2011 00:48:00 +0200
+Subject: Better reporting for security checks
+
+Split the error conditions for the WRONG INODE INFO into separate tests with
+more informative error messages (wrong mode, wrong owner, wrong link count).
+
+Bug-Debian: http://bugs.debian.org/625491
+Bug-Debian: http://bugs.debian.org/625493
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/27520
+
+Last-Update: 2011-09-24
+Index: sf3/database.c
+===================================================================
+--- sf3.orig/database.c	2011-09-24 00:45:48.392004747 +0200
++++ sf3/database.c	2011-09-24 00:45:58.744004760 +0200
+@@ -360,12 +360,24 @@
+                 log_it(fname, getpid(), "WRONG FILE OWNER", tabname);
+ 		goto next_crontab;
+             }
+-            if (!S_ISREG(statbuf->st_mode) ||
+-                statbuf->st_nlink != 1 ||
+-                (statbuf->st_mode & 07777) != 0600) {
+-                log_it(fname, getpid(), "WRONG INODE INFO", tabname);
+- 		goto next_crontab;
+-            }
++
++	    /* Check to make sure that the crontab is a regular file */
++            if (!S_ISREG(statbuf->st_mode)) {
++		log_it(fname, getpid(), "NOT A REGULAR FILE", tabname);
++		goto next_crontab;
++	    }
++
++	    /* Check to make sure that the crontab's permissions are secure */
++            if ((statbuf->st_mode & 07777) != 0600) {
++		log_it(fname, getpid(), "INSECURE MODE (mode 0600 expected)", tabname);
++		goto next_crontab;
++	    }
++
++	    /* Check to make sure that there are no hardlinks to the crontab */
++            if (statbuf->st_nlink != 1) {
++		log_it(fname, getpid(), "NUMBER OF HARD LINKS > 1", tabname);
++		goto next_crontab;
++	    }
+         } else {
+             /* System crontab path. These can be symlinks, but the
+                symlink and the target must be owned by root. */
+@@ -388,22 +400,39 @@
+ 		log_it(fname, getpid(), "FSTAT FAILED", tabname);
+ 		goto next_crontab;
+             }
++
+             /* Check to make sure that the crontab is owned by root */
+             if (statbuf->st_uid != ROOT_UID) {
+                 log_it(fname, getpid(), "WRONG FILE OWNER", tabname);
+ 		goto next_crontab;
+             }
+-            /* Check to make sure that the crontab is writable only by root */
+-            if ((statbuf->st_mode & S_IWGRP) || (statbuf->st_mode & S_IWOTH))  {
+-                log_it(fname, getpid(), "WRONG INODE INFO", tabname);
++
++            /* Check to make sure that the crontab is a regular file */
++            if (!S_ISREG(statbuf->st_mode)) {
++		log_it(fname, getpid(), "NOT A REGULAR FILE", tabname);
+ 		goto next_crontab;
+-            }
++	    }
++
++            /* Check to make sure that the crontab is writable only by root
++	     * This should really be in sync with the check for users above
++	     * (mode 0600). An upgrade path could be implemented for 4.1
++	     */
++	    if ((statbuf->st_mode & S_IWGRP) || (statbuf->st_mode & S_IWOTH)) {
++		log_it(fname, getpid(), "INSECURE MODE (group/other writable)", tabname);
++		goto next_crontab;
++	    }
+             /* Technically, we should also check whether the parent dir is
+  	     * writable, and so on. This would only make proper sense for
+  	     * regular files; we can't realistically check all possible
+  	     * security issues resulting from symlinks. We'll just assume that
+  	     * root will handle responsible when creating them.
+ 	     */
++
++	    /* Check to make sure that there are no hardlinks to the crontab */
++            if (statbuf->st_nlink != 1) {
++		log_it(fname, getpid(), "NUMBER OF HARD LINKS > 1", tabname);
++		goto next_crontab;
++	    }
+         }
+         /*
+          * The link count check is not sufficient (the owner may
diff --git a/debian/patches/series b/debian/patches/series
index e4e0988..68101c6 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -40,6 +40,7 @@ features/debian-logging-configuration
 features/properly-handle-time-skips
 features/swap-both-uid-and-gid
 features/security-make-crontab-setgid-crontab
+features/better-reporting-for-security-checks
 features/auditlog-support
 features/run-on-reboot
 features/set-contenttype-in-mail

-- 
Git repository for pkg-cron