Subject: Working Group Last Call for draft-ietf-httpbis-expect-ct-05

From: Mark Nottingham <[email-masked]>
Date: Thu, 31 May 2018 13:27:21 +1000

Everyone,

Emily has indicated that she thinks this document is ready for WGLC, and there are no open issues.

Please take a look at:
  https://tools.ietf.org/html/draft-ietf-httpbis-expect-ct-05

And bring up and issues / concerns / suggestions here or on the issues list. Statements of implementation or support for publication would also be appreciated.

Working Group Last Call will end on 8 June 2018.

--
Mark Nottingham   https://www.mnot.net/

---

From: Martin Thomson <[email-masked]>
Date: Thu, 31 May 2018 16:08:49 +1000

This looks mostly fine.  It's a little surprising how many words there
are in here when you first look at it, but it's hard to see a shorter
document being better than this.  It's pretty comprehensive.

Major things

The HSTS-like behaviour (p1 of S2.4) isn't necessary.  Disabling
user-level overrides for things like certificate validation errors is
a surprise to me.  Did we discuss this at all?

Minor things

CAs can (and do) issue IP certificates, so why does this specifically
exclude those?  If this is a requirement imposed by CT, then please
cite that.  Otherwise, I think that this should allow IP literals.

The storage model doesn't account for the Age of a response.  If that
is intentional, please call it out, otherwise, cite RFC 7234.

The document doesn't account for Expect-CT in the presence of
intermediation.  This is very likely to fail when there are additional
roots installed for the purposes of TLS MitM.

What is the interaction model of Expect-CT reporting with other
similar features.  Say we have 10 reasons for rejecting a connection,
of which Expect-CT and a small subset of others could both trigger
errors and reports.  Do all report if they fail?  Just the first?  The
document is currently written with the assumption that there is a
bunch of checks that either pass or fail and that Expect-CT is the
very last check, but that's unlikely to be the case forever.

Do you really believe that all that reporting information is
necessary?  Isn't enough to know that Host X had N errors?  These are
almost big enough to constitute a self DoS if a mess-up occurs.  Two
copies of the certification path in base64!

application/expect-ct-report+json needs to be registered.  There's a
whole process and it's annoying, but necessary.  You can crib the text
from other recently successful examples (I recently shepherded RFC
8142, but you can probably find a few others), and someone needs to
send an email to a mailing list somewhere.

Nits

The drafts doesn't explicitly say that the report URI is a quoted
string, nor that the max-age is (I assume) either a quoted string or
token.

Why use "host" rather than the accepted term "server"?  Noting the
equivalence of terms in S1.2 might be sensible (host is also a valid
generalization for client, so this usage might otherwise be
confusing).

What is the difference between "Known CT Host " and "Known Expect-CT Host"?

2.2.1, the "SHOULD" here isn't a real requirement.  You can drop it
and just say "an Expect-CT host includes exactly one Expect-CT header
field in its response".  A host that isn't Expect-CT won't include the
header field.  (Using "SHOULD" invites all sorts of awkward questions
about when it might be appropriate not to include the value, but I
concluded that most can be answered with "just use HPACK" in this
case).

2.3.1 contains two lead-ins to the list.  I think that the processing
order is a little misleading.  In particular, the second lead-in
doesn't say that Expect-CT is present.  You want to say something like
"If a host is CT-qualified, then process a response that contains a
Expect-CT header field in one of the two following ways:"

Overall, I think that 2.3 could be structured differently.  This
section includes cases for when the Expect-CT header field is
malformed (equivalent to not being present) as well as for when the
host is CT-qualified.  I would move the processing for absent and
invalid Expect-CT header fields into another section that simply says
that no change is made to the value of "Expect-CT metadata" for that
host.  Then you only have to worry about updating Expect-CT metadata -
which can be moved to 2.3.2, and reporting, which deserves its own
section.

Something like:

- Responses Without Expect-CT
  When the header is absent or malformed, the status of the host is
unchanged.  That is, any Expect-CT metadata for the host is unchanged.

- Updating Expect-CT Metadata
  The Expect-CT header field is ignored if a connection is not
CT-qualified (see previous section), but it might generate a report
(see next section).
  If the connection is CT-qualified, then CT-metadata is created or
updated for the corresponding host based on the value of the Expect-CT
header field.

- Reporting Expect-CT Problems
  If a connection is not CT-qualified, but the Expect-CT header field
is present and contains a report-uri, then a report is generated
(Section 3).

- Storage

For 2.4, I would start by saying that when evaluating a connection,
the UA determines if the UA is a Known Expect-CT Host.  It does this
by looking for records that aren't expired.  Then talk about the extra
checks.  The current arrangement of paragraphs doesn't have a
consistent flow, it's currently: extra-checks, expiration, not
CT-qualified, more on reporting for not-CT-qualified hosts,
extra-checks again, disabling CT.  The bit on disabling CT belongs
elsewhere, I think, because it risks being missed.

"i.e." -> "i.e.,", same for "e.g."

3.1 should fix date-time to Zulu, and not permit time zone variations.

Section 3.1 cites Section 4.6 of RFC6962-bis, but this moved to 4.5.
Also, there is a "Section 4 **or** [RFC6962]" to fix here.

Section 3.1 needs to cite JSON (of some version or other).

Promote Section 8.1 to Section 8.