On 03/09/14 05:13, Viktor Dukhovni wrote:
> On Wed, Sep 03, 2014 at 12:27:16AM +0000, Phil Pennock wrote:
>
>> My jaw dropped when I returned to my list index view and saw just how
>> many patches came in from work this weekend by Todd and Jeremy to
>> implement DANE in Exim.
>>
>> Seriously good work which makes me very happy and feeling that I owe
>> drinks to you both. Thank you for picking up where I totally slacked
>> off and working on implementing this. (Thank you for removing one of my
>> guilt layers of "I need to find time to work on X too").
>
> Yes, thanks a bunch. I have a few questions about the code.
> Specifically:
>
> verify_callback_client_dane(), (lines 469--471)
> tlsa_lookup(), (lines 1691--1713)
> dane_tlsa_load(), (all lines)
>
> transports/smtp.c: (lines 3297--3306,
> tempfail_try_clear logic)
>
> Since I only read the commit diff, I am missing some context,
> so please pardon my ignorance... I don't know where the logic
> is that tracks the DNSSEC status of the MX lookup to determine
> whether the subsequent per MX-host lookups can be considered
> secure.
There's a flag "dnssec" on a host struct, filled in by
host_find_bydns() called (usually) from the dnslookup
router, and checked by tls_client_start()
[ in the version you have, but I'm about to pull it
up a layer to smtp_deliver() ]
in deciding whether to try DANE.
> I am not sure that DNS lookup errors are handled correctly. I
> could not find code that distinguishes between lookups failure and
> NXDOMAIN (which is not a lookup error).
Nor me. What action do you suggest for which?
>
> I don't at first glance see code that enforces STARTTLS, without
> authentication, when a secure TLSA RRset exists, but all records
> are "unusable". I did not find any code to coerce PKIX-EE(1)
> and PKIX-TA(0) to unusable.
>
> I did notice that unsupported matching types seem to be needlessly
> treated as errors, rather than as an unusable record among possibly
> usable other records.
Those are all whatever your library does.
>
> I am confused by the interaction with "tempfail try clear".
That'll be dealt with by the change above; intent is to
disable the try-in-clear possibility if dane was requested
and the tlsa lookup succeeded.
>
> The DANE-specific verify callback seems to assume that any call in
> which the verification status is OK is sufficient to make the peer's
> chain valid, but that need not be the case when the chain length
> is greater than 1. It may be best to not infer any final status
> in that callback, and check the verification state after the
> connection completes, sending a "QUIT" (to the MiTM perhaps)
> and gracefully closing the connection if verification failed.
>
My understanding was that any "fail" return code to the sequence
of callbacks was enough to tell the OpenSSL library to fail the
TLS startup, and that there would be at least one callback per
chain element. Is this not the case?
--
Cheers,
Jeremy
