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

I've pushed fixes for all the github issues now.

On Mon, Jun 4, 2018 at 1:56 PM Emily Stark <estark@google.com> wrote:

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