Subject: Re: [websec] Strict-Transport-Security syntax redux

From: =JeffH <[email-masked]>
Date: Fri, 28 Oct 2011 19:36:35 -0700

>> Strict-Transport-Security = "Strict-Transport-Security" ":"
 >> directive *( ";" [ directive ] )
 >>
 >> STS directives:
 >>
 >> directive = max-age | includeSubDomains | STS-d-ext
 >>
 >> max-age = "max-age" "=" delta-seconds
 >
 > What happens with
 >
 >    max-age="1"
 >
 > ?
 >
 > Do you expect all recipients to reject this? Depending on the parsing
 > API they use they might not even know that the value was quoted on the wire.

Offhand, I'm not super sure, but after thinking about it while 
researching/writing the below, I'm thinking "yes", max-age="1" is invalid 
according to the ABNF and we should do whatever we do in error cases (which is 
a separate open question). But implementors' parsing API and its problems are 
out-of-scope for a spec such as this.

This obviously isn't the first HTTP response header field with such a syntax 
and thus these potential issues (this one with a delta-seconds value, and the 
issue you note below wrt "includeSubDomains"), yes?

In doing a quick grep on RFCs for delta-seconds, I note that some of the specs 
using it (I didn't look at them all) appear to not directly address the case above.

Except for RFC6265, which in the algorithm for parsing "Max-Age=", it 
algorithmically provides for ignoring a value that doesn't match the effective 
value ABNF of..

   ["-"]*DIGIT

..which would catch the max-age="1" case, but doesn't seem to explicitly address..

   max-age=

But in any case, perhaps (additional) browser implementor folk could chime in 
here -- do we really need to address such detail-level issues (both of the 
examples above and below) in the syntax/grammar we specify in specs such as 
these? Or is the new ABNF proposed in the original message in this thread 
sufficient?

 > > includeSubDomains = "includeSubDomains"
 >
 > There's a tiny risk that some implementations will handle value-less
 > parameters the same way as parameters with empty values, such as
 >
 >    includeSubDomains=
 >
 > or
 >
 >    includeSubDomains=""
 >
 > but maybe I'm over-engineering here.

Yes, I'm wondering if this might be over-engineering -- I note that in
RFC 6266 you didn't distinguish/address this sort of case for "inline" or 
"attachment" -- are you feeling now that you should have, and thus we ought to 
do so going forward?

 > Also, identifiers "max-age" and "includeSubDomains" are
 > case-insensitive, right? This follows from the ABNF,

yes, and yes.

 > but might be worth
 > saying again in prose; in particular because it also needs to be the
 > case for all future extensions.

Agreed. I see how you did it in RFC 6266 and will endeavor to do similarly.

thanks again,

=JeffH

---

From: Adam Barth <[email-masked]>
Date: Fri, 28 Oct 2011 20:08:34 -0700

On Fri, Oct 28, 2011 at 7:36 PM, =JeffH <[email-masked]> wrote:
> according to the ABNF and we should do whatever we do in error cases (which
> is a separate open question). But implementors' parsing API and its problems
> are out-of-scope for a spec such as this.
>
> and thus these potential issues (this one with a delta-seconds value, and
> the issue you note below wrt "includeSubDomains"), yes?
>
> In doing a quick grep on RFCs for delta-seconds, I note that some of the
> specs using it (I didn't look at them all) appear to not directly address
> the case above.
>
> algorithmically provides for ignoring a value that doesn't match the
> effective value ABNF of..
>
>  ["-"]*DIGIT
>

That's handled by some more general processing rules in the spec.  The
net result is that it's ignored.

> But in any case, perhaps (additional) browser implementor folk could chime
> in here -- do we really need to address such detail-level issues (both of
> the examples above and below) in the syntax/grammar we specify in specs such
> as these? Or is the new ABNF proposed in the original message in this thread
> sufficient?

Generally, we prefer to be instructed exactly how to behave for every
possible input (even illegal ones).  There's a long history of
quoted-string not being implemented correctly by browsers.  I spec
this as just splitting the string on ; and then processing each
substring separately, ignoring bogus/future ones.  I know Julian has a
dream that all HTTP headers will be parsed the same, but quoted-string
is sufficiently ill-defined w.r.t. error handling that I prefer to
avoid it.

> "attachment" -- are you feeling now that you should have, and thus we ought
> to do so going forward?
>
>
> _______________________________________________
> websec mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/websec
>

---

From: Julian Reschke <[email-masked]>
Date: Sat, 29 Oct 2011 09:59:26 +0200

On 2011-10-29 04:36, =JeffH wrote:
> researching/writing the below, I'm thinking "yes", max-age="1" is
> invalid according to the ABNF and we should do whatever we do in error
> cases (which is a separate open question). But implementors' parsing API
> and its problems are out-of-scope for a spec such as this.

They are out-of-scope in that we don't specify them. But we still should 
consider what's likely to be implemented, and the cost of not being able 
to re-use a generic parser.

> This obviously isn't the first HTTP response header field with such a
> syntax and thus these potential issues (this one with a delta-seconds
> value, and the issue you note below wrt "includeSubDomains"), yes?

Indeed. It's just I've been paying more attention to these kinds of 
issues recently :-)

> RFC 6266 you didn't distinguish/address this sort of case for "inline"
> or "attachment" -- are you feeling now that you should have, and thus we
> ought to do so going forward?
> ...

That case is a bit different as this is a stand-alone token, and there 
do not seem to be any header fields that are similar to C-D but allow a 
quoted-string in the first position.

Also see <http://greenbytes.de/tech/tc2231/#inlonlyquoted> -- some 
recipients reject it, some don't. This is not a big issue per se, unless 
those you reject it are forced to follow suite because senders start to 
rely on it.

> ...

Best regards, Julian

---

From: Julian Reschke <[email-masked]>
Date: Sat, 29 Oct 2011 10:07:28 +0200

> is sufficiently ill-defined w.r.t. error handling that I prefer to

- when discussing generic parsing, we need to distinguish between legacy 
cases like cookies, and new headers, where we can do better

- standardizing handling of broken headers is one thing (and in general 
I prefer not to), but that doesn't mean that when defining a new header 
field we shouldn't minimize the things a sender can get wrong; if we 
know that some recipients will accept both token and quoted-string 
anyway, then it seems like a good thing to simply allow them both, 
reducing the number of special-cases in parsing

- not sure what you mean by "ill-defined w.r.t. error handling"; it's 
defined just like most other syntax elements in HTTP -- is there 
something *specific* to quoted-string you have in mind?

Best regards, Julian

---

From: Adam Barth <[email-masked]>
Date: Sat, 29 Oct 2011 01:10:30 -0700

On Sat, Oct 29, 2011 at 1:07 AM, Julian Reschke <[email-masked]> wrote:
> - standardizing handling of broken headers is one thing (and in general I
> prefer not to), but that doesn't mean that when defining a new header field
> we shouldn't minimize the things a sender can get wrong; if we know that
> some recipients will accept both token and quoted-string anyway, then it
> seems like a good thing to simply allow them both, reducing the number of
> special-cases in parsing
>
> defined just like most other syntax elements in HTTP -- is there something
> *specific* to quoted-string you have in mind?

Most of HTTP is ill-defined w.r.t. error handling.  We muddle through
with reverse engineering.

Adam

---

From: Julian Reschke <[email-masked]>
Date: Sat, 29 Oct 2011 10:19:30 +0200

On 2011-10-29 10:10, Adam Barth wrote:
> On Sat, Oct 29, 2011 at 1:07 AM, Julian Reschke<[email-masked]>  wrote:

It's not ill-defined, but undefined (by design) - see 
<http://trac.tools.ietf.org/wg/httpbis/trac/ticket/186>.

What I was trying to understand whether there's something special with 
respect to quoted-string?

Best regards, Julian

---

From: Adam Barth <[email-masked]>
Date: Sat, 29 Oct 2011 01:21:32 -0700

On Sat, Oct 29, 2011 at 1:19 AM, Julian Reschke <[email-masked]> wrote:
>>>>> ..which would catch the max-age="1" case, but doesn't seem to
>>>>> explicitly
>>>>> address..
>>>>>
>>>>>  max-age=
>>>>
>>> something

Quoted string is particularly bad because it's hard to know what to do
with unbalanced quotation marks.

Anyway, I'm resigned to lose this argument in this forum.  It just
means we'll need to clean up the mess later in another forum.

Adam

---

From: Julian Reschke <[email-masked]>
Date: Sat, 29 Oct 2011 11:06:41 +0200

> with unbalanced quotation marks.

So your points were about quoted-string in general, not the question of 
allowing them for max-age, right?

I'm asking because due the possible presence of extension parameters, 
recipients need to deal with quoted-string anyway, no matter whether 
they are allowed for max-age.

Best regards, Julian

---

From: Adam Barth <[email-masked]>
Date: Sat, 29 Oct 2011 11:50:16 -0700

On Sat, Oct 29, 2011 at 2:06 AM, Julian Reschke <[email-masked]> wrote:
> recipients need to deal with quoted-string anyway, no matter whether they
> are allowed for max-age.

I'm saying we shouldn't use quoted-string anywhere in the grammar
because it makes the error-handling ill-defined.

Here's the code I wrote to parse the header:

http://src.chromium.org/viewvc/chrome/trunk/src/net/base/transport_security_state.cc?revision=106333&view=markup

TransportSecurityState::ParseHeader

I'm happy to change that code to allow the parameters to appear in any
order and to allow other semi-colin separated fields.  Here's the
parsing algorithm I'm willing to implement:

1) Split the header field value on ";".
2) Process each substring in sequence:
  a) Remove leading and trailing LWS from the substring.
  b) If the substring is a case-insensitive match for
"includeSubDomains", set the include-subdomains flag and continue to
the next substring.
  c) If the substring contains at least one "=", let the characters
before the first "=" be the parameter-name and let the characters
after the first "=" be the parameter-value:
    i) Strip leading and trailing LWS from both the parameter-name and
the parameter-value.
    ii) If the parameter-name is a case insensitive match for
"max-age" and the parameter-value is non-empty and consists entirely
of digits:
      A) Let the max-age be the number represented by the digits and
continue to the next substring.
  d) Continue to the next substring.

Notice that this algorithm defines behavior for all inputs and doesn't
balance quotation marks in extension parameters.  Here's a
representation of that algorithm in ABNF:

value = paramater *(";" parameter)
paramater = "includeSubDomains" / "max-age" "=" 1*DIGIT *LWS / extension
extension = <any character aside from ";">

If you want the specification to match the code, you need to take into
account the desires of implementors.  In particular, you need to take
into account the fact that implementations need to do something for
every possible input and that browser implementors wish to be
instructed how to handle every possible input.

The net-net of this discussion is that's what the code is going to do.
 If you'd like the specification to match the code, I'd recommend
against adding aspirational quoted-strings to the grammar.

Adam

---

From: Adam Barth <[email-masked]>
Date: Sat, 29 Oct 2011 11:51:07 -0700

> extension = <any character aside from ";">

Sorry,

extension = *<any character aside from ";">

of course.

---

From: Phillip Hallam-Baker <[email-masked]>
Date: Sun, 30 Oct 2011 08:31:36 -0400

I think that the field is going to match the requirements of the HTTP spec:

http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2

       message-header = field-name ":" [ field-value ]
       field-name     = token
       field-value    = *( field-content | LWS )
       field-content  = <the OCTETs making up the field-value
                        and consisting of either *TEXT or combinations
                        of token, separators, and quoted-string>

That takes precedence over the desire to scrape together naive parsers in
my view.

Most people writing code are going to want to skip the need to write a
parser altogether by using existing parser methods built in to their
platform.

This is a standards based specification and so it has to match the format
already defined by the HTTP specification and aspirational or not, any code
that fails to match quoted string is likely to break in future spec
versions.

-- 
Website: http://hallambaker.com/