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

Suborigin names can only start with letters. #62

Merged
merged 1 commit into from
Oct 27, 2016

Conversation

joelweinberger
Copy link
Contributor

@devd, can you take a look (available for viewing at https://metromoxie.github.io/webappsec-suborigins/)? This is meant to start addressing #47. In particular, this change:

  • Requires that suborigin names start with a letter. However, it does not ban numbers for the 2nd character onward. I'm very sympathetic to the use case listed in Suborigin names should not be allowed to start with number #47, and it seems that allowing easy base 32 encoding would be extremely useful (e.g. encoding hashes in the suborigin name). That having been said, we'll still need to do a more thorough review per @annevk's suggestion to make sure that's a safe thing to do in all contexts.
  • It adds an allowed suborigin name value of 'null' in the header which is meant to be equivalent to not providing a suborigin name at all. This serves two purposes: (1) if we need to send suborigin headers even if a suborigin isn't explicitly assigned, this allows that more easily, and (2) it allows policies to be set even if there isn't a suborigin. Neither one of these is explicitly useful in this version of the spec, but I don't want to outright disallow them.

@joelweinberger
Copy link
Contributor Author

Addition: It also disallows hyphens in suborigin names.

@devd
Copy link
Contributor

devd commented Oct 27, 2016

I am not sure why we need to add 'null' right now to the grammer: the day we use it, we can add it? the single quotes means that it won't conflict with existing grammar?

other changes look fine.

@devd
Copy link
Contributor

devd commented Oct 27, 2016

although, maybe add a comment in the spec that current suborigins need to start with lowercase letter

@joelweinberger
Copy link
Contributor Author

Fair enough re: 'null', although it wouldn't conflict because of the way I wrote it (a suborigin name is either lower alpha digit OR it's the string 'null'). I guess your point is that today a parser that checks the suborigin header would just ignore any header that comes in with an invalid value, so it will just silently reject 'null' if we add it later?

Also, why do I need to a comment in the spec about suborigins starting with lowercase letter when that's what the change to the grammar does? Do you feel that it's just not obvious in the grammar and you just want a plain English description?

@joelweinberger
Copy link
Contributor Author

@devd, I've simplified the change to just require lowercase alpha first, remove the hyphen, and add a note explaining what's going on.

@devd devd merged commit dc4e8c8 into w3c:master Oct 27, 2016
@devd
Copy link
Contributor

devd commented Oct 27, 2016

lgtm

@hillbrad
Copy link

Sorry to be late, but does this have any impact on the possibilities for #40 ?

@joelweinberger
Copy link
Contributor Author

To the contrary, I think that makes this future easier. The only thing grammar currently recognized as a suborigin name starts with a letter followed by alphanumeric. Anything else is ignored. Thus, in the future, if we want to use a special prefix (such as -sha256-deadbeef-), we haven't used hyphens, so we can adopt that (or any other delimeter).

I suppose the only place this is annoying is that Chrome gives a console error message if you give a bad suborigin name, and that might be annoying in the future, but yay for auto-upgrading browsers?

@joelweinberger
Copy link
Contributor Author

Or, if we'd rather just reserve something now, that's also fine by me :-)

@hillbrad
Copy link

:) I didn't say prevent, I said impact. Happy to hear that it makes it easier. I think I agree that reserving a secure prefix in the future will be easier if we can take over a part of the namespace that wouldn't previously have been valid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants