Subject: Adam Roach's Discuss on draft-ietf-httpbis-expect-ct-07: (with DISCUSS and COMMENT)

From: Adam Roach <[email-masked]>
Date: Wed, 12 Sep 2018 23:54:52 -0700

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.

---

From: Julian Reschke <[email-masked]>
Date: Thu, 13 Sep 2018 13:23:07 +0200

On 9/13/2018 8:54 AM, Adam Roach wrote:
> ... > §2.1:
> ...

Disagreed. We have stopped doing this in the HTTP specs for a reason - 
it conflates two different thing: parsing, and detecting certain 
predefined tokens.

 > ...

Best regards, Julian

---

From: Adam Roach <[email-masked]>
Date: Thu, 13 Sep 2018 08:45:23 -0500

> it conflates two different thing: parsing, and detecting certain 

Thanks. If HTTP-related RFCs do this consistently, then I agree that you 
should keep with that convention.

/a

---

From: Patrick McManus <[email-masked]>
Date: Fri, 14 Sep 2018 15:14:06 +0000 (UTC)

> https://datatracker.ietf.org/doc/draft-ietf-httpbis-expect-ct/
agree that should be experimental

---

From: Alexey Melnikov <[email-masked]>
Date: Fri, 14 Sep 2018 16:23:02 +0100

Hi all,

On 14/09/2018 16:14, Patrick McManus wrote:
> On Thu, Sep 13, 2018 at 2:54 AM, Adam Roach <[email-masked] 
> <mailto:[email-masked]>> wrote:
>

This should already be fixed in the editor's copy.

Best Regards,

Alexey

---

From: Julian Reschke <[email-masked]>
Date: Wed, 19 Sep 2018 18:24:52 +0200

> appear in each Expect-CT header field. This should probably be clarified.

That's another case where progress on 
<https://github.com/httpwg/http-core/issues/111> would help. This 
plagues other WG drafts as well.

Best regards, Julian