Intercept chown()/fchown() failure and emit a pointer to the bugreport. Closes 2391

In a specific NFS setup we experienced a failing chown(). As it is not
clear, whether this was due to a misconfiguration or if this may happen in
other environments too, we behave as usual (abort the operation), but
issue a MAIN_LOG and PANIC_LOG entry pointing to this Bugreport.

You're encouraged to contact the developers, if you hit this issue.

diff --git a/src/src/dbfn.c b/src/src/dbfn.c
index 336cfe73e262119c53f3322c73af77c336de9a03..5555c710b86fbb5e1b0e2cfa23547f495df93f1f 100644 (file)
--- a/src/src/dbfn.c
+++ b/src/src/dbfn.c
@@ -209,7 +209,7 @@ if (created && geteuid() == root_uid)
       if (Ustat(filename, &statbuf) >= 0 && statbuf.st_uid != exim_uid)
         {
         DEBUG(D_hints_lookup) debug_printf_indent("ensuring %s is owned by exim\n", filename);
       if (Ustat(filename, &statbuf) >= 0 && statbuf.st_uid != exim_uid)
         {
         DEBUG(D_hints_lookup) debug_printf_indent("ensuring %s is owned by exim\n", filename);

-        if (Uchown(filename, exim_uid, exim_gid))
+        if (exim_chown(filename, exim_uid, exim_gid))

           DEBUG(D_hints_lookup) debug_printf_indent("failed setting %s to owned by exim\n", filename);
         }
       }
           DEBUG(D_hints_lookup) debug_printf_indent("failed setting %s to owned by exim\n", filename);
         }
       }

diff --git a/src/src/deliver.c b/src/src/deliver.c
index c1396a7f7d754384450924733ad8d378f9fbeb37..696effdeedfea0a6c21d6ca756b25fe76f5858f4 100644 (file)
--- a/src/src/deliver.c
+++ b/src/src/deliver.c
@@ -347,7 +347,7 @@ for (int i = 2; i > 0; i--)
 #ifndef O_CLOEXEC
     (void)fcntl(fd, F_SETFD, fcntl(fd, F_GETFD) | FD_CLOEXEC);
 #endif
 #ifndef O_CLOEXEC
     (void)fcntl(fd, F_SETFD, fcntl(fd, F_GETFD) | FD_CLOEXEC);
 #endif

-    if (fchown(fd, exim_uid, exim_gid) < 0)
+    if (exim_fchown(fd, exim_uid, exim_gid, filename) < 0)

       {
       *error = US"chown";
       return -1;
       {
       *error = US"chown";
       return -1;


@@ -367,7 +367,7 @@ for (int i = 2; i > 0; i--)
                        MSGLOG_DIRECTORY_MODE, TRUE);
   }
 
                        MSGLOG_DIRECTORY_MODE, TRUE);
   }
 

-*error = US"create";
+*error = US"create or open";

 return -1;
 }
 
 return -1;
 }
 


@@ -7072,7 +7072,7 @@ if (addr_local || addr_remote)
     that the mode is correct - the group setting doesn't always seem to get
     set automatically. */
 
     that the mode is correct - the group setting doesn't always seem to get
     set automatically. */
 

