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

Replace CSP 'nonce-<base64>' directive with 'self' directive #43553

Merged
merged 4 commits into from
Aug 21, 2019

Conversation

joshdover
Copy link
Contributor

@joshdover joshdover commented Aug 19, 2019

Summary

Closes #42497

This replaces our usage of the 'nonce-{nonce}' directive with 'self' which will allow us to support dynamic imports more easily.

This change is BWC by adding fixes for any invalid rules in the csp.rules config option:

  • If any CSP rules contain the {nonce} template, that source will be removed and a warning will be logged.
  • If a CSP rule does not contain 'self', it will be added and a warning will be logged.

Dev Docs

Kibana no longer supports the {nonce} notation in the csp.rules configuration. These will be replaced with the 'self' source directive automatically and log a deprecation warning. The {nonce} notation must be removed before upgrading to 8.0.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@joshdover joshdover added Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. Feature:Security/CSP Platform Security - Content Security Policy labels Aug 19, 2019
@joshdover joshdover requested review from epixa and kobelb August 19, 2019 22:06
@joshdover joshdover requested review from a team as code owners August 19, 2019 22:06
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security

@kobelb
Copy link
Contributor

kobelb commented Aug 19, 2019

ACK: reviewing now

src/legacy/server/config/transform_deprecations.js Outdated Show resolved Hide resolved
sourceList = sourceList.filter(source => !source.includes(NONCE_STRING));
}

if (SELF_POLICIES.includes(policy) && !sourceList.find(source => source.includes(SELF_STRING))) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone happened to set the following, we'll likely be breaking this install csp.rules: ["default-src 'unsafe-eval' 'nonce-{nonce}'"]. What if instead whenever we removed a nonce source, we added the 'self' source?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should work. I think we should still also add it if script-src or style-src don't have self or nonce.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable to me.

src/legacy/server/csp/index.test.ts Show resolved Hide resolved
test/api_integration/apis/general/csp.js Outdated Show resolved Hide resolved
src/legacy/ui/ui_render/views/ui_app.pug Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@joshdover joshdover requested a review from kobelb August 20, 2019 22:30
@joshdover joshdover requested a review from mistic August 21, 2019 15:36
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Member

@mistic mistic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the operations side it looks good to me! 👍

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

return h.response(`
<!DOCTYPE html>
<title>Kibana OpenID Connect Login</title>
<script nonce="${nonce}">
<script>
Copy link
Member

@azasypkin azasypkin Aug 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: hmmm @joshdover @kobelb aren't we effectively disabling execution of this script here assuming our default CSP rule is script-src 'unsafe-eval' 'self'; now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened an issue to discuss: #44668

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Security/CSP Platform Security - Content Security Policy release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v7.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DISCUSS] Use script-src 'self' instead of script-src 'nonce-<base64-value>'
5 participants