So, CSP is pretty cool. Would be handy if the devtools had a way to help figure out what CSP is blocking/allowing.

Even just lumping it into Page Info (*shudder*) would be a fine start. AFAIK this isn't user-visible anywhere yet?

CSP already dumps warnings and errors into the JS/error console (shift-ctrl-j)... and if you set "security.csp.debug" to true, you get a bunch more info there. Those error messages got a little better with the result of bug 600584, but they are what they are and could surely use some design/polish to make them better for devs.

(In reply to Sid Stamm [:geekboy] from comment #2) > CSP already dumps warnings and errors into the JS/error console > (shift-ctrl-j)... and if you set "security.csp.debug" to true, you get a > bunch more info there. Those error messages got a little better with the > result of bug 600584, but they are what they are and could surely use some > design/polish to make them better for devs. At the very least we could show these errors in the web console, if we're not doing it already. Furthermore, it might be a good idea to indicate forbidden content in the inspector (fonts, images) and in the debugger (scripts), assuming there's some way to retrieve this information. A suitable indicator could be a stricken through text, a red font, an exclamation mark or an icon with an explanatory tooltip.

yeah, this seems like a fine place to put this sort of information. Sid, Dolske, do you have any test pages for CSP we could use to check the content of the Web Console?

There are a bunch in the unit (mochi) tests, but there's this one Brandon set up too: http://people.mozilla.org/~bsterne/content-security-policy/demo.cgi

ah, that's cool. With JS messages enabled in the web console we see: [13:10:18.549] call to eval() blocked by CSP @ http://people.mozilla.org/~bsterne/content-security-policy/tests/script/eval-script-test.js:2 Unfortunately there Error Console reports a lot more. We should at least replicate what's in that list. e.g., Timestamp: 2011-12-22 13:12:06 Warning: CSP: 'allow' or 'default-src' directive required but not present. Reverting to "default-src 'none'"

+1 Just spent a while trying to figure out why Firefox was ignoring a script tag (tabzilla js). Would have been extremely helpful if this showed in the web console instead of the only indication being its absence from the list under Net.

Hey Mark, are you still hacking on this?

I am, yes.

This currently only deals with CSP violations (not issues with the policy itself). I'll get around to that, but first I need to resolve bug 766569.

Comment on attachment 634957 [details] [diff] [review] Initial kick at getting CSP violation information in the web developer console. Review of attachment 634957 [details] [diff] [review]: ----------------------------------------------------------------- Looks pretty good to me overall - please create patches with the standard settings in your .hgrc as discussed on irc :) ::: content/base/src/CSPUtils.jsm @@ +51,5 @@ > > }; > > > +function CSPWarning(aMsg, windowID, aSource, aScriptSample, aLineNum) { nit : should be aWindowID like the other args ::: content/base/src/contentSecurityPolicy.js @@ +24,1 @@ > Cu.import("resource://gre/modules/XPCOMUtils.jsm"); curious, why is the gre omitted here (and should there be 3 slashes ? ) @@ +106,5 @@ > > + get windowID() { > + let win = NetworkHelper.getWindowForRequest(this._docRequest); > + let winUtils = win.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils); > + return winUtils.currentInnerWindowID; can getting the window for the request ever fail ? can the QI fail ?

(In reply to Ian Melven :imelven from comment #13) > Looks pretty good to me overall - please create patches with the standard > settings in your .hgrc as discussed on irc :) These should be OK; I checked my .hgrc: [defaults] # ... diff = -p -U 8 qdiff = -p -U 8 > curious, why is the gre omitted here (and should there be 3 slashes ? ) Ah, that's because I was trying to use devtools stuff in core. See new patch. > can getting the window for the request ever fail ? can the QI fail ? Yes. See new patch.

Comment on attachment 635685 [details] [diff] [review] show CSP policy violation messages in the web developer console (take 2) Review of attachment 635685 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me with some minor nits - Mounir will likely have more authoritative suggestions on what the correct style is :) ::: content/base/src/contentSecurityPolicy.js @@ +109,5 @@ > + let loadContext = null; > + if (this._docRequest && this._docRequest.notificationCallbacks) { > + try { > + loadContext = this._docRequest.notificationCallbacks.getInterface(Ci.nsILoadContext); > + } catch (ex) { } i think Mozilla style is try { ... } catch (ex) { } @@ +112,5 @@ > + loadContext = this._docRequest.notificationCallbacks.getInterface(Ci.nsILoadContext); > + } catch (ex) { } > + } else { > + if (this._docRequest && this._docRequest.loadGroup > + && this._docRequest.loadGroup.notificationCallbacks) { if (this._docRequest && this._docRequest.loadGroup && this._docRequest.loadGroup.notificationCallbacks) { @@ +116,5 @@ > + && this._docRequest.loadGroup.notificationCallbacks) { > + try { > + loadContext = this._docRequest.loadGroup.notificationCallbacks.getInterface(Ci.nsILoadContext); > + } catch (ex) { } > + } same style nit as above @@ +122,5 @@ > + if (loadContext) { > + win = loadContext.associatedWindow; > + } > + if (win) { > + try{ missing space after try @@ +125,5 @@ > + if (win) { > + try{ > + let winUtils = win.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils); > + winID = winUtils.currentInnerWindowID; > + } catch (ex) { } as above

drive by nit: The windowID getter could be simplified with an early return if !this._docRequest (and subsequent early returns if !loadContext or !win).

Comment on attachment 635685 [details] [diff] [review] show CSP policy violation messages in the web developer console (take 2) Review of attachment 635685 [details] [diff] [review]: ----------------------------------------------------------------- I'm not familiar with our CSP code. Boris might be.

Comment on attachment 635685 [details] [diff] [review] show CSP policy violation messages in the web developer console (take 2) The code in the windowID getter is wrong: it should fall back to the loadgroup if the getInterface on the notificationCallbacks throws. So it would make more sense like so, imo: try { loadContext = this._docRequest .notificationCallbacks.getInterface(Ci.nsILoadContext); } catch (ex) { try { loadContext = this._docRequest.loadGroup .notificationCallbacks.getInterface(Ci.nsILoadContext); } catch (ex) { } } with no other null-checks. r=me with that change.

Tweaked some formatting (in response to Ian's nits) and added some early returns (in response to Gavin's).

(In reply to Boris Zbarsky (:bz) from comment #19) > Comment on attachment 635685 [details] [diff] [review] > show CSP policy violation messages in the web developer console (take 2) > > The code in the windowID getter is wrong: it should fall back to the > loadgroup if the getInterface on the notificationCallbacks throws. So it > would make more sense like so, imo: > > try { > loadContext = this._docRequest > .notificationCallbacks.getInterface(Ci.nsILoadContext); > } catch (ex) { > try { > loadContext = this._docRequest.loadGroup > > .notificationCallbacks.getInterface(Ci.nsILoadContext); > } catch (ex) { > } > } > > with no other null-checks. r=me with that change. Ah, missed that. I'll sort that out now.

Modified as per Boris' suggestion to fall back to the loadgroup. Also renamed windowID to innerWindowID to better reflect usage elsewhere.

Comment on attachment 636418 [details] [diff] [review] show CSP policy violation messages in the web developer console (take 4) r=me

Comment on attachment 636418 [details] [diff] [review] show CSP policy violation messages in the web developer console (take 4) hg.mozilla.org/try/pushloghtml?changeset=c3baa99944ef - I think we're good to go

Try run for c3baa99944ef is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=c3baa99944ef Results (out of 222 total builds): success: 194 warnings: 27 failure: 1 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mgoodwin@mozilla.com-c3baa99944ef

Pushed to inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/29d7b50bc993

(In reply to Ed Morley [:edmorley] from comment #27) > https://hg.mozilla.org/mozilla-central/rev/29d7b50bc993 Reopening; the above only addresses violation information. I also need to address issues with broken directives and report POST failures.

Can you do that in another bug? Tracking multiple landings in one bug is generally not a good idea.

(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #29) > Can you do that in another bug? Tracking multiple landings in one bug is > generally not a good idea. Done; tracking directive and report failures in bug 770099.