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

On Wed, May 30, 2018 at 11:09 PM Martin Thomson <martin.thomson@gmail.com>
wrote:

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

Might have been blindly cribbed from HSTS/HPKP -- I don't remember
discussing it specifically for Expect-CT. Filed
https://github.com/httpwg/http-extensions/issues/637


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

This was also cribbed from HSTS/HPKP. I'll try to find out the motivation
for including it in those specs; I'm a little wary of dropping it from
Expect-CT without understanding why it's there for the other two...


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

Not sure I quite follow this, can you clarify? Are you expecting that an
Expect-CT header should be disregarded if Age is present?


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

Section 2.4 is intended to cover this: "It is acceptable to skip this CT
compliance check for some hosts according to local policy. For example, a
UA may disable CT compliance checks for hosts whose validated certificate
chain terminates at a user-defined trust anchor, rather than a trust anchor
built-in to the UA (or underlying platform)."


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

I think this should probably be up to the UA. I'm not sure I see where the
document assumes that Expect-CT is the very last check; Section 2.4 is
intended to say that the UA evaluates CT compliance if it is able to
successfully set up a TLS connection, but intentionally doesn't specify
exactly when except that it should be before beginning an HTTP
conversation, and that if CT compliance isn't evaluated for whatever
reason, reports aren't sent.


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

I think we could reasonably drop one copy of the certificate chain, and
maybe just limit it to the leaf cert... but the SCT list and other
information is certainly useful for debugging misconfigurations. I'm not
sure it's worth making the backwards-incompatible change of reducing the
cert chains at this point -- it doesn't remove enough data to remove the
risk of a self DoS. Sites are advised to use separate reporting
infrastructure for this reason.


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

Errr... how do I find out who needs to send what email to what mailing list?


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

I'm not sure what specifically needs to change here (also note that
report-uri is not required to be quoted,
https://github.com/httpwg/http-extensions/issues/312, unless we want to
revisit that). directive-value is defined as token / quoted-string in
Figure 1... is there something I'm supposed to have in Figure 3 to say that
max-age-value is a directive-value?


>
> 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).
>
>
Both HSTS and HPKP use "host". Filed
https://github.com/httpwg/http-extensions/issues/638


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

One is accidentally a word,
https://github.com/httpwg/http-extensions/issues/639


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

https://github.com/httpwg/http-extensions/issues/640


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

https://github.com/httpwg/http-extensions/issues/644


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

https://github.com/httpwg/http-extensions/issues/644


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

Done (
https://github.com/httpwg/http-extensions/commit/68e30c45c81b72e8aed166e114d2ae460bcf942a
)


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

https://github.com/httpwg/http-extensions/issues/643


>
> Section 3.1 cites Section 4.6 of RFC6962-bis, but this moved to 4.5.
>

Done (
https://github.com/httpwg/http-extensions/commit/5ca7c4e8218bf2c8a8194ac68c66518fbba5b809
)

Also, there is a "Section 4 **or** [RFC6962]" to fix here.
>

Done (
https://github.com/httpwg/http-extensions/commit/68e30c45c81b72e8aed166e114d2ae460bcf942a
)


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

https://github.com/httpwg/http-extensions/issues/642


>
> Promote Section 8.1 to Section 8.
>

Done (
https://github.com/httpwg/http-extensions/commit/68e30c45c81b72e8aed166e114d2ae460bcf942a
)


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