Subject: draft-ietf-httpbis-replay-03, "5.1. The Early-Data Header Field"

From: Julian Reschke <[email-masked]>
Date: Fri, 11 May 2018 13:53:41 +0200

"The Early-Data header field carries a single bit of information and 
clients MUST include at most one instance. Multiple instances MUST be 
treated as equivalent to a single instance by a server."

What would that mean if what's sent is:

   Early-Data: 1
   Early-Data: 0

?

Or

   Early-Data: 1
   Early-Data: 0

?

Best regards, Julian

---

From: Willy Tarreau <[email-masked]>
Date: Fri, 11 May 2018 14:13:40 +0200

Hi Julian,

On Fri, May 11, 2018 at 01:53:41PM +0200, Julian Reschke wrote:
> "The Early-Data header field carries a single bit of information and clients
> MUST include at most one instance. Multiple instances MUST be treated as
> equivalent to a single instance by a server."
> 

I'd argue it's similar to :

  Early-Data: 1
  Early-Data: foobar

In that only the token "1" is defined as a valid value for this header field.
At this point I think it will depend on the implementation. Some will find
"Early-Data: 1" and be happy with it. Others will look closer and will find
an invalid value, either discarding it or deciding that Early-Data doesn't
carry a valid value (eg: "Early-Data: 1, foobar").

I would personally suggest that we propose that the presence of any non-empty
value in this header field MUST be considered as a boolean "true" value and
that implementation SHOULD write 1 there. It would be safer in my opinion.

Willy

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 11 May 2018 21:17:04 +0900

> What would that mean if what's sent is:

Early-Data header field only takes one value: "1". Therefore, having
multiple instances can be equal to having just one.

-- 
Kazuho Oku

---

From: Julian Reschke <[email-masked]>
Date: Fri, 11 May 2018 14:35:30 +0200

On 2018-05-11 14:17, Kazuho Oku wrote:
> ...

Understood. I'm just worried about error handling being unspecified in 
cases like the above.

Best regards, Julian

---

From: Martin Thomson <[email-masked]>
Date: Sat, 12 May 2018 10:44:08 +1000

We're already saying this about invalid constructions:

The Early-Data header field carries a single bit of information and clients
MUST include at most one instance. Multiple instances MUST be treated as
equivalent to a single instance by a server.

Why not go all out?

Multiple or invalid instances of the header field MUST be treated as
equivalent to a single instance with a value of 1 by a server.
On Fri, May 11, 2018 at 10:42 PM Julian Reschke <[email-masked]>

> >> "The Early-Data header field carries a single bit of information and
clients

---

From: Willy Tarreau <[email-masked]>
Date: Sat, 12 May 2018 07:00:51 +0200

> equivalent to a single instance with a value of 1 by a server.

That sounds perfect to me and allows us to extend it in the future if we
feel like we need it.

+1

Willy

---

From: Martin Thomson <[email-masked]>
Date: Sun, 13 May 2018 08:25:50 +1000

PR: https://github.com/httpwg/http-extensions/pull/622

---

From: Mark Nottingham <[email-masked]>
Date: Sun, 13 May 2018 12:41:25 +1000

This sounds like a fine resolution.

Just to make sure that Structured Headers is fit for purpose (i.e., not trying to get adoption here!), *if* this header were based upon SH, would its current design be adequate?

I think so, just want to make sure.

Regards,

> On 12 May 2018, at 3:00 pm, Willy Tarreau <[email-masked]> wrote:
> 

--
Mark Nottingham   https://www.mnot.net/

---

From: Willy Tarreau <[email-masked]>
Date: Sun, 13 May 2018 06:38:35 +0200

> I think so, just want to make sure.

I think so as well.

Willy

---

From: Martin Thomson <[email-masked]>
Date: Sun, 13 May 2018 20:10:34 +1000

On Sun, May 13, 2018 at 2:38 PM Willy Tarreau <[email-masked]> wrote:
> > Just to make sure that Structured Headers is fit for purpose (i.e., not
trying to get adoption here!), *if* this header were based upon SH, would
its current design be adequate?

> I think so as well.

Quoting: "header field authors are encouraged to clearly state additional
constraints upon the syntax, as well as the consequences when those
constraints are violated"

