Re: [exim-dev] Candidate patches for privilege escalation

Top Page
Delete this message
Reply to this message
Author: David Woodhouse
Date:  
To: Andreas Metzler
CC: exim-dev
Subject: Re: [exim-dev] Candidate patches for privilege escalation
On Thu, 2010-12-16 at 20:25 +0000, David Woodhouse wrote:
> Screw it, let's forget the prefix and regex options. The
> TRUSTED_CONFIG_LIST can just be a list of simple filenames. Must be a
> *precise* match to be honoured as root. Why do anything more?


?

From 90b6341f7282beed1175e942a113c30c212425c9 Mon Sep 17 00:00:00 2001
From: David Woodhouse <David.Woodhouse@???>
Date: Thu, 16 Dec 2010 22:29:53 +0000
Subject: [PATCH] Turn TRUSTED_CONFIG_PREFIX_LIST into TRUSTED_CONFIG_LIST. No prefix or regexes

---
 doc/doc-docbook/spec.xfpt       |   45 +++++++++++++++++++--------------------
 doc/doc-txt/ChangeLog           |    2 +
 doc/doc-txt/IncompatibleChanges |    6 ++--
 doc/doc-txt/NewStuff            |   20 ++++++++--------
 src/src/EDITME                  |   15 ++++++-------
 src/src/config.h.defaults       |    2 +-
 src/src/exim.c                  |   34 +++++++++++++---------------
 7 files changed, 61 insertions(+), 63 deletions(-)


diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index c9b77b8..22815a9 100644
--- a/doc/doc-docbook/spec.xfpt
+++ b/doc/doc-docbook/spec.xfpt
@@ -3334,13 +3334,13 @@ proceeding any further along the list, and an error is generated.
When this option is used by a caller other than root, and the list is different
from the compiled-in list, Exim gives up its root privilege immediately, and
runs with the real and effective uid and gid set to those of the caller.
-However, if a TRUSTED_CONFIG_PREFIX_LIST file is defined in &_Local/Makefile_&,
-root privilege is retained for any configuration file which matches a prefix
-listed in that file as long as the caller is the Exim user (or the user
-specified in the CONFIGURE_OWNER option, if any).
+However, if a TRUSTED_CONFIG_LIST file is defined in &_Local/Makefile_&, root
+privilege is retained for any configuration file which is listed in that file
+as long as the caller is the Exim user (or the user specified in the
+CONFIGURE_OWNER option, if any).

-Leaving TRUSTED_CONFIG_PREFIX_LIST unset precludes the possibility of testing
-a configuration using &%-C%& right through message reception and delivery,
+Leaving TRUSTED_CONFIG_LIST unset precludes the possibility of testing a
+configuration using &%-C%& right through message reception and delivery,
even if the caller is root. The reception works, but by that time, Exim is
running as the Exim user, so when it re-executes to regain privilege for the
delivery, the use of &%-C%& causes privilege to be lost. However, root can
@@ -4537,17 +4537,16 @@ A one-off alternate configuration can be specified by the &%-C%& command line
option, which may specify a single file or a list of files. However, when
&%-C%& is used, Exim gives up its root privilege, unless called by root (or
unless the argument for &%-C%& is identical to the built-in value from
-CONFIGURE_FILE), or matches a prefix listed in the TRUSTED_CONFIG_PREFIX_LIST
-file and the caller is the Exim user or the user specified in the
-CONFIGURE_OWNER setting. &%-C%& is useful mainly for checking the syntax of
-configuration files before installing them. No owner or group checks are done
-on a configuration file specified by &%-C%&, if root privilege has been
-dropped.
+CONFIGURE_FILE), or is listed in the TRUSTED_CONFIG_LIST file and the caller
+is the Exim user or the user specified in the CONFIGURE_OWNER setting. &%-C%&
+is useful mainly for checking the syntax of configuration files before
+installing them. No owner or group checks are done on a configuration file
+specified by &%-C%&, if root privilege has been dropped.

 Even the Exim user is not trusted to specify an arbitrary configuration file
 with the &%-C%& option to be used with root privileges, unless that file is
-listed in the TRUSTED_CONFIG_PREFIX_LIST file. This locks out the possibility
-of testing a configuration using &%-C%& right through message reception and
+listed in the TRUSTED_CONFIG_LIST file. This locks out the possibility of
+testing a configuration using &%-C%& right through message reception and
 delivery, even if the caller is root. The reception works, but by that time,
 Exim is running as the Exim user, so when it re-execs to regain privilege for
 the delivery, the use of &%-C%& causes privilege to be lost. However, root
@@ -33824,15 +33823,15 @@ into the Exim account from running a privileged Exim with an arbitrary
 configuration file, and using it to break into other accounts.
 .next
 If a non-trusted configuration file (i.e. not the default configuration file
-or one which is trusted by virtue of matching a prefix listed in the
-TRUSTED_CONFIG_PREFIX_LIST file) is specified with &%-C%&, or if macros are
-given with &%-D%& (but see the next item), then root privilege is retained only
-if the caller of Exim is root. This locks out the possibility of testing a
-configuration using &%-C%& right through message reception and delivery, even
-if the caller is root. The reception works, but by that time, Exim is running
-as the Exim user, so when it re-execs to regain privilege for the delivery, the
-use of &%-C%& causes privilege to be lost. However, root can test reception and
-delivery using two separate commands.
+or one which is trusted by virtue of being listed in the TRUSTED_CONFIG_LIST
+file) is specified with &%-C%&, or if macros are given with &%-D%& (but see
+the next item), then root privilege is retained only if the caller of Exim is
+root. This locks out the possibility of testing a configuration using &%-C%&
+right through message reception and delivery, even if the caller is root. The
+reception works, but by that time, Exim is running as the Exim user, so when
+it re-execs to regain privilege for the delivery, the use of &%-C%& causes
+privilege to be lost. However, root can test reception and delivery using two
+separate commands.
 .next
 The WHITELIST_D_MACROS build option declares some macros to be safe to override
 with &%-D%& if the real uid is one of root, the Exim run-time user or the
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index f405cda..07501bb 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -106,6 +106,8 @@ DW/30 Allow TRUSTED_CONFIG_PREFIX_FILE only for Exim or CONFIGURE_OWNER, not
       for other users. Others should always drop root privileges if they use
       -C on the command line, even for a whitelisted configure file.


+DW/31 Turn TRUSTED_CONFIG_PREFIX_FILE into TRUSTED_CONFIG_FILE. No prefixes.
+

 Exim version 4.72
 -----------------
diff --git a/doc/doc-txt/IncompatibleChanges b/doc/doc-txt/IncompatibleChanges
index 8f07d78..50bf186 100644
--- a/doc/doc-txt/IncompatibleChanges
+++ b/doc/doc-txt/IncompatibleChanges
@@ -39,9 +39,9 @@ Exim version 4.73
    on; the Exim user can, by default, no longer use -C/-D and retain privilege.
    Two new build options mitigate this.


-    * TRUSTED_CONFIG_PREFIX_LIST defines a path prefix within which files
-      owned by root can be used by the Exim user; this is the recommended
-      approach going forward.
+    * TRUSTED_CONFIG_LIST defines a file containing a whitelist of config
+      files that are trusted to be selected by the Exim user; this is the
+      recommended approach going forward.


     * WHITELIST_D_MACROS defines a colon-separated list of macro names which
       the Exim run-time user may safely pass without dropping privileges.
diff --git a/doc/doc-txt/NewStuff b/doc/doc-txt/NewStuff
index b9d88ff..a732d9b 100644
--- a/doc/doc-txt/NewStuff
+++ b/doc/doc-txt/NewStuff
@@ -102,19 +102,19 @@ Version 4.73


 12. [POSSIBLE CONFIG BREAKAGE] ALT_CONFIG_ROOT_ONLY is no longer optional and
     is forced on.  This is mitigated by the new build option
-    TRUSTED_CONFIG_PREFIX_LIST which defines a list of pathname prefices which
-    are trusted; if a config file is owned by root and is under that prefix,
-    then it may be used by the Exim run-time user.
+    TRUSTED_CONFIG_LIST which defines a list of configuration files which
+    are trusted; if a config file is owned by root and matches a pathname in
+    the list, then it may be invoked by the Exim build-time user without Exim
+    relinquishing root privileges.


 13. [POSSIBLE CONFIG BREAKAGE] The Exim user is no longer automatically
     trusted to supply -D<Macro[=Value]> overrides on the command-line.  Going
-    forward, we recommend using TRUSTED_CONFIG_PREFIX_LIST with shim configs
-    that include the main config.  As a transition mechanism, we are
-    temporarily providing a work-around: the new build option
-    WHITELIST_D_MACROS provides a colon-separated list of macro names which
-    may be overriden by the Exim run-time user.  The values of these macros
-    are constrained to the regex ^[A-Za-z0-9_/.-]*$ (which explicitly does
-    allow for empty values).
+    forward, we recommend using TRUSTED_CONFIG_LIST with shim configs that
+    include the main config.  As a transition mechanism, we are temporarily
+    providing a work-around: the new build option WHITELIST_D_MACROS provides
+    a colon-separated list of macro names which may be overriden by the Exim
+    run-time user.  The values of these macros are constrained to the regex
+    ^[A-Za-z0-9_/.-]*$ (which explicitly does allow for empty values).



Version 4.72
diff --git a/src/src/EDITME b/src/src/EDITME
index ade6a7c..1bb60be 100644
--- a/src/src/EDITME
+++ b/src/src/EDITME
@@ -476,14 +476,13 @@ FIXED_NEVER_USERS=root
# When a user other than root uses the -C option to override the configuration
# file (including the Exim user when re-executing Exim to regain root
# privileges for local message delivery), this will normally cause Exim to
-# drop root privileges. The TRUSTED_CONFIG_PREFIX_LIST option, specifies
-# a file which contains a list of trusted configuration prefixes (like the
-# ALT_CONFIG_PREFIX above), one per line. If the -C option is used by the Exim
-# user or by the user specified in the CONFIGURE_OWNER setting, to specify a
-# configuration file which matches a trusted prefix, root privileges are not
-# dropped by Exim.
-
-# TRUSTED_CONFIG_PREFIX_LIST=/usr/exim/trusted_configs
+# drop root privileges. The TRUSTED_CONFIG_LIST option, specifies a file which
+# contains a list of trusted configuration filenames, one per line. If the -C
+# option is used by the Exim user or by the user specified in the
+# CONFIGURE_OWNER setting, to specify a configuration file which is listed in
+# the TRUSTED_CONFIG_LIST file, then root privileges are not dropped by Exim.
+
+# TRUSTED_CONFIG_LIST=/usr/exim/trusted_configs


#------------------------------------------------------------------------------
diff --git a/src/src/config.h.defaults b/src/src/config.h.defaults
index b4e2c6d..5cff6ad 100644
--- a/src/src/config.h.defaults
+++ b/src/src/config.h.defaults
@@ -13,7 +13,7 @@ in config.h unless some value is defined in Local/Makefile. If there is data,
it's a default value. */

#define ALT_CONFIG_PREFIX
-#define TRUSTED_CONFIG_PREFIX_LIST
+#define TRUSTED_CONFIG_LIST

 #define APPENDFILE_MODE            0600
 #define APPENDFILE_DIRECTORY_MODE  0700
diff --git a/src/src/exim.c b/src/src/exim.c
index d506721..dce42f0 100644
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -1971,17 +1971,17 @@ for (i = 1; i < argc; i++)
       #endif
       if (real_uid != root_uid)
         {
-        #ifdef TRUSTED_CONFIG_PREFIX_LIST
+        #ifdef TRUSTED_CONFIG_LIST


-        if ((real_uid != exim_uid
-             #ifdef CONFIGURE_OWNER
-             && real_uid != config_uid
-             #endif
-             ) || Ustrstr(argrest, "/../"))
+        if (real_uid != exim_uid
+            #ifdef CONFIGURE_OWNER
+            && real_uid != config_uid
+            #endif
+            )
           trusted_config = FALSE;
         else
           {
-          FILE *trust_list = Ufopen(TRUSTED_CONFIG_PREFIX_LIST, "rb");
+          FILE *trust_list = Ufopen(TRUSTED_CONFIG_LIST, "rb");
           if (trust_list)
             {
             struct stat statbuf;
@@ -2007,8 +2007,8 @@ for (i = 1; i < argc; i++)
               {
               /* Well, the trust list at least is up to scratch... */
               void *reset_point = store_get(0);
-              uschar *trusted_prefixes[32];
-              int nr_prefixes = 0;
+              uschar *trusted_configs[32];
+              int nr_configs = 0;
               int i = 0;


               while (Ufgets(big_buffer, big_buffer_size, trust_list))
@@ -2021,13 +2021,13 @@ for (i = 1; i < argc; i++)
                 nl = Ustrchr(start, '\n');
                 if (nl)
                   *nl = 0;
-                trusted_prefixes[nr_prefixes++] = string_copy(start);
-                if (nr_prefixes == 32)
+                trusted_configs[nr_configs++] = string_copy(start);
+                if (nr_configs == 32)
                   break;
                 }
               fclose(trust_list);


-              if (nr_prefixes)
+              if (nr_configs)
                 {
                 int sep = 0;
                 uschar *list = argrest;
@@ -2035,14 +2035,12 @@ for (i = 1; i < argc; i++)
                 while (trusted_config && (filename = string_nextinlist(&list,
                         &sep, big_buffer, big_buffer_size)) != NULL)
                   {
-                  for (i=0; i < nr_prefixes; i++)
+                  for (i=0; i < nr_configs; i++)
                     {
-                    int len = Ustrlen(trusted_prefixes[i]);
-                    if (Ustrlen(filename) >= len &&
-                        Ustrncmp(filename, trusted_prefixes[i], len) == 0)
+                    if (Ustrcmp(filename, trusted_configs[i]) == 0)
                       break;
                     }
-                  if (i == nr_prefixes)
+                  if (i == nr_configs)
                     {
                     trusted_config = FALSE;
                     break;
@@ -3487,7 +3485,7 @@ if (removed_privilege && (!trusted_config || macros != NULL) &&
   else
     log_write(0, LOG_MAIN|LOG_PANIC,
       "exim user lost privilege for using %s option",
-      (int)exim_uid, trusted_config? "-D" : "-C");
+      trusted_config? "-D" : "-C");
   }


/* Start up Perl interpreter if Perl support is configured and there is a
--
1.7.3.3




--
dwmw2