Re: [exim-dev] [PATCH] Implement -D filtering, first pass.

Páxina inicial
Borrar esta mensaxe
Responder a esta mensaxe
Autor: David Woodhouse
Data:  
Para: Phil Pennock
CC: exim-dev
Asunto: Re: [exim-dev] [PATCH] Implement -D filtering, first pass.
On Tue, 2010-12-14 at 11:11 +0000, David Woodhouse wrote:
> I did the same thing with -C (allowing users other than Exim
> to specify a config as long as it was in the trusted list). It's less
> obviously wrong for -C than it is for -D, but it's been bothering me,
> and I think we should change it too.


Thus:

diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index b2c40e4..c9b77b8 100644
--- a/doc/doc-docbook/spec.xfpt
+++ b/doc/doc-docbook/spec.xfpt
@@ -3336,7 +3336,8 @@ 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.
+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,
@@ -4536,10 +4537,12 @@ 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. &%-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 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.

 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
@@ -33820,17 +33823,16 @@ which only root has access, this guards against someone who has broken
 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. the default configuration file or
-one which is trusted by virtue of matching a prefix listed in the
+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.
+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 201d961..f405cda 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -102,6 +102,10 @@ DW/29 Remove use of va_copy() which breaks pre-C99 systems. Duplicate the
       result string, instead of calling string_vformat() twice with the same
       arguments.


+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.
+


Exim version 4.72
-----------------
diff --git a/src/src/EDITME b/src/src/EDITME
index d093eb6..ade6a7c 100644
--- a/src/src/EDITME
+++ b/src/src/EDITME
@@ -478,8 +478,9 @@ FIXED_NEVER_USERS=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 to specify
-# a configuration file which matches a trusted prefix, root privileges are not
+# 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
diff --git a/src/src/exim.c b/src/src/exim.c
index 7498682..d506721 100644
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -1973,7 +1973,11 @@ for (i = 1; i < argc; i++)
         {
         #ifdef TRUSTED_CONFIG_PREFIX_LIST


-    if (Ustrstr(argrest, "/../"))
+        if ((real_uid != exim_uid
+             #ifdef CONFIGURE_OWNER
+             && real_uid != config_uid
+             #endif
+             ) || Ustrstr(argrest, "/../"))
           trusted_config = FALSE;
         else
           {


--
dwmw2