Ticket #89: fopen-use-statok.diff

libvuurmuur/src/io.c

@@ -22 +22 @@
 #include "vuurmuur.h"
 
 
-//
+/*  vuurmuur_fopen
+
+    A wrapper around fopen which can be used to open config files. This
+    function performs additionals checks on the file, appropriate for
+    configuration files (such as checking the owner, the permissions, etc.)
+
+    This wrapper only works on a regular file and only when it already exists
+    (even when opening for writing!).
+
+    The path and mode parameters are identical to the fopen(3) libc function.
+*/
 FILE *
-vuurmuur_fopen(const char *path, const char *mode)
+vuurmuur_fopen(const int debuglvl, const char *path, const char *mode)
 {
     FILE        *fp=NULL;
-    struct stat stat_buf;
-    int         statted=0;  // can 'path' be stat-ed? 0: no, 1: yes
 
-    // check if we can lstat the file. If not, we assume file doens't exist.
-    if(lstat(path, &stat_buf) == -1)
-        statted = 0 ;
-    else
-        statted = 1;
+    // Stat the file
+    if (!stat_ok(debuglvl, path, STATOK_WANT_FILE, STATOK_VERBOSE))
+        // File not OK? Don't open it. stat_ok will have printed an error message already.
+        return NULL;
 
-    // now look at the results
-    if(statted && S_ISLNK(stat_buf.st_mode) == 1)
-    {
-        (void)vrprint.error(-1, "Error", "opening '%s': For security reasons Vuurmuur will not allow following symbolic-links.", path);
-    }
-    else if(statted && (stat_buf.st_mode & S_IWGRP || stat_buf.st_mode & S_IWOTH))
+    // now open the file, this should not fail because if we get here it exists and is readable,
+    // but we check to be sure.
+    if(!(fp=fopen(path, mode)))
     {
-        (void)vrprint.error(-1, "Error", "opening '%s': For security reasons Vuurmuur will not open files that are writable by 'group' or 'other'. Check the file content & permissions.", path);
+        (void)vrprint.error(-1, "Error", "opening '%s' failed: %s (in: vuurmuur_fopen).", path, strerror(errno));
+        return NULL;
     }
-    else if(statted && (stat_buf.st_uid != 0 || stat_buf.st_gid != 0))
-    {
-        (void)vrprint.error(-1, "Error", "opening '%s': For security reasons Vuurmuur will not open files that are not owned by root.", path);
-    }
-    else
-    {
-        // check if group and others can read the file. If so, fix the permissions.
-        if(statted && (stat_buf.st_mode & S_IRGRP || stat_buf.st_mode & S_IROTH))
-        {
-            (void)vrprint.info("Info", "'%s' is readable by 'group' and 'other'. This is not recommended. Fixing.", path);
-            if(chmod(path, 0600) == -1)
-            {
-                (void)vrprint.error(-1, "Error", "failed to repair file permissions for file '%s': %s.", path, strerror(errno));
-                return(NULL);
-            }
-        }
-        // check if group and others can execute the file. If so, fix the permissions.
-        if(statted && (stat_buf.st_mode & S_IXGRP || stat_buf.st_mode & S_IXOTH))
-        {
-            (void)vrprint.info("Info", "'%s' is executable by 'group' and 'other'. This is not recommended. Fixing.", path);
-            if(chmod(path, 0600) == -1)
-            {
-                (void)vrprint.error(-1, "Error", "failed to repair file permissions for file '%s': %s.", path, strerror(errno));
-                return(NULL);
-            }
-        }
 
-        // now open the file, this should not fail because if we get here it exists and is readable,
-        // but we check to be sure.
-        if(!(fp=fopen(path, mode)))
-        {
-            (void)vrprint.error(-1, "Error", "opening '%s' failed: %s (in: vuurmuur_fopen).", path, strerror(errno));
-        }
-        else
-        {
-            // return our succes
-            return(fp);
-        }
-    }
-
-    // if we get here, there was an error
-    return(NULL);
+    // return our succes
+    return(fp);
 }
 
 
