gwtproject / gwt

GWT Open Source Project
http://www.gwtproject.org
1.52k stars 374 forks source link

GWT should support "Content Security Policy" #8197

Closed dankurka closed 7 years ago

dankurka commented 9 years ago

Originally reported on Google Code with ID 8207

Found in GWT Release   : any
Encountered on OS      : any OS
Encountered on Browser : FF, Chrome

Detailed description (please be as specific as possible):

GWT should support CSP

https://dvcs.w3.org/hg/content-security-policy/raw-file/tip/csp-specification.dev.html

Current issues:
- Devmode uses eval, which is forbidden by CSP
- History support with iFrame is not accepted by CSP

Used CSP HTTP Header:
Content-Security-Policy:   default-src 'self' gwt.google.com; img-src 'self'
X-Content-Security-Policy: default-src 'self' gwt.google.com; img-src 'self'

GWT History iFrame in Hostpage:
<iframe src="javascript:''" id="__gwt_historyFrame" style="width:0;height:0;border:0"></iframe>

Exception:
08:30:06.117 [ERROR] [csptest] Unable to load module entry point class null (see associated
exception for details)

com.google.gwt.core.client.JavaScriptException: (Error) @com.google.gwt.core.client.impl.Impl::registerEntry()([]):
call to eval() blocked by CSP
    at com.google.gwt.dev.shell.BrowserChannelServer.invokeJavascript(BrowserChannelServer.java:249)
    at com.google.gwt.dev.shell.ModuleSpaceOOPHM.doInvoke(ModuleSpaceOOPHM.java:136)
    at com.google.gwt.dev.shell.ModuleSpace.invokeNative(ModuleSpace.java:571)
    at com.google.gwt.dev.shell.ModuleSpace.invokeNativeObject(ModuleSpace.java:279)
    at com.google.gwt.dev.shell.JavaScriptHost.invokeNativeObject(JavaScriptHost.java:91)
    at com.google.gwt.core.client.impl.Impl.registerEntry(Impl.java)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:601)
    at com.google.gwt.dev.shell.ModuleSpace.onLoad(ModuleSpace.java:357)
    at com.google.gwt.dev.shell.OophmSessionHandler.loadModule(OophmSessionHandler.java:200)
    at com.google.gwt.dev.shell.BrowserChannelServer.processConnection(BrowserChannelServer.java:526)
    at com.google.gwt.dev.shell.BrowserChannelServer.run(BrowserChannelServer.java:364)
    at java.lang.Thread.run(Thread.java:722)

Firebug Console Output:
CSP WARN: Directive inline script base restriction violated
javascript:''
CSP WARN: Directive eval script base restriction violated
call to eval() or related function blocked by CSP

Workaround if you have one: not using CSP

Reported by daniel.gerber2 on 2013-06-19 06:50:37

dankurka commented 9 years ago
The iframe for history is only needed for IE6 and IE7 (or later IEs in those legacy
modes) which will no longer be supported in the next GWT version; so you can safely
remove that iframe (and we should update the docs about history).

I think DevMode (hosted.html and devmode.js) can be change to avoid using eval(), though
the DevMode plugin does use eval() to install the JSNI methods, and I don't know how
it'd play with CSP (eval()ing from a plugin).

We've already fixed a few things re. CSP in 2.5.1 (see issue 7682 and issue 7894),
but there are a few others to fix (see issue 8130 and issue 5735).