-    if(  fchown(journal_fd, exim_uid, exim_gid)
+    if(  exim_fchown(journal_fd, exim_uid, exim_gid, fname)

       || fchmod(journal_fd, SPOOL_MODE)
 #ifndef O_CLOEXEC
       || fcntl(journal_fd, F_SETFD, fcntl(journal_fd, F_GETFD) | FD_CLOEXEC)
       || fchmod(journal_fd, SPOOL_MODE)
 #ifndef O_CLOEXEC
       || fcntl(journal_fd, F_SETFD, fcntl(journal_fd, F_GETFD) | FD_CLOEXEC)

diff --git a/src/src/directory.c b/src/src/directory.c
index e5b655186c35abf314a14f228453cfcca3ecae08..2d4d565f4ede31504743571a5b1ded263d0ebc42 100644 (file)
--- a/src/src/directory.c
+++ b/src/src/directory.c
@@ -69,7 +69,7 @@ while (c && *p)
 
     /* Set the ownership if necessary. */
 
 
     /* Set the ownership if necessary. */
 

-    if (use_chown && Uchown(path, exim_uid, exim_gid))
+    if (use_chown && exim_chown(path, exim_uid, exim_gid))

       { p = US"set owner on"; goto bad; }
 
     /* It appears that any mode bits greater than 0777 are ignored by
       { p = US"set owner on"; goto bad; }
 
     /* It appears that any mode bits greater than 0777 are ignored by

diff --git a/src/src/exim.c b/src/src/exim.c
index 7c9aa0e3fff2004a6654f3ef0a26d551c70488ca..2dbc41162513504d9c5df13b8815df743b6a0e8d 100644 (file)
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -473,8 +473,6 @@ return f;
 }
 
 
 }
 
 

-
-

 /*************************************************
 *   Ensure stdin, stdout, and stderr exist       *
 *************************************************/
 /*************************************************
 *   Ensure stdin, stdout, and stderr exist       *
 *************************************************/


@@ -687,6 +685,36 @@ vfprintf(stderr, fmt, ap);
 exit(EXIT_FAILURE);
 }
 
 exit(EXIT_FAILURE);
 }
 

+/* exim_chown_failure() called from exim_chown()/exim_fchown() on failure
+of chown()/fchown().  See src/functions.h for more explanation */
+int
+exim_chown_failure(int fd, const uschar *name, uid_t owner, gid_t group)
+{
+#if 1
+log_write(0, LOG_MAIN|LOG_PANIC,
+  __FILE__ ":%d: chown(%s, %d:%d) failed (%s)."
+  " Please contact the authors and refer to https://bugs.exim.org/show_bug.cgi?id=2391",
+  __LINE__, name?name:US"<unknown>", owner, group, strerror(errno));
+#else
+/* I leave this here, commented, in case the "bug"(?) comes up again.
+   It is not an Exim bug, but we can provide a workaround.
+   See Bug 2391
+   HS 2019-04-18 */
+
+int saved_errno = errno;  /* from the preceeding chown call */
+struct stat buf;
+
+if (0 == (fd < 0 ? stat(name, &buf) : fstat(fd, &buf)))
+{
+  if (buf.st_uid == owner && buf.st_gid == group) return 0;
+  log_write(0, LOG_MAIN|LOG_PANIC, "Wrong ownership on %s", name);
+}
+else log_write(0, LOG_MAIN|LOG_PANIC, "Stat failed on %s: %s", name, strerror(errno));
+
+errno = saved_errno;
+return -1;
+#endif
+}

 
 
 /*************************************************
 
 
 /*************************************************

diff --git a/src/src/functions.h b/src/src/functions.h
index 4193caccbd2c3f9e0d4c3b6587ea9fe1f0137bcf..c5af51652030ac8e8f08487faadbe2aa285edf69 100644 (file)
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -10,6 +10,8 @@
 to avoid having a lot of tiddly little headers with only a couple of lines in
 them. However, some functions that are used (or not used) by utility programs
 are in in fact in separate headers. */
 to avoid having a lot of tiddly little headers with only a couple of lines in
 them. However, some functions that are used (or not used) by utility programs
 are in in fact in separate headers. */

+#ifndef _FUNCTIONS_H_
+#define _FUNCTIONS_H_

 
 
 #ifdef EXIM_PERL
 
 
 #ifdef EXIM_PERL


@@ -209,6 +211,10 @@ extern BOOL    enq_start(uschar *, unsigned);
 extern uschar *event_raise(uschar *, const uschar *, uschar *);
 extern void    msg_event_raise(const uschar *, const address_item *);
 #endif
 extern uschar *event_raise(uschar *, const uschar *, uschar *);
 extern void    msg_event_raise(const uschar *, const address_item *);
 #endif

