Subject: Comments on draft-ietf-httpbis-replay-01.txt

From: Eric Rescorla <[email-masked]>
Date: Fri, 17 Nov 2017 10:02:17 +0800

This document seems like it's in good shape and is mostly ready for WGLC.
Some detailed comments below.

-Ekr

   The words "MUST", "MUST NOT", "SHOULD", and "MAY" are used in this
   document.  It's not shouting; when they are capitalized, they have
   the special meaning defined in [RFC2119].

You should just cite RFC 8174.

S 3.
   A server decides whether or not to offer a client the ability to send
   early data on future connections when sending the TLS session ticket.

This reveals an ambiguity in the QUIC spec. See
https://github.com/quicwg/base-drafts/issues/942

   A request might be sent partially in early data with the remainder of
   the request being sent after the handshake completes.  This does not
   necessarily affect handling of that request; what matters is when the
   server starts acting upon the contents of a request.  Any time a
   server might initiate processing prior to completion of the handshake
   it needs to consider how a possible replay of early data could affect
   that processing (see also Section 6.2).

   A server can partially process requests that are incomplete.  Parsing
   header fields - without acting on the values - and determining
   request routing is likely to be safe from side-effects, but other
   actions might not be.

This text seems like it might be sharper. If you receive a request that
is partly in early data, but you don't process it until you have
the whole request, this guarantees that it has not been replayed
to you but not to others, if they start processing earlier. So maybe
say:

   what matters is when servers starts acting upon the contents
   of a request.  Any time any server instance might initiate
   processing prior to completion of the handshake, all server instances
   need to consider how a possible replay of early data could affect that
   processing (see also Section 6.2).

S 4.
   If the server rejects early data at the TLS layer, a client MUST
   start sending again as though the connection was new.  For HTTP/2,
   this means re-sending the connection preface.  Any requests sent in
   early data MUST be sent again, unless the client decides to abandon
   those requests.

You should mention here that this might need ALPN negotiation.

This automatic retry exposes the request to a potential replay
   attack.  An attacker sends early data to one server instance that
   accepts and processes the early data, but allows that connection to
   proceed no further.  The attacker then forwards the same messages
   from the client to another server instance that will reject early
   data.  The client then retries the request, resulting in the request
   being processed twice.  Replays are also possible if there are
   multiple server instances that will accept early data, or if the same
   server accepts early data multiple times (though this would be in
   violation of requirements in TLS).

You should cite the specific section in TLS (S 8).

S 5.
      The "Early-Data" header field is included in requests that are
      received in early data.

Do you mean "partially received" here?

S 5.2.
   Clients (user-agents and intermediaries) that sent the request in
   early data MUST automatically retry the request when receiving a 425
   (Too Early) response status code.  Such retries MUST NOT be sent in
   early data.

   Intermediaries that receive a 425 (Too Early) status code MAY
   automatically retry requests after allowing the handshake to complete
   unless the original request contained the "Early-Data" header field
   when it was received.  Otherwise, an intermediary MUST forward the
   425 (Too Early) status code.

I am having trouble reading this text. It seems to me that the first
graf says that intermediaries MUST retry when I receive 425, and then
the second graf says that it MUST instead forward 425 if the
Early-Data header was not in the request.

---

From: Martin Thomson <[email-masked]>
Date: Fri, 17 Nov 2017 10:19:01 +0800

These are all good comments, thanks.  These are all actionable, so I
should have a PR (or PRs) to review soon.  Just not this week :)

---

From: Willy Tarreau <[email-masked]>
Date: Fri, 17 Nov 2017 06:40:32 +0100

Hi Eric,

> Some detailed comments below.

Thanks for your comments, here are some of mine below.

> https://github.com/quicwg/base-drafts/issues/942

Wasn't this max_early_data server parameter initially designed to
advertise how many bytes of early data the server is able to store ?
We probably need to be careful regarding this, as some implementations
may experience difficulties if they rely on separate buffers. I believe
that openssl's client code uses this as a max for the number of bytes
to send.

>    processing (see also Section 6.2).

I think it's indeed better this way. Probably that we can add that a
completed handshake confirms the validity of initially seen data, as
some implementers may wonder if they need to restart processing from
scratch once the handshake completes.

> You should mention here that this might need ALPN negotiation.

Then we should mention it before the HTTP/2 sentence so that the text
flows according to the operations flow.

> Do you mean "partially received" here?

It's "at least partially received", or "at least one bit from this request
was determined based on contents received in early data". I remember my
coworker who implemented it in haproxy asked me a similar question, we
need to reformulate this one I think.

> Early-Data header was not in the request.

You're right, I think we should simply remove the parenthesis in the
first paragraph and leave "clients".

Thanks!
Willy

---

From: Martin Thomson <[email-masked]>
Date: Tue, 21 Nov 2017 16:44:52 +1100

Proposed changes are here:  https://github.com/httpwg/http-extensions/pull/423

I've trimmed my response to just those that I think need some
attention.  In particular, see the last change, which is substantial.
Though I think that it remains editorial in nature, I really want to
make sure that I haven't made a mistake.

> Do you mean "partially received" here?

I've changed the definition to:

* The `Early-Data` header field is included in requests that might have been
  forwarded by an intermediary prior to the TLS handshake completing.

I think that covers your question better, and should address Willy's
concerns too.  I don't want to overload this too much with detail,
which is easy, but I think that this is accurate enough.

> Early-Data header was not in the request.

At Willy's prompting, I removed the parenthetical on the first
paragraph, but then realized that our recent changes to how this works
make that first bit quite misleading.  I've changed it to concentrate
on User Agents instead of clients.  That only helps a little though.

I think that I have something that makes the intermediary text much
clearer.  That is, concentrate first on forwarding 425.  This is
always possible because we have taken steps to ensure that early data
is opt in along the whole intermediary chain.  Then, add a note about
when it is possible to hold and retry requests at an intermediary, but
that is the exceptional case.

~~~
User agents that send a request in early data MUST automatically retry the
request when receiving a 425 (Too Early) response status code. Such retries MUST
NOT be sent in early data.

In all cases, an intermediary can forward a 425 (Too Early) status code.
Intermediaries MUST forward a 425 (Too Early) status code if the request that it
received and forwarded contained an `Early-Data` header field. An intermediary
that receives a request in early data MAY automatically retry that request in
response to a 425 (Too Early) status code, but it MUST wait for the TLS
handshake to complete on the connection where it received the request.
~~~

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 21 Nov 2017 15:51:41 -0600

> Proposed changes are here:  https://github.com/httpwg/http-extensions/pull/423

I left a couple of minor notes there.

> handshake to complete on the connection where it received the request.

I think this captures the intended semantics.

-Ben