getstation / electron-chrome-extension

Implementation of Chrome extension APIs for Electron
139 stars 26 forks source link

Gmelius not working #4

Closed alexstrat closed 6 years ago

alexstrat commented 6 years ago

I don't have the stack-trace at the time of writing, but please complete with it. The issue seem to come from CSP problematics.

Gmelius use the content_security_policy key in its manifest: see here.

I think this issue is linked: https://github.com/electron/electron/issues/10180

I realized that started working on this and let let hanging. I committed as it (don't remember if it worked) and push on this branch: add-web-frame-add-origin-access-whitelist-entry.

This should be used on used in the electron-chrome-extension pacakage to support the content_security_policy key.

hugomano commented 6 years ago

Strange, I don't reproduce the gmelius bug. Use case should be cool for reproduce the bug. The code looks good after reading this

hugomano commented 6 years ago

We have tried to inject CSP policies with meta[http-equiv="Content-Security-Policy"] but we are fucked up because we can't add all type of keys and if the server return nonce, we can't add neither.

hugomano commented 6 years ago

The expected code that doesn't work https://github.com/getstation/electron-chrome-extension/compare/feature/csp?expand=1

(to test, you will need to build electron locally with python script/build.py and run ../electron/out/D/Electron.app/Contents/MacOS/Electron example/main.js)

hugomano commented 6 years ago

We tried to replace GURL(sourceOrigin.c_str()) with blink::WebURL url((GURL(sourceOrigin))); because the header file use the WebURL type. After long minutes of compilation et running it, the error still the same.

https://github.com/electron/electron/commit/d4b257e3621d2237e4cfc750dcca08d07554cc78#diff-893a7c52739f0cd0d58e20ddce38dd1fR210

void WebFrame::AddOriginAccessWhitelistEntry(
    const std::string& sourceOrigin,
    const std::string& destinationProtocol,
    const std::string& destinationHost,
    bool allowDestinationSubdomains) {
    blink::WebURL sourceOrigin((GURL(sourceOrigin)));
    blink::WebSecurityPolicy::AddOriginAccessWhitelistEntry(
        sourceOrigin,
        blink::WebString::FromUTF8(destinationProtocol),
        blink::WebString::FromUTF8(destinationHost),
        allowDestinationSubdomains);
};
alexstrat commented 6 years ago

@hugomano I only had a time to do a static exploration. Here are few things I hope will help you:

I may have misguided you: the support for content_security_policy manifest key may not be solved by adding the AddOriginAccessWhitelistEntry.

Indeed, in Chromium code, tracing the handling of the content_security_policy manifest key leads me to the use of the WebKit's WebFrame's method SetIsolatedWorldContentSecurityPolicy which is probably the one we need to use.

_Btw the lines that actually do the script injection in Chromium are worth looking closely_

At this point then, you need to rely on WebKit's "isolated world" to do script injection which is not the case today but is definitely the right thing to do to avoid problems. The good news is that I had started working on this and that it's feasible, the bad news is that it's not finished.

Therefore you might need to continue from there:

That being said I'm not sure Gmelius's bug is directly linked to the non-support of content_security_policy manifest key. Would you mind copy pasting the actual bug report in this thread?

Good luck

hugomano commented 6 years ago
alexstrat commented 6 years ago

I think you need to webFrame.setIsolatedWorldSecurityOrigin(0, 'chrome-extension://${cs.chromeStoreExtensionId}\') in the background page