From 7009f3d26e87aa427f52f05c159395eafafed5ff Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Thu, 5 Feb 2026 16:42:57 +0200 Subject: [PATCH] Bug 38365: Require a token for using public CSP reports endpoint This patch protects the endpoint with a token that is valid for only 15 seconds, so that one can't simply keep POST'ing indefinitely, filling CSP violation logs. It adds a new placeholder, _CSP_API_TOKEN_, to KOHA_CONF, that will be replaced by this token. To test: 1. prove t/db_dependent/Koha/Middleware/ContentSecurityPolicy.t 2. prove t/db_dependent/api/v1/public/csp_reports.t 3. prove t/Koha/ContentSecurityPolicy.t --- Koha/ContentSecurityPolicy.pm | 20 +++++++ Koha/Middleware/ContentSecurityPolicy.pm | 11 ++-- Koha/REST/V1/CSPReports.pm | 34 ++++++++++++ api/v1/swagger/paths/public_csp_reports.yaml | 10 ++++ debian/templates/koha-conf-site.xml.in | 3 +- etc/koha-conf.xml | 3 +- t/Koha/ContentSecurityPolicy.t | 17 +++++- .../Koha/Middleware/ContentSecurityPolicy.t | 27 +++++++--- t/db_dependent/api/v1/public/csp_reports.t | 53 +++++++++++++++---- 9 files changed, 153 insertions(+), 25 deletions(-) diff --git a/Koha/ContentSecurityPolicy.pm b/Koha/ContentSecurityPolicy.pm index d112b586154..02ca3b6a4f6 100644 --- a/Koha/ContentSecurityPolicy.pm +++ b/Koha/ContentSecurityPolicy.pm @@ -19,6 +19,10 @@ package Koha::ContentSecurityPolicy; use Modern::Perl; +use Digest::MD5; +use Encode; +use Mojo::JWT; + use C4::Context; use Koha::Cache::Memory::Lite; use Koha::Token; @@ -129,7 +133,9 @@ sub header_value { my $csp_header_value = $conf_csp->{$interface}->{csp_header_value}; my $csp_nonce = $self->get_nonce; + my $api_token = $self->api_token; $csp_header_value =~ s/_CSP_NONCE_/$csp_nonce/g; + $csp_header_value =~ s/_CSP_API_TOKEN_/$api_token/g; return $csp_header_value; } @@ -214,4 +220,18 @@ sub set_nonce { return 1; } +sub api_token { + my $time = time(); + return Mojo::JWT->new( + claims => { + type => 'csp-violation', + timestamp => $time, + }, + expires => $time + 15, # the endpoint is valid for 15 seconds + secret => Digest::MD5::md5_base64( + Encode::encode( 'UTF-8', C4::Context->config('api_secret_passphrase') || 'unsafe' ) + ) + )->encode; +} + 1; diff --git a/Koha/Middleware/ContentSecurityPolicy.pm b/Koha/Middleware/ContentSecurityPolicy.pm index 49fe6379926..258fcf1301e 100644 --- a/Koha/Middleware/ContentSecurityPolicy.pm +++ b/Koha/Middleware/ContentSecurityPolicy.pm @@ -54,7 +54,7 @@ sub call { # if reporting-endpoints already exists, append it if ( lc( $headers->[$i] ) eq 'reporting-endpoints' ) { - $headers->[ $i + 1 ] = _add_csp_to_reporting_endpoints( $headers->[ $i + 1 ] ); + $headers->[ $i + 1 ] = _add_csp_to_reporting_endpoints( $csp, $headers->[ $i + 1 ] ); $add_reporting_endpoints = 0; last; } @@ -62,7 +62,7 @@ sub call { # reporting-endpoints is not yet defined, so let's define it if ($add_reporting_endpoints) { - push @$headers, ( 'Reporting-Endpoints' => _add_csp_to_reporting_endpoints() ); + push @$headers, ( 'Reporting-Endpoints' => _add_csp_to_reporting_endpoints($csp) ); } push @$headers, ( $csp->header_name => $csp->header_value ); } @@ -70,14 +70,15 @@ sub call { } sub _add_csp_to_reporting_endpoints { - my ($value) = @_; + my ( $csp, $value ) = @_; if ( $value && $value =~ /^\w+/ ) { $value = $value . ', '; } else { $value = ''; } - $value = $value . 'csp-violations="/api/v1/public/csp-reports"'; - return $value; + my $token = $csp->api_token; + return $value . "csp-violations=\"/api/v1/public/csp-reports?token=$token\""; } + 1; diff --git a/Koha/REST/V1/CSPReports.pm b/Koha/REST/V1/CSPReports.pm index 0040208852b..2b193450ce0 100644 --- a/Koha/REST/V1/CSPReports.pm +++ b/Koha/REST/V1/CSPReports.pm @@ -20,9 +20,12 @@ package Koha::REST::V1::CSPReports; use Modern::Perl; use Mojo::Base 'Mojolicious::Controller'; +use Mojo::JWT; use Koha::Logger; +use Try::Tiny qw( try catch ); + =head1 NAME Koha::REST::V1::CSPReports - Controller for Content-Security-Policy violation reports @@ -46,6 +49,37 @@ is violated. This endpoint logs those reports for administrator review. sub add { my $c = shift->openapi->valid_input or return; + my $token = $c->param('token'); + my $decoded_jwt; + my $expired = 0; + try { + $decoded_jwt = Mojo::JWT->new( + secret => Digest::MD5::md5_base64( + Encode::encode( 'UTF-8', C4::Context->config('api_secret_passphrase') || 'unsafe' ) + ) + )->decode($token); + } catch { + if ( $_ && ref($_) eq 'Mojo::Exception' ) { + $expired = 1 if $_->message =~ /^JWT has expired/; + } + }; + return $c->render( + status => 400, + openapi => { error => 'This token has expired.' } + ) if $expired; + + return $c->render( + status => 401, + openapi => { error => 'Invalid token.' } + ) unless $decoded_jwt; + + if ( !$decoded_jwt->{type} || $decoded_jwt->{type} ne 'csp-violation' ) { + return $c->render( + status => 400, + openapi => { error => 'Invalid token type.' } + ); + } + my $report = $c->req->json; # CSP reports come wrapped in a 'csp-report' key diff --git a/api/v1/swagger/paths/public_csp_reports.yaml b/api/v1/swagger/paths/public_csp_reports.yaml index 735b5c81ca4..ccf4746568d 100644 --- a/api/v1/swagger/paths/public_csp_reports.yaml +++ b/api/v1/swagger/paths/public_csp_reports.yaml @@ -26,6 +26,12 @@ required: true schema: $ref: "../swagger.yaml#/definitions/csp_report" + - name: token + in: query + description: Temporary token found in Reporting-Endpoints response header + required: true + type: string + minLength: 4 responses: "204": description: Report received successfully @@ -33,6 +39,10 @@ description: Bad request schema: $ref: "../swagger.yaml#/definitions/error" + "401": + description: Invalid token + schema: + $ref: "../swagger.yaml#/definitions/error" "500": description: Internal server error schema: diff --git a/debian/templates/koha-conf-site.xml.in b/debian/templates/koha-conf-site.xml.in index 9b09b89ecf2..ce566e0f100 100644 --- a/debian/templates/koha-conf-site.xml.in +++ b/debian/templates/koha-conf-site.xml.in @@ -513,6 +513,7 @@ __END_SRU_PUBLICSERVER__ csp_header_value: The CSP policy directives. Special placeholders: - _CSP_NONCE_: Replaced with a unique nonce for each request + - _CSP_API_TOKEN_: Replaced with a token for verifying the CSP violation report payload Recommended workflow: 1. Start with report-only mode to identify violations @@ -521,7 +522,7 @@ __END_SRU_PUBLICSERVER__ To enable violation reporting, add report-to to your policy pointing to csp-violations. Example with reporting: - default-src 'self'; script-src 'self' 'nonce-_CSP_NONCE_'; style-src 'self' 'nonce-_CSP_NONCE_'; style-src-attr 'unsafe-inline'; img-src 'self' data:; font-src 'self'; object-src 'none'; report-uri /api/v1/public/csp-reports; report-to csp-violations + default-src 'self'; script-src 'self' 'nonce-_CSP_NONCE_'; style-src 'self' 'nonce-_CSP_NONCE_'; style-src-attr 'unsafe-inline'; img-src 'self' data:; font-src 'self'; object-src 'none'; report-uri /api/v1/public/csp-reports?token=_CSP_API_TOKEN_; report-to csp-violations Reports are logged via Koha's logging system. Configure log4perl to capture them: log4perl.logger.api.csp = WARN, CSP diff --git a/etc/koha-conf.xml b/etc/koha-conf.xml index 97108498fd9..45a3145cfca 100644 --- a/etc/koha-conf.xml +++ b/etc/koha-conf.xml @@ -324,6 +324,7 @@ csp_header_value: The CSP policy directives. Special placeholders: - _CSP_NONCE_: Replaced with a unique nonce for each request + - _CSP_API_TOKEN_: Replaced with a token for verifying the CSP violation report payload Recommended workflow: 1. Start with report-only mode to identify violations @@ -332,7 +333,7 @@ To enable violation reporting, add report-to to your policy pointing csp-violations. Example with reporting: - default-src 'self'; script-src 'self' 'nonce-_CSP_NONCE_'; style-src 'self' 'nonce-_CSP_NONCE_'; style-src-attr 'unsafe-inline'; img-src 'self' data:; font-src 'self'; object-src 'none'; report-uri /api/v1/public/csp-reports; report-to csp-violations + default-src 'self'; script-src 'self' 'nonce-_CSP_NONCE_'; style-src 'self' 'nonce-_CSP_NONCE_'; style-src-attr 'unsafe-inline'; img-src 'self' data:; font-src 'self'; object-src 'none'; report-uri /api/v1/public/csp-reports?token=_CSP_API_TOKEN_; report-to csp-violations Reports are logged via Koha's logging system. Configure log4perl to capture them: log4perl.logger.api.csp = WARN, CSP diff --git a/t/Koha/ContentSecurityPolicy.t b/t/Koha/ContentSecurityPolicy.t index b24f26e9acf..33415b63e3a 100755 --- a/t/Koha/ContentSecurityPolicy.t +++ b/t/Koha/ContentSecurityPolicy.t @@ -19,7 +19,7 @@ use Modern::Perl; use Test::NoWarnings; use Test::MockModule; -use Test::More tests => 6; +use Test::More tests => 7; use Test::Exception; BEGIN { use_ok('Koha::ContentSecurityPolicy') } @@ -172,4 +172,19 @@ subtest 'nonce tests' => sub { is( $csp->get_nonce, 'cached value', 'nonce is not re-generated as it was previously cached' ); }; +subtest 'api_token() tests' => sub { + t::lib::Mocks::mock_config( + $conf_csp_section, + { opac => { csp_header_value => 'begin |_CSP_NONCE_| |_CSP_API_TOKEN_| end' } } + ); + + my $csp = Koha::ContentSecurityPolicy->new; + + $csp->set_nonce('cached value'); + like( + $csp->header_value, qr/^begin |cached value| |.{22,}| end$/, + '_CSP_API_TOKEN_ got replaced, together with the nonce' + ); +}; + 1; diff --git a/t/db_dependent/Koha/Middleware/ContentSecurityPolicy.t b/t/db_dependent/Koha/Middleware/ContentSecurityPolicy.t index 7d12f3cd9c4..8b5f1c33025 100755 --- a/t/db_dependent/Koha/Middleware/ContentSecurityPolicy.t +++ b/t/db_dependent/Koha/Middleware/ContentSecurityPolicy.t @@ -165,11 +165,11 @@ subtest 'test CSP in staff client' => sub { }; subtest 'test Reporting-Endpoints for CSP violation reports' => sub { - plan tests => 1; + 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'"; + "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'; report-uri /api/v1/public/csp-reports?token=_CSP_API_TOKEN_"; t::lib::Mocks::mock_preference( 'SessionStorage', 'tmp' ); @@ -195,14 +195,27 @@ subtest 'test Reporting-Endpoints for CSP violation reports' => sub { }; }; - my $test = Plack::Test->create($app); - my $res = $test->request( GET "/opac/opac-main.pl" ); - my $expected_csp_header_value = $csp_header_value; - $expected_csp_header_value =~ s/_CSP_NONCE_/$test_nonce/g; + my $test = Plack::Test->create($app); + my $res = $test->request( GET "/opac/opac-main.pl" ); like( - $res->header('reporting-endpoints'), qr/^csp-violations="\/api\/v1\/public\/csp-reports"$/, + $res->header('content-security-policy'), qr/report-uri \/api\/v1\/public\/csp-reports\?token=[\w\.\-]{22,}$/, + "Response contains Content-Security-Policy header when it is enabled in koha-conf.xml" + ); + like( + $res->header('reporting-endpoints'), qr/^csp-violations="\/api\/v1\/public\/csp-reports\?token=[\w\.\-]+"$/, "Response contains Reporting-Endpoints header with the expected value" ); + my ($token) = $res->header('reporting-endpoints') =~ /token=([\w\.\-]+)"$/; + + my $decoded_jwt = Mojo::JWT->new( + secret => Digest::MD5::md5_base64( + Encode::encode( 'UTF-8', C4::Context->config('api_secret_passphrase') || 'unsafe' ) + ) + )->decode($token); + + cmp_ok( $decoded_jwt->{timestamp}, '<=', time(), 'JWT timestamp is less or equal to current timestamp' ); + cmp_ok( $decoded_jwt->{exp}, '>=', time(), 'JWT expires in the future' ); + is( $decoded_jwt->{type}, 'csp-violation', 'JWT type is csp-violation' ); $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/api/v1/public/csp_reports.t b/t/db_dependent/api/v1/public/csp_reports.t index 40da813de6a..aaa4b21eb8f 100755 --- a/t/db_dependent/api/v1/public/csp_reports.t +++ b/t/db_dependent/api/v1/public/csp_reports.t @@ -28,15 +28,19 @@ use Log::Log4perl; use t::lib::Mocks; use Koha::Database; +use Koha::ContentSecurityPolicy; my $schema = Koha::Database->new->schema; my $t = Test::Mojo->new('Koha::REST::V1'); t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 ); +t::lib::Mocks::mock_config( 'api_secret_passphrase', 'test' ); + +my $csp = Koha::ContentSecurityPolicy->new; subtest 'add() tests' => sub { - plan tests => 7; + plan tests => 16; $schema->storage->txn_begin; @@ -57,12 +61,41 @@ subtest 'add() tests' => sub { } }; - # Anonymous request should work (browsers send these without auth) + # Anonymous request requires token $t->post_ok( '/api/v1/public/csp-reports' => { 'Content-Type' => 'application/csp-report' } => json => $csp_report ) + ->status_is( 400, 'CSP report denied (400)' ) + ->content_like( qr/Missing property.*\/token"/, '...because of a missing token' ); + + # Anonymous request requires valid token + $t->post_ok( '/api/v1/public/csp-reports?token=invalid' => { 'Content-Type' => 'application/csp-report' } => json => + $csp_report )->status_is( 401, 'CSP report denied (401)' ) + ->json_is( '/error', 'Invalid token.', '...because of an invalid token' ); + + # Anonymous request requires a non-expired token + my $expired_token = Mojo::JWT->new( + claims => { + type => 'csp-violation', + timestamp => 0, + }, + expires => 0, + secret => Digest::MD5::md5_base64( + Encode::encode( 'UTF-8', C4::Context->config('api_secret_passphrase') || 'unsafe' ) + ) + )->encode; + + $t->post_ok( '/api/v1/public/csp-reports?token=' + . $expired_token => { 'Content-Type' => 'application/csp-report' } => json => $csp_report ) + ->status_is( 400, 'CSP report denied (400)' ) + ->json_is( '/error', 'This token has expired.', '...because of an expired token' ); + + # Anonymous request should work (browsers send these without auth) + $t->post_ok( '/api/v1/public/csp-reports?token=' + . $csp->api_token => { 'Content-Type' => 'application/csp-report' } => json => $csp_report ) ->status_is( 204, 'CSP report accepted' ); # Test with application/json content type (also valid) - $t->post_ok( '/api/v1/public/csp-reports' => { 'Content-Type' => 'application/json' } => json => $csp_report ) + $t->post_ok( '/api/v1/public/csp-reports?token=' + . $csp->api_token => { 'Content-Type' => 'application/json' } => json => $csp_report ) ->status_is( 204, 'CSP report accepted with application/json content type' ); # Test with minimal report @@ -74,7 +107,7 @@ subtest 'add() tests' => sub { } }; - $t->post_ok( '/api/v1/public/csp-reports' => json => $minimal_report ) + $t->post_ok( '/api/v1/public/csp-reports?token=' . $csp->api_token => json => $minimal_report ) ->status_is( 204, 'Minimal CSP report accepted' ); subtest 'make sure log entries are being written' => sub { @@ -114,8 +147,8 @@ subtest 'add() tests' => sub { is( $appender->buffer, '', 'Nothing in log buffer yet' ); is( $appenderplack->buffer, '', 'Nothing in plack log buffer yet' ); - $t->post_ok( - '/api/v1/public/csp-reports' => { 'Content-Type' => 'application/csp-report' } => json => $csp_report ) + $t->post_ok( '/api/v1/public/csp-reports?token=' + . $csp->api_token => { 'Content-Type' => 'application/csp-report' } => json => $csp_report ) ->status_is( 204, 'CSP report accepted' ); my $expected_log_entry = sprintf( @@ -137,8 +170,8 @@ subtest 'add() tests' => sub { $appender->clear(); $ENV{'plack.is.enabled.for.this.test'} = 1; # tricking C4::Context->psgi_env - $t->post_ok( - '/api/v1/public/csp-reports' => { 'Content-Type' => 'application/csp-report' } => json => $csp_report ) + $t->post_ok( '/api/v1/public/csp-reports?token=' + . $csp->api_token => { 'Content-Type' => 'application/csp-report' } => json => $csp_report ) ->status_is( 204, 'CSP report accepted' ); like( $appenderplack->buffer, qr/$expected_log_entry/ @@ -157,8 +190,8 @@ log4perl.appender.API.utf8=1 HERE ); $appender = Log::Log4perl->appenders()->{API}; - $t->post_ok( - '/api/v1/public/csp-reports' => { 'Content-Type' => 'application/csp-report' } => json => $csp_report ) + $t->post_ok( '/api/v1/public/csp-reports?token=' + . $csp->api_token => { 'Content-Type' => 'application/csp-report' } => json => $csp_report ) ->status_is( 204, 'CSP report returns 204 even when no CSP loggers are defined' ); is( $appender->buffer, '', 'Nothing in the only defined log buffer, because it is unrelated to CSP' ); -- 2.34.1