SecurityEngineering/WorkingSessions/03-01-12

From MozillaWiki
Jump to: navigation, search

Random CSP & C++ Questions

Question 1

https://bugzilla.mozilla.org/show_bug.cgi?id=570505 Point 4:

> ::: content/base/src/contentSecurityPolicy.js
> @@ +238,5 @@
> >  
> >      // If there is a policy-uri, fetch the policy, then re-call this function.
> >      // (1) parse and create a CSPRep object
> >      var newpolicy = CSPRep.fromString(aPolicy,
> > +                      selfURI,
> 
> Please put a comment here that says something along the lines of: "we can
> pass the whole URI because when it's parsed as 'self' to construct a CSP Source, only the scheme host
> and port are kept." Just as a friendly reminder.

What Sid is saying here is that their is no reason to strip out parts of the uri. when create csprep; when converted to cspsource everything but the scheme, host, and port get thrown away. before uri->string->uri->source now just uri->source and when we go from uri->source only the scheme, host, and port are kept.

Question 2

https://bugzilla.mozilla.org/show_bug.cgi?id=548984 instead of using javascript:

/* URIs for this protocol execute script when they are opened. */
const unsigned long URI_OPENING_EXECUTES_SCRIPT = 8192
/*  The URIs for this protocol have no inherent security context, so
 documents loaded via this protocol should inherit the security context
 from the document that loads them. */
const unsigned long URI_INHERITS_SECURITY_CONTEXT = 16

Question: What about "chrome:"? That is hardcoded too.

Answer: Ask BZ.

Question 3

https://bugzilla.mozilla.org/show_bug.cgi?id=548984

> 1) We need to allow javascript: in nsIContentPolicy (the way chrome: is
> allowed):
>  if (aURI.schemeIs("javascript")){
>     nsIContentPolicy.ALLOW
>  }
> We can add this code to nsContentUtils.cpp in the URIIsChromeOrInPref(nsIURI *aURI, const char *aPref) function.
> This function is called two times.  Once in IsWhiteListed() in nsScreen.cpp and once in BrowserFrameSecurityCheck() in
> nsGenericHTMLFrameElement.cpp.  Allowing javascript in addition to chrome seems to make sense here.  However, perhaps we should
> change the function name, since javascript is neither Chrome or in the Pref)

I was editing nsContentUtils but that's the the wrong file! Content Policy is not specific to CSP. Should be editing here: http://mxr.mozilla.org/mozilla-central/source/content/base/src/contentSecurityPolicy.js#385 http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsCSPService.cpp ^^ These are the bits of CSP that make decisions about loads.

never touched nsContentUtils when wrote CSP. nsCSPServie content policy that sits and watches the things that load. which calls the javascript function in contentSecurityPolicy #385. Only have to do the check for certain types of URIs, not every URI. URI's that aren't subject to any CSP, we don't have to check. IF there is a CSP on the document, then in line 121, we get the CSP out, if it exists we call the javascript function from CSPService. In the javascript function, we can decide to skip certain types of uris like chrome: or javascript:. I was looking at the infrastructure for content policys, and not for csp. A lot of addons use content policys. they are basically load watchers. They make a decision for everything that is requested. Example: image blocker is a content policy. CSP USES a contentpolicy. So this is an instance of an nsIContentPolicy. (http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentUtils.cpp#690

We can add this code to nsContentUtils.cpp in the URIIsChromeOrInPref(nsIURI *aURI, const char *aPref) function. This function is called two times. Once in IsWhiteListed() in nsScreen.cpp and once in BrowserFrameSecurityCheck() in nsGenericHTMLFrameElement.cpp. Allowing javascript in addition to chrome seems to make sense here. However, perhaps we should change the function name, since javascript is neither Chrome or in the Pref)

Question 4

https://bugzilla.mozilla.org/show_bug.cgi?id=548984 nsJSProtocolHandler EvaluateScript function mxr.mozilla.org/mozilla-central/source/dom/src/jsurl/nsJSProtocolHandler.cpp#171 the check for inline script is already done here; nothing to do.

Question 4