In the mean time, add 'unsafe-eval' to your CSP, it's not that bad according to web
security expert Adam Barth (http://www.youtube.com/watch?v=pocsv39pNXA). You could
even have your server add 'unsafe-eval' to the CSP only in the presence of ?gwt.codesvr
in the URL.

SuperDevMode might also be an alternative as I don't think it uses eval() at all.

@Ray, Brian & Matthew: do we want to support 'unsafe-eval'-less CSP with DevMode? Also,
should we create more specific issues? and/or maybe add a CSP or Security label to
the issues I linked above?

Reported by t.broyer on 2013-06-19 09:39:57

dankurka commented 9 years ago
I'd be happy if we can get DevMode to work with CSP, but I don't think it's a must-have
feature.  If you're already using DevMode, CSP probably isn't beneficial to you.  (I
can be convinced otherwise here if someone makes a compelling argument.)

A "Security" label seems appropriate to me.

Reported by mdempsky@google.com on 2013-06-19 20:47:37

dankurka commented 9 years ago
Someone who wants CSP in production might also want to use dev mode for development.
But they might have to be mutually exclusive, at least at stricter settings, meaning
you can't use dev mode with your production server, but you can set a flag when setting
up a dev or QA server.

That makes the dev mode environment different from production which tends to be a pain
if you want to debug issues that happen only in production. For example, what happens
if there's a bug that happens due to the CSP?

(There are similar issues with https and Super Dev Mode since it's http only.)

Reported by skybrian@google.com on 2013-06-19 21:54:23

dankurka commented 9 years ago

Reported by t.broyer on 2013-06-20 09:00:27

dankurka commented 9 years ago
I was playing with GWT+CSP again this morning and rediscovered this bug.  So currently
in a lot of places we use

    <iframe src="javascript:''"></iframe>

to create a blank iframe, but CSP doesn't like this because it's basically an inline
JavaScript snippet.

I think we should be able to change everything to src="about:blank" and still work
and make CSP happy.  According to Wikipedia (http://en.wikipedia.org/wiki/About_URI_scheme),
about:blank is supported in every major browser and IE at least as far back as IE6.
 Beyond that, I think we'll need to look at how the linker startup scripts actually
install the fragment code.

E.g., the xsiframe linker works by loading a .cache.js file that looks like:

    installCode("...actual code...");

into the main frame, which creates a <script> tag with "...actual code..." as the text
content, and then inserts it into the iframe's DOM.

What we could probably do to be CSP compliant is to change the .cache.js file to look
like:

    beforeInstallCode();
    ...actual code...
    afterInstallCode();

and we load the .cache.js file directly into the iframe DOM instead of the main frame.
 Also, before loading the code, we can use cross-frame scripting to setup the beforeInstallCode()
and afterInstallCode() functions.

Reported by mdempsky@google.com on 2013-09-18 20:23:57

dankurka commented 9 years ago
Oh, so that's actually what the direct_install linker already does.  If I change all
the javascript:'' URLs to about:empty and use direct_install instead of xsiframe, I'm
able to load a simple skeleton app.

Next issue trying to use GWT-RPC is that this line from XMLHttpRequest.java:167 triggers
an unsafe-eval warning:

      self.onreadystatechange = new Function();

Changing this from "new Function()" to "function() {}" seems to work.

And now I'm stuck at "Server Error : com.google.gwt.core.client.JavaScriptException:
(EvalError) __gwt$exception: <skipped>: Refused to evaluate a string as JavaScript
because 'unsafe-eval' is not an allowed source of script in the following Content Security
Policy directive: "script-src 'self'"."

I'm guessing somewhere in GWT-RPC we're calling eval(), and I'll need to change it
to JSON.parse().

Reported by mdempsky@google.com on 2013-09-18 20:57:25

dankurka commented 9 years ago
And lastly, if I change

  private static native JavaScriptObject eval(String encoded) /*-{
    return eval(encoded);
  }-*/;

to

  private static native JavaScriptObject eval(String encoded) /*-{
    return JSON.parse(encoded);
  }-*/;

in user/super/com/google/gwt/user/translatable/com/google/gwt/user/client/rpc/impl/ClientSerializationStreamReader.java,
then I'm able to decode (at least simple) GWT-RPC responses with a strict CSP policy.

I seem to recall though that the GWT-RPC protocol isn't strictly JSON, so this isn't
necessarily a valid change to make.

Reported by mdempsky@google.com on 2013-09-18 21:34:39

dankurka commented 9 years ago
This patch is relevant for converting GWT-RPC to use JSON.parse(): https://gwt-review.googlesource.com/#/c/2900/
 (Proposed for IE9 compatibility, but with the side benefit of improving CSP compatibility.)

Reported by mdempsky@google.com on 2013-09-18 21:38:21

dankurka commented 9 years ago
@mdempsky @ #7 - IIRC, there are two places where the RPC payload isn't JSON:  a) the
dead direct-eval RPC that BobV worked on;  b) the payload gets split into multiple
pieces when it is really large and includes actually JS code 

My memory here is getting increasingly dated, so this may not apply any more.

Reported by jat@jaet.org on 2014-02-27 00:10:02

akbertram commented 8 years ago

Has this been released as part of 2.7.0? According to the linked code review the patch seems to have been merged?

akbertram commented 8 years ago

Oh actually this look like it has been imported from much earlier work. Is there any reason it is still open?

tbroyer commented 8 years ago

The linked review has been merged and is in 2.8.0-beta1 (but not 2.7.0). There are still a few widgets making use of src="javascript:''" though (namely FormPanel, NamedFrame, and RichTextArea –in IE8/9/10 only though), or href="javascript:''" or similar (namely CellTree –for its “load more” link'y false-button–, Anchor –if its href is left empty–, and DisclosurePanel). I'm hesitant to close this issue given those, and the fact you need to use the direct_install linker instead of the (now-default) xsiframe linker. @dankurka ?

tbroyer commented 7 years ago

FormPanel just got updated to use href="about:blank": https://gwt-review.googlesource.com/17740

nandakishoremutyala commented 7 years ago

Do we have CSP support for GWT in latest version (2.7/2.8). Please confirm?

tbroyer commented 7 years ago

@nandakishoremutyala Depends what you mean by "CSP support" (depends what you put into your CSP). Please read above comments; there are still instances of javascript:'' in the code, including the xsiframe linker (but not in the direct_install linker), so that depends what you're using and what you put in your CSP.

tbroyer commented 7 years ago

https://gwt-review.googlesource.com/c/gwt/+/19620

acrestan commented 6 years ago

This issue should not be closed as even version 2.8.2 still uses 'eval' javascript call. (ie ClientSerializationStreamReader) This forces the use of CSP directive 'unsafe-eval'... which is not very good.

GWT_CSP.txt

Regards,

niloc132 commented 6 years ago

Good catch. Here is the code that invokes that eval, and only does so if the rpc stream format version is less than 8, the current version:

/*
 * Need to read the version from the string since version is later set in
 * super.prepareToRead() from the evaluated payload but that is too late.
 */
if (readVersion(encoded) < SERIALIZATION_STREAM_JSON_VERSION) {
  // Versions prior to 8 uses almost JSON with Javascript is special cases; e.g., using ].concat([,
  // non-stringified NaN/Infinity or ''+'' concatenated strings.
  results = eval(encoded);
} else {
  results = JsonUtils.safeEval(encoded);
}

On the server, that version is set only if some other part of the stream required an old format:

/**
 * Notice that the field are written in reverse order that the client can just
 * pop items out of the stream.
 */
private void writeHeader(LengthConstrainedArray stream) {
  stream.addToken(getFlags());
  if (stream.isJavaScript() && getVersion() >= SERIALIZATION_STREAM_JSON_VERSION) {
    // Ensure we are not using the JSON supported version if stream is Javascript instead of JSON
    stream.addToken(SERIALIZATION_STREAM_JSON_VERSION - 1);
  } else {
    stream.addToken(getVersion());
  }
}

This is set in two cases - when strings are longer than 64kb (to correctly deal with a hosted mode issue), and if more than 32k values are in the stream (to deal with a bug in IE6/7). I think we can probably safely remove that ie6/7 support (and when I do so, I will test with IE8 to ensure we are correctly compatible).

Hosted mode is a different story: from my (very quick) read of the legacy dev mode version of the ClientSerializationStreamReader, it actually doesn't need that 64kb string limitation any more, and in fact strips out the + tokens automatically before parsing the string:

@Override
public void prepareToRead(String encoded) throws SerializationException {
  try {
    List<JsStatement> stmts = JsParser.parse(SourceOrigin.UNKNOWN, JsRootScope.INSTANCE,
        new StringReader(encoded));
    ArrayConcatEvaler arrayConcatEvaler = new ArrayConcatEvaler();
    arrayConcatEvaler.acceptList(stmts);
    StringConcatEvaler stringConcatEvaler = new StringConcatEvaler();
    stringConcatEvaler.acceptList(stmts);

I'm going to split this off into its own issue for its own fix/test/verification, since this current one was already closed for 2.8.2, and this is the only case remaining (either huge payloads with more than 32k values or object references, or very large strings) case. If we see more issues, perhaps we'll need to reevaluate.