8000 [WebProfiler] don't add inline javascript · Issue #15397 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

[WebProfiler] don't add inline javascript #15397

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.

Sign up for GitHub

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

Jump to bottom
Closed
thkoch2001 opened this issue Jul 28, 2015 · 5 comments
Closed

[WebProfiler] don't add inline javascript #15397

thkoch2001 opened this issue Jul 28, 2015 · 5 comments
Labels
Enhancement WebProfilerBundle

Comments

@thkoch2001
Copy link

Inline JavaScript (and CSS) is a security and performance issue. Content-Security-Policy exists to tell browsers not to execute inline JavaScript.

The Web Profiler Toolbar however uses inline JavaScript. Why? It would also be possible to add a script tag to load the missing JavaScript.

Informations can be passed from the server to the JavaScript code easily without inline JavaScript:

<script class="embedded-json-data" type="application/json" data-name="myActiveProfile">
   {"id": 123, "name": "ido", "language": "en"}
</script>

and in your JavaScrip (angular here)t:

var selector = 'script.embedded-json-data[data-name="' + name + '"]',
    node = document.querySelector(selector),
    data = angular.fromJson(node.innerHTML);
@jakzal
Copy link
Contributor
jakzal commented Jul 30, 2015

Does it really matter in case of the web toolbar which is meant to be used in development only?

@stof
Copy link
Member
stof commented Jul 30, 2015

@jakzal this forces you to disable CSP headers during development, meaning that any CSP-incompatible code would only be detected in prod.

@stof
Copy link
Member
stof commented Sep 22, 2015

I have some idea about how this could be done.
This would involve a big change in the WebProfilerBundle, and require bundles to update their code to register their JS and custom styles in a new way (otherwise, they would continue to work as long as you don't have CSP in place, but would not allow CSP). But thanks to the new system we put in places in the WebProfilerBundle to allow bundles to support different versions of the profiler in parallel while knowing which one is used, it is now possible to introduce such refactoring in a BC way IMO (I need to confirm it though to be sure).
I have no time to work on this before the feature freeze of Symfony 2.8 and 3.0 (which is at the end of the month), but I'm putting this on my todo-list for Symfony 3.1.

@romainneutron romainneutron mentioned this issue Apr 4, 2016
Closed
1 task
@romainneutron romainneutron mentioned this issue Apr 16, 2016
Merged
@vhejda
Copy link
vhejda commented Jun 1, 2016 •
edited
Loading

For now, I am using this workaround (Symfony 2.7) - in Kernel Response event listener with lower priority than "WebDebugToolbarListener", I am adding a CSP "nonce" exception like this:

if ($this->kernel->getEnvironment() == "dev") {
    $crawler = new Crawler($response->getContent());
    $scripts = $crawler->filter('script');
    foreach($scripts as $script) {
        if ($script->textContent) {
            if (strpos($script->textContent, "Sfjs") !== false) {
                $nonce = hash('sha256', random_bytes(32));
                $script->setAttribute("nonce", $nonce);
                $security_policy["script-src"][] = "'nonce-".$nonce."'";
            }
        }
    }
    $response->setContent($crawler->html());
}

$security_policy also contains other CSP rules, and it is parsed and set into Content-Security-Policy HTTP header. There is probably a better/safer way to identify toolbar scripts than searching for "Sfjs", but this is executed only for development, so (as @stof pointed out) you don't have to disable CSP during development and you are able to detect incompatibilities in dev.

@stof
Copy link
Member
stof commented Jun 1, 2016

@vooj-tae see #18568 for the PR implementing support for CSP in the toolbar (this still requires disabling CSP for the profiler pages themselves, which is OK IMO and makes things much easier than what I thought about in September 2015 (no need to change external bundles hooking into the profiler to add new panels)

fabpot added a commit that referenced this issue Jun 9, 2016
@fabpot
feature #18568 [3.2][WebProfilerBundle] Fix bundle usage in Content-S…
856c9f6 
…ecurity-Policy context without unsafe-inline (romainneutron)

This PR was merged into the 3.2-dev branch.

Discussion
----------

[3.2][WebProfilerBundle] Fix bundle usage in Content-Security-Policy context without unsafe-inline

| Q             | A
| ------------- | ---
| Branch?       | 3.2
| Bug fix?      | yes
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #15397
| License       | MIT
| Doc PR        | N/A

Hello, this PR fixes the compatibility of the WebprofilerBundle in a context where Content-Security-Policy headers are could prevent `unsafe-inline` of `script-src` or `style-src` directives.

This PR has been originally proposed in 2.8 in #18434

Commits
-------

571a1f2 [WebProfilerBundle] Fix bundle usage in Content-Security-Policy context without unsafe-inline
@fabpot fabpot closed this as completed Jun 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Enhancement WebProfilerBundle
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@fabpot @javiereguiluz @thkoch2001 @jakzal @stof @xabbuh @vhejda
0