Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Conversation

skepticspriggan
Copy link
Contributor

@skepticspriggan skepticspriggan commented Jan 27, 2024

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

  • Nonce use-case is easy to find in the API docs
  • Less flexible

Custom attributes

  • Nonce use-case is easy to find if explained in the general docs
  • 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

Set custom script attributes centrally in web.php:

'components' => [
    'view' => [
        'class' => 'yii\web\View',
        'scriptOptions' => ['nonce' => $nonce],
    ],
],

Updates required to make this work:

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

Copy link

what-the-diff bot commented Jan 27, 2024

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.

Copy link

codecov bot commented Jan 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rob006
Copy link
Contributor

rob006 commented Jan 30, 2024

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.

@skepticspriggan
Copy link
Contributor Author

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
Copy link
Contributor Author

skepticspriggan commented Feb 19, 2024

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 skepticspriggan force-pushed the 20087-add-csp-nonce-attribute-to-script-tags branch from 5778147 to 676a6ec Compare March 23, 2024 23:23
@samdark
Copy link
Member

samdark commented Mar 24, 2024

It looks OK.

@skepticspriggan
Copy link
Contributor Author

skepticspriggan commented Mar 25, 2024

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 skepticspriggan force-pushed the 20087-add-csp-nonce-attribute-to-script-tags branch from 28dc476 to 838dd65 Compare March 25, 2024 18:35
@skepticspriggan skepticspriggan force-pushed the 20087-add-csp-nonce-attribute-to-script-tags branch from 361dd46 to 3071e20 Compare April 3, 2024 12:15
framework/web/View.php Show resolved Hide resolved
@samdark samdark merged commit bf3ada1 into yiisoft:master Apr 3, 2024
20 of 25 checks passed
@samdark
Copy link
Member

samdark commented Apr 3, 2024

Thanks!

@samdark samdark added this to the 2.0.50 milestone Apr 3, 2024
@skepticspriggan
Copy link
Contributor Author

Glad to contribute :)

@skepticspriggan skepticspriggan deleted the 20087-add-csp-nonce-attribute-to-script-tags branch June 29, 2024 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants