4c1f82306c
Looks ok. Thx! OBS-URL: https://build.opensuse.org/request/show/55623 OBS-URL: https://build.opensuse.org/package/show/server:mail/exim?expand=0&rev=91
129 lines
6.1 KiB
Diff
129 lines
6.1 KiB
Diff
Bugzilla: bnc#658731
|
||
Author: David Woodhouse
|
||
Date: 2010-12-09 17:22 UTC
|
||
To: exim-dev
|
||
Subject: Re: [exim-dev] [Exim-maintainers] Remote root vulnerability in Exim
|
||
[Resend to exim-dev for comment, since exim-maintainers is quiet]
|
||
|
||
On Thu, 2010-12-09 at 10:18 +0000, David Woodhouse wrote:
|
||
> In the meantime, I'm looking at the second bug; the privilege
|
||
> escalation. I think the best answer to that is to kill the
|
||
> ALT_CONFIG_ROOT_ONLY option completely, and *always* allow the trusted
|
||
> user to use an alternative config but *only* if that config is listed in
|
||
> a file such as /etc/exim/allowed-configs. Does that seem sane?
|
||
|
||
Hm, we already have ALT_CONFIG_PREFIX which *almost* does this. It's
|
||
just that when defined, it refuses to run at all with a config that
|
||
doesn't match the prefix; rather than simply running without privileges.
|
||
|
||
I don't really see the point in refusing to run at all; the user can
|
||
always build their *own* Exim binary and run it without privileges.
|
||
|
||
So I've come up with the patch below. It kills the ALT_CONFIG_ROOT_ONLY
|
||
option, by making it effectively always set and never allowing even the
|
||
trusted Exim user to use arbitrary config files.
|
||
|
||
It repurposes the existing ALT_CONFIG_PREFIX, rather than adding a new
|
||
option and making things more complex. Instead of refusing to run with
|
||
config files that don't match the prefix, Exim will simply run without
|
||
root privileges. And it *will* run with root privileges when invoked
|
||
with the -C option for a file which *does* match the prefix.
|
||
|
||
Important user-visible changes would be:
|
||
|
||
- If you previously had ALT_CONFIG_ROOT_ONLY unset (the default), then
|
||
you must set ALT_CONFIG_PREFIX and use matching files if you want
|
||
to use alternative config files.
|
||
|
||
- If you previously had ALT_CONFIG_PREFIX set, then you should be aware
|
||
that matching config files will now be invoked with root privileges
|
||
regardless of the uid of the invoking user.
|
||
|
||
We could change the latter so that non-root and non-exim users invoking
|
||
config files in ALT_CONFIG_PREFIX are *never* granted root privs, but
|
||
I'm not sure we should. Comments?
|
||
|
||
Index: exim-4.71/src/exim.c
|
||
===================================================================
|
||
--- exim-4.71.orig/src/exim.c
|
||
+++ exim-4.71/src/exim.c
|
||
@@ -3158,12 +3158,9 @@ if (setgroups(0, NULL) != 0)
|
||
}
|
||
|
||
/* If the configuration file name has been altered by an argument on the
|
||
-command line (either a new file name or a macro definition) and the caller is
|
||
-not root or the exim user, or if this is a filter testing run, remove any
|
||
-setuid privilege the program has, and run as the underlying user.
|
||
-
|
||
-If ALT_CONFIG_ROOT_ONLY is defined, the exim user is locked out of this, which
|
||
-severely restricts the use of -C for some purposes.
|
||
+command line (either a new file name not matching ALT_CONFIG_PREFIX or a macro
|
||
+definition) and the caller is not root, or if this is a filter testing run,
|
||
+remove any setuid privilege the program has, and run as the underlying user.
|
||
|
||
Otherwise, set the real ids to the effective values (should be root unless run
|
||
from inetd, which it can either be root or the exim uid, if one is configured).
|
||
@@ -3177,9 +3174,6 @@ configuration file changes and macro def
|
||
if (( /* EITHER */
|
||
(config_changed || macros != NULL) && /* Config changed, and */
|
||
real_uid != root_uid && /* Not root, and */
|
||
- #ifndef ALT_CONFIG_ROOT_ONLY /* (when not locked out) */
|
||
- real_uid != exim_uid && /* Not exim, and */
|
||
- #endif
|
||
!running_in_test_harness /* Not fudged */
|
||
) || /* OR */
|
||
expansion_test /* expansion testing */
|
||
@@ -3367,47 +3361,16 @@ else
|
||
}
|
||
|
||
/* Handle the case when we have removed the setuid privilege because of -C or
|
||
--D. This means that the caller of Exim was not root, and, provided that
|
||
-ALT_CONFIG_ROOT_ONLY is not defined, was not the Exim user that is built into
|
||
-the binary.
|
||
-
|
||
-If ALT_CONFIG_ROOT_ONLY is not defined, there is a problem if it turns out we
|
||
-were running as the exim user defined in the configuration file (different to
|
||
-the one in the binary). The sysadmin may expect this case to retain privilege
|
||
-because "the binary was called by the Exim user", but it hasn't, because of the
|
||
-order in which it handles this stuff. There are two possibilities:
|
||
-
|
||
- (1) If deliver_drop_privilege is set, Exim is not going to re-exec in order
|
||
- to do message deliveries. Thus, the fact that it is running as a
|
||
- non-privileged user is plausible, and might be wanted in some special
|
||
- configurations. However, really_exim will have been set false when
|
||
- privilege was dropped, to stop Exim trying to write to its normal log
|
||
- files. Therefore, re-enable normal log processing, assuming the sysadmin
|
||
- has set up the log directory correctly.
|
||
-
|
||
- (2) If deliver_drop_privilege is not set, the configuration won't work as
|
||
- apparently intended, and so we log a panic message. In order to retain
|
||
- root for -C or -D, the caller must either be root or the Exim user
|
||
- defined in the binary (when deliver_drop_ privilege is false).
|
||
-
|
||
-If ALT_CONFIG_ROOT_ONLY is defined, we don't know whether we were called by the
|
||
-built-in exim user or one defined in the configuration. In either event,
|
||
-re-enable log processing, assuming the sysadmin knows what they are doing. */
|
||
+-D. This means that the caller of Exim was not root.
|
||
+
|
||
+We don't know whether we were called by the built-in exim user or one defined
|
||
+in the configuration. In either event, re-enable log processing, assuming the
|
||
+sysadmin knows what they are doing. */
|
||
|
||
if (removed_privilege && (config_changed || macros != NULL) &&
|
||
real_uid == exim_uid)
|
||
{
|
||
- #ifdef ALT_CONFIG_ROOT_ONLY
|
||
really_exim = TRUE; /* let logging work normally */
|
||
- #else
|
||
-
|
||
- if (deliver_drop_privilege)
|
||
- really_exim = TRUE; /* let logging work normally */
|
||
- else
|
||
- log_write(0, LOG_MAIN|LOG_PANIC,
|
||
- "exim user (uid=%d) is defined only at runtime; privilege lost for %s",
|
||
- (int)exim_uid, config_changed? "-C" : "-D");
|
||
- #endif
|
||
}
|
||
|
||
/* Start up Perl interpreter if Perl support is configured and there is a
|