It seems to me like we've done that.  Note that we're suggesting a
contradictions with this though: "If parsing fails – including when calling
another algorithm – the entire header field’s value MUST be discarded."
  And I'm OK with that in this case.  Were we to cite structured headers, we
would probably want to call out that direct contravention.

---

From: Willy Tarreau <[email-masked]>
Date: Sun, 13 May 2018 12:16:57 +0200

On Sun, May 13, 2018 at 08:10:34PM +1000, Martin Thomson wrote:
> contradictions with this though: "If parsing fails - including when calling
> another algorithm - the entire header field's value MUST be discarded."

Indeed, this rule may have some nasty side effects if it causes the loss
of a header field serving as a signal.

> would probably want to call out that direct contravention.

I agree.

Willy

---

From: Mark Nottingham <[email-masked]>
Date: Mon, 14 May 2018 08:45:11 +1000

> On 13 May 2018, at 8:16 pm, Willy Tarreau <[email-masked]> wrote:
> 

The idea here is that if you're using SH, you defer to it for error handling of this nature, and that error handling is, shall we say, quite "crisp." As long as parser end of the connection is using SH implementations and those implementations are reasonably conformant, you should get decent interop, since non-conformant headers will fail consistently and hard.

>> would probably want to call out that direct contravention.

My assumption was that if this spec were using SH, you wouldn't need to say anything beyond 'this is the structure we expect:...".

Cheers,

---

From: Julian Reschke <[email-masked]>
Date: Mon, 14 May 2018 07:00:19 +0200

> My assumption was that if this spec were using SH, you wouldn't need to say anything beyond 'this is the structure we expect:...".

Agreed. If we need a special case for something simple like that, one of 
the two specs needs to change.

Best regards, Julian

---

From: Willy Tarreau <[email-masked]>
Date: Mon, 14 May 2018 07:36:43 +0200

On Mon, May 14, 2018 at 07:00:19AM +0200, Julian Reschke wrote:
> Agreed. If we need a special case for something simple like that, one of the
> two specs needs to change.

I was trying to figure what other very sensitive header field we have
which falls in this situation where we don't want to simply ignore nor
discard it. Transfer-Encoding is one such header field. What we do
there for non-100% valid requests is to return 400 (cf 7230#3.3.3).

Given the sensitifivity of the Early-Data header field, I'm now more
convinced that the risk of being too tolerant with it could lead to
some agents setting it lazily and causing interoperability issues or
opening replay attacks. Thus I would suggest the following instead :

  If a server receives a request message containing more than one
  Early-Data header field or one such header field with a value that
  is not exactly "1", the server MUST respond with a 400 (Bad Request)
  status code and thne close the connection.

Optionally we could try to deduplicate identical values like we already
do for Content-Length, but I'm inclined to think that it's up to the
intermediaries which decide to add this field to first verify if another
one was already present.

I think we should always keep SH in mind when adding new fields, and try
hard not to open new cans of worms all the time. That doesn't mean it
will always work but at least we have to try.

Cheers,
Willy

---

From: Mark Nottingham <[email-masked]>
Date: Mon, 14 May 2018 15:56:35 +1000

The general discussion around SH error handling has been that if we're able to establish good interop early on with clear algorithms and a test suite, it's not necessary to have messy post-hoc special casing like we do with many existing HTTP headers.

See also <https://tools.ietf.org/html/draft-iab-protocol-maintenance-00>.

Cheers,

> On 14 May 2018, at 3:36 pm, Willy Tarreau <[email-masked]> wrote:
>

---

From: Martin Thomson <[email-masked]>
Date: Mon, 14 May 2018 16:21:00 +1000

Mark, are you advocating for removing the text around duplicate values and
falling back to something like:

"If the header field is repeated or contains a value other than "1" it is
invalid.  An invalid header field SHOULD be disregarded."

There's an interesting question here about how the requirement to discard
malformed header fields generates appropriate feedback signals, but I think
that you have a fair point.  If we stop assuming that we have to deal with
actors that fail to implement correctly, then we exert pressure on those
actors to comply.  And everyone benefits because our code is simpler and
more consistent overall.

I was initially attracted to the idea that maybe *just this one time* the
use case was "special".  But that's dangerous thinking.

One comment for SH.  You permit additional constraints on syntax (like
ranges on integers).  You separately note that if parsing fails the header
field is dropped.  You should connect the two explicitly. If a header field
is valid according to SH, but invalid according to constraints on the
specific header field what happens?
   * it needs to be dropped because malformed header fields are dropped
according to SH
   * the definition of the specific header field needs to define what
happens in this case

I think that your assumed position here is the former, but that needs to be
really crisp.

https://github.com/httpwg/http-extensions/issues/623
On Mon, May 14, 2018 at 3:56 PM Mark Nottingham <[email-masked]> wrote:

> The general discussion around SH error handling has been that if we're
able to establish good interop early on with clear algorithms and a test
suite, it's not necessary to have messy post-hoc special casing like we do
with many existing HTTP headers.

> >>> My assumption was that if this spec were using SH, you wouldn't need
to say anything beyond 'this is the structure we expect:...".

---

From: Mark Nottingham <[email-masked]>
Date: Mon, 14 May 2018 16:29:02 +1000

> On 14 May 2018, at 4:21 pm, Martin Thomson <[email-masked]> wrote:
> 

As I said at the start, I'm not asking for any changes to the early-data header field; just trying to validate that in theory, SH would be usable for it (if the timing worked out).

Of course, we *can* make changes if we wish.

> actors to comply.  And everyone benefits because our code is simpler and

*If* it's consistent. We're putting mechanisms in place for SH to help that (generic implementations and a test suite); not sure that applies here.

> use case was "special".  But that's dangerous thinking.

:)

> https://github.com/httpwg/http-extensions/issues/623

Section 2 of the editors' draft already says:

"However, header field authors are encouraged to clearly state additional constraints upon the syntax, as well as the consequences when those constraints are violated."

https://httpwg.org/http-extensions/draft-ietf-httpbis-header-structure.html#specify

Happy to take PRs to expand upon that; if not, I'll try to tease it out.

Cheers,

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 14 May 2018 15:35:22 +0900

Maybe I am missing some context, but the question here (for SH) seems
to me that how we should carry a boolean flag.

I think that there could be two ways:
1. use the existence of the header as the flag, as Early-Data draft does
2. always require the header to carry a boolean value

I do not mind whichever we choose, but I would prefer having one way
of doing that defined in SH.

-- 
Kazuho Oku

---

From: Willy Tarreau <[email-masked]>
Date: Mon, 14 May 2018 18:29:49 +0200

> to me that how we should carry a boolean flag.

Absolutely agreed.

> of doing that defined in SH.

I *tend* to think that being a bit stricter (ie value must match) is better
for the long term : over time we've seen many times a boolean become an enum
and that becomes a problem because anyone sends anything if the presence only
suffices. By enforcing strictness (400 bad req if doesn't match) we reserve
all unspecified values for later use if required.

Willy

---

From: Mark Nottingham <[email-masked]>
Date: Tue, 15 May 2018 15:42:10 +1000

SH doesn't allow empty header fields, FWIW.

We also don't have boolean values, but it seems trivial to use an integer.

> On 14 May 2018, at 4:35 pm, Kazuho Oku <[email-masked]> wrote:
>

---

From: Willy Tarreau <[email-masked]>
Date: Tue, 15 May 2018 08:11:51 +0200

> We also don't have boolean values, but it seems trivial to use an integer.

Sure but integer also means parsing, deduplicating, and error path for
what doesn't parse according to expectations. Supporting boolean values
"header field is present and non empty" could really be useful IMHO.

Willy

---

From: Mark Nottingham <[email-masked]>
Date: Tue, 15 May 2018 16:39:05 +1000

> On 15 May 2018, at 4:11 pm, Willy Tarreau <[email-masked]> wrote:
> 
> what doesn't parse according to expectations.

But all of those things are already defined by SH for integers..

> Supporting boolean values

I think the question is whether they're useful enough -- as compared to just using an integer with a value constrained to 0 or 1 -- that it justifies the extra machinery (in the spec and in implementations).

Cheers,

---

From: Julian Reschke <[email-masked]>
Date: Tue, 15 May 2018 09:00:51 +0200

> SH doesn't allow empty header fields, FWIW.

...which makes it incompatible with the list rule in RFC 7230, no?

---

From: Mark Nottingham <[email-masked]>
Date: Tue, 15 May 2018 17:03:13 +1000

> ...which makes it incompatible with the list rule in RFC 7230, no?

In what sense?

---

From: Julian Reschke <[email-masked]>
Date: Tue, 15 May 2018 09:13:30 +0200

>> ...which makes it incompatible with the list rule in RFC 7230, no?

Because it says, that if you header is defined as

   #token

you can send

   Foo: a
   Foo:
   Foo: b

and it would be equivalent to

   Foo: a, b

Best regards, Julian

---

From: Mark Nottingham <[email-masked]>
Date: Tue, 15 May 2018 17:19:15 +1000

> On 15 May 2018, at 5:13 pm, Julian Reschke <[email-masked]> wrote:
> 

Ah. 

I didn't mean "empty header fields" in that sense -- rather, just

Foo:

(we REALLY need some terminology to cleanly differentiate between the various senses of "header")

That said, SH doesn't allow that either, or:

Foo: a,,b

---

From: Julian Reschke <[email-masked]>
Date: Tue, 15 May 2018 09:27:37 +0200

> (we REALLY need some terminology to cleanly differentiate between the various senses of "header")

"single field value" vs "aggregated field value"?

Best regards, Julian

---

From: Willy Tarreau <[email-masked]>
Date: Tue, 15 May 2018 11:28:28 +0200

> I didn't mean "empty header fields" in that sense -- rather, just

FWIW I've used many times "Host:" with no name when connecting to an
HTTP/1.1 server without knowing its name, and rarely faced any issue
with this. I'm not saying it's right or wrong, but that many
implementations support an empty header field and distinguish it from
an absent one.

Willy

---

From: Willy Tarreau <[email-masked]>
Date: Tue, 15 May 2018 11:34:46 +0200

On Tue, May 15, 2018 at 04:39:05PM +1000, Mark Nottingham wrote:
> I think the question is whether they're useful enough -- as compared to just
> using an integer with a value constrained to 0 or 1 -- that it justifies the
> extra machinery (in the spec and in implementations).

But we're back to the problem Julian raised which is :

    When Structured Headers parsing fails, the header is discarded

Booleans are different from integers. Booleans indicate a status. Eg:
"safe" vs "unsafe". Discarding the "unsafe" status because we failed to
parse it is dangerous. Actually I'm starting to think that the rule
consisting in silently discarding a header field that fails to parse is
the problem here. Normally we're supposed to return "400 bad req" on
parsing errors, and I fear we're becoming too lenient on parsing and
introduce a new class of problems.

I'd rather be a bit stricter by suggesting that if a recipient fails
to parse a header field, it may attempt to fix it according to a number
of well-established rules (eg deduplicate it if it's supposed to be
unique as 7230 does for Content-Length), or fail with 400 bad req.

Anyway I'm really not fine anymore with imagining that an agent along
the chain would silently remove "Early-Data: 1, 1". This causes a big
issue. Either we fix it or we reject. And given that this error might
be caused by an intermediary possibly not really impacted by Early-Data
but just aware that it's supposed to be an integer and wanting to fix
it, there's no way we can even expect this intermediary to comply with
specific provisions made in the Early-Data spec, so it clearly is SH
that must prevent this from happening.

Cheers,
Willy

---

From: Mark Nottingham <[email-masked]>
Date: Tue, 15 May 2018 19:38:11 +1000

> On 15 May 2018, at 7:28 pm, Willy Tarreau <[email-masked]> wrote:
> 

Sure - although that's pretty broken for Host, this isn't a completely uncommon thing. When we did SOAPAction way back when (shudder), we came to the conclusion that some HTTP implementations might drop empty headers, so it wasn't safe -- but I don't remember how much data that was based upon.

That said, just because it's possible with HTTP headers doesn't mean it needs to be possible with structured headers.

Cheers,

---

From: Mark Nottingham <[email-masked]>
Date: Tue, 15 May 2018 19:46:02 +1000

> parse it is dangerous.
> Actually I'm starting to think that the rule

400 is for HTTP-level parsing issues; e.g., message delimitation -- at least from generic HTTP implementations. Not being able to parse an extension header is a totally different thing. 

By their nature, extensions need to be "safe" by default; i.e., if the extension isn't in use, the resulting behaviour shouldn't be bad -- because most of the Web won't exhibit the extension, at least at first.  So, if there's a parsing error, dropping the header *should* be safe as well.

I can see an argument that we should allow headers to say that in event of parsing failure, some status code should be generated, etc.; we should talk about that and figure out if it's a good idea. What I'd push back on is the notion that a header using SH should define more complex error handling such as dedup or fixup of the SH-defined parts; that's removing a good amount of value from SH.

> unique as 7230 does for Content-Length), or fail with 400 bad req.

Those things are necessary because CL et al were loosely specified at the start, and saw significant divergence in implementation at the same time that they saw wide deployment. That's not the case with SH; we're trying to define it tightly up-front so that such things are unnecessary. 

> the chain would silently remove "Early-Data: 1, 1".

In SH, this would only be the parser that's actually consuming the header, not anyone along the chain, FWIW.

> This causes a big

Early-Data doesn't use SH.

And, as above, intermediaries aren't expected to implement SH -- at least yet.

Cheers,

---

From: Willy Tarreau <[email-masked]>
Date: Tue, 15 May 2018 12:00:13 +0200

On Tue, May 15, 2018 at 07:38:11PM +1000, Mark Nottingham wrote:
> Sure - although that's pretty broken for Host, this isn't a completely
> uncommon thing. When we did SOAPAction way back when (shudder), we came to
> the conclusion that some HTTP implementations might drop empty headers, so it
> wasn't safe -- but I don't remember how much data that was based upon.
> 

I agree. I've also seen some components drop them and we'd rather not
rely on them.

Willy

---

From: Julian Reschke <[email-masked]>
Date: Tue, 15 May 2018 12:06:30 +0200

> 400 is for HTTP-level parsing issues; e.g., message delimitation -- at least from generic HTTP implementations. Not being able to parse an extension header is a totally different thing.

400 is for anything caused by the client, and not covered by a more 
specific 4xx code.

It might make sense to define a new 4xx code to cover the case of a 
syntax error in a request header field value.

Best regards, Julian

---

From: Mark Nottingham <[email-masked]>
Date: Tue, 15 May 2018 20:18:18 +1000

> On 15 May 2018, at 8:06 pm, Julian Reschke <[email-masked]> wrote:
> 
> 400 is for anything caused by the client, and not covered by a more specific 4xx code.

Yes, but its use is not required upon any error by the client.

> It might make sense to define a new 4xx code to cover the case of a syntax error in a request header field value.

Do we have examples of extension headers needing / doing this? 

None of the ones in recent memory do (e.g., Access-Control-Allow-*, Accept-Patch, Accept-Post, ALPN, Alt-Used, Cookie (bis), HTTP2-Settings, Origin, Prefer).

That's not to say that we'll never encounter another header that justifies a hard error reflected in a status code, just that it doesn't seem common, so we might be optimising (well, specifying) prematurely here.

I would say we could define a "SH-Errors-Encountered" header to stick onto a 400, but I think that creates a more complex, coupled protocol than we want here (YMMV), and it would encourage header authors to generate that 400, rather than having safe defaults -- which is I think the pattern we want them to be following.

Cheers,

---

From: Julian Reschke <[email-masked]>
Date: Tue, 15 May 2018 12:34:36 +0200

> Yes, but its use is not required upon any error by the client.

Right. I was just disagreeing with "400 is for HTTP-level parsing 
issues; e.g., message delimitation -- at least from generic HTTP 
implementations".

> I would say we could define a "SH-Errors-Encountered" header to stick onto a 400, but I think that creates a more complex, coupled protocol than we want here (YMMV), and it would encourage header a…

...well, we got here because we were discussing error handling for 
malformed Early-Data header fields. If Early-Data was using SH, this 
would be an example.

Best regards, Julian

---

From: Mark Nottingham <[email-masked]>
Date: Tue, 15 May 2018 20:56:24 +1000

> On 15 May 2018, at 8:34 pm, Julian Reschke <[email-masked]> wrote:
> 
> ...well, we got here because we were discussing error handling for malformed Early-Data header fields. If Early-Data was using SH, this would be an example.

I could see doing that if we perhaps added an appendix on error handling that explained the nuances here.