-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Add API to render sandboxed iframe with inline scripts #132595
Comments
Pinging @elastic/kibana-core (Team:Core) |
Pinging @elastic/kibana-security (Team:Security) |
Thanks for the detailed explanations. Given I couldn't attend to the meeting, I got a few questions:
Did you discuss by any chance which core service it would make the most sense to expose this new API from?
Unless we're expecting all consumers to call the API during their lifecycle methods, I don't think this would work? I would assume that consumers will call
What |
I think this was a typo -- |
We didn't discuss this, I'm not an expert but in my opinion a new top level service (could be called
Apologies, should have explained that better. In addition to Joes explanations, I don't have a strong opinion of how this registration process should look like, open to suggestions. What I had in mind was indeed forcing the user to generate the renderer during the class MyPlugin {
setup(core) {
this.myIFrameRenderer = core.sandbox.generateSandboxIframeRenderer('myPlugin');
}
start(core) {
core.application.register({
mount: ({ mountpoint }) => {
this.myIFrameRenderer.render(mountpoint, theSandboxedScript);
}
})
}
} This would keep the API relatively simple (no multi step registration plus usage) and allow the core services to drop every reference to the nonce value except for the JS closure which is providing the renderer, minimizing the chances for accidentally leaking it due to refactoring etc.
Yes, exactly. Not sure about that part and how it would interact with the Simplified implementation of the iframe renderer: render(el, myScript) {
// TODO - maybe use react.renderToString for that?
const srcDoc = `<html><body><script nonce="${this.nonce}">${myScript}</script></body></html>`;
ReactDom.render(el, <iframe sandbox="allow-scripts" srcDoc={srcDoc} />);
} |
@flash1293 what is the priority regarding this? |
@pgayvallet Low priority at the moment, I will update the issue in case we get closer being ready to consume an API like this |
@elastic/kibana-security do you know if that's still something we might want, or should I just close this one? |
@pgayvallet I know this was needed for script-based visualizations (#131321), but I don't believe this is on the roadmap, and I'm unsure about its future. From the security side, we found alternate solutions to hardening our third-party dependencies, which removed our own need for this. So -- unless we have feedback from the viz team, I think we are ok to close this. |
I'm not part of the visualizations team anymore, but it looks like it has been deprioritized: #126748 (comment) I'll pour one out for this feature, would have loved to see it land at one point :) |
Thank you both for your feedback. Closing then |
Discussed with @lukeelmers and @jportner - FYI @pgayvallet
There should be an API exposed as part of core services to render a sandboxed iframe with a supplied inline script in order to execute this inline script in a safe way as well as allowing the script to render dom elements etc. within the iframe.
To make sure this API isn't abused, any consuming plugin has to register itself in its setup phase. A functional test including a whitelist of all registered plugins will be maintained in order to pull in the security and core teams as a mandatory reviewer of each feature using this service.
Design
kibanaHost/app/<xyz>
generateSandboxIframeRenderer(pluginId: string): SandboxIframeRenderer
SandboxIframeRenderer#render
method is wrapping the supplied script into an inline script tag within thesrcDoc
of a newly generated iframe element with the nonce passed via injected meta data. The plugin never has access to the nonce itself.sandbox="allow-scripts"
which disallows everything except for running local scripts.<meta http-equiv="content-security-policy" content="default-src none; script-src 'nonce-${nonce}'; style-src 'nonce-${nonce}'">
generateSandboxIframeRenderer
- by configuring the security team as code owner of the functional test, a review of the security team for each plugin using the new API can be enforcedReferences
See POC here https://github.com/elastic/kibana/pull/131321/files#diff-4825eb8ea1b3f99945f4c0347d8f99081bc698ccfa29428b86f044fa59b7f806R97
The text was updated successfully, but these errors were encountered: