Re: [exim-dev] fopen(), umask, and file permissions

Top Page
Delete this message
Reply to this message
Author: Philip Hazel
Date:  
To: Marc Haber
CC: exim-dev
Subject: Re: [exim-dev] fopen(), umask, and file permissions
On Thu, 16 Feb 2006, Marc Haber wrote:

> > Near the start of Exim there is this code:
> >
> > /* Set the umask to zero so that any files that Exim creates are created
> > with the modes that it specifies. */
>
> That comment should be "can later be chmodded to what exim specified".


Not really. You see, the main code of Exim creates files with open(),
not fopen(). With open(), a mode can be specified directly in the open()
call. The fopen() calls to create files have crept in with the content
scanning code.

I have created a function called modefopen() which takes an additional
mode argument. It sets the umask to 777, creates the file, chmods it,
and then resets the umask. This avoids having to change any of the
existing open() functionality.

Patch for 4.60 is below, if anyone wants to try it out.

Philip

-- 
Philip Hazel            University of Cambridge Computing Service,
ph10@???      Cambridge, England. Phone: +44 1223 334714.




*** exim-4.60/src/functions.h        Mon Nov 28 10:57:32 2005
--- functions.h Wed Feb 22 12:01:54 2006
***************
*** 166,171 ****
--- 166,172 ----
  extern void    moan_tell_someone(uschar *, address_item *, uschar *, char *,
                   ...);
  extern BOOL    moan_to_sender(int, error_block *, header_line *, FILE *, BOOL);
+ extern FILE   *modefopen(uschar *, char *, mode_t);


  extern uschar *parse_extract_address(uschar *, uschar **, int *, int *, int *,
                   BOOL);
*** exim-4.60/src/exim.c    Mon Nov 28 10:57:32 2005
--- exim.c    Wed Feb 22 12:16:04 2006
***************
*** 376,381 ****
--- 376,414 ----



  /*************************************************
+ *   Call fopen() with umask 777 and adjust mode  *
+ *************************************************/
+ 
+ /* Exim runs with umask(0) so that files created with open() have the mode that 
+ is specified in the open() call. However, there are some files, typically in 
+ the spool directory, that are created with fopen(). They end up world-writeable 
+ if no precautions are taken. Although the spool directory is not accessible to 
+ the world, this is an untidiness. So this is a wrapper function for fopen() 
+ that sorts out the mode of the created file.
+ 
+ Arguments:
+    filename       the file name
+    options        the fopen() options
+    mode           the required mode
+    
+ Returns:          the fopened FILE or NULL
+ */
+ 
+ FILE *
+ modefopen(uschar *filename, char *options, mode_t mode)
+ {
+ FILE *f;
+ umask(0777);
+ f = Ufopen(filename, options);
+ umask(0);
+ if (f != NULL) (void)fchmod(fileno(f), mode);
+ return f;
+ }   
+ 
+ 
+ 
+ 
+ /*************************************************
  *   Ensure stdin, stdout, and stderr exist       *
  *************************************************/


***************
*** 1435,1442 ****
message_id = message_id_external + 1;
message_id[0] = 0;

! /* Set the umask to zero so that any files that Exim creates are created
! with the modes that it specifies. */

umask(0);

--- 1473,1487 ----
message_id = message_id_external + 1;
message_id[0] = 0;

! /* Set the umask to zero so that any files that Exim creates using open() are
! created with the modes that it specifies. NOTE: Files created with fopen() have
! a problem, which was not recognized till rather late (February 2006). With this
! umask, such files will be world writeable. (They are all content scanning files
! in the spool directory, which isn't world-accessible, so this is not a
! disaster, but it's untidy.) I don't want to change this overall setting,
! however, because it will interact badly with the open() calls. Instead, there's
! now a function called modefopen() that fiddles with the umask while calling
! fopen(). */

umask(0);

*** exim-4.60/src/spool_mbox.c        Mon Nov 28 10:57:32 2005
--- spool_mbox.c    Wed Feb 22 14:29:11 2006
***************
*** 56,62 ****


      /* open [message_id].eml file for writing */
      (void)string_format(mbox_path, 1024, "%s/scan/%s/%s.eml", spool_directory, message_id, message_id);
!     mbox_file = Ufopen(mbox_path,"wb");


      if (mbox_file == NULL) {
        debug_printf("unable to open file for writing: %s\n", mbox_path);
--- 56,62 ----


      /* open [message_id].eml file for writing */
      (void)string_format(mbox_path, 1024, "%s/scan/%s/%s.eml", spool_directory, message_id, message_id);
!     mbox_file = modefopen(mbox_path,"wb",SPOOL_MODE);


      if (mbox_file == NULL) {
        debug_printf("unable to open file for writing: %s\n", mbox_path);
*** exim-4.60/src/mime.c    Mon Nov 28 10:57:32 2005
--- mime.c    Wed Feb 22 14:33:43 2006
***************
*** 241,250 ****


    if ((pname != NULL) && (fname != NULL)) {
      (void)string_format(filename, 2048, "%s/%s", pname, fname);
!     f = fopen(CS filename,"wb+");
    }
    else if (pname == NULL) {
!     f = fopen(CS fname,"wb+");
    }
    else if (fname == NULL) {
      int file_nr = 0;
--- 241,250 ----


    if ((pname != NULL) && (fname != NULL)) {
      (void)string_format(filename, 2048, "%s/%s", pname, fname);
!     f = modefopen(filename,"wb+",SPOOL_MODE);
    }
    else if (pname == NULL) {
!     f = modefopen(fname,"wb+",SPOOL_MODE);
    }
    else if (fname == NULL) {
      int file_nr = 0;
***************
*** 261,267 ****
        result = stat(CS filename,&mystat);
      }
      while(result != -1);
!     f = fopen(CS filename,"wb+");
    };


    /* set expansion variable */
--- 261,267 ----
        result = stat(CS filename,&mystat);
      }
      while(result != -1);
!     f = modefopen(filename,"wb+",SPOOL_MODE);
    };


    /* set expansion variable */
*** exim-4.60/src/malware.c Mon Nov 28 10:57:32 2005
--- malware.c    Wed Feb 22 14:34:19 2006
***************
*** 873,879 ****
        };


        (void)string_format(file_name,1024,"%s/scan/%s/%s_scanner_output", spool_directory, message_id, message_id);
!       scanner_record = fopen(CS file_name,"wb");


        if (scanner_record == NULL) {
          log_write(0, LOG_MAIN|LOG_PANIC,
--- 873,879 ----
        };


        (void)string_format(file_name,1024,"%s/scan/%s/%s_scanner_output", spool_directory, message_id, message_id);
!       scanner_record = modefopen(file_name,"wb",SPOOL_MODE);


        if (scanner_record == NULL) {
          log_write(0, LOG_MAIN|LOG_PANIC,
*** exim-4.60/src/demime.c  Mon Nov 28 10:57:32 2005
--- demime.c    Wed Feb 22 14:34:48 2006
***************
*** 256,262 ****
    }
    while(result != -1);


!   *f = fopen(CS file_name,"wb+");
    if (*f == NULL) {
      /* cannot open new dump file, disk full ? -> soft error */
      (void)string_format(info, 1024,"unable to open dump file");
--- 256,262 ----
    }
    while(result != -1);


!   *f = modefopen(file_name,"wb+",SPOOL_MODE);
    if (*f == NULL) {
      /* cannot open new dump file, disk full ? -> soft error */
      (void)string_format(info, 1024,"unable to open dump file");