Problem/Motivation

The comment under the "default-src" field at /admin/config/system/seckit is wrong / misleading.
It says:
> Specify security policy for all types of content, which are not specified further (frame-ancestors excepted). Default is 'self'.

But its default is not 'self'. Its default value is empty, meaning the "default-src" definition is not present in the generated Content-Security-Policy header.

Steps to reproduce

1) configure the secktit module

Go to /admin/config/system/seckit
Configure the "Content Security Policy" section under "Cross-site Scripting"
Send HTTP response header - enable
You can see the "default-src" field has empty value (after the fresh module installation)
Verify the "report-uri" field has the default "/report-csp-violation" value.
Save configuration.

2) observe the content security policy header:

for example, using curl and a cache buster: 

$ curl -sI http://my-d8.site/?ab1 | grep Content-Security-Policy
Content-Security-Policy: report-uri /report-csp-violation

conclusion: The default value was empty and the "default-src" directive is not present in the header.

3) reconfigure the seckit module

Set the "default-src" to 'self' and save.

4) Observe the header

$ curl -sI http://my-d8.site/?ab2 | grep Content-Security-Policy
Content-Security-Policy: default-src 'self'; report-uri /report-csp-violation

conclusion: after *explicitly* setting the value of "default-src" to 'self', this made it to the header.

Proposed resolution

Change the comment under the field
from: "Default is 'self'."
to: "Default is empty, meaning the default-src directive won't be present in the generated header."

Remaining tasks

Find the wrong comment in the module code. Create a patch.

User interface changes

Changed user interface text

API changes

N/A

Data model changes

N/A

CommentFileSizeAuthor
#16 Screenshot 2025-02-28 at 17.35.37.png150.55 KBthe_g_bomb
#2 3193697-2.patch890 bytesmohit.bansal623

Issue fork seckit-3193697

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

marji created an issue. See original summary.

mohit.bansal623’s picture

mohit.bansal623 commented
Status: Active » Needs review
StatusFileSize
new 3193697-2.patch890 bytes

As per the comment, updated the description and created the patch. Please review.

richardgaunt’s picture

richardgaunt commented
Priority: Minor » Critical
Status: Needs review » Reviewed & tested by the community

I've updated priority to critical as the critical setting has a misleading description.

If someone was to read this description and assume the default was 'self' then their site would have an ineffective Content Security Policy.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Priority: Critical » Major
Status: Reviewed & tested by the community » Needs work

It would be worth looking at how it's handled in the D7 branch.

Reducing the priority to "major" as nothing is broken, the UI just could be better.

ramonvasconcelos’s picture

ramonvasconcelos commented
Assigned: Unassigned » ramonvasconcelos

I had a look at the D7 branch and the description is the same.
Specify security policy for all types of content, which are not specified further (frame-ancestors excepted). Default is 'self'.
I think the changes suggested in this issue are applicable.

ramonvasconcelos’s picture

ramonvasconcelos commented
Assigned: ramonvasconcelos » Unassigned
Status: Needs work » Needs review

Moving it to needs review so we can get a third opinion.

pflora’s picture

pflora commented
Status: Needs review » Reviewed & tested by the community

I wonder if we should set self as default as the original description intended. In any case, the patch provided by #2 solves the description problem, so I'm moving this to RTBC so a maintainer can give us some input.

rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented

Agree description needs update.

anoopsingh92’s picture

anoopsingh92
Location Rajasthan, India
commented

Agree +1 RTBC

mcdruid’s picture

mcdruid commented
Version: 8.x-1.x-dev » 2.x-dev
Status: Reviewed & tested by the community » Needs work

re. #4 and #5 yes, the description is the same in the D7 branch but the default is correctly applied to match the description.

https://git.drupalcode.org/project/seckit/-/blob/7.x-1.12/seckit.module#...

Let's fix 2.x rather than changing the description to match the bug.

https://git.drupalcode.org/project/seckit/-/blob/2.0.1/config/install/se...

bharath-kondeti made their first commit to this issue’s fork.

bharath-kondeti opened merge request !16

bharath-kondeti’s picture

bharath-kondeti commented
Status: Needs work » Needs review

Addressed #10 and raised a PR. Please review

mcdruid’s picture

mcdruid commented
Status: Needs review » Needs work

Thanks. However, a clean install of the module still seems to have an empty value for default-src:

$ drush cget seckit.settings

_core:
  default_config_hash: **SNIP**
seckit_xss:
  csp:
    checkbox: false
    vendor-prefix:
      x: false
      webkit: false
    report-only: false
    default-src: ''   <====
    script-src: ''
    object-src: ''

..snip..

The same thing shows up in the admin UI, as we'd expect.

I'm not sure why yet, but just adding the default to config/install/seckit.settings.yml doesn't seem to be enough to fix this.

the_g_bomb made their first commit to this issue’s fork.

the_g_bomb’s picture

the_g_bomb commented
StatusFileSize
new Screenshot 2025-02-28 at 17.35.37.png150.55 KB

drush en seckit using this latest patch and seckit: 2.0.3 or seckit dev-2.x seems to work ok for me when you view the admin form.

  - Installing drupal/seckit (dev-2.x f15d470): Cloning f15d470265 from cache
  - Applying patches for drupal/seckit
    https://git.drupalcode.org/project/seckit/-/merge_requests/16.diff (3193697: default-src has wrong description)

Screenshot of SecKit config form showing self set in default-src
And also when running:

 % drush cget seckit.settings
_core:
  default_config_hash: **SNIP**
seckit_xss:
  csp:
    checkbox: false
    vendor-prefix:
      x: false
      webkit: false
    report-only: false
    default-src: "'self'"
    script-src: ''
    object-src: ''
..snip..
the_g_bomb’s picture

the_g_bomb commented
Status: Needs work » Needs review