Bug 38365: Fix nonce caching and add to framework plugin scripts

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Lari Taskula <lari.taskula@hypernova.fi>
Signed-off-by: David Cook <dcook@prosentient.com.au>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Koha/ContentSecurityPolicy.pm

@@ -57,7 +57,7 @@ sub new {
     my ( $class, $params ) = @_;
     my $self = bless $params // {}, $class;
 
-    $self->nonce( $params->{nonce} );
+    $self->set_nonce( $params->{nonce} ) if $params->{nonce};
     $self->{config} = Koha::Config->get_instance;
     return $self;
 }
@@ -127,7 +127,7 @@ sub header_value {
         unless exists $conf_csp->{$interface}->{csp_header_value};
 
     my $csp_header_value = $conf_csp->{$interface}->{csp_header_value};
-    my $csp_nonce        = $self->nonce;
+    my $csp_nonce        = $self->get_nonce;
 
     $csp_header_value =~ s/_CSP_NONCE_/$csp_nonce/g;
 
@@ -166,44 +166,52 @@ sub is_enabled {
     return 0;
 }
 
-=head2 nonce
+=head2 get_nonce
 
-    $csp->nonce();
+    $csp->get_nonce();
 
-    Generates and returns the nonce.
-
-    $csp->nonce($nonce);
-
-    Sets and returns the nonce.
+    Returns the previously set nonce.
 
     A CSP nonce is a random token that is used both in the inline scripts
     and the Content-Security-Policy[-Report-Only] response header.
 
 =cut
 
-sub nonce {
+sub get_nonce {
+    my ($self) = @_;
+
+    #Koha::Middleware::ContentSecurityPolicy sets an environmental variable
+    #We cannot use the L1 cache since it is cleared after the middleware is applied
+    my $env_value = $ENV{CSP_NONCE};
+
+    return $env_value;
+}
+
+=head2 set_nonce
+
+    $csp->set_nonce($nonce);
+
+    Set the nonce value.
+
+    If value is provided, that value is used. Otherwise, a value is generated.
+
+    A CSP nonce is a random token that is used both in the inline scripts
+    and the Content-Security-Policy[-Report-Only] response header.
+
+=cut
+
+sub set_nonce {
     my ( $self, $nonce ) = @_;
-    my $cache = Koha::Cache::Memory::Lite->new;
 
+    #Koha::Middleware::ContentSecurityPolicy sets an environmental variable
+    #We cannot use the L1 cache since it is cleared after the middleware is applied
     if ($nonce) {
-        $cache->set_in_cache( 'CSP-NONCE', $nonce );
-        $self->{nonce} = $nonce;
-        return $self->{nonce};
+        $ENV{CSP_NONCE} = $nonce;
+    } else {
+        my $nonce = Koha::Token->new()->generate( { pattern => '\w{22}' } );
+        $ENV{CSP_NONCE} = $nonce;
     }
-
-    if ( $nonce = $cache->get_from_cache('CSP-NONCE') ) {
-        $self->{nonce} = $nonce;
-        return $self->{nonce};
-    }
-
-    unless ( $self->{nonce} ) {
-        $nonce = Koha::Token->new()->generate( { pattern => '\w{22}' } );
-        $self->{nonce} = $nonce;
-    }
-
-    $cache->set_in_cache( 'CSP-NONCE', $self->{nonce} );
-
-    return $self->{nonce};
+    return 1;
 }
 
 1;

Koha/FrameworkPlugin.pm

@@ -117,6 +117,7 @@ use Cwd qw//;
 use base qw(Class::Accessor);
 
 use C4::Context;
+use Koha::ContentSecurityPolicy;
 
 __PACKAGE__->mk_ro_accessors(
     qw|
@@ -137,7 +138,11 @@ __PACKAGE__->mk_ro_accessors(
 
 sub new {
     my ( $class, $params ) = @_;
-    my $self = $class->SUPER::new();
+    my $self      = $class->SUPER::new();
+    my $csp_nonce = Koha::ContentSecurityPolicy->new->get_nonce;
+    if ($csp_nonce) {
+        $self->{csp_nonce} = $csp_nonce;
+    }
     if ( ref($params) eq 'HASH' ) {
         foreach ( 'name', 'path', 'item_style' ) {
             $self->{$_} = $params->{$_};
@@ -356,6 +361,8 @@ sub _generate_js {
 sub _process_javascript {
     my ( $self, $params, $script ) = @_;
 
+    my $csp_nonce = $self->{csp_nonce};
+
     #remove the script tags; we add them again later
     $script =~ s/\<script[^>]*\>\s*(\/\/\<!\[CDATA\[)?\s*//s;
     $script =~ s/(\/\/\]\]\>\s*)?\<\/script\>//s;
@@ -372,7 +379,7 @@ sub _process_javascript {
     }
     $self->{noclick}    = !$clickfound;
     $self->{javascript} = <<JS;
-<script>
+<script nonce="$csp_nonce">
 \$(document).ready(function () {
 $script
 });

Koha/Middleware/ContentSecurityPolicy.pm

@@ -39,6 +39,7 @@ sub call {
     my $req = Plack::Request->new($env);
 
     my $csp = Koha::ContentSecurityPolicy->new;
+    $csp->set_nonce();
 
     my $res = $self->app->($env);
     return $res unless $csp->is_enabled;

Koha/Template/Plugin/Koha.pm

@@ -172,7 +172,7 @@ It should be cached by Koha::Middleware::ContentSecurityPolicy.
 =cut
 
 sub CSPNonce {
-    return Koha::ContentSecurityPolicy->new->nonce;
+    return Koha::ContentSecurityPolicy->new->get_nonce;
 }
 
 1;

t/Koha/ContentSecurityPolicy.t

@@ -140,10 +140,8 @@ subtest 'header_value tests' => sub {
         $conf_csp_section,
         { opac => { csp_header_value => 'begin nonce-_CSP_NONCE_ end' } }
     );
-    my $cache = Koha::Cache::Memory::Lite->get_instance();
-    $cache->set_in_cache( 'CSP-NONCE', 'cached value' );
+    $csp->set_nonce('cached value');
     is( $csp->header_value, 'begin nonce-cached value end', 'Cached csp_header_value' );
-    $cache->flush;
 
     $csp = Koha::ContentSecurityPolicy->new( { nonce => 'forced value' } );
     is( $csp->header_value, 'begin nonce-forced value end', 'Forced csp_header_value' );
@@ -158,21 +156,20 @@ subtest 'nonce tests' => sub {
     my $csp = Koha::ContentSecurityPolicy->new;
 
     $csp = Koha::ContentSecurityPolicy->new( { nonce => 'forced value' } );
-    is( $csp->nonce, 'forced value', 'nonce is not re-generated as it was passed to new()' );
+    is( $csp->get_nonce, 'forced value', 'nonce is not re-generated as it was passed to new()' );
 
     $csp = Koha::ContentSecurityPolicy->new( { nonce => 'forced value' } );
-    is( $csp->nonce, 'forced value', 'nonce is not re-generated as was cached in memory' );
+    is( $csp->get_nonce, 'forced value', 'nonce is not re-generated as was cached in memory' );
 
-    my $cache = Koha::Cache::Memory::Lite->get_instance();
-    $cache->flush;
+    $csp->set_nonce();
 
     $csp = Koha::ContentSecurityPolicy->new();
-    my $nonce = $csp->nonce;
+    my $nonce = $csp->get_nonce;
     like( $nonce, qr/\w{22}/, 'nonce is a random string of 22 characters (128+ bits entropy)' );
-    is( $nonce, $csp->nonce, 're-calling nonce() returns the same randomly generated cached value' );
+    is( $nonce, $csp->get_nonce, 're-calling nonce() returns the same randomly generated cached value' );
 
-    $cache->set_in_cache( 'CSP-NONCE', 'cached value' );
-    is( $csp->nonce, 'cached value', 'nonce is not re-generated as it was previously cached' );
+    $csp->set_nonce('cached value');
+    is( $csp->get_nonce, 'cached value', 'nonce is not re-generated as it was previously cached' );
 };
 
 1;

t/Koha/Middleware/ContentSecurityPolicy.t

@@ -41,7 +41,6 @@ my $conf_csp_section = 'content_security_policy';
 subtest 'test Content-Security-Policy header' => sub {
     plan tests => 3;
 
-    my $test_nonce = 'TEST_NONCE';
     my $csp_header_value =
         "default-src 'self'; script-src 'self' 'nonce-_CSP_NONCE_'; style-src 'self' 'nonce-_CSP_NONCE_'; img-src 'self' data:; font-src 'self'; object-src 'none'";
 
@@ -50,9 +49,6 @@ subtest 'test Content-Security-Policy header' => sub {
         { opac => { csp_mode => 'enabled', csp_header_value => $csp_header_value } }
     );
 
-    # Set nonce
-    Koha::ContentSecurityPolicy->new->nonce($test_nonce);
-
     t::lib::Mocks::mock_config( 'opachtdocs', C4::Context->config('intranetdir') . '/t/mock_templates/opac-tmpl' );
 
     my $env = {};
@@ -79,6 +75,7 @@ subtest 'test Content-Security-Policy header' => sub {
 
     my $test                      = Plack::Test->create($app);
     my $res                       = $test->request( GET "/" );
+    my $test_nonce                = Koha::ContentSecurityPolicy->new->get_nonce();
     my $expected_csp_header_value = $csp_header_value;
     $expected_csp_header_value =~ s/_CSP_NONCE_/$test_nonce/g;
     is(

t/db_dependent/Koha/Middleware/ContentSecurityPolicy.t

@@ -37,7 +37,6 @@ my $conf_csp_section = 'content_security_policy';
 subtest 'test CSP in OPAC' => sub {
     plan tests => 5;
 
-    my $test_nonce = 'TEST_NONCE';
     my $csp_header_value =
         "default-src 'self'; script-src 'self' 'nonce-_CSP_NONCE_'; style-src 'self' 'nonce-_CSP_NONCE_'; img-src 'self' data:; font-src 'self'; object-src 'none'";
 
@@ -47,9 +46,6 @@ subtest 'test CSP in OPAC' => sub {
         { opac => { csp_mode => 'enabled', csp_header_value => $csp_header_value } }
     );
 
-    # Set nonce
-    Koha::ContentSecurityPolicy->new->nonce($test_nonce);
-
     my $env  = {};
     my $home = $ENV{KOHA_HOME};
     my $app  = Plack::App::CGIBin->new( root => $ENV{GIT_INSTALL} ? "$home/opac" : "$home/opac/cgi-bin/opac" )->to_app;
@@ -63,6 +59,7 @@ subtest 'test CSP in OPAC' => sub {
 
     my $test                      = Plack::Test->create($app);
     my $res                       = $test->request( GET "/opac/opac-main.pl" );
+    my $test_nonce                = Koha::ContentSecurityPolicy->new->get_nonce();
     my $expected_csp_header_value = $csp_header_value;
     $expected_csp_header_value =~ s/_CSP_NONCE_/$test_nonce/g;
     is(
@@ -100,7 +97,6 @@ subtest 'test CSP in OPAC' => sub {
 subtest 'test CSP in staff client' => sub {
     plan tests => 5;
 
-    my $test_nonce = 'TEST_NONCE';
     my $csp_header_value =
         "default-src 'self'; script-src 'self' 'nonce-_CSP_NONCE_'; style-src 'self' 'nonce-_CSP_NONCE_'; img-src 'self' data:; font-src 'self'; object-src 'none'";
 
@@ -109,9 +105,6 @@ subtest 'test CSP in staff client' => sub {
         { opac => { csp_mode => 'enabled', csp_header_value => $csp_header_value } }
     );
 
-    # Set nonce
-    Koha::ContentSecurityPolicy->new->nonce($test_nonce);
-
     my $env  = {};
     my $home = $ENV{KOHA_HOME};
     my $app  = Plack::App::CGIBin->new( root => $ENV{GIT_INSTALL} ? $home : "$home/intranet/cgi-bin/" )->to_app;
@@ -125,6 +118,7 @@ subtest 'test CSP in staff client' => sub {
 
     my $test                      = Plack::Test->create($app);
     my $res                       = $test->request( GET "/intranet/mainpage.pl" );
+    my $test_nonce                = Koha::ContentSecurityPolicy->new->get_nonce();
     my $expected_csp_header_value = $csp_header_value;
     $expected_csp_header_value =~ s/_CSP_NONCE_/$test_nonce/g;
     is(
@@ -154,7 +148,10 @@ subtest 'test CSP in staff client' => sub {
         { intranet => { csp_mode => 'enabled', csp_header_value => $csp_header_value } }
     );
 
-    $res = $test->request( GET "/intranet/mainpage.pl" );
+    $res                       = $test->request( GET "/intranet/mainpage.pl" );
+    $test_nonce                = Koha::ContentSecurityPolicy->new->get_nonce();
+    $expected_csp_header_value = $csp_header_value;
+    $expected_csp_header_value =~ s/_CSP_NONCE_/$test_nonce/g;
     is(
         $res->header('content-security-policy'), $expected_csp_header_value,
         "Response contains Content-Security-Policy header when it is enabled in koha-conf.xml"

t/db_dependent/Koha/Template/Plugin/Koha.t

@@ -91,7 +91,7 @@ subtest 'CSPNonce' => sub {
     my $plugin = Koha::Template::Plugin::Koha->new($context);
 
     my $csp   = Koha::ContentSecurityPolicy->new;
-    my $nonce = $csp->nonce;
+    my $nonce = $csp->get_nonce;
 
     my $token = $plugin->CSPNonce;