Mailing List Archive: De-tainting with ${sg} expansion

De-tainting with ${sg} expansion

Jul 27, 2020, 11:45 AM

Post #1 of 3 (391 views)

Hi all;

Firstly, just to note that I understand the purpose of tainting data, and appreciate any improvements to security within Exim.

That said, I was already aware of the potential for bad variable data being exposed to the server, and was removing non-alphanumeric characters from $local_part (or, at least, attempting to) with the expansion ${sg{$local_part}{\N[^A-Za-z0-9_.-]\N}{_}}. However, this still appears to fail in my transports.

Is ${sg} not a suitable expansion to de-taint $local_part or $domain? If not, that massively screws with the long-term archival that I am required to do with my Exim mail server (which I appreciate may not have the same use-case as a normal mail server).

Regex replacement as de-taint operation is the typical approach in Perl (where they also apply a tainting principle), so I would have reasonably expected it to be the same here.

If not, I am desperately in need of an alternative for the following two transports, where I need to be able to store *any* received mail (not handled by earlier routers/transports) in a browsable directory structure, and so don't have valid lookups that I can do:

-----------------
BADFILECHARS = \N[^A-Za-z0-9_.-]\N

local_unhandled:
driver = appendfile
create_directory = yes
directory = /var/spool/exim/unhandled/\
${sg{$domain}{BADFILECHARS}{_}}/\
${sg{$local_part}{BADFILECHARS}{_}}/\
$tod_logfile
user = exim
group = mail
mode = 0660
### end local_unhandled

local_delivery:
driver = appendfile
file = ${if or{{bool{$acl_m_localdiscard}} \
{bool{${lookup{$local_part} \
lsearch{/etc/passwd} \
{no} \
{yes}}}}} \
{/dev/null} \
{/var/spool/mail/${sg{$local_part}{BADFILECHARS}{_}}}}
user = ${if or{{bool{$acl_m_localdiscard}} \
{eqi {$local_part}{root}} \
{bool{${lookup{$local_part} \
lsearch{/etc/passwd} \
{no} \
{yes}}}}} \
{mail} \
{$local_part}}
group = mail
mode = 0620
delivery_date_add
envelope_to_add
return_path_add
notify_comsat
### end local_delivery
-----------------

I've been avoiding check_local_user (since it tries to chdir into home directories that the exim user has no access to), so I don't think I have access to $local_part_data (as nothing populates it).

I would dearly love to avoid downgrading to 4.93, off the back of this change.

Regards.
J.
-----------------
~# exim -bV
Exim version 4.94 #2 built 01-Jun-2020 19:51:21
Copyright (c) University of Cambridge, 1995 - 2018
(c) The Exim Maintainers and contributors in ACKNOWLEDGMENTS file, 2007 - 2018
Berkeley DB: Berkeley DB 5.3.21: (May 11, 2012)
Support for: crypteq iconv() IPv6 PAM Perl Expand_dlfunc OpenSSL Content_Scanning DANE DKIM DNSSEC Event OCSP PIPE_CONNECT PRDR PROXY SOCKS SPF DMARC TCP_Fast_Open
Lookups (built-in): lsearch wildlsearch nwildlsearch iplsearch cdb dbm dbmjz dbmnz dnsdb dsearch ldap ldapdn ldapm nis nis0 nisplus passwd sqlite
Authenticators: cram_md5 cyrus_sasl dovecot gsasl plaintext spa tls
Routers: accept dnslookup ipliteral manualroute queryprogram redirect
Transports: appendfile/maildir/mailstore/mbx autoreply lmtp pipe smtp
Malware: f-protd f-prot6d drweb fsecure sophie clamd avast sock cmdline
Fixed never_users: 0
Configure owner: 0:0
Size of off_t: 8
Configuration file is /etc/exim/exim.conf

~# yum list installed | grep exim
exim.x86_64 4.94-1.el7 @epel

--
## List details at https://lists.exim.org/mailman/listinfo/exim-users
## Exim details at http://www.exim.org/
## Please use the Wiki with this list - http://wiki.exim.org/

Re: De-tainting with ${sg} expansion [ In reply to ]

exim-users at exim

Jul 28, 2020, 12:25 PM

Post #2 of 3 (391 views)

Permalink

On 27/07/2020 19:45, Jamie Barnes via Exim-users wrote:> I've been avoiding check_local_user (since it tries to chdir into home directories that the exim user has no access to), so I don't think I have access to $local_part_data (as nothing populates it).
Not so. Any lookup done by a local_parts= condition on a router
also sets it.

You didn't show the routers calling these transports.

> in need of an alternative for the following two transports, where I need to be able to store *any* received mail (not handled by earlier routers/transports)

Transports are only run when called by a router...

> local_delivery:
> driver = appendfile
> file = ${if or{{bool{$acl_m_localdiscard}} \
> {bool{${lookup{$local_part} \
> lsearch{/etc/passwd} \
> {no} \
> {yes}}}}} \
> {/dev/null} \
> {/var/spool/mail/${sg{$local_part}{BADFILECHARS}{_}}}}

The discard invoked by the /dev/null choice could be done by
a router (redirect, to :blackhole:). Doing that would simplify this to

file = /var/spool/mail/${sg{$local_part}{BADFILECHARS}{_}}

... and then we can detaint easily with a lookup rather than the ${sg}

file = /var/spool/mail/${lookup{$local_part}lsearch,ret=key{/etc/passwd}}

(whether you need any further cleaning depends on your policies on username
creation).

Depending on the router calling this transport, you might be able to do the
lookup there, populating $local_part_data for use here.

> user = ${if or{{bool{$acl_m_localdiscard}} \
> {eqi {$local_part}{root}} \
> {bool{${lookup{$local_part} \
> lsearch{/etc/passwd} \
> {no} \
> {yes}}}}} \
> {mail} \
> {$local_part}}

This looks like it uses the same lookup or value as the file= option.

> local_unhandled:
> driver = appendfile
> create_directory = yes
> directory = /var/spool/exim/unhandled/\
> ${sg{$domain}{BADFILECHARS}{_}}/\
> ${sg{$local_part}{BADFILECHARS}{_}}/\
> $tod_logfile

This transport is much harder to deal with. Currently you have to use
one of the gross hacks presented as a get-out-of-jail-free, to
obtain untainted strings for that path. I hate them, but there were
always going to be ways for shooting oneself in the foot.

I fully expect them to be cargo-culted, probably via serverfault.

Hint: Use a lookup which matches anything, and returns the key.

I'm not happy with the situation either - but I've not been able to
think of a principled way of dealing with it. I do not regard the
above hack as principled because there are zero limits on it.

> Is ${sg} not a suitable expansion to de-taint $local_part or $domain?

No - because there are no limits on it, and I see no way for Exim
to interpret the replacement pattern in a $(sg) to infer useful limits.

Possibly some form of operator which had the semantics of
"anything below this given path is fair game" would work. But it
could not be a general de-tainting string expansion, because taint
not only prevents use as a file path but also prevents further
expansion.
--
Cheers,
Jeremy

--
## List details at https://lists.exim.org/mailman/listinfo/exim-users
## Exim details at http://www.exim.org/
## Please use the Wiki with this list - http://wiki.exim.org/

Re: De-tainting with ${sg} expansion [ In reply to ]

exim-users at exim

Jul 30, 2020, 7:06 PM

Post #3 of 3 (389 views)

Permalink

