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

Provide hooks for Content Security Policy (CSP) #450

Closed
Ms2ger opened this Issue Mar 3, 2016 · 20 comments

Comments

Projects
None yet
7 participants
@Ms2ger

Ms2ger commented Mar 3, 2016

In particular, CSP wants to do something to eval() and new Function().

http://w3c.github.io/webappsec-csp/ is a specification implemented in web browsers that provides a feature that makes eval() and new Function() no-ops. Currently it uses very hand-wavy prose for that. I believe it is better for the definition of those features to acknowledge that feature, to ensure that the expected result is well-defined.

@michaelficarra

This comment has been minimized.

Show comment
Hide comment
@michaelficarra

michaelficarra Mar 3, 2016

Member

@Ms2ger This is not the place for these discussions. Please email es-discuss.

Member

michaelficarra commented Mar 3, 2016

@Ms2ger This is not the place for these discussions. Please email es-discuss.

@domenic

This comment has been minimized.

Show comment
Hide comment
@domenic

domenic Mar 3, 2016

Member

This is 100% the place for this "discussion".

Member

domenic commented Mar 3, 2016

This is 100% the place for this "discussion".

@domenic

This comment has been minimized.

Show comment
Hide comment
@domenic

domenic Mar 3, 2016

Member

I will work on a pull request for this today.

Member

domenic commented Mar 3, 2016

I will work on a pull request for this today.

@annevk

This comment has been minimized.

Show comment
Hide comment
@annevk
Contributor

annevk commented Mar 3, 2016

@michaelficarra

This comment has been minimized.

Show comment
Hide comment
@michaelficarra

michaelficarra Mar 3, 2016

Member

@domenic As I understand it, this repo is for concrete proposals and bug fixes. Not an open-ended discussion.

Member

michaelficarra commented Mar 3, 2016

@domenic As I understand it, this repo is for concrete proposals and bug fixes. Not an open-ended discussion.

@domenic

This comment has been minimized.

Show comment
Hide comment
@domenic

domenic Mar 3, 2016

Member

Yes, this is a concrete bugfix to match implementations. No discussion is needed.

Member

domenic commented Mar 3, 2016

Yes, this is a concrete bugfix to match implementations. No discussion is needed.

@erights

This comment has been minimized.

Show comment
Hide comment
@erights

erights Mar 3, 2016

Yes, this is a concrete bugfix to match implementations. No discussion is needed.

WHAT? Such notions have not previously been part of EcmaScript. I understand why they probably should be, but this is absolutely a substantive discussion, not a bug fix.

erights commented Mar 3, 2016

Yes, this is a concrete bugfix to match implementations. No discussion is needed.

WHAT? Such notions have not previously been part of EcmaScript. I understand why they probably should be, but this is absolutely a substantive discussion, not a bug fix.

@domenic

This comment has been minimized.

Show comment
Hide comment
@domenic

domenic Mar 3, 2016

Member

This is like the other host hooks. It's already implemented everywhere, too. It has no normative consequences, only layering ones. There's no need for discussion; the proper place for discussion if you wanted to change the behavior was in the webappsec working group.

Member

domenic commented Mar 3, 2016

This is like the other host hooks. It's already implemented everywhere, too. It has no normative consequences, only layering ones. There's no need for discussion; the proper place for discussion if you wanted to change the behavior was in the webappsec working group.

@domenic

This comment has been minimized.

Show comment
Hide comment
@domenic

domenic Mar 3, 2016

Member

This was also resolved to be a trivial change in https://bugs.ecmascript.org/show_bug.cgi?id=2494 and desired for inclusion in the spec. Allen says there are enough hooks already but they aren't very explicit, if they exist. (I guess he is referring to hooking into the creation of new execution contexts.)

Member

domenic commented Mar 3, 2016

This was also resolved to be a trivial change in https://bugs.ecmascript.org/show_bug.cgi?id=2494 and desired for inclusion in the spec. Allen says there are enough hooks already but they aren't very explicit, if they exist. (I guess he is referring to hooking into the creation of new execution contexts.)

@zenparsing

This comment has been minimized.

Show comment
Hide comment
@zenparsing

zenparsing Mar 3, 2016

Contributor

@Ms2ger Out of respect to those readers (like me) who don't have the necessary context, please take the time to write a full description with appropriate links when creating an issue like this.

Contributor

zenparsing commented Mar 3, 2016

@Ms2ger Out of respect to those readers (like me) who don't have the necessary context, please take the time to write a full description with appropriate links when creating an issue like this.

@Ms2ger

This comment has been minimized.

Show comment
Hide comment
@Ms2ger

Ms2ger Mar 3, 2016

Added some more background to the summary. I may have mistakenly assumed that a search engine of your choice would have been able to find sufficient context.

Ms2ger commented Mar 3, 2016

Added some more background to the summary. I may have mistakenly assumed that a search engine of your choice would have been able to find sufficient context.

@zenparsing

This comment has been minimized.

