I need sleep, so I'm sending out what I have.
This change goes too far and lets users other than the Exim user also
specify -D options without causing a privilege drop.
That should be a trivial change around line 3254 (post-patch) but I'm no
longer focusing enough to be sure I'm not missing other stuff.
So, besides that, and the lack of documentation, do any maintainers have
any comments on this approach? David?
-Phil
---
src/src/EDITME | 25 ++++++++++
src/src/buildconfig.c | 6 ++-
src/src/config.h.defaults | 2 +
src/src/exim.c | 108 ++++++++++++++++++++++++++++++++++++++++++++-
src/src/globals.c | 3 +
src/src/globals.h | 3 +
6 files changed, 144 insertions(+), 3 deletions(-)
diff --git a/src/src/EDITME b/src/src/EDITME
index 1ae1399..d093eb6 100644
--- a/src/src/EDITME
+++ b/src/src/EDITME
@@ -494,6 +494,31 @@ FIXED_NEVER_USERS=root
#------------------------------------------------------------------------------
+# By contrast, you might be maintaining a system which relies upon the ability
+# to override values with -D and assumes that these will be passed through to
+# the delivery processes. As of Exim 4.73, this is no longer the case by
+# default. Going forward, we strongly recommend that you use a shim Exim
+# configuration file owned by root stored under TRUSTED_CONFIG_PREFIX_LIST.
+# That shim can set macros before .include'ing your main configuration file.
+#
+# As a strictly transient measure to ease migration to 4.73, the
+# WHITELIST_D_MACROS value definies a colon-separated list of macro-names
+# which are permitted to be overriden from the command-line which will be
+# honoured by the Exim user. So these are macros that can persist to delivery
+# time.
+# Examples might be -DTLS or -DSPOOL=/some/dir. The values on the
+# command-line are filtered to only permit: [A-Za-z0-9_/.-]*
+#
+# This option is highly likely to be removed in a future release. It exists
+# only to make 4.73 as easy as possible to migrate to. If you use it, we
+# encourage you to schedule time to rework your configuration to not depend
+# upon it. Most people should not need to use this.
+#
+# By default, no macros are whitelisted for -D usage.
+
+# WHITELIST_D_MACROS=TLS:SPOOL
+
+#------------------------------------------------------------------------------
# Exim has support for the AUTH (authentication) extension of the SMTP
# protocol, as defined by RFC 2554. If you don't know what SMTP authentication
# is, you probably won't want to include this code, so you should leave these
diff --git a/src/src/buildconfig.c b/src/src/buildconfig.c
index 7e2790b..49d8c41 100644
--- a/src/src/buildconfig.c
+++ b/src/src/buildconfig.c
@@ -800,11 +800,13 @@ while (fgets(buffer, sizeof(buffer), base) != NULL)
fprintf(new, "\"%s\"\n", value);
}
- /* Timezone values HEADERS_CHARSET, and TCP_WRAPPERS_DAEMON_NAME get quoted */
+ /* Timezone values HEADERS_CHARSET, TCP_WRAPPERS_DAEMON_NAME and
+ WHITELIST_D_MACROS get quoted */
else if (strcmp(name, "TIMEZONE_DEFAULT") == 0||
strcmp(name, "TCP_WRAPPERS_DAEMON_NAME") == 0||
- strcmp(name, "HEADERS_CHARSET") == 0)
+ strcmp(name, "HEADERS_CHARSET") == 0||
+ strcmp(name, "WHITELIST_D_MACROS") == 0)
fprintf(new, "\"%s\"\n", value);
/* For others, quote any paths and don't quote anything else */
diff --git a/src/src/config.h.defaults b/src/src/config.h.defaults
index d3d7d65..b4e2c6d 100644
--- a/src/src/config.h.defaults
+++ b/src/src/config.h.defaults
@@ -145,6 +145,8 @@ it's a default value. */
#define USE_TCP_WRAPPERS
#define USE_TDB
+#define WHITELIST_D_MACROS
+
#define WITH_CONTENT_SCAN
#define WITH_OLD_DEMIME
#define WITH_OLD_CLAMAV_STREAM
diff --git a/src/src/exim.c b/src/src/exim.c
index 729114c..f50a62b 100644
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -1132,6 +1132,102 @@ exit(EXIT_FAILURE);
/*************************************************
+* Validate that the macros given are okay *
+*************************************************/
+
+/* Typically, Exim will drop privileges if macros are supplied. In some
+cases, we want to not do so.
+
+Arguments: none (macros is a global)
+Returns: true if trusted, false otherwise
+*/
+
+static BOOL
+macros_trusted(void)
+{
+#ifdef WHITELIST_D_MACROS
+macro_item *m;
+uschar *whitelisted, *end, *p, **whites, **w;
+int white_count, i, n;
+size_t len;
+BOOL prev_char_item, found;
+#endif
+
+if (macros == NULL)
+ return TRUE;
+#ifndef WHITELIST_D_MACROS
+return FALSE;
+#else
+
+/* Get a list of macros which are whitelisted */
+whitelisted = string_copy_malloc(US WHITELIST_D_MACROS);
+prev_char_item = FALSE;
+white_count = 0;
+for (p = whitelisted; *p != '\0'; ++p)
+ {
+ if (*p == ':' || isspace(*p))
+ {
+ *p = '\0';
+ if (prev_char_item)
+ ++white_count;
+ prev_char_item = FALSE;
+ continue;
+ }
+ if (!prev_char_item)
+ prev_char_item = TRUE;
+ }
+end = p;
+if (prev_char_item)
+ ++white_count;
+if (!white_count)
+ return FALSE;
+whites = store_malloc(sizeof(uschar *) * (white_count+1));
+for (p = whitelisted, i = 0; (p != end) && (i < white_count); ++p)
+ {
+ if (*p != '\0')
+ {
+ whites[i++] = p;
+ if (i == white_count)
+ break;
+ while (*p != '\0' && p < end)
+ ++p;
+ }
+ }
+whites[i] = NULL;
+
+/* The list of macros should be very short. Accept the N*M complexity. */
+for (m = macros; m != NULL; m = m->next)
+ {
+ found = FALSE;
+ for (w = whites; *w; ++w)
+ if (Ustrcmp(*w, m->name) == 0)
+ {
+ found = TRUE;
+ break;
+ }
+ if (!found)
+ return FALSE;
+ if (m->replacement == NULL)
+ continue;
+ len = Ustrlen(m->replacement);
+ if (len == 0)
+ continue;
+ n = pcre_exec(regex_whitelisted_macro, NULL, CS m->replacement, len,
+ 0, PCRE_EOPT, NULL, 0);
+ if (n < 0)
+ {
+ if (n != PCRE_ERROR_NOMATCH)
+ debug_printf("macros_trusted checking %s returned %d\n", m->name, n);
+ return FALSE;
+ }
+ }
+debug_printf("macros_trusted overriden to true by whitelisting\n");
+return TRUE;
+#endif
+}
+
+
+/*************************************************
* Entry point and high-level code *
*************************************************/
@@ -1417,6 +1513,15 @@ regex_smtp_code =
regex_must_compile(US"^\\d\\d\\d\\s(?:\\d\\.\\d\\d?\\d?\\.\\d\\d?\\d?\\s)?",
FALSE, TRUE);
+#ifdef WHITELIST_D_MACROS
+/* Precompile the regular expression used to filter the content of macros
+given to -D for permissibility. */
+
+regex_whitelisted_macro =
+ regex_must_compile(US"^[A-Za-z0-9_/.-]*$", FALSE, TRUE);
+#endif
+
+
/* If the program is called as "mailq" treat it as equivalent to "exim -bp";
this seems to be a generally accepted convention, since one finds symbolic
links called "mailq" in standard OS configurations. */
@@ -3145,7 +3250,8 @@ values (such as the path name). If running in the test harness, pretend that
configuration file changes and macro definitions haven't happened. */
if (( /* EITHER */
- (!trusted_config || macros != NULL) && /* Config changed, and */
+ (!trusted_config || /* Config changed, or */
+ !macros_trusted()) && /* impermissible macros and */
real_uid != root_uid && /* Not root, and */
!running_in_test_harness /* Not fudged */
) || /* OR */
diff --git a/src/src/globals.c b/src/src/globals.c
index 500691c..71de52a 100644
--- a/src/src/globals.c
+++ b/src/src/globals.c
@@ -930,6 +930,9 @@ const pcre *regex_PIPELINING = NULL;
const pcre *regex_SIZE = NULL;
const pcre *regex_smtp_code = NULL;
const pcre *regex_ismsgid = NULL;
+#ifdef WHITELIST_D_MACROS
+const pcre *regex_whitelisted_macro = NULL;
+#endif
#ifdef WITH_CONTENT_SCAN
uschar *regex_match_string = NULL;
#endif
diff --git a/src/src/globals.h b/src/src/globals.h
index 62dd6b0..b4e3f2a 100644
--- a/src/src/globals.h
+++ b/src/src/globals.h
@@ -594,6 +594,9 @@ extern const pcre *regex_PIPELINING; /* For recognizing PIPELINING */
extern const pcre *regex_SIZE; /* For recognizing SIZE settings */
extern const pcre *regex_smtp_code; /* For recognizing SMTP codes */
extern const pcre *regex_ismsgid; /* Compiled r.e. for message it */
+#ifdef WHITELIST_D_MACROS
+extern const pcre *regex_whitelisted_macro; /* For -D macro values */
+#endif
#ifdef WITH_CONTENT_SCAN
extern uschar *regex_match_string; /* regex that matched a line (regex ACL condition) */
#endif
--
1.7.3.2
