Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Definition of Expect-CT is a little unclear #318

Closed
martinthomson opened this issue Mar 31, 2017 · 10 comments
Closed

Definition of Expect-CT is a little unclear #318

martinthomson opened this issue Mar 31, 2017 · 10 comments

Comments

@martinthomson
Copy link
Contributor

The convention for HTTP is to define header fields as follows:

<header-field-name> = #<header-field-name>-fields

Which leads to me suggesting:

Expect-CT = #Expect-CT-Directives
Expect-CT-Directives = [ parameter *( OWS ";" OWS parameter ) ]

Note that this doesn't follow the common structure, which uses:

common-structure = 1* ( identifier dictionary )

Ignore the obvious errors in the common structure syntax, the intent is to have a list of identifiers followed by dictionaries.

I've marked this as editorial, I think that the outcome will not affect the wire syntax at all.

@royfielding
Copy link
Member

If it is supposed to be a list of directives (some of which may be name=value), then it should be

   Expect-CT       = #directive
   directive       = directive-name [ "=" directive-value ]
   directive-name  = token
   directive-value = token / quoted-string

If it is supposed to be a list of directives with optional name=value parameters to each directive, then

   Expect-CT       = #directive
   directive       = directive-name *( OWS ";" OWS parameter ) 
   directive-name  = token
   parameter       = token / ( token "=" quoted-string )

I guess we could also use some examples in the specification.

@reschke
Copy link
Contributor

reschke commented Mar 31, 2017

@martinthomson what you suggested does change the wire syntax...

@reschke
Copy link
Contributor

reschke commented Mar 31, 2017

I also disagree with the statement:

The convention for HTTP is to define header fields as follows:

That depends on the type of the field. It's definitively not true universally right now.

@royfielding
Copy link
Member

BTW, that would be to match the standard HTTP/1.x syntax for most header fields. I don't think it is appropriate to use the experimental common structure, which is still just a proposal, unless folks want to make this header field a guinea pig.

@reschke
Copy link
Contributor

reschke commented Mar 31, 2017

https://greenbytes.de/tech/webdav/rfc7838.html#rfc.section.3 might be a good template.

@estark37
Copy link
Contributor

estark37 commented Apr 14, 2017

Sorry for my delay here.

If I'm understanding correctly, then @royfielding's first suggestion is the intention: it's supposed to be a list of directives, some of which may be name=value:

   Expect-CT       = #directive
   directive       = directive-name [ "=" directive-value ]
   directive-name  = token
   directive-value = token / quoted-string

However this changes the format from semicolon-separated directives (Expect-CT: enforce; max-age=23) to comma-separated (Expect-CT: enforce, max-age=23); is that correct?

If so, it's a little unfortunate that it'll differ from HPKP/HSTS syntax, which is semicolon-separated directives, but I suppose that's okay. @jcjones how do you feel about that from a Firefox implementation perspective?

@jcjones
Copy link

jcjones commented Apr 14, 2017

@estark37: Implementation-wise, it's not much work to accept a comma-delimited list instead of semicolons. However, since this is so similar to CSP/HPKP/HSTS, I'd prefer it to have a similar structure of semicolon delineation, particularly since the reporting mechanism looks so much like CSP. It feels like it'd be weird to see the different delimiters side-by-side:

    add_header	           Strict-Transport-Security "max-age=31536000; includeSubDomains; preload";
    add_header             Content-Security-Policy "default-src 'self'; script-src 'unsafe-eval' 'unsafe-inline' 'self' ... ; img-src data: 'self'; ... ";
    add_header             Expect-CT "enforce, max-age=1";

@royfielding
Copy link
Member

Yes, comma-separated lists is the normal form in HTTP (inherited from the Internet message syntax of email and netnews) because it allows multiple header fields to be appended without changing the semantics of the field. In other words,

Expect-CT: enforce, max-age=23

is defined by HTTP to be equivalent to

Expect-CT: enforce
Expect-CT: max-age=23

which allows various implementation layers/filters to add to a message without rewriting the entire header block.

If the comma-separated list syntax is not used, then implementations are forbidden from sending the directives in more than one field and recipients must fail-with-error if more than one field is received.

@martinthomson
Copy link
Contributor Author

STS, like Cookie, is a bad example of a microsyntax that doesn't take advantage of the HTTP-native features.

Using the non-standard syntax can run afoul of some of the compression measures in HTTP/2.

CSP is actually specifically defined as a single string, so the semi-colons are internal to the header field. Note that CSP actively forbids multiple header field values (or makes them all apply equally, I can't remember). In the ways that matter, CSP is actually perfectly compatible with Roy's suggestion.

estark37 added a commit that referenced this issue Apr 18, 2017
This changes Expect-CT to be a comma-separated list of directives (which may be
name=value pairs), and adds some examples.

Also allows the server to send multiple header fields so that they can be
combined, as per
#318 (comment)

Fixes issue #318
estark37 added a commit that referenced this issue Apr 18, 2017
This changes Expect-CT to be a comma-separated list of directives (which may be
name=value pairs), and adds some examples.

Also allows the server to send multiple header fields so that they can be
combined, as per
#318 (comment)

Fixes issue #318
@estark37
Copy link
Contributor

Alright, like @jcjones I find it weird that the syntax will be different than HSTS/HPKP/CSP, but I suppose that's okay; Expect-CT will already differ in a couple other subtle ways so I think the syntax difference is not the end of the world.

@martinthomson @royfielding @reschke could you please check if #327 seems reasonable?

estark37 added a commit that referenced this issue Apr 19, 2017
This changes Expect-CT to be a comma-separated list of directives (which may be name=value pairs), and adds some examples.

Also allows the server to send multiple header fields so that they can be combined, as per #318 (comment)

Fixes issue #318, #321
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants