Discussion:
Adam Roach's Discuss on draft-ietf-httpbis-expect-ct-07: (with DISCUSS and COMMENT)
Adam Roach
2018-09-13 06:54:52 UTC
Permalink
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.
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.

---------------------------------------------------------------------------
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.

---------------------------------------------------------------------------
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.

---------------------------------------------------------------------------
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?

---------------------------------------------------------------------------
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.
Julian Reschke
2018-09-13 11:23:07 UTC
Permalink
Post by Adam Roach
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,
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
directive-name = "report-uri" / "enforce" / "max-age" / token
...
Disagreed. We have stopped doing this in the HTTP specs for a reason -
it conflates two different thing: parsing, and detecting certain
predefined tokens.
Post by Adam Roach
...
Best regards, Julian
Adam Roach
2018-09-13 13:45:23 UTC
Permalink
Post by Julian Reschke
Post by Adam Roach
  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,
    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
    directive-name      = "report-uri" / "enforce" / "max-age" / token
...
Disagreed. We have stopped doing this in the HTTP specs for a reason -
it conflates two different thing: parsing, and detecting certain
predefined tokens.
Thanks. If HTTP-related RFCs do this consistently, then I agree that you
should keep with that convention.

/a
Patrick McManus
2018-09-14 15:14:06 UTC
Permalink
Post by Adam Roach
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.
https://datatracker.ietf.org/doc/draft-ietf-httpbis-expect-ct/
----------------------------------------------------------------------
----------------------------------------------------------------------
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.
Status: standard
My reading of RFC 3864 does not allow Experimental RFCs to register HTTP header
fields as "Status: Standard."
agree that should be experimental
Alexey Melnikov
2018-09-14 15:23:02 UTC
Permalink
Hi all,
Post by Adam Roach
----------------------------------------------------------------------
----------------------------------------------------------------------
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.
  Status:  standard
My reading of RFC 3864 does not allow Experimental RFCs to
register HTTP header
fields as "Status: Standard."
agree that should be experimental
This should already be fixed in the editor's copy.

Best Regards,

Alexey
Julian Reschke
2018-09-19 16:24:52 UTC
Permalink
Post by Adam Roach
...
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.
...
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
Emily Stark
2018-11-10 17:31:10 UTC
Permalink
Hi Adam,
Thanks for the review. I've addressed your comments in
https://github.com/httpwg/http-extensions/commit/d6e31360bdeeace4b8271162b0a8d25b836eb446.
(Some of them had already been fixed during other reviews.)
Emily
Post by Adam Roach
Post by Adam Roach
...
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
Post by Adam Roach
that contains an Expect-CT header field MUST contain an Expect-CT header
field
Post by Adam Roach
with a max-age directive, although that directive does not necessarily
need to
Post by Adam Roach
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
Adam Roach
2018-11-15 22:28:21 UTC
Permalink
Thanks. This addresses my concern, and I will clear my discuss once a
new version of the document is in the i-d repository.

/a
Post by Emily Stark
Hi Adam,
Thanks for the review. I've addressed your comments in
https://github.com/httpwg/http-extensions/commit/d6e31360bdeeace4b8271162b0a8d25b836eb446.
(Some of them had already been fixed during other reviews.)
Emily
Post by Adam Roach
...
   The "max-age" directive is REQUIRED to be present within an
"Expect-
Post by Adam Roach
   CT" header field.
This doesn't appear to be true as stated; or, at least, it is
stated in a
Post by Adam Roach
somewhat confusing way. A casual reading of this requirement is
that an
Post by Adam Roach
"Expect-CT" header field is noncompliant if it is missing this
directive.
Post by Adam Roach
Based on the examples given, the actual requirement here is that
a response
Post by Adam Roach
that contains an Expect-CT header field MUST contain an
Expect-CT header field
Post by Adam Roach
with a max-age directive, although that directive does not
necessarily need to
Post by Adam Roach
appear in each Expect-CT header field. This should probably be
clarified.
Post by Adam Roach
...
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
Loading...