+
+extern int     exim_chown_failure(int, const uschar*, uid_t, gid_t);
+inline int     exim_chown(const uschar*, uid_t, gid_t);
+inline int     exim_fchown(int, uid_t, gid_t, const uschar*);

 extern const uschar * exim_errstr(int);
 extern void    exim_exit(int, const uschar *);
 extern void    exim_nullstd(void);
 extern const uschar * exim_errstr(int);
 extern void    exim_exit(int, const uschar *);
 extern void    exim_nullstd(void);


@@ -585,6 +591,39 @@ extern void    version_init(void);
 extern BOOL    write_chunk(transport_ctx *, uschar *, int);
 extern ssize_t write_to_fd_buf(int, const uschar *, size_t);
 
 extern BOOL    write_chunk(transport_ctx *, uschar *, int);
 extern ssize_t write_to_fd_buf(int, const uschar *, size_t);
 

+/* exim_chown - in some NFSv4 setups *seemes* to be an issue with
+   chown(<exim-uid>, <exim-gid>).
+
+   Probably because the idmapping is broken, misconfigured or set up in
+   an unusal way. (see Bug 2931). As I'm not sure, if this was a single
+   case of misconfiguration, or if there are more such broken systems
+   out, I try to impose as least impact as possible and for now just write
+   a panic log entry pointing to the bug report. You're encouraged to
+   contact the developers, if you experience this issue.
+
+   fd     the file descriptor (or -1 if not valid)
+   name   the file name for error messages or for file operations,
+          if fd is < 0
+   owner  the owner
+   group  the group
+
+   returns 0 on success, -1 on failure */
+
+inline int
+exim_fchown(int fd, uid_t owner, gid_t group, const uschar *name)
+{
+return (0 == fchown(fd, owner, group))
+  ? 0 : exim_chown_failure(fd, name, owner, group);
+}
+
+inline int
+exim_chown(const uschar *name, uid_t owner, gid_t group)
+{
+return (0 == chown(name, owner, group))
+  ? 0 : exim_chown_failure(-1, name, owner, group);
+}
+
+#endif  /* _FUNCTIONS_H_ */

 
 /* vi: aw
 */
 
 /* vi: aw
 */

diff --git a/src/src/mytypes.h b/src/src/mytypes.h
index ef455958cc487a5810b6331a2e52aa4c3d8b6e55..4234574c9c852919f5bfdc3cdd8b11f418321460 100644 (file)
--- a/src/src/mytypes.h
+++ b/src/src/mytypes.h
@@ -79,7 +79,6 @@ functions that are called quite often; for other calls to external libraries
 #define Uatol(s)           atol(CCS(s))
 #define Uchdir(s)          chdir(CCS(s))
 #define Uchmod(s,n)        chmod(CCS(s),n)
 #define Uatol(s)           atol(CCS(s))
 #define Uchdir(s)          chdir(CCS(s))
 #define Uchmod(s,n)        chmod(CCS(s),n)

-#define Uchown(s,n,m)      chown(CCS(s),n,m)

 #define Ufgets(b,n,f)      fgets(CS(b),n,f)
 #define Ufopen(s,t)        fopen(CCS(s),CCS(t))
 #define Ulink(s,t)         link(CCS(s),CCS(t))
 #define Ufgets(b,n,f)      fgets(CS(b),n,f)
 #define Ufopen(s,t)        fopen(CCS(s),CCS(t))
 #define Ulink(s,t)         link(CCS(s),CCS(t))

diff --git a/src/src/receive.c b/src/src/receive.c
index 64f62757d78ffddf456fc20419a4657f78ad4b4d..701d540b09121d66e542550bde6bd68cb315c06f 100644 (file)
--- a/src/src/receive.c
+++ b/src/src/receive.c
@@ -3086,7 +3086,7 @@ if ((data_fd = Uopen(spool_name, O_RDWR|O_CREAT|O_EXCL, SPOOL_MODE)) < 0)
 /* Make sure the file's group is the Exim gid, and double-check the mode
 because the group setting doesn't always get set automatically. */
 
 /* Make sure the file's group is the Exim gid, and double-check the mode
 because the group setting doesn't always get set automatically. */
 

