From f42deca923414cedcbb6d6646afbef460f50080c Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Fri, 6 May 2016 13:07:18 +0100 Subject: [PATCH 1/1] Avoid exposing passwords in log, on failing ldap lookup expansion. Bug 165 --- doc/doc-txt/ChangeLog | 2 +- src/src/deliver.c | 20 +++----------------- src/src/expand.c | 23 +++++++++++++++++++++++ src/src/functions.h | 1 + src/src/rewrite.c | 17 +---------------- src/src/route.c | 17 ++--------------- 6 files changed, 31 insertions(+), 49 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 4171d61c4..3bf6fc908 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -27,7 +27,7 @@ JH/05 If main configuration option tls_certificate is unset, generate a selfsigned certificate for inbound TLS connections. JH/06 Bug 165: hide more cases of password exposure - this time in expansions - in rewrites. + in rewrites and routers. Exim version 4.87 diff --git a/src/src/deliver.c b/src/src/deliver.c index c6de1b901..743fc83e8 100644 --- a/src/src/deliver.c +++ b/src/src/deliver.c @@ -1076,23 +1076,9 @@ malformed, it won't ever have gone near LDAP.) */ if (addr->message) { const uschar * s = string_printing(addr->message); - if (s != addr->message) - addr->message = US s; - /* deconst cast ok as string_printing known to have alloc'n'copied */ - if ( ( Ustrstr(s, "failed to expand") != NULL - || Ustrstr(s, "expansion of ") != NULL - ) - && ( Ustrstr(s, "mysql") != NULL - || Ustrstr(s, "pgsql") != NULL - || Ustrstr(s, "redis") != NULL - || Ustrstr(s, "sqlite") != NULL - || Ustrstr(s, "ldap:") != NULL - || Ustrstr(s, "ldaps:") != NULL - || Ustrstr(s, "ldapi:") != NULL - || Ustrstr(s, "ldapdn:") != NULL - || Ustrstr(s, "ldapm:") != NULL - ) ) - addr->message = US"Temporary internal error"; + + /* deconst cast ok as string_printing known to have alloc'n'copied */ + addr->message = expand_hide_passwords(US s); } /* If we used a transport that has one of the "return_output" options set, and diff --git a/src/src/expand.c b/src/src/expand.c index fbbc56316..f783682b4 100644 --- a/src/src/expand.c +++ b/src/src/expand.c @@ -7631,6 +7631,29 @@ return OK; +/* Avoid potentially exposing a password in a string about to be logged */ + +uschar * +expand_hide_passwords(uschar * s) +{ +return ( ( Ustrstr(s, "failed to expand") != NULL + || Ustrstr(s, "expansion of ") != NULL + ) + && ( Ustrstr(s, "mysql") != NULL + || Ustrstr(s, "pgsql") != NULL + || Ustrstr(s, "redis") != NULL + || Ustrstr(s, "sqlite") != NULL + || Ustrstr(s, "ldap:") != NULL + || Ustrstr(s, "ldaps:") != NULL + || Ustrstr(s, "ldapi:") != NULL + || Ustrstr(s, "ldapdn:") != NULL + || Ustrstr(s, "ldapm:") != NULL + ) ) + ? US"Temporary internal error" : s; +} + + + /************************************************* ************************************************** diff --git a/src/src/functions.h b/src/src/functions.h index 454037fe1..f690e9fd9 100644 --- a/src/src/functions.h +++ b/src/src/functions.h @@ -180,6 +180,7 @@ extern int exp_bool(address_item *addr, extern BOOL expand_check_condition(uschar *, uschar *, uschar *); extern uschar *expand_string(uschar *); /* public, cannot make const */ extern const uschar *expand_cstring(const uschar *); /* ... so use this one */ +extern uschar *expand_hide_passwords(uschar * ); extern uschar *expand_string_copy(const uschar *); extern int_eximarith_t expand_string_integer(uschar *, BOOL); extern void modify_variable(uschar *, void *); diff --git a/src/src/rewrite.c b/src/src/rewrite.c index f2a7ff273..830d2bb8d 100644 --- a/src/src/rewrite.c +++ b/src/src/rewrite.c @@ -206,22 +206,7 @@ for (rule = rewrite_rules; if (expand_string_forcedfail) { if ((rule->flags & rewrite_quit) != 0) break; else continue; } - /* Avoid potentially exposing a password */ - - if ( ( Ustrstr(expand_string_message, "failed to expand") != NULL - || Ustrstr(expand_string_message, "expansion of ") != NULL - ) - && ( Ustrstr(expand_string_message, "mysql") != NULL - || Ustrstr(expand_string_message, "pgsql") != NULL - || Ustrstr(expand_string_message, "redis") != NULL - || Ustrstr(expand_string_message, "sqlite") != NULL - || Ustrstr(expand_string_message, "ldap:") != NULL - || Ustrstr(expand_string_message, "ldaps:") != NULL - || Ustrstr(expand_string_message, "ldapi:") != NULL - || Ustrstr(expand_string_message, "ldapdn:") != NULL - || Ustrstr(expand_string_message, "ldapm:") != NULL - ) ) - expand_string_message = US"Temporary internal error"; + expand_string_message = expand_hide_passwords(expand_string_message); log_write(0, LOG_MAIN|LOG_PANIC, "Expansion of %s failed while rewriting: " "%s", rule->replacement, expand_string_message); diff --git a/src/src/route.c b/src/src/route.c index b289b0f34..f7a532567 100644 --- a/src/src/route.c +++ b/src/src/route.c @@ -1897,21 +1897,8 @@ if (unseen && r->next) /* Unset the address expansions, and return the final result. */ ROUTE_EXIT: -if ( yield == DEFER - && addr->message - && ( Ustrstr(addr->message, "failed to expand") != NULL - || Ustrstr(addr->message, "expansion of ") != NULL - ) - && ( Ustrstr(addr->message, "mysql") != NULL - || Ustrstr(addr->message, "pgsql") != NULL - || Ustrstr(addr->message, "redis") != NULL - || Ustrstr(addr->message, "sqlite") != NULL - || Ustrstr(addr->message, "ldap:") != NULL - || Ustrstr(addr->message, "ldapdn:") != NULL - || Ustrstr(addr->message, "ldapm:") != NULL - ) - ) - addr->message = string_sprintf("Temporary internal error"); +if (yield == DEFER && addr->message) + addr->message = expand_hide_passwords(addr->message); deliver_set_expansions(NULL); router_name = NULL; -- 2.30.2