nelmio / NelmioSecurityBundle

Adds extra security-related features in your Symfony application
https://symfony.com/bundles/NelmioSecurityBundle/
MIT License
652 stars 85 forks source link

Reusing nonce for Turbo Drive integration #321

Open florimondmanca opened 1 year ago

florimondmanca commented 1 year ago

Hey there,

I'm hitting an issue similar to #136 just now. I am opening this new issue to discuss.

I am using symfony-ux, which integrates with Turbo.

Turbo Drive manipulates the DOM and inserts stylesheets as part of normal operation. It supports using a nonce read from <meta name="nonce" />.

The first obstacle I'm encountering is that Turbo uses the same nonce for both scripts and styles. But the bundle suggests a different usage. Not sure I understand why that is, but there must be history about this choice.

Second, this bundle applies a new nonce for each request/response cycle. This comment https://github.com/hotwired/turbo/issues/294#issuecomment-877842232 suggests doing similar to OP in #136, and send the nonce read initially by Turbo as a header that the server-side nonce generator should reuse for the CSP header.

I managed to make things work partially by doing the following.

First, apply the recommended Turbo patch:

// Somewhere in main app js

// See: https://github.com/hotwired/turbo/issues/294#issuecomment-877842232
document.addEventListener('turbo:before-fetch-request', (event) => {
    event.detail.fetchOptions.headers['X-CSP-Nonce'] = document.querySelector('meta[name="csp-nonce"]').content;
});
document.addEventListener('turbo:before-cache', () => {
    document.querySelectorAll('script[nonce]').forEach((element) => {
        element.setAttribute('nonce', element.nonce);
    });
});

Second, add the <meta name="csp-nonce" /> to the base template. Notice that I call csp_nonce() twice, once per usage, so that the CSP listener is tasked to set the nonce on both script-src and style-src.

<head>
    <!-- ... -->

    {% set _nonce = csp_nonce('script') %}
    {% set _nonce = csp_nonce('style') %}
    <meta name="csp-nonce" content="{{ _nonce }}">

    <!-- ... -->
</head>

Third, override the nonce generator to have it reuse the nonce if present, like so:

// src/Infrastructure/Security/NonceGenerator.php
<?php

declare(strict_types=1);

namespace App\Infrastructure\Security;

use Nelmio\SecurityBundle\ContentSecurityPolicy\NonceGenerator as ParentNonceGenerator;
use Nelmio\SecurityBundle\ContentSecurityPolicy\NonceGeneratorInterface;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\RequestEvent;
use Symfony\Component\HttpKernel\Event\ResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;

class NonceGenerator implements NonceGeneratorInterface, EventSubscriberInterface
{
    private ?string $requestNonce = null;

    public function __construct(
        private readonly ParentNonceGenerator $parent,
    )
    {
    }

    public function generate(): string
    {
        if ($this->requestNonce) {
            return $this->requestNonce;
        } else {
            return $this->parent->generate();
        }
    }

    public static function getSubscribedEvents(): array
    {
        return [
            KernelEvents::REQUEST => ['onKernelRequest', 400],
            KernelEvents::RESPONSE => 'onKernelResponse',
        ];
    }

    public function onKernelRequest(RequestEvent $event): void
    {
        if ($event->getRequest()->headers->has('X-CSP-Nonce')) {
            $this->requestNonce = $event->getRequest()->headers->get('X-CSP-Nonce');
        }
    }

    public function onKernelResponse(ResponseEvent $event): void {
        $this->requestNonce = null;
    }
}

Service declaration:

services:
  # ...
  App\Infrastructure\Security\NonceGenerator:
    decorates: 'nelmio_security.nonce_generator'
    arguments: ['@.inner']

Now, on pages where I get CSP errors upon navigating, I can see that the <meta name="csp-nonce" /> reuses the same nonce (before the patch, it would send a new nonce). I also checked that my custom nonce generator is called with a dd() call.

Unfortunately, the CSP errors didn't go away, even though the csp-nonce value and the nonce values in the CSP header match.

This might need more investigation on my side, perhaps setting a proper reproduction repo, but I thought I'd open this for other community members to discuss.

DavidPetrasek commented 10 months ago

Are you sure that both of these nonces are equal?

{% set _nonce = csp_nonce('script') %}
{% set _nonce = csp_nonce('style') %}

And also try removing this part (see https://github.com/hotwired/turbo/issues/294#issuecomment-1869754841):

document.addEventListener('turbo:before-cache', () => {
    document.querySelectorAll('script[nonce]').forEach((element) => {
        element.setAttribute('nonce', element.nonce);
    });
});