Content Security Policy (CSP) Implementation a new approach

[empiricompany]

Content Security Policy (CSP) Implementation in OpenMage

This PR adds comprehensive support for Content Security Policy (CSP) headers in OpenMage, significantly improving application security against XSS attacks and other client-side security vulnerabilities. The module was developed based on the PR #4753 by @Hanmac, to whom all credit should be given. I decided to create a completely separate PR because there were many changes and the approach is very different. I don't want to impose my solution but rather propose a more robust implementation following the OpenMage standards that guarantees the same functionality, such as the ability to specify directives per module and the separation of frontend and backend directives.

Features

• Modular configuration: Each module can specify its own CSP directives in the default section areas:

  • global/csp/<directive-name>: Applied to both frontend and backend
  • frontend/csp/<directive-name>: Applied only to the frontend
  • adminhtml/csp/<directive-name>: Applied only to the backend (administration panel)

• Administration interface: Administrators can:

  • Enable/disable CSP separately for frontend and admin areas
  • Configure "Report Only" mode for each area
  • Configure "Report URI" for each area
  • Split headers for each directive to avoid potential web server/browser limitations

  
  IMPORTANT: Read this https://content-security-policy.com/examples/multiple-csp-headers/

  • Add custom directives
  • View directives added by modules (shown as readonly and non-modifiable) and scope (<global />, <frontend />, <admihtml />)
  • All CSP configuration for Frontend can be custimezed per store view
  • Ability to convert directives into HTTP headers

  
  

• Pre-configured security: The module includes safe directives in the global section (like 'self' for most directives) providing a secure baseline configuration

• Support for all standard CSP directives:

  • default-src
  • script-src
  • style-src
  • img-src
  • connect-src
  • font-src
  • frame-src
  • object-src
  • media-src
  • form-action

Advantages of This Approach

The key technical advantages of this implementation include:

• Efficient configuration caching: Directives from config.xml are properly cached, minimizing performance impact
• Strategic event integration with presentation layer separation: Using http_response_send_before ensures headers are applied at the optimal moment in the request lifecycle while preventing themes from injecting potentially malicious directives
• Developer-friendly format: The configuration structure aligns with OpenMage conventions, making adoption intuitive
• Safe defaults: The module is disabled by default with "Report Only" mode enabled to prevent breaking functionality

Configuration Example in a Module

Modules can specify CSP directives in their config.xml file in three different areas. This configuration approach follows the same pattern used for global>ignore_user_agents in OpenMage:

<config>
    <!-- Global directives applied to both frontend and backend -->
    <global>
        <csp>
            <script-src>
                <my-service>example.com</my-service>
                <my-cdn>cdn.example.com</my-cdn>
            </script-src>
            <img-src>
                <my-images>images.example.com</my-images>
            </img-src>
        </csp>
    </global>
    
    <!-- Frontend-specific directives -->
    <frontend>
        <csp>
            <connect-src>
                <my-api>api.example.com</my-api>
            </connect-src>
            <frame-src>
                <my-widget>widgets.example.com</my-widget>
            </frame-src>
        </csp>
    </frontend>
    
    <!-- Admin-specific directives -->
    <adminhtml>
        <csp>
            <script-src>
                <admin-tool>admin-tools.example.com</admin-tool>
            </script-src>
            <style-src>
                <admin-styles>admin-styles.example.com</admin-styles>
            </style-src>
        </csp>
    </adminhtml>
</config>

Each directive can have multiple entries, and they will be properly merged with directives from other modules and user configurations.
You can see for each directive in which area it was added:

Resources

• https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Security-Policy
• https://csp-evaluator.withgoogle.com/

Open Questions

Ability to add directives from views

The ability to add directives from the presentation layer was deliberately left out. It could be easily implemented, but IMHO views should not have this kind of "power". In cases where themes include external resources, directives can be easily added by checking the browser console.

= UPDATE =
I implemented support for adding directives via <meta>, but it’s strongly discouraged according to the documentation.

For this reason, all <meta> directives added via layout update are merged into the HTTP header by default. If you want to enable directive insertion through <meta>, you must explicitly it in the configuration.

It’s possible to add them through a layout update like this:

