Last Comment Bug 783497 - refinePolicy doesn't respect other directives except default-src
: refinePolicy doesn't respect other directives except default-src
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: 14 Branch
: x86 Linux
: -- normal (vote)
: mozilla17
Assigned To: Sid Stamm [:geekboy or :sstamm]
:
: Andrew Overholt [:overholt]
Mentors:
Depends on: 764937
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-17 02:08 PDT by Kailas
Modified: 2012-08-18 04:27 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
test and patch (9.37 KB, patch)
2012-08-17 14:09 PDT, Sid Stamm [:geekboy or :sstamm]
jst: review+
Details | Diff | Splinter Review

Description Kailas 2012-08-17 02:08:15 PDT
User Agent: Mozilla/5.0 (X11; Linux i686) AppleWebKit/537.1 (KHTML, like Gecko) Chrome/21.0.1180.79 Safari/537.1

Steps to reproduce:

suppose policy_1 sets, default-src 'self' script-src 'self'
policy_2 sets script-src 'self'
If I used 
csp.refinePolicy(policy_1, selfURI);
csp.refinePolicy(policy_2, selfURI);





Actual results:

it sets
 script-src 'none' 



Expected results:

The expected ourcome for script-src should be :
script-src 'self'
Comment 1 Tanvi Vyas [:tanvi] 2012-08-17 02:36:29 PDT
There is a bug in our refinePolicy implementation.
Comment 2 Daniel Veditz [:dveditz] 2012-08-17 08:58:00 PDT
If we're going to fix this we should consider going ahead and implementing the spec's algorithm now rather than having to change things again later. I think we just store the policies separately in a list and then apply each one in order. This will work fine for the content loading policies and get the reporting correct, that is, a violation of policy B should not be sent to the report-uri from policy A.

This gets tricky with the inline/eval restrictions because for perf reasons the value is cached. We will eventually need a way to get the engine to reset its cached value (esp. when we support <meta> policies), although until then we might just be able to check through the policies to find the appropriate report-uris when a violation is signaled.
Comment 3 Sid Stamm [:geekboy or :sstamm] 2012-08-17 09:39:40 PDT
The way I'm understanding the problem here, I don't think this is a bug.  The policy "script-src 'self'" turns into "default-src 'none'" as intended.  Then, when refine-policy happens, everything gets essentially nullified.

The old CSP spec used to require a default-src (er, "allow") directive in *all* policies when they're created.  This was intended to require policy writers to be very specific about their controls.

If we do want to make a change here, it is bug 764937, and should follow with CSP 1.0 conformance and not the default.

Kailas: can you confirm that "script-src 'self'", when parsed (due to makeExplicit()), ends up with all directives set to 'none'?
Comment 4 Kailas 2012-08-17 11:26:43 PDT
Sid: The  "https://csptest.computerist.org" website set following CSP rules:

allow 'self'; img-src 'self'; script-src 'self'; options 'bogus-option'; report-uri https://unknown.computerist.org:8443/report 

If user defined policy is as follows:
default-src 'none' ; script-src 'self' ; 

Then the output of refinePolicy method
"csp.refinePolicy(websiteRules,selfURI); 
csp.refinePolicy(userRules,seflURI); "
is as follows:

default-src 'none'; script-src *; style-src 'none'; media-src 'none'; img-src 'none'; object-src 'none'; frame-src 'none'; frame-ancestors *; font-src 'none'; xhr-src 'none' 

I don't understand, why "script-src *" is set to *. Both website and user CSP policy contains "script-src 'self'". It should be either 'self' or 'none' if default-src is enforced. 

Suppose user policy is as follows:

default-src * ; script-src 'self' 

Then the outcome of refinePolicy method calls on website and user rules is as follows:

default-src https://csptest.computerist.org:443; script-src *; style-src https://csptest.computerist.org:443; media-src https://csptest.computerist.org:443; img-src https://csptest.computerist.org:443; object-src https://csptest.computerist.org:443; frame-src https://csptest.computerist.org:443; frame-ancestors *; font-src https://csptest.computerist.org:443; xhr-src https://csptest.computerist.org:443
Comment 5 Sid Stamm [:geekboy or :sstamm] 2012-08-17 14:07:47 PDT
Ah, yes, that more complete policy makes sense.

Looks like when we intersect two sources we ignore whether they are 'self' or not.  That's the crux of the problem: a source knows if it is self, but intersectWith ignores that.
Comment 6 Sid Stamm [:geekboy or :sstamm] 2012-08-17 14:09:52 PDT
Created attachment 652912 [details] [diff] [review]
test and patch

Hey dveditz: can you review this?  It's pretty trivial -- I just have to tweak the intersection logic to resolve "self" information in the CSPSources.
Comment 7 Sid Stamm [:geekboy or :sstamm] 2012-08-17 15:02:37 PDT
Thanks, jst.  Pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d3bc83c14073
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-08-18 04:27:06 PDT
https://hg.mozilla.org/mozilla-central/rev/d3bc83c14073

Note You need to log in before you can comment on or make changes to this bug.