On 2018-12-16 at 10:42 +0000, Jeremy Harris via Exim-dev wrote:
> On 16/12/2018 10:20, Andreas Metzler via Exim-dev wrote:
> > 4.92rc1 adds this to the smarthost_smtp transport:
> >
> > tls_sni = $host
> >
> > I do not think that always works as expected. Depending on the DNS setup
> > (CNAME, round robin) $host will not contain the name of the selected
> > smarthost anymore but a different value.
>
> Phil - that went in at 26739076ae in the example config; could you
> comment?
I think that I just missed that we might adapt `$host` during the life
of the Transport.
<
https://www.exim.org/exim-html-current/doc/html/spec_html/ch-the_smtp_transport.html>
30.2
Absent `hosts_override` or `hosts` directly on the Transport, Round
Robin A records have no cause to change the host _name_. So the only
issue should be CNAME records?
If we're changing `$host` based upon CNAMEs in DNS, then yes this will
do The Wrong Thing. It might be a security problem then, because the
normally-insecure DNS changes the name we validate the certificate
against. We can't rely upon DNSSEC for this default example config.
It's an example, which can be fixed, so it's not major. This isn't the
built-in default for the option, but the default example configuration.
Short of messing with `$address_data`, the only fix I can immediately
think of would be to record `$router_host` or `$original_host` as the
value of `$host` when the Transport is entered, and then make that the
configured example. That's some more coding changes.
Is this enough of a problem in real world scenarios to be worth the
coding work? More code for hypothetical problems is complexity to debug
and maintain. I'm inclined to have our example configuration gently
push back against that and encourage people to reduce CNAMEs, or accept
that this will require explicit configuration.
But this is a taste issue; if it's a big problem then I strongly suspect
we'll accept a codebase patch upstream to add `$router_host` and use
that as the example config default, or some other appropriate solution
if someone spots something good which I've missed. Which is possible,
because I missed the CNAME issue at the time.
The cop-out would be to change to using a macro at the top of the file,
`SMARTHOST_NAME`, and .ifdef guards to define the Router only if that
macro is defined, and then reuse `SMARTHOST_NAME` as the default for
`tls_sni`. That would be more secure, but perhaps a little harder to
talk people through changing. But hey, we can uncomment the Router by
default, in that case. Just leave the macro commented-out by default.
So a small win?
Hrm. Perhaps the macro approach for the imminent release, and consider
a new variable for the next release? Heiko's discretion at this point.
-Phil
