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

Incompatible with a content-security-policy which does not allow script-src: unsafe-inline #234

Closed
ragnarkarlsson opened this issue May 31, 2018 · 4 comments

Comments

@ragnarkarlsson
Copy link

Whilst trying to tighten up my CSP I've noticed that the plugin generates an inline script which is unique on each page load to define u2fL10n. Without running script-src: unsafe-inline this means it is impossible to provide a hash, and the inclusion of https://core.trac.wordpress.org/ticket/39941 script nonce's has potential problems. Can this js not be created on the fly and included rather than inlined?

@kasparsd
Copy link
Collaborator

kasparsd commented Jun 1, 2018

Thanks for opening the issue @ragnarkarlsson!

The plugin uses core wp_localize_script() to include some of the dynamic variables necessary for the U2F to work:

https://github.com/georgestephanis/two-factor/blob/c220afbc9464f07abef6dd961da7d09857544272/providers/class.two-factor-fido-u2f-admin.php#L83-L102

Since all of this is dynamic for each user we can't make it static or move to a file. Looks like fixing this for core with the added nonce attribute to <script> would also solve it here. Do you have other suggestions on how to approach this?

@ragnarkarlsson
Copy link
Author

Hmm, I'm afraid my development days are a little behind me and I'm now in security so I may not be much help!

Would it be possible to ajax the u2fL10n var? https://developer.wordpress.org/reference/functions/wp_localize_script/#comment-1391 that way it'd be possible a single hash required in the CSP, instead of unsafe-inline ... I think?

unsafe-inline presents various XSS possibilities, hence why I'd like to eliminate it. I don't like the idea of dropping U2F in favour of a tighter CSP.

I've made comment regarding the nonce value on the trac, it could easily be overwritten (I tested this last night) if a security plugin then defines a new CSP after core outputs a very slimline CSP, so not convinced atm it'll solve this issue.

@kkmuffme
Copy link
Contributor

Atm Wordpress is not compatible with CSP that has unsafe-inline disabled.

The wp localize script does not yet support the nonce(s) required for CSP with unsafe-inline disabled, so there isn't anything that can be done to fix this when Wordpress compatibility should be kept (the only thing would be a custom localize script, but then again this is against WP core policies for scripts to be completely dequeueable using standard measures)

Furthermore WP uses so many inline scripts, you will not only have a problem here, but in like 100 other places of WP too. So I think it's not worth it to fix that now, but rather to promote WP core to implement CSP nonce in their wp_localize_script function.

@iandunn
Copy link
Member

iandunn commented Oct 20, 2022

U2F is deprecated and no longer works in Chrome, so the provider is being removed in #439 . Given that, there's probably no reason to keep this open anymore.

@iandunn iandunn closed this as completed Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants