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

From: =JeffH <[email-masked]>
Date: Thu, 06 Oct 2011 16:23:55 -0700

Hi Ryan, thanks for the detailed feedback.

> Following Julian Reschke's questions, I also had a few questions related
> to the draft-02 syntax. I've been basing my understanding on RFC 5234,
> which I understand to be the most current/relevant to understanding the
> syntax.
>
>
> First, maxAge and includeSubDomains both include value extension points,
> defined as:
>
> 	maxAge     = "max-age"  OWS  "="  OWS  delta-seconds  [ OWS v-ext ]
> 	includeSubDomains =  "includeSubDomains"  [ OWS v-ext ]
> 	v-ext      = value     ; STS extension value
>
> However, the rule for OWS is specified as:
>
> 	OWS       = *( [ CRLF ] WSP )
>
> As written, it would seem that the OWS between either delta-seconds or
> "includeSubDomains" can legitimately be omitted before the v-ext. As best
> I can tell, this would mean that the following values would still be
> valid:
>
> 	max-age=123.456
> 	includeSubDomainsabc
>
> In the first case ".456" is the v-ext, while in the second, "abc" is. In
> both cases, because the OWS is truly optional (valid to have 0
> occurrences), only the v-ext is present.

yes, trying to use the OWS construction there is broken as you point out.

<snip/>
> To remedy this, I believe some form of explicit delimiter between the
> existing values and the v-ext should be defined for the existing headers.
> If the intent was to have extension values whitespace separated, then
> would the following modification to introduce required whitespace solve
> it?
>
> 	maxAge     = "max-age"  OWS  "="  OWS  delta-seconds  [ RWS v-ext ]
> 	includeSubDomains =  "includeSubDomains"  [ RWS v-ext ]
> 	v-ext      = value     ; STS extension value
> 	OWS       = *( [ CRLF ] WSP )
> 	RWS       = 1*( [ CRLF ] WSP )

Yes, using a required whitespace (RWS) construction would seem to fix it. 
However, since we've migrated to base the ABNF on RFC2616 and not HTTPbis, we 
should perhaps whole-hog and use the former's LWS (linear whitespace) construct 
which has the "implied whitespace" notion, and get rid of the OWS stuff.

> Alternatively, can/should the v-ext be dropped entirely, and extensions to
> the STS header be accomplished via defining a new STS-d-ext?

yes, that's a possibility. I put the v-ext thing in there with max-age for 
completeness without a good use case, and maybe it'll just cause more trouble 
than its worth.

> Second, as currently specified, extension directives take the form of:
>
> 	STS-d      = STS-d-cur / STS-d-ext
> 	STS-d-ext  = name      ; STS extension directive
> 	name       = token
> 	token      = 1*tchar
> 	tchar      = "!" / "#" / "$" / "%" / "&" / "'" / "*"
>       	     / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~"
> 	           / DIGIT / ALPHA
>
> As currently written, STS-d-ext doesn't seem to be able to contain name
> "=" value pairs such as those used by Evans' and Palmer's pinning draft,
> because "=" is not a valid token character. Likewise, it wouldn't be able
> to contain "#rule" style lists, because comma is not a valid tchar.

good catch.

> Should STS-d-ext be defined as "value" instead, so that it contain a wider
> range of characters?

offhand, "value" seems the way to go to me.

Again, thanks much for your review, I'm working on getting new rev out with 
repaired ABNF amongst other things.

=JeffH

---

From: Julian Reschke <[email-masked]>
Date: Sat, 08 Oct 2011 20:22:29 +0200

On 2011-10-07 01:23, =JeffH wrote:
> ...

Trying to summarize my concerns and proposals...

(1) RFC2616-or-HTTPbis

In a perfect world I could give you an exact time-of-arrival for 
HTTPbis. But the world is not perfect; and progress on HTTPbis mainly 
depends on the availability of the editors, and, more importantly, 
people showing up on the HTTPbis mailing list to help in closing issues 
and reviewing the existing text. So help *is* appreciated. -> 
<http://trac.tools.ietf.org/wg/httpbis/trac/wiki>

(2) Multiple header instances

Header fields can occur multiple times, even when they were intended not 
to. Examples: Location, Content-Length. When this happens, we usually 
see interop problems because some consumers pick the first, some pick 
the second, and some combine them using a comma, as described in HTTP spec.

This can be dangerous when the repetition makes the message format 
ambiguous (Content-Length, for instance). It also means that it allows 
smuggling, relying on checkers/intermediaries only seeing one of the 
headers.

Note that Chrome and FF have become very strict in checking for this for 
C-L, and FF does even more checks (syntax of C-L, also checks for 
Location and Content-Disposition).

So *if* STS doesn't allow multiple header fields (which would require 
switching to a comma-separated syntax), the spec should be crystal clear 
what to do when multiple instances are encountered; in many cases the 
default should be "ignore the header field" or even "consider the 
message to be broken". Not sure what's best here.

Also note that somebody else may be combining multiple fields into a 
single one, and the recipient will see those concatenated with commas as 
separators. Optimally the format is robust enough to detect things like 
that.

(3) ABNF organization

Any header field that is extensible needs to define the syntax for the 
extension point, so existing code can parse them. Don't make extensions 
different from the builtins with respect to syntax. It just makes the 
parser more complicated.

So just define a single ABNF for STS directives in ABNF.

For instance, using RFC 2616 ABNF and list syntax:

Strict-Transport-Security =
   "Strict-Transport-Security" ":" 1#directive

...or when sticking to semicolon:

Strict-Transport-Security =
   "Strict-Transport-Security" ":" directive *( ";" directive )

Then

directive = token( "=" ( token / quoted-string ) )

In prose, add additional constrains, such as "MUST contain FOOBAR 
directive exactly once".

Finally, for each builtin directive state the individual syntax of the 
param *value*, and do not make the ABNF vary based on the directive.

Hope this helps.

Also worth reading: 
<http://greenbytes.de/tech/webdav/draft-ietf-httpbis-p2-semantics-latest.html#considerations.for.creating.header.fields>

Best regards, Julian

---

From: Julian Reschke <[email-masked]>
Date: Sat, 08 Oct 2011 20:23:50 +0200

> directive = token( "=" ( token / quoted-string ) )

And this should have been...:

  directive = token [ "=" ( token / quoted-string ) ]

Best regards, Julian