http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentPolicy.cpp#130

   /*
    * There might not be a requestinglocation. This can happen for
    * iframes with an image as src. Get the uri from the dom node.
    * See bug 254510
    */
   if (!requestingLocation) {
       nsCOMPtr<nsIDocument> doc;
       nsCOMPtr<nsIContent> node = do_QueryInterface(requestingContext);
       if (node) {
           doc = node->OwnerDoc();
       }
       if (!doc) {
           doc = do_QueryInterface(requestingContext);
           //TNV: why don't we set doc = node here?
       }
       if (doc) {
           requestingLocation = doc->GetDocumentURI();
       }
   }

requestingContext willalways either be nsIContent or an nsIDocument

Question 6

http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentPolicy.cpp#147

   /* 
    * Enumerate mPolicies and ask each of them, taking the logical AND of
    * their permissions.
    */
   nsresult rv;
   const nsCOMArray<nsIContentPolicy>& entries = mPolicies.GetEntries(); //returns a pointer
   
   PRInt32 count = entries.Count();
   for (PRInt32 i = 0; i < count; i++) {
       /* check the appropriate policy */
       rv = (entries[i]->*policyMethod)(contentType, contentLocation,
                                        requestingLocation, requestingContext,
                                        mimeType, extra, decision);
       //TNV: What does this do?

deference each entry and call the function on those parameters. nscomarray what is the order of operations? dereference first, then the arrow. policyMethod is a pointer to a function name. so *policyMethod gives you the function. policyMethod is a CPMethod (which we can't find in mxr)

Question 7

http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentPolicy.cpp#174

174 //uses the parameters from ShouldXYZ to produce and log a message
175 //logType must be a literal string constant
176 #define LOG_CHECK(logType)                                                    \
177   PR_BEGIN_MACRO                                                              \
178     /* skip all this nonsense if the call failed */                           \
179     if (NS_SUCCEEDED(rv)) {                                                   \
180       const char *resultName;                                                 \
181       if (decision) {                                                         \
182         resultName = NS_CP_ResponseName(*decision);                           \
183       } else {                                                                \
184         resultName = "(null ptr)";                                            \
185       }                                                                       \
186       nsCAutoString spec("None");                                             \
187       if (contentLocation) {                                                  \
188           contentLocation->GetSpec(spec);                                     \
189       }                                                                       \
190       nsCAutoString refSpec("None");                                          \
191       if (requestingLocation) {                                               \
192           requestingLocation->GetSpec(refSpec);                               \
193       }                                                                       \
194       PR_LOG(gConPolLog, PR_LOG_DEBUG,                                        \
195              ("Content Policy: " logType ": <%s> <Ref:%s> result=%s",         \
196               spec.get(), refSpec.get(), resultName)                          \
197              );                                                               \
198     }                                                                         \
199   PR_END_MACRO
200 //TNV: what does spec() and refSpec() do?

They are variable names.

Question 8

http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentPolicy.cpp#207

207 NS_IMETHODIMP
208 nsContentPolicy::ShouldLoad(PRUint32          contentType,
209                             nsIURI           *contentLocation,
210                             nsIURI           *requestingLocation,
211                             nsISupports      *requestingContext,
212                             const nsACString &mimeType,
213                             nsISupports      *extra,
214                             PRInt16          *decision)
215 {
216     // ShouldProcess does not need a content location, but we do
217     NS_PRECONDITION(contentLocation, "Must provide request location");
218     nsresult rv = CheckPolicy(&nsIContentPolicy::ShouldLoad, contentType,
219                               contentLocation, requestingLocation,
220                               requestingContext, mimeType, extra, decision);
221     //TNV: what does the first parameter here mean?
222     LOG_CHECK("ShouldLoad");
223 
224     return rv;
225 }

v table index of the function javascript equivalent checkpolicy("shouldload", ...) in checkpolicy... entries[i][policyMethod] where "shouldload" is policymethod

Question 9

Bug 570505 feedback:

  • conclusion is to change the test cases and only allow string when needed in the code.
  • remove unnecessary else clauses
  • create stringtouri and pass each of these into

NetUtil.newURI Or I can create a global function named _U or something that does this.