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

Add CSP header if available #1013

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add CSP header if available #1013

wants to merge 4 commits into from

Conversation

barryvdh
Copy link
Owner

@barryvdh barryvdh commented Mar 3, 2020

TODO; get the source of the CSP in a callable?
Counterpart of maximebf/php-debugbar#439

@barryvdh
Copy link
Owner Author

barryvdh commented Mar 3, 2020

Also need to modify the VarDumper to inject CSP attributes..

@savage-eagle
Copy link

Any news on?

@barryvdh
Copy link
Owner Author

@savage-eagle This is merged on the php-debugbar. Still need to see how we're going to add the actual nonce though.

@rdevroede
Copy link

@barryvdh the stylesheet needs a nonce as well.

Haven't properly dug into the matter yet, but here's some first thoughts for adding the nonce. First I thought to add it to the config like this "'nonce' => csp_nonce(),". That would give flexibility to devs for implementing other packages as well. But that throws a BindingResolutionException on app('csp-nonce'). I did manage to set that config from the middleware I wrote to programmatically enable Debugbar like this "config(['debugbar.nonce' => csp_nonce()]);".

Hope to find some time to dig a bit deeper later on.

@rdevroede
Copy link

rdevroede commented Jun 28, 2021

Got a working proof-of-concept.

config/debugbar.php:

    'csp_callback' => 'csp_nonce',

src/JavascriptRenderer.php > renderHead():

        $cspString = '';
        $cspCallback = config('debugbar.csp_callback');
        if ($cspCallback) {
            $cspString = sprintf('nonce="%s"', call_user_func($cspCallback));
        }

        $html  = "<link {$cspString} rel='stylesheet' type='text/css' property='stylesheet' href='{$cssRoute}'>";
        $html .= "<script {$cspString} type='text/javascript' src='{$jsRoute}'></script>";

        if ($this->isJqueryNoConflictEnabled()) {
            $html .= '<script ' . $cspString . 'type="text/javascript">jQuery.noConflict(true);</script>' . "\n";
        }

However, I ran into the next issue with the inline script tag of the Symfony HtmlDumper. This raises the following question: should vendor packages even implement this kind of change?
I am leaning towards this being the responsibility of the package that implements CSP (i.e. spatie/laravel-csp) or the local developer. It should propably be hooked somewhere in the view renderer to add it everywhere it has not been set yet.

Regards,
Richard

PS Thanks for your awesome work!

@rdevroede
Copy link

Addendum to my previous statement: ... unless the package injects content through middleware added by an auto-discovered ServiceProvider.

I could not find a way to modify the response content after the middlewares and middlewarePriority wasn't sufficient either.

@rdevroede
Copy link

The inline html also needs to be modified.

src/JavascriptRenderer.php > renderHead():

        $html .= preg_replace_callback('/<(script|style)(.*?)>/', function ($matches) {
            if (strpos($matches[2], 'nonce=') !== false) {
                return $matches[0];
            }

            return sprintf('<%s nonce="%s"%s>', $matches[1], csp_nonce(), $matches[2]);
        }, $this->getInlineHtml());

And the render method.

    /**
     * {@inheritdoc}
     */
    public function render($initialize = true, $renderStackedData = true)
    {
        return str_replace('<script ', '<script nonce="'.csp_nonce().'" ', parent::render());
    }

But that still isn't enough. The inline script creates a style element without a nonce. And assets/javascripts creates a script element without a nonce. What a can of worms...

@barryvdh
Copy link
Owner Author

Yeah now you mention it, I guess this is why I didn't merge this sooner :P
Easiest is just to set CSP to warnings on dev, but not ideal.

@rdevroede
Copy link

Yeah. I managed to scotchtape the inline scipt to work as well with another preg_replace by injecting a setAttribute, but assets/javascript gave me a headache.
Also experimented a bit with strict-dynamic [1], but could not get that to work.

I have to park this for now.

[1] https://content-security-policy.com/strict-dynamic/

@bretto36
Copy link

bretto36 commented Nov 7, 2022

@barryvdh @rdevroede Will there by any more work done on this to get it to work with a CSP header?

@rdevroede
Copy link

@barryvdh @rdevroede Will there by any more work done on this to get it to work with a CSP header?

Sadly, I have not made any progress. I ended up following Barry's suggestion of setting it to warnings on dev and moved on to things with higher priorities.

@dasch88
Copy link

dasch88 commented Jan 18, 2023

It feels like this is close. Is it possible to get this working without falling back to the less ideal warnings mode in dev? Making CSP failures work in dev the same as prod are really important to help prevent issues from being accidentally released. Any thoughts, @barryvdh?

@Virus5600
Copy link

Virus5600 commented Mar 8, 2023

Building up to @rdevroede codes, I've managed to put this one up:

src/JavascriptRenderer.php > renderHead():

    public function renderHead()
    {
        $cssRoute = route('debugbar.assets.css', [
            'v' => $this->getModifiedTime('css'),
            'theme' => config('debugbar.theme', 'auto'),
        ]);

        $jsRoute = route('debugbar.assets.js', [
            'v' => $this->getModifiedTime('js')
        ]);

        $cssRoute = preg_replace('/\Ahttps?:/', '', $cssRoute);
        $jsRoute  = preg_replace('/\Ahttps?:/', '', $jsRoute);

        $cspString = '';
        $cspCallback = config('debugbar.csp_callback');
        if ($cspCallback) {
            $this->setCspNonce(call_user_func($cspCallback));
        }

        $nonce = isset($this->cspNonce) ? $this->getNonceAttribute() : '';

        $html  = "<link{$nonce} rel='stylesheet' type='text/css' property='stylesheet' href='{$cssRoute}' data-turbolinks-eval='false' data-turbo-eval='false'>";
        $html .= "<script{$nonce} src='{$jsRoute}' data-turbolinks-eval='false' data-turbo-eval='false'></script>";

        if ($this->isJqueryNoConflictEnabled()) {
            $html .= "<script{$nonce} data-turbo-eval='false'>jQuery.noConflict(true);</script>\n";
        }

        $inlineHtml = $this->getInlineHtml();
        if (isset($this->cspNonce)) {
            $inlineHtml = preg_replace("/(<(script|style))/", "$1 {$nonce}", $inlineHtml);
            $inlineHtml = preg_replace("/(doc\.head\.appendChild\(refStyle\);)/", "refStyle.setAttribute('nonce', '{$this->cspNonce}');\n", $inlineHtml);
        }
        
        $html .= $inlineHtml;

        return $html;
    }

config/debugbar.php:

    /*
     |--------------------------------------------------------------------------
     | CSP Setter
     |--------------------------------------------------------------------------
     |
     | DebugBar injects scripts and styles to show the debug bar. However, this causes lots of policy violations
     | when CSP is used. To prevent these violations, a callback will be identified to apply a nonce or hash to the
     | elements. A good example of this would be the Laravel CSP by spatie, utilizing the `csp_nonce()` function.
     */
    'csp_callback' => env('DEBUGBAR_CSP_CALLBACK', null),

.env:

CSP_ENABLED=true
DEBUGBAR_INJECT=true
DEBUGBAR_CSP_CALLBACK=csp_nonce

The inline part was brute forced, but it did manage to run my debug bar with CSP on since the dumper was only injecting style elements. The script part was already fixed by rdevroede and that's where I built it from.

I also want to add that the PHP Debugbar from maximebf is using jQuery 3.3.1 which also does not support CSP when creating or appending elements. This was discussed in this jQuery 3.1.1 Issue.

@erikn69
Copy link
Contributor

erikn69 commented Feb 22, 2024

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.

7 participants