<layout version="0.1.0">
    <checkout_onepage_index>
        <reference name="csp_meta">
            <action method="addDirective"><directive>default-src</directive><value>*.default-from-layout-update.com</value></action>
            <action method="addDirective"><directive>script-src</directive><value>*.script-from-layout-update.com</value></action>
            <action method="addDirective"><directive>style-src</directive><value>*.style-from-layout-update.com'</value></action>
        </reference>
    </checkout_onepage_index>
</layout>

This will print in the head (in report-only or enforce mode according to the configuration):

<meta http-equiv="Content-Security-Policy-Report-Only" content="default-src *.default-from-layout-update.com; script-src *.script-from-layout-update.com; style-src *.style-from-layout-update.com'; report-uri https://xxx.report-uri.com/r/d/csp/reportOnly" />

These directives will be difficult to debug because they won’t appear in the configuration like those declared by the modules.
IMPORTANT: Read this https://content-security-policy.com/examples/meta/

API Responses and Event Handling

CSP headers are currently added to all responses, including API responses, by observing the http_response_send_before event. This shouldn't create problems since CSP directives are only interpreted by browsers and add minimal overhead to API responses.
However, it raises the question:
Is it better to hook into an event that only affects frontend/HTML responses, or is it preferable to have CSP headers consistently applied to all responses regardless of type?
Netalico solution use "controller_action_predispatch"
https://github.com/netalico/magento-1-csp/blob/master/app/code/local/Netalico/Csp/etc/config.xml

Report URI Implementation

The current implementation include configuration for a report-to directive declared in Reporting-Endpoints header, which could be valuable for collecting violation reports to external service like https://report-uri.com.

Future enhancements could include:

• Implementing an internal reporting system that collects violation reports and notifies administrators
• Creating an interface that allows administrators to easily whitelist reported domains after verification

Enforce only specific frontend area

I'm thinking about implementing the possibility to enable CSP enforcement only in specific areas, for example on the checkout_onepage_index controller — although I'm not sure I'll go through with it, since I believe the entire site should include security headers.

Default directives

Does it make sense to include default directives for services like Google Analytics, PayPal, Stripe, etc.?
These can change over time and there will never be a complete list — it will always be specific to each use case.
It feels like the same discussion we had about ignore_user_agents.

Nonce helper

A nonce helper could also be implemented to use in the views for scripts and automatically add it to the directives.
Resource: https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/CSP#nonces

[Hanmac]

I like the different Input fields, but I would have tried it with less classes. (I might try to get it working in my MR)

One of the reasons why I wanted the layout update approach was so I could enable different urls depending on the page. Like if Captcha is enabled and it uses external source, I could enable it only for the ones with captcha.
(Maybe even with Events)

[empiricompany]

Hey @Hanmac, thanks for your work and inspiration!

I like the different Input fields, but I would have tried it with less classes. (I might try to get it working in my MR)

One of the reasons why I wanted the layout update approach was so I could enable different urls depending on the page. Like if Captcha is enabled and it uses external source, I could enable it only for the ones with captcha. (Maybe even with Events)

In my view, CSPs should be set at the controller level, where the response and its headers are handled.
To me, it doesn’t make sense to enable a host only for certain pages—if it’s safe, it can be whitelisted for the entire site.
Also, this way I can't get a clear backend-side view of which hosts are active, who is whitelisting them and where.
I prefer this separation of responsibilities.

[empiricompany]

@Hanmac, it’s not exactly what you wanted, but I’ve added the option to include directives via <meta> tag.
You can find the documentation in the updated PR description.

[colinmollenhour]

Awesome, work guys! CSP is the only real way to stop MageCart style attacks so this is major.

I think allowing the rules to be added via layouts and events is useful so that extensions can inject their CSP policies without overriding controllers or requiring manual update to the config or global config. For example, you may want an obnoxious add-on the main site, but exclude it from the checkout or account pages, or it may only apply to product pages (e.g. a reviews plugin) so adding it everywhere just bloats your header sizes. Keep in mind that some third-party add-ons don't just require a single rule, some of the worst ones may require a bunch of rules (Facebook is one of the worst if I recall correctly).

[colinmollenhour]

These are both big PRs so it is hard to review and compare without a deep dive.. @Hanmac you are very clearly credited in the description of the PR, does it matter whose PR is finally merged? If you truly think this is a worse PR than yours then just state the reasons so everyone can understand and help decide. But, if you think it is generally an improvement or at least can live with the changes and recognize that to some users it may be an improvement, then just get onboard and help iron out the issues on this PR and close yours - everyone will still recognize the hard work you put into laying the foundation and getting it to completion.

[sreichel]

Cant create a PR to your repo ... PhpSran Level 10 ... https://github.com/OpenMage/magento-lts/commit/8b501d87d611f42376acd10c68c0318194046ca5.patch

Please check commented lines.

[empiricompany]

I've added a default setting to convert and merge all <meta> directives into HTTP headers.
The behavior is described at the point "Ability to add directives from views".

This allows adding:

<checkout_onepage_index>
    <reference name="csp_meta">
        <action method="addDirective"><directive>default-src</directive><value>*.default-from-layout-update.com</value></action>
        <action method="addDirective"><directive>script-src</directive><value>*.script-from-layout-update.com</value></action>
        <action method="addDirective"><directive>style-src</directive><value>*.style-from-layout-update.com</value></action>
    </reference>
</checkout_onepage_index>

Then they are read and added here, and the block output is disabled:
f33f2f6#diff-379c816be58daa7145011f49adf0aa9a9d899bbed7923d7f4f44a63accc70a57

if ($helper->shouldMergeMeta($area)) {
    $blockCspMeta = Mage::app()->getLayout()->getBlock('csp_meta');
    if ($blockCspMeta && $blockCspMeta instanceof Mage_Csp_Block_Meta) {
        $metaDirectives = $blockCspMeta->getDirectives();
        foreach ($metaDirectives as $directive => $values) {
            $directives[$directive] = array_unique(
                array_merge($directives[$directive] ?? [], $values),
            );
        }
    }
}

[Hanmac]

@empiricompany the only tricky one is this Route: adminhtml_index_login
Because it doesn't really have a Head-Block you can access. (and can't really add meta tags)

But you could add it to form.additional.info if shouldMergeMeta is enabled?

[empiricompany]

@Hanmac For the adminhtml_index_login and adminhtml_index_forgotpassword handles, if I add it to form.additional.info when in <meta> mode, the meta tag output is incorrectly added to the <body>.

[Hanmac]

@Hanmac For the adminhtml_index_login and adminhtml_index_forgotpassword handles, if I add it to form.additional.info when in <meta> mode, the meta tag output is incorrectly added to the <body>.

Yeah, that's what i mean.

But if i would enable, shouldMergeMeta then the Update should be moved to the Header

[sreichel]

Please grant permission to create PRs. I'd like to add some tests.

Two small bugs in admin config:

• there is no form validation, so its possible to save empty input
• self or data: appears twice. (global and ...)

[sreichel]

Some code could be shorter, but .... https://youtrack.jetbrains.com/issue/WI-81337/Argument-type-does-not-match-the-declared-when-using-class-alias

[empiricompany]

Please grant permission to create PRs. I'd like to add some tests.

Two small bugs in admin config:

• there is no form validation, so its possible to save empty input
• self or data: appears twice. (global and ...)

sure, you can create PR on my branch

[sreichel]

• revert visibility change from private to protected, add new private property?
• avoid new entries in phpstan baseline
• add form validation, avoid empty/duplicated entries
• see comment from @colinmollenhour, do we need all that default values in xml?
• chore: update labeler config
• tests: add some tests (unit/e2e)

My checklist for next days :)

[sreichel]

sure, you can create PR on my branch

Mhh, i cant select your repo.

[sreichel]

See latest commits https://github.com/sreichel/magento-lts/tree/Mage_Csp

[empiricompany]

strange, if you look there is also a PR of @Hanmac that I have just merged

[Hanmac]

sure, you can create PR on my branch

Mhh, i cant select your repo.

I somehow had the same problem, but i don't remember how i fixed it.

I think I changed it to myRepo:Mage_Csp <= empiricompany:Mage_Csp,
and then told github to reverse the comparison

@sreichel you can try to click on this Link: empiricompany/openmage@Mage_Csp...sreichel:magento-lts:Mage_Csp

[empiricompany]

@sreichel simply fork my branch https://github.com/empiricompany/openmage/tree/Mage_Csp and create a pull request
This branch is not behind the upstream OpenMage/magento-lts:main

[sreichel]

@empiricompany thats what I did. I'll test it again.