-if (fchown(data_fd, exim_uid, exim_gid))
+if (0 != exim_fchown(data_fd, exim_uid, exim_gid, spool_name))

   log_write(0, LOG_MAIN|LOG_PANIC_DIE,
     "Failed setting ownership on spool file %s: %s",
     spool_name, strerror(errno));
   log_write(0, LOG_MAIN|LOG_PANIC_DIE,
     "Failed setting ownership on spool file %s: %s",
     spool_name, strerror(errno));


@@ -4098,7 +4098,7 @@ if (message_logs && !blackholed_by)
   {
   int fd;
   uschar * m_name = spool_fname(US"msglog", message_subdir, message_id, US"");
   {
   int fd;
   uschar * m_name = spool_fname(US"msglog", message_subdir, message_id, US"");

-  
+

   if (  (fd = Uopen(m_name, O_WRONLY|O_APPEND|O_CREAT, SPOOL_MODE)) < 0
      && errno == ENOENT
      )
   if (  (fd = Uopen(m_name, O_WRONLY|O_APPEND|O_CREAT, SPOOL_MODE)) < 0
      && errno == ENOENT
      )


@@ -4257,7 +4257,7 @@ if(!smtp_reply)
   if (f.deliver_freeze) log_write(0, LOG_MAIN, "frozen by %s", frozen_by);
   if (f.queue_only_policy) log_write(L_delay_delivery, LOG_MAIN,
     "no immediate delivery: queued%s%s by %s",
   if (f.deliver_freeze) log_write(0, LOG_MAIN, "frozen by %s", frozen_by);
   if (f.queue_only_policy) log_write(L_delay_delivery, LOG_MAIN,
     "no immediate delivery: queued%s%s by %s",

-    *queue_name ? " in " : "", *queue_name ? CS queue_name : "",       
+    *queue_name ? " in " : "", *queue_name ? CS queue_name : "",

     queued_by);
   }
 f.receive_call_bombout = FALSE;
     queued_by);
   }
 f.receive_call_bombout = FALSE;

diff --git a/src/src/spool_out.c b/src/src/spool_out.c
index a4a734a1ad196a5ba9661713d90279682ab2168a..46a490a9324266ead100f2adf3e1d05643192eda 100644 (file)
--- a/src/src/spool_out.c
+++ b/src/src/spool_out.c
@@ -92,7 +92,7 @@ double-check the mode because the group setting doesn't always get set
 automatically. */
 
 if (fd >= 0)
 automatically. */
 
 if (fd >= 0)

