- From: Martin Thomson <martin.thomson@gmail.com>
- Date: Thu, 31 May 2018 16:08:49 +1000
- To: Mark Nottingham <mnot@mnot.net>
- Cc: HTTP Working Group <ietf-http-wg@w3.org>, Patrick McManus <mcmanus@ducksong.com>, "Emily Stark (Dunn)" <estark@google.com>
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. On Thu, May 31, 2018 at 1:38 PM Mark Nottingham <mnot@mnot.net> wrote: > > 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/ > >
Received on Thursday, 31 May 2018 06:09:31 UTC