- From: Adam Roach <adam@nostrum.com>
- Date: Wed, 12 Sep 2018 23:54:52 -0700
- To: "The IESG" <iesg@ietf.org>
- Cc: draft-ietf-httpbis-expect-ct@ietf.org, Mark Nottingham <mnot@mnot.net>, httpbis-chairs@ietf.org, mnot@mnot.net, ietf-http-wg@w3.org
Adam Roach has entered the following ballot position for draft-ietf-httpbis-expect-ct-07: Discuss When responding, please keep the subject line intact and reply to all email addresses included in the To and CC lines. (Feel free to cut this introductory paragraph, however.) Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html for more information about IESG DISCUSS and COMMENT positions. The document, along with other ballot positions, can be found here: https://datatracker.ietf.org/doc/draft-ietf-httpbis-expect-ct/ ---------------------------------------------------------------------- DISCUSS: ---------------------------------------------------------------------- Thanks for the work done on defining this mechanism! I think it's quite useful, and I plan to ballot "Yes" as soon as the minor but important issue below is fixed. §6.1: > Status: standard My reading of RFC 3864 does not allow Experimental RFCs to register HTTP header fields as "Status: Standard." ---------------------------------------------------------------------- COMMENT: ---------------------------------------------------------------------- I also have a number of non-blocking comments that range from editorial to substantial-but-not-critical. They appear below in document order. --------------------------------------------------------------------------- ID Nits reports: -- Obsolete informational reference (is this intentional?): RFC 5246 (Obsoleted by RFC 8446) Given that this document doesn't seem to be tied to any specific version of TLS, I suspect this should be updated. --------------------------------------------------------------------------- §1.1: This document makes use of non-captialized versions of terms like "should." Please consider using the RFC 8174 boilerplate. --------------------------------------------------------------------------- §2.1: > Expect-CT = #expect-ct-directive > expect-ct-directive = directive-name [ "=" directive-value ] > directive-name = token > directive-value = token / quoted-string I note that there is no registry for directive names in the IANA section, so presumably there is a small, closed set of directives allowed here. Typically, when this is the case, the ABNF includes the permissible values; e.g.: directive-name = "report-uri" / "enforce" / "max-age" ...although I also note that list item (5) under the ABNF implies that the intention here is to be extensible. If such is the case, I would suggest adding an IANA registry that records Expect-CT directives, and specifying the ABNF as: directive-name = "report-uri" / "enforce" / "max-age" / token NOTE that not specifying a registry in this document is practically identical to specifying a registry with a policy of "RFC Required," as adding new values will require a new RFC that updates this one. That's an exceptionally restrictive policy, and I would hope that the working group would make such a decision with intention rather than letting it happen through inaction. --------------------------------------------------------------------------- §2.1.3: > The "max-age" directive is REQUIRED to be present within an "Expect- > CT" header field. This doesn't appear to be true as stated; or, at least, it is stated in a somewhat confusing way. A casual reading of this requirement is that an "Expect-CT" header field is noncompliant if it is missing this directive. Based on the examples given, the actual requirement here is that a response that contains an Expect-CT header field MUST contain an Expect-CT header field with a max-age directive, although that directive does not necessarily need to appear in each Expect-CT header field. This should probably be clarified. --------------------------------------------------------------------------- §3.1: These reports indicate hostname and port, but omit scheme. RFC 6454 defines origin (which is what you're really getting at here) as scheme/host/port. Clearly, it doesn't make sense to report on http, so I presume that the thought process here is that "https" is the only scheme in play. The worry I have is that the current design is not particularly future-proof. For example, it would take only a modest adaptation for this mechanism to work with "coaps" URIs, except that the collecting server wouldn't be able to distinguish between "coaps" and "https" resources on the same device. Note that port number isn't necessarily a valid distinguisher here, as both HTTP and COAP use ALPN, and could conceivably run on the same port as a consequence. It seems that it would be easy to add "scheme" as an optional field at this point in time, to head off potential confusion in the future. --------------------------------------------------------------------------- §3.3: > Upon receiving an Expect-CT violation report, the report server MUST > respond with a 2xx (Successful) status code if it can parse the > request body as valid JSON and recognizes the hostname in the > "hostname" field of the report. It seems to me that "port" should be treated the same as "hostname" here -- that is, if the host:port combination (or scheme:host:port, if you make changes based on my preceding comment) isn't expected, then the result should be a 4xx. > If the report body cannot be parsed > or the report server does not expect to receive reports for the > hostname in the "hostname" field, the report server MUST respond with > a 4xx (Client Error) status code. Which one? --------------------------------------------------------------------------- §5: > Implementations must store state about Known Expect-CT Hosts, and > hence which domains the UA has contacted. The "must" here (even if non-normative) seems to overstep a boundary. For example, when in Incognito/Private Browsing mode, browsers will take special pains not to persistently store any information related to the domains visited. It should probably be noted that persistent caching of this information is subject to local policy (either here or elsewhere), and the "must" in this sentence should be softened or qualified.
Received on Thursday, 13 September 2018 06:55:16 UTC