yii2 icon indicating copy to clipboard operation
yii2 copied to clipboard

Mark client-side scripts as safe to use for browsers (#20087)

Open skepticspriggan opened this issue 1 year ago • 5 comments

Q A
Is bugfix?
New feature? ✔️
Breaks BC?
Fixed issues #20087

The important choices made and the reasons why are summarized below. I'm open to suggestions for improvements.

What attribute should be added to the script tag?

Only the nonce

Features:

  • Nonce use-case is easy to find in the API docs
  • Slightly more cohesive as it must be enabled in the application configuration
  • Less flexible

Custom attributes

  • Nonce use-case is easy to find if explained in the general docs
  • Slightly less cohesive as it must be enabled in the entry script
  • More flexible

Solution 1 -- Only the nonce

This section assumes only the nonce is added.

Where should the nonce script attribute be added?

In the BaseHtml helper

Set the nonce attribute in all script tags via yii\helpers\BaseHtml::script() when it exists. This is similar to how the hidden CSRF token input is added to all forms via yii\helpers\BaseHtml::beginForm().

Adding it at a low level ensures all script tags are whitelisted and makes it easier to maintain.

How should the nonce be called?

From a function in the Response component

$nonce = Yii::$app->response->getCspNonce();

Where should the nonce be set?

In the application configuration

'components' => [
    'response' => [
        'class' => 'yii\web\Response',
        'cspNonce' => $nonce,
    ],
],

Solution 2 -- Custom attributes

DI could be used to inject custom attributes into the script tags:

\Yii::$container->set('yii\web\View', ['jsOptions' => ['nonce' => $nonce]]);

Updates required to make this work:

  • Add the variable jsOptions to yii\web\View.
  • Load the default options from jsOptions inside yii\web\View::registerJsFile().

skepticspriggan avatar Jan 27 '24 13:01 skepticspriggan

PR Summary

  • Improved Security Feature in Changelog The update includes a notable enhancement to make client-side scripts safer for browsers. This is a notable change as it bolsters the overall security of the application.

  • Added Support for Content Security Policy (CSP) Nonce This update now allows the integration of a nonce (a randomly generated string) in the <script> tags within our HTML code. This drastically improves our content security policy, making our web content more resilient to malicious attacks.

  • Enhanced Response Handling The PR has updated how responses are handled by including a property for the CSP nonce. This includes getter and setter methods for the nonce and a method to load the nonce from the header.

  • Additional Unit Tests Finally, the PR includes additional unit tests to make sure the new changes about CSP nonce are working as intended. These tests work by checking every small unit of our code, especially the ones that have just been updated, ensuring the entire system is working correctly.

what-the-diff[bot] avatar Jan 27 '24 13:01 what-the-diff[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 63.42%. Comparing base (f7cba1b) to head (3071e20).

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #20107      +/-   ##
============================================
- Coverage     63.64%   63.42%   -0.22%     
- Complexity    11376    11378       +2     
============================================
  Files           429      429              
  Lines         37073    37077       +4     
============================================
- Hits          23594    23515      -79     
- Misses        13479    13562      +83     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 27 '24 14:01 codecov[bot]

Easier to use when nonce is generated at webserver level: No configuration required as the nonce is automatically read from the header.

Are you sure about that? AFAIK if header is added by webserver, it is done after generating response by PHP, so PHP does not have access to this header.

rob006 avatar Jan 30 '24 17:01 rob006

AFAIK if header is added by webserver, it is done after generating response by PHP, so PHP does not have access to this header.

You are right, that was an incorrect assumption. I will revise the request. This will make the comparison easier.

skepticspriggan avatar Feb 02 '24 09:02 skepticspriggan

The initial solution of setting only the CSP nonce via the Response property in the application configuration has become less attractive after review. What do you think about the new version?

skepticspriggan avatar Feb 19 '24 13:02 skepticspriggan

It looks OK.

samdark avatar Mar 24 '24 21:03 samdark

It seems the scriptOptions variable got lost somewhere causing an error:

yii\base\UnknownPropertyException: Setting unknown property: yii\web\View::scriptOptions

I will add the missing variable shortly.

skepticspriggan avatar Mar 25 '24 15:03 skepticspriggan

Thanks!

samdark avatar Apr 03 '24 13:04 samdark

Glad to contribute :)

skepticspriggan avatar Apr 04 '24 18:04 skepticspriggan