Re: [exim] De-tainting with ${sg} expansion

Αρχική Σελίδα
Delete this message
Reply to this message
Συντάκτης: Jamie Barnes
Ημερομηνία:  
Προς: exim-users
Αντικείμενο: Re: [exim] De-tainting with ${sg} expansion
(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.