@@ -340 +306 @@
     Returns the pointer to the file, or NULL if failed.
 */
 FILE *
-rules_file_open(const char *path, const char *mode, int caller)
+rules_file_open(const int debuglvl, const char *path, const char *mode, int caller)
 {
     FILE    *lock_fp = NULL,
             *fp = NULL;
@@ -431 +397 @@
         free(lock_path);
     }
 
-    fp = vuurmuur_fopen(path, mode);
+    fp = vuurmuur_fopen(debuglvl, path, mode);
     return(fp);
 }
 

libvuurmuur/plugins/textdir/textdir_ask.c

@@ -96 +96 @@
     /* now open and read the file, but only if it is not already open */
     if(ptr->file == NULL)
     {
-        if(!(ptr->file = vuurmuur_fopen(file_location, "r")))
+        if(!(ptr->file = vuurmuur_fopen(debuglvl, file_location, "r")))
         {
             (void)vrprint.error(-1, "Error", "Unable to open file '%s'.", file_location);
 

libvuurmuur/plugins/textdir/textdir_tell.c

@@ -85 +85 @@
     /*
         first open the file for reading
     */
-    if(!(fp = vuurmuur_fopen(file_location, "r")))
+    if(!(fp = vuurmuur_fopen(debuglvl, file_location, "r")))
     {
         (void)vrprint.error(-1, "Error", "unable to open file '%s' for reading: %s.", file_location, strerror(errno));
 
@@ -321 +321 @@
     /*
         now open the file for writing
     */
-    if(!(fp = vuurmuur_fopen(file_location, "w+")))
+    if(!(fp = vuurmuur_fopen(debuglvl, file_location, "w+")))
     {
         (void)vrprint.error(-1, "Error", "unable to open file '%s' for writing: %s (in: %s).", file_location, strerror(errno), __FUNC__);
         

libvuurmuur/src/config.c

@@ -1558 +1558 @@
     if(!question || !file_location || size == 0)
         return(-1);
 
-    if(!(fp = vuurmuur_fopen(file_location,"r")))
+    if(!(fp = vuurmuur_fopen(debuglvl, file_location,"r")))
     {
         (void)vrprint.error(-1, "Error", "unable to open configfile '%s': %s (in: ask_configfile).", file_location, strerror(errno));
         return(-1);

libvuurmuur/src/rules.c

@@ -1371 +1371 @@
     }
 
     /* open the rulesfile */
-    if(!(fp = rules_file_open(rulesfile_location, "w+", 0)))
+    if(!(fp = rules_file_open(debuglvl, rulesfile_location, "w+", 0)))
     {
         (void)vrprint.error(-1, "Error", "opening rulesfile '%s' failed: %s (in: %s).",
                 rulesfile_location, strerror(errno), __FUNC__);

libvuurmuur/src/vuurmuur.h

@@ -1401 +1401 @@
 /*
     io.c
 */
-FILE *vuurmuur_fopen(const char *path, const char *mode);
+FILE *vuurmuur_fopen(const int debuglvl, const char *path, const char *mode);
 DIR *vuurmuur_opendir(const int, const char *);
 int stat_ok(const int, const char *, char, char);
 int check_pidfile(char *pidfile_location);
 int create_pidfile(char *pidfile_location, int shm_id);
 int remove_pidfile(char *pidfile_location);
-FILE * rules_file_open(const char *path, const char *mode, int caller);
+FILE * rules_file_open(const int debuglvl, const char *path, const char *mode, int caller);
 int rules_file_close(FILE *file, const char *path);
 int pipe_command(const int, struct vuurmuur_config *, char *, char);
 int libvuurmuur_exec_command(const int, struct vuurmuur_config *, char *, char **, char *);