(Apologies if this doesn't thread properly -- I am writing via email and reading via Lurker...)

On 27/07/2020 19:45, Jamie Barnes via Exim-users wrote:
>> I've been avoiding check_local_user (since it tries to chdir into home directories that the exim user has no access to), so I don't think I have access to $local_part_data (as nothing populates it).

On 2020-07-28 20:25 +100, Jeremy Harris via Exim-users wrote:
> Not so. Any lookup done by a local_parts= condition on a router also sets it.

That's nice to know, thanks, but I'm not doing any lookup in a `local_parts` router directive either. I'm literally storing everything that comes in, in a domain/localpart folder tree.

> You didn't show the routers calling these transports...
> Transports are only run when called by a router...

There were a lot of routers, with a lot of logic in between, and I didn't expect everyone to attempt to parse it all. In summary, when I was putting it all together originally, I short-circuited all of that routing logic for mail sent locally (e.g. by daemons to the root user), so I have this one first...

store_local_user:
driver = accept
condition = ${if bool{$acl_c_islocal}}
transport = local_delivery
log_as_local
no_more

...and then, as a fallback to catch all the places where logic hasn't routed something, the last router is...

unhandled:
driver = accept
transport = local_unhandled
verify = false
no_more

>> local_delivery:
>> driver = appendfile
>> file = ${if or{{bool{$acl_m_localdiscard}} \
>> {bool{${lookup{$local_part} \
>> lsearch{/etc/passwd} \
>> {no} \
>> {yes}}}}} \
>> {/dev/null} \
>> {/var/spool/mail/${sg{$local_part}{BADFILECHARS}{_}}}}
>
> The discard invoked by the /dev/null choice could be done by a router (redirect, to :blackhole:).
> Doing that would simplify this to
>
> file = /var/spool/mail/${sg{$local_part}{BADFILECHARS}{_}}
>
> ... and then we can detaint easily with a lookup rather than the ${sg}
>
> file = /var/spool/mail/${lookup{$local_part}lsearch,ret=key{/etc/passwd}}
>
> (whether you need any further cleaning depends on your policies on username creation).

To be honest, despite the documentation, I've never really understood the redirect router. If there is any reasonable gain to be had from switching this logic to redirect, I might re-evaluate it.

> Depending on the router calling this transport, you might be able to do the
> lookup there, populating $local_part_data for use here.
>
>> user = ${if or{{bool{$acl_m_localdiscard}} \
>> {eqi {$local_part}{root}} \
>> {bool{${lookup{$local_part} \
>> lsearch{/etc/passwd} \
>> {no} \
>> {yes}}}}} \
>> {mail} \
>> {$local_part}}
>
> This looks like it uses the same lookup or value as the file= option.

It is. The irony is that local mail is the least of my worries here, but Exim wasn't originally the server's default system mail handler and this was mostly an attempt to get that facility back when we switched away from Postfix. It's just that the local mail was clogging up the queue (defers, because of the Tainted errors).

> local_unhandled:
> driver = appendfile
> create_directory = yes
> directory = /var/spool/exim/unhandled/\
> ${sg{$domain}{BADFILECHARS}{_}}/\
> ${sg{$local_part}{BADFILECHARS}{_}}/\
> $tod_logfile
>
> This transport is much harder to deal with. Currently you have to use
> one of the gross hacks presented as a get-out-of-jail-free, to
> obtain untainted strings for that path. I hate them, but there were
> always going to be ways for shooting oneself in the foot.
>
> I fully expect them to be cargo-culted, probably via serverfault.
>
> Hint: Use a lookup which matches anything, and returns the key.

I missed the fact that Exim-users is moderated, so after not seeing my email in Lurker, I went back over the documentation after posting and came up with adding the below to the above routers:

set = r_fs_local_part=${lookup{${sg{$local_part}{BADFILECHARS}{_}}}\
lsearch*,ret=key{$config_dir/detaint}}
set = r_fs_domain=${lookup{${sg{$domain}{BADFILECHARS}{_}}}\
lsearch*,ret=key{$config_dir/detaint}}
(where /detaint is a file containing `*`)

I suppose I could use a macro, such as...

SAFELOCALPART = ${lookup{${sg{$local_part}{BADFILECHARS}{_}}} lsearch*,ret=key{$config_dir/detaint}}
SAFEDOMAIN = ${lookup{${sg{$domain}{BADFILECHARS}{_}}} lsearch*,ret=key{$config_dir/detaint}}

...if I find myself needing it more often?

I assume this is one of the gross hacks you are referring to (it certainly felt like one to me, when I wrote it), but it appears to be working as expected. If there is something dangerous I'm missing with the above approach (assuming I trust my BADFILECHARS regex macro), I would appreciate learning it.

> I'm not happy with the situation either - but I've not been able to
> think of a principled way of dealing with it. I do not regard the
> above hack as principled because there are zero limits on it.
>
>> Is ${sg} not a suitable expansion to de-taint $local_part or $domain?
>
> No - because there are no limits on it, and I see no way for Exim
> to interpret the replacement pattern in a $(sg) to infer useful limits.

Do you really have to infer useful limits, rather than implementing a sane default and just documenting the risk? There's always going to be someone who has a different definition of "useful".

I guess I originally came at this from a Perl perspective, where a similar "taint" concept exists. They also suffer their concerns with dangerous patterns (and the possibility of a generic detainting regex pattern, such as `s/(.+)/$1/g`) but, at the end of the day, they document the risk and then trust their end-users to do the right thing regarding their patterns.

I appreciate Exim is not Perl but, since someone is always going to attempt to work around this, maybe ${sg} offers a less-gross hack than a hard-to-parse/reason-about ${lookup} with an external file dependency.

> Possibly some form of operator which had the semantics of
> "anything below this given path is fair game" would work. But it
> could not be a general de-tainting string expansion, because taint
> not only prevents use as a file path but also prevents further
> expansion.

Maybe just an operator ${safe:$...}, which is a shortcut for ${sg{...}{PREDEFINED_REGEX}{_}}...? I've been using `[A-Za-z0-9._-]`, but that's just because I know where I'm using it and why (...he says!)

> --
> Cheers,
> Jeremy

Thanks for your help.

--
## List details at https://lists.exim.org/mailman/listinfo/exim-users
## Exim details at http://www.exim.org/
## Please use the Wiki with this list - http://wiki.exim.org/