-  if (fchown(fd, exim_uid, exim_gid) || fchmod(fd, SPOOL_MODE))
+  if (exim_fchown(fd, exim_uid, exim_gid, temp_name) || fchmod(fd, SPOOL_MODE))

     {
     DEBUG(D_any) debug_printf("failed setting perms on %s\n", temp_name);
     (void) close(fd); fd = -1;
     {
     DEBUG(D_any) debug_printf("failed setting perms on %s\n", temp_name);
     (void) close(fd); fd = -1;

diff --git a/src/src/tls-gnu.c b/src/src/tls-gnu.c
index ad55f95cdae975fc8085b43e4b54fdff4265eab0..fe048ba62b417b171aad613b681a8d22754974aa 100644 (file)
--- a/src/src/tls-gnu.c
+++ b/src/src/tls-gnu.c
@@ -705,7 +705,7 @@ if (rc < 0)
   temp_fn = string_copy(US"%s.XXXXXXX");
   if ((fd = mkstemp(CS temp_fn)) < 0)  /* modifies temp_fn */
     return tls_error_sys(US"Unable to open temp file", errno, NULL, errstr);
   temp_fn = string_copy(US"%s.XXXXXXX");
   if ((fd = mkstemp(CS temp_fn)) < 0)  /* modifies temp_fn */
     return tls_error_sys(US"Unable to open temp file", errno, NULL, errstr);

-  (void)fchown(fd, exim_uid, exim_gid);   /* Probably not necessary */
+  (void)exim_chown(temp_fn, exim_uid, exim_gid);   /* Probably not necessary */

 
   /* GnuTLS overshoots!
    * If we ask for 2236, we might get 2237 or more.
 
   /* GnuTLS overshoots!
    * If we ask for 2236, we might get 2237 or more.

diff --git a/src/src/transports/appendfile.c b/src/src/transports/appendfile.c
index 1e92add35ddf1610a5be3ee19ddfd119a0b25b8a..d9f8d4989a7161763afb2090aaa92a2ae75a5e6b 100644 (file)
--- a/src/src/transports/appendfile.c
+++ b/src/src/transports/appendfile.c
@@ -1798,7 +1798,7 @@ if (!isdirectory)
       /* We have successfully created and opened the file. Ensure that the group
       and the mode are correct. */
 
       /* We have successfully created and opened the file. Ensure that the group
       and the mode are correct. */
 

-      if(Uchown(filename, uid, gid) || Uchmod(filename, mode))
+      if(exim_chown(filename, uid, gid) || Uchmod(filename, mode))

         {
         addr->basic_errno = errno;
         addr->message = string_sprintf("while setting perms on mailbox %s",
         {
         addr->basic_errno = errno;
         addr->message = string_sprintf("while setting perms on mailbox %s",


@@ -2606,7 +2606,7 @@ else
     /* Why are these here? Put in because they are present in the non-maildir
     directory case above. */
 
     /* Why are these here? Put in because they are present in the non-maildir
     directory case above. */
 

-    if(Uchown(filename, uid, gid) || Uchmod(filename, mode))
+    if(exim_chown(filename, uid, gid) || Uchmod(filename, mode))

       {
       addr->basic_errno = errno;
       addr->message = string_sprintf("while setting perms on maildir %s",
       {
       addr->basic_errno = errno;
       addr->message = string_sprintf("while setting perms on maildir %s",


@@ -2652,7 +2652,7 @@ else
     /* Why are these here? Put in because they are present in the non-maildir
     directory case above. */
 
     /* Why are these here? Put in because they are present in the non-maildir
     directory case above. */
 

-    if(Uchown(filename, uid, gid) || Uchmod(filename, mode))
+    if(exim_chown(filename, uid, gid) || Uchmod(filename, mode))

       {
       addr->basic_errno = errno;
       addr->message = string_sprintf("while setting perms on file %s",
       {
       addr->basic_errno = errno;
       addr->message = string_sprintf("while setting perms on file %s",


@@ -2749,7 +2749,7 @@ else
       Uunlink(filename);
       return FALSE;
       }
       Uunlink(filename);
       return FALSE;
       }

-    if(Uchown(dataname, uid, gid) || Uchmod(dataname, mode))
+    if(exim_chown(dataname, uid, gid) || Uchmod(dataname, mode))

       {
       addr->basic_errno = errno;
       addr->message = string_sprintf("while setting perms on file %s",
       {
       addr->basic_errno = errno;
       addr->message = string_sprintf("while setting perms on file %s",


@@ -2764,7 +2764,7 @@ else
   /* In all cases of writing to a new file, ensure that the file which is
   going to be renamed has the correct ownership and mode. */
 
   /* In all cases of writing to a new file, ensure that the file which is
   going to be renamed has the correct ownership and mode. */
 

-  if(Uchown(filename, uid, gid) || Uchmod(filename, mode))
+  if(exim_chown(filename, uid, gid) || Uchmod(filename, mode))

     {
     addr->basic_errno = errno;
     addr->message = string_sprintf("while setting perms on file %s",
     {
     addr->basic_errno = errno;
     addr->message = string_sprintf("while setting perms on file %s",