Show comment
Hide comment
@zenparsing

zenparsing Mar 3, 2016

Contributor

@Ms2ger Awesome, but I'm curious what's with all the terseness and snark?

Contributor

zenparsing commented Mar 3, 2016

@Ms2ger Awesome, but I'm curious what's with all the terseness and snark?

@domenic

This comment has been minimized.

Show comment
Hide comment
@domenic

domenic Mar 3, 2016

Member

It probably has something to do with this contributor's first welcome to the tc39 github repo being told to go away in a terse and snarky way.

Member

domenic commented Mar 3, 2016

It probably has something to do with this contributor's first welcome to the tc39 github repo being told to go away in a terse and snarky way.

@zenparsing

This comment has been minimized.

Show comment
Hide comment
@zenparsing

zenparsing Mar 3, 2016

Contributor

Ah, that would do it. Well, chalk it up to a misunderstanding, and good to have you here, @Ms2ger !

Contributor

zenparsing commented Mar 3, 2016

Ah, that would do it. Well, chalk it up to a misunderstanding, and good to have you here, @Ms2ger !

@erights

This comment has been minimized.

Show comment
Hide comment
@erights

erights Mar 3, 2016

Yes @Ms2ger , welcome!

erights commented Mar 3, 2016

Yes @Ms2ger , welcome!

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Mar 3, 2016

Member

@domenic

This was also resolved to be a trivial change in (https://bugs.ecmascript.org/show_bug.cgi?id=2494) and desired for inclusion in the spec. Allen says there are enough hooks already but they aren't very explicit, if they exist. (I guess he is referring to hooking into the creation of new execution contexts.)

The hook is step 11 of [InitializeHostDefineRealm()]](http://tc39.github.io/ecma262/#sec-initializehostdefinedrealm)

11. Create any implementation defined global object properties on globalObj.

Arguably this might be better worded as:

11. Define any implementation defined global object properties on globalObj.

to cover the cases of eval and function which have already been created prior to step 11.

In addition, the following paragraph should be added to Clause 16

An implementation may extend the behaviour of eval, the Function constructor, and %GeneratorFunction% that restricts where they may be used or the ECMAScript source code that they will accept.

All the other details should be left to the CSPspec.

Member

allenwb commented Mar 3, 2016

@domenic

This was also resolved to be a trivial change in (https://bugs.ecmascript.org/show_bug.cgi?id=2494) and desired for inclusion in the spec. Allen says there are enough hooks already but they aren't very explicit, if they exist. (I guess he is referring to hooking into the creation of new execution contexts.)

The hook is step 11 of [InitializeHostDefineRealm()]](http://tc39.github.io/ecma262/#sec-initializehostdefinedrealm)

11. Create any implementation defined global object properties on globalObj.

Arguably this might be better worded as:

11. Define any implementation defined global object properties on globalObj.

to cover the cases of eval and function which have already been created prior to step 11.

In addition, the following paragraph should be added to Clause 16

An implementation may extend the behaviour of eval, the Function constructor, and %GeneratorFunction% that restricts where they may be used or the ECMAScript source code that they will accept.

All the other details should be left to the CSPspec.

@domenic

This comment has been minimized.

Show comment
Hide comment
@domenic

domenic Mar 3, 2016

Member

Thanks Allen. I'd rather add specific restricted and explicit hooks as in #451 (which e.g. clarifies which steps in which algorithms, including direct eval, are affected) rather than use those kind of blanket permissions.

Member

domenic commented Mar 3, 2016

Thanks Allen. I'd rather add specific restricted and explicit hooks as in #451 (which e.g. clarifies which steps in which algorithms, including direct eval, are affected) rather than use those kind of blanket permissions.

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Mar 3, 2016

Member

I agree that a hook for direct eval is needed. I think the right place to put it is in PerformEval rather than at the call sites to PerformEval.

I don't think need a "hook" operation like HostEnsureCompileStrings is need. A prose line in PerformEval is more readable and can be as specific as you want to make it.

Member

allenwb commented Mar 3, 2016

I agree that a hook for direct eval is needed. I think the right place to put it is in PerformEval rather than at the call sites to PerformEval.

I don't think need a "hook" operation like HostEnsureCompileStrings is need. A prose line in PerformEval is more readable and can be as specific as you want to make it.

@domenic

This comment has been minimized.

Show comment
Hide comment
@domenic

domenic Mar 3, 2016

Member

I think a hook operation is more precise and clearer for host environments (both implementations and specs). It certainly maps better to the existing open source engines. We can let the editor decide.

PerformEval is not sufficient since we need both the calleeRealm and the callerRealm to match web reality.

Member

domenic commented Mar 3, 2016

I think a hook operation is more precise and clearer for host environments (both implementations and specs). It certainly maps better to the existing open source engines. We can let the editor decide.

PerformEval is not sufficient since we need both the calleeRealm and the callerRealm to match web reality.

@bterlson bterlson closed this in a34229a Mar 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment