[dane] AD review of draft-ietf-dane-ops-12

[Stephen Farrell <[e-mail]>]

Hiya,

My review of this is below. I'm fine with starting the IETF LC
and you handling these as last call comments along with others
but since there are a bunch of 'em (and I bet Viktor will not
be silent anyway:-) please tell me (Warren, as shepherd) if you
prefer me to start the IETF LC now, or wait until we've chatted
about these a bit. (And I'm fine either way.)

Cheers and thanks for the work on this,
S.

- general: this was a bit of a slog - the draft is
both dense (loads of 2119 terms) and long, which
makes it a hard read. I'm not saying it's possible
to do better, but just noting that in case someone
has a good idea about how to make it easier to read.

- intro, 3rd para: I don't think this is very clear
and it'd be great if it were - isn't there a way to
provide clarity about the name that goes with the
TLS session in which cases?

- 1.1, I think you need to define "TLSA base domain"
here (or is it elsewhere? I didn't find it in 6698)

- section 2: Why duplicate so much of 7218?  I think
it'd be better not to (or to obsolete that with
this) but I don't care that much really.

- section 2: last sentence before 2.1 - is that
saying that the MUST applies to section 9? If not,
I'm not sure what it means, but it's not quite
clear.

- section 3: saying TLS1.2 is a SHOULD is fine but
what about when TLS1.3 is done (next year).
Wouldn't it be better to say that the latest TLS
version is a SHOULD and is currently 1.2?

- section 4: I read this as you calling DNSSEC
"illusory incremental security" which I don't think
is really what you mean and which interpretation
calls into question the strength of the
RECOMMENDATION here.  4.1 similarly says "PKIX-TA(0)
and PKIX-EE(1) TLSA records do not provide
additional security" which is too definitive, where
is the evidence? (Not opinion or argument, but
evidence.) In the absence of evidence I think we
should omit these (and they are not needed.)

- 4.2: Did we check that CT is unlikely to make
sense with DANE-TA records? In principle, public
logs could add enterprise CAs without having real
spam issues. I agree it's unlikely to happen in
practice, and it's probably fine to revise this if
that starts happening a lot, but since it could in
principle, I wondered if we'd checked with the trans
list (and I don't recall that we have)

- 5.1, last para: that's a bit coy about the "extra
logic" - wouldn't it be better to explain? If not,
why not?

- 5.2.1: 2nd para says "do honour constraints" but
earlier (for DANE-EE+Cert) you explicitly said to
ignore validity, subject etc. I think it'd be better
to explicitly say here whether or not validity and
naming need to match and if they do, then how.
As-is, I suspect developers are likely to randomly
either ignore validity and naming here too, or
not;-) And if enough of 'em do different things,
then that'd be bad. (If there were a good list of
subsections of 5280 to honor or ignore that might
work, but would be a bit tedious to figure out and
might not work - I've not checked.)

- 5.2.3, last para: What is "are encouraged to" in
2119 terms?

- 5.4: "SHOULD NOT always stop" isn't clear, and the
next sentence seems to have the MUST that clarified,
so maybe drop the 2119 "SHOULD NOT"?

- section 6: what is a "master" TLSA record?  Maybe
better to not introduce a new term like that.

- section 8, 2nd para: saying "are only compatible
with" seems like an overstatement to me

- 8.2: I think in this example you generate a new
key pair for the TLS server at the time that your
are transitioning from DANE-EE to DANE-TA. Saying
that would help, as there could be another
transition case where the same TLS server key pair
is maintained and an x.509 certificate (issued under
the DANE-TA) for that same key is deployed on the
TLS server. I'm not saying that's a better
transition but only that it could presumably be done
and arguably easier if it worked. But the only
change I'd suggest for now is to just point out that
a new key pair is generated for this example.

- 8.3: lowercase "should" often confuses as to
whether that means 2119 or not. I don't care how you
resolve.

- 8.4, I think last sentence of 1st bullet could be
worth a bullet of its own

- 8.4, I'm not sure the last bullet is clear enough
to be a useful summary - its a doozy of a sentence
in any case;-)

- section 9: your background is showing:-) "by the
MTA administrator" should presumably be "on the TLS
client (e.g. by an MTA administrator)" or similar.

- 10.1.1: I guess we don't consider that it'd be
good to give a reference to the TC bit? I'm ok with
that but someone else might like to have one.

- 10.2: "The server name used for this comparison
SHOULD be the base domain of the TLSA RRset." I
think that works ok, but isn't that a change in
client behaviour compared to a non-DANE TLS client?
If to, then don't you need to call that out much
more clearly? Do we actually need a "differences
between DANE TLS clients and non-DANE TLS clients"
section?

- section 11: I think s/public CA PKI/public CA
WebPKI/ would be more accurate

- section 11: I don't get the logic for recommending
weekly signing - once you automate signing (which
you have to) then daily seems like it should work
for everyone and weekly hasn't any benefit. So I
don't get the reasoning here. (And a week seems bad
when I read section 13 too.)