Re: [exim-dev] Preliminary dane_require_tls_ciphers support

2018-03-29 Thread Phil Pennock via Exim-dev
On 2018-03-29 at 10:33 +0100, Jeremy Harris via Exim-dev wrote:
> I'm unsure about the philosophy of the interface; having one option
> override another.  You mentioned "complex expansions" before in the
> discussion but without detail.  I assume that's the same consideration
> as "lots of conditional logic" above.  Was that discarding the solution
> of dnsdb-lookup expansions selecting values for the original
> tls_require_ciphers option?

We already just ignore so many TLS options when DANE is determined to be
in effect, so it seemed cleanest to make it easier to have a different
option, both for the code-base and for postmasters maintaining configs.
Otherwise, we're looking at a new variable exposed once DANE is seen.

The Exim on my laptop has remote_smtp using:

  tls_require_ciphers = ${extract{tlshigh}{$address_data}{${if 
eq{$value}{yes}{TLS_CLIENT_HIGHSEC_CIPHERSPEC}{TLS_CLIENT_DEFAULT_CIPHERSPEC}}}{TLS_CLIENT_DEFAULT_CIPHERSPEC}}

(full transport at end)

There, `tlshigh` is a possible key from a "K1=V1 K2=V2 K3=V3" list
populated from other sources, letting me say "gmail.com: tlshigh=yes"
and so forth.

Clearly, I am capable of changing that to use a variable, but ... this
is security, and complexity is the enemy of security.  If we require
everyone to do this, then an uncomfortably large percentage of installs
will get it wrong.

If we say "DANE will use this other option by preference, just set that
to something strong", that's much harder to get wrong.  We can have an
easy win for most people, while those who want manually
configured/derived per-domain properties can continue to do so.

It's a judgement call, but I think here, DANE-specific is the way to go.
Off-list, there was a suggestion of renaming it to not mention DANE but
be for whenever TLS is mandatory.  I think leave that for a future
reconfig, because at present the Exim way would be to either have two
Routers, with distinct smtp Transports, with different TLS configs on
the two transports, the "hosts_require_tls = *" one requiring better
ciphers, or to use address_data for dynamic property binding, as I do.

> I'd prefer testing was in place before merge if at all possible.
> Certainly in place before it hits a release.

We have improved here, mostly thanks to you.  Thank you.  :)

> I'll have a go, planning to push into the dane_require_tls_ciphers
> branch.

Thanks!
-Phil

---8< pdp laptop exim remote_smtp >8
remote_smtp:
  driver = smtp
  port = ${extract{port}{$address_data}{$value}{25}}
  hosts_require_auth = ${extract{authreq}{$address_data}{${if 
eq{$value}{yes}{*}{$value}}}{}}
  hosts_require_tls = ${extract{tls}{$address_data}{${if 
eq{$value}{yes}{*}{$value}}}{}}
  hosts_avoid_tls = ${extract{tls}{$address_data}{${if eq{$value}{no}{*}{}}}{}}
  tls_sni = ${extract{tlssni}{$address_data}{$value}{}}
  tls_require_ciphers = ${extract{tlshigh}{$address_data}{${if 
eq{$value}{yes}{TLS_CLIENT_HIGHSEC_CIPHERSPEC}{TLS_CLIENT_DEFAULT_CIPHERSPEC}}}{TLS_CLIENT_DEFAULT_CIPHERSPEC}}
  tls_verify_certificates = 
${extract{tlsverify}{$address_data}{/usr/local/etc/openssl/certs}fail}
  hosts_try_dane = *
  dane_require_tls_ciphers = TLS_CLIENT_HIGHSEC_CIPHERSPEC
  no_multi_domain
  no_delay_after_cutoff
  helo_data = ${extract{helo}{$address_data}{$value}{$primary_hostname}}
  hide socks_proxy = ${extract{socks}{$address_data}{<; 8

8< pdp laptop smtp client TLS macros >8-
TLS_CLIENT_DEFAULT_CIPHERSPEC=DEFAULT:!SSLv2:!LOW:aNULL:!eNULL
TLS_CLIENT_HIGHSEC_CIPHERSPEC=TLS13-CHACHA20-POLY1305-SHA256:TLS13-AES-128-GCM-SHA256:TLS13-AES-256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:!3DES:!DSS:!aNULL:!eNULL
8< pdp laptop smtp client TLS macros >8-

-- 
## List details at https://lists.exim.org/mailman/listinfo/exim-dev Exim 
details at http://www.exim.org/ ##


Re: [exim-dev] Preliminary dane_require_tls_ciphers support

2018-03-29 Thread Jeremy Harris via Exim-dev
On 29/03/18 04:08, Phil Pennock via Exim-dev wrote:
> I've written support for a new SMTP Transport option
> dane_require_tls_ciphers which is like tls_require_ciphers but is used
> in _preference_ to tls_require_ciphers when DANE enabled.
> 
> This seemed much saner than requiring lots of conditional logic,
> especially since we already ignore most of the TLS options once DANE is
> in play anyway.
> 
> I wrote code for OpenSSL and GnuTLS and tested compilation with OpenSSL.
> 
> I wrote docs.  I did not write tests, I'm way out of practice on the
> Exim test suite.
> 
> Pushed to dane_require_tls_ciphers in the main git repo.

The coding is nicely selfcontained, and at a quick glance should do
the job.

I'm unsure about the philosophy of the interface; having one option
override another.  You mentioned "complex expansions" before in the
discussion but without detail.  I assume that's the same consideration
as "lots of conditional logic" above.  Was that discarding the solution
of dnsdb-lookup expansions selecting values for the original
tls_require_ciphers option?


> Jeremy, does this look mergeable/sane?  Did we get as far as pre-merge
> testing at any point, rather than post-merge testing?

I'd prefer testing was in place before merge if at all possible.
Certainly in place before it hits a release.

> What sort of coverage do we need from tests?  It's honestly going to be
> faster if someone else writes them

I'll have a go, planning to push into the dane_require_tls_ciphers
branch.
-- 
Cheers,
  Jeremy


-- 
## List details at https://lists.exim.org/mailman/listinfo/exim-dev Exim 
details at http://www.exim.org/ ##