Bug 1026520 - CSP: Inline report sending into allows - callsite updates (r=dveditz)
authorChristoph Kerschbaumer <mozilla@christophkerschbaumer.com>
Thu, 17 Sep 2015 22:34:34 -0700
changeset 295794 19eb4586448f4a9c5272a057218f63956ac8b8ca
parent 295793 8f860449c1a96be345671ca3b7b92beff4fa7511
child 295795 778c2ddda4439667680bb08ac29a6e7aba63b12c
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdveditz
bugs1026520
milestone43.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1026520 - CSP: Inline report sending into allows - callsite updates (r=dveditz)
dom/base/nsScriptLoader.cpp
dom/events/EventListenerManager.cpp
dom/jsurl/nsJSProtocolHandler.cpp
layout/style/nsStyleUtil.cpp
--- a/dom/base/nsScriptLoader.cpp
+++ b/dom/base/nsScriptLoader.cpp
@@ -431,109 +431,31 @@ CSPAllowsInlineScript(nsIScriptElement *
   nsresult rv = aDocument->NodePrincipal()->GetCsp(getter_AddRefs(csp));
   NS_ENSURE_SUCCESS(rv, false);
 
   if (!csp) {
     // no CSP --> allow
     return true;
   }
 
-  // An inline script can be allowed because all inline scripts are allowed,
-  // or else because it is whitelisted by a nonce-source or hash-source. This
-  // is a logical OR between whitelisting methods, so the allowInlineScript
-  // outparam can be reused for each check as long as we stop checking as soon
-  // as it is set to true. This also optimizes performance by avoiding the
-  // overhead of unnecessary checks.
-  bool allowInlineScript = true;
-  nsAutoTArray<unsigned short, 3> violations;
-
-  bool reportInlineViolation = false;
-  rv = csp->GetAllowsInlineScript(&reportInlineViolation, &allowInlineScript);
-  NS_ENSURE_SUCCESS(rv, false);
-  if (reportInlineViolation) {
-    violations.AppendElement(static_cast<unsigned short>(
-          nsIContentSecurityPolicy::VIOLATION_TYPE_INLINE_SCRIPT));
-  }
-
+  // query the nonce
+  nsCOMPtr<nsIContent> scriptContent = do_QueryInterface(aElement);
   nsAutoString nonce;
-  if (!allowInlineScript) {
-    nsCOMPtr<nsIContent> scriptContent = do_QueryInterface(aElement);
-    bool foundNonce = scriptContent->GetAttr(kNameSpaceID_None,
-                                             nsGkAtoms::nonce, nonce);
-    if (foundNonce) {
-      bool reportNonceViolation;
-      rv = csp->GetAllowsNonce(nonce, nsIContentPolicy::TYPE_SCRIPT,
-                               &reportNonceViolation, &allowInlineScript);
-      NS_ENSURE_SUCCESS(rv, false);
-      if (reportNonceViolation) {
-        violations.AppendElement(static_cast<unsigned short>(
-              nsIContentSecurityPolicy::VIOLATION_TYPE_NONCE_SCRIPT));
-      }
-    }
-  }
-
-  if (!allowInlineScript) {
-    bool reportHashViolation;
-    nsAutoString scriptText;
-    aElement->GetScriptText(scriptText);
-    rv = csp->GetAllowsHash(scriptText, nsIContentPolicy::TYPE_SCRIPT,
-                            &reportHashViolation, &allowInlineScript);
-    NS_ENSURE_SUCCESS(rv, false);
-    if (reportHashViolation) {
-      violations.AppendElement(static_cast<unsigned short>(
-            nsIContentSecurityPolicy::VIOLATION_TYPE_HASH_SCRIPT));
-    }
-  }
+  scriptContent->GetAttr(kNameSpaceID_None, nsGkAtoms::nonce, nonce);
 
-  // What violation(s) should be reported?
-  //
-  // 1. If the script tag has a nonce attribute, and the nonce does not match
-  // the policy, report VIOLATION_TYPE_NONCE_SCRIPT.
-  // 2. If the policy has at least one hash-source, and the hashed contents of
-  // the script tag did not match any of them, report VIOLATION_TYPE_HASH_SCRIPT
-  // 3. Otherwise, report VIOLATION_TYPE_INLINE_SCRIPT if appropriate.
-  //
-  // 1 and 2 may occur together, 3 should only occur by itself. Naturally,
-  // every VIOLATION_TYPE_NONCE_SCRIPT and VIOLATION_TYPE_HASH_SCRIPT are also
-  // VIOLATION_TYPE_INLINE_SCRIPT, but reporting the
-  // VIOLATION_TYPE_INLINE_SCRIPT is redundant and does not help the developer.
-  if (!violations.IsEmpty()) {
-    MOZ_ASSERT(violations[0] == nsIContentSecurityPolicy::VIOLATION_TYPE_INLINE_SCRIPT,
-               "How did we get any violations without an initial inline script violation?");
-    // gather information to log with violation report
-    nsIURI* uri = aDocument->GetDocumentURI();
-    nsAutoCString asciiSpec;
-    uri->GetAsciiSpec(asciiSpec);
-    nsAutoString scriptText;
-    aElement->GetScriptText(scriptText);
-    nsAutoString scriptSample(scriptText);
+  // query the scripttext
+  nsAutoString scriptText;
+  aElement->GetScriptText(scriptText);
 
-    // cap the length of the script sample at 40 chars
-    if (scriptSample.Length() > 40) {
-      scriptSample.Truncate(40);
-      scriptSample.AppendLiteral("...");
-    }
-
-    for (uint32_t i = 0; i < violations.Length(); i++) {
-      // Skip reporting the redundant inline script violation if there are
-      // other (nonce and/or hash violations) as well.
-      if (i > 0 || violations.Length() == 1) {
-        csp->LogViolationDetails(violations[i], NS_ConvertUTF8toUTF16(asciiSpec),
-                                 scriptSample, aElement->GetScriptLineNumber(),
-                                 nonce, scriptText);
-      }
-    }
-  }
-
-  if (!allowInlineScript) {
-    NS_ASSERTION(!violations.IsEmpty(),
-        "CSP blocked inline script but is not reporting a violation");
-   return false;
-  }
-  return true;
+  bool allowInlineScript = false;
+  rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_SCRIPT,
+                            nonce, scriptText,
+                            aElement->GetScriptLineNumber(),
+                            &allowInlineScript);
+  return allowInlineScript;
 }
 
 bool
 nsScriptLoader::ProcessScriptElement(nsIScriptElement *aElement)
 {
   // We need a document to evaluate scripts.
   NS_ENSURE_TRUE(mDocument, false);
 
--- a/dom/events/EventListenerManager.cpp
+++ b/dom/events/EventListenerManager.cpp
@@ -717,52 +717,47 @@ EventListenerManager::SetEventHandler(ns
   // 'doc' is fetched above
   if (doc) {
     // Don't allow adding an event listener if the document is sandboxed
     // without 'allow-scripts'.
     if (doc->GetSandboxFlags() & SANDBOXED_SCRIPTS) {
       return NS_ERROR_DOM_SECURITY_ERR;
     }
 
+    // Perform CSP check
     nsCOMPtr<nsIContentSecurityPolicy> csp;
     rv = doc->NodePrincipal()->GetCsp(getter_AddRefs(csp));
     NS_ENSURE_SUCCESS(rv, rv);
 
     if (csp) {
-      bool inlineOK = true;
-      bool reportViolations = false;
-      rv = csp->GetAllowsInlineScript(&reportViolations, &inlineOK);
+      // let's generate a script sample and pass it as aContent,
+      // it will not match the hash, but allows us to pass
+      // the script sample in aCOntent.
+      nsAutoString scriptSample, attr, tagName(NS_LITERAL_STRING("UNKNOWN"));
+      aName->ToString(attr);
+      nsCOMPtr<nsIDOMNode> domNode(do_QueryInterface(mTarget));
+      if (domNode) {
+        domNode->GetNodeName(tagName);
+      }
+      // build a "script sample" based on what we know about this element
+      scriptSample.Assign(attr);
+      scriptSample.AppendLiteral(" attribute on ");
+      scriptSample.Append(tagName);
+      scriptSample.AppendLiteral(" element");
+
+      bool allowsInlineScript = true;
+      rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_SCRIPT,
+                                EmptyString(), // aNonce
+                                scriptSample,
+                                0,             // aLineNumber
+                                &allowsInlineScript);
       NS_ENSURE_SUCCESS(rv, rv);
 
-      if (reportViolations) {
-        // gather information to log with violation report
-        nsIURI* uri = doc->GetDocumentURI();
-        nsAutoCString asciiSpec;
-        if (uri)
-          uri->GetAsciiSpec(asciiSpec);
-        nsAutoString scriptSample, attr, tagName(NS_LITERAL_STRING("UNKNOWN"));
-        aName->ToString(attr);
-        nsCOMPtr<nsIDOMNode> domNode(do_QueryInterface(mTarget));
-        if (domNode)
-          domNode->GetNodeName(tagName);
-        // build a "script sample" based on what we know about this element
-        scriptSample.Assign(attr);
-        scriptSample.AppendLiteral(" attribute on ");
-        scriptSample.Append(tagName);
-        scriptSample.AppendLiteral(" element");
-        csp->LogViolationDetails(nsIContentSecurityPolicy::VIOLATION_TYPE_INLINE_SCRIPT,
-                                 NS_ConvertUTF8toUTF16(asciiSpec),
-                                 scriptSample,
-                                 0,
-                                 EmptyString(),
-                                 EmptyString());
-      }
-
       // return early if CSP wants us to block inline scripts
-      if (!inlineOK) {
+      if (!allowsInlineScript) {
         return NS_OK;
       }
     }
   }
 
   // This might be the first reference to this language in the global
   // We must init the language before we attempt to fetch its context.
   if (NS_FAILED(global->EnsureScriptEnvironment())) {
--- a/dom/jsurl/nsJSProtocolHandler.cpp
+++ b/dom/jsurl/nsJSProtocolHandler.cpp
@@ -173,37 +173,25 @@ nsresult nsJSThunk::EvaluateScript(nsICh
     nsresult rv;
 
     // CSP check: javascript: URIs disabled unless "inline" scripts are
     // allowed.
     nsCOMPtr<nsIContentSecurityPolicy> csp;
     rv = principal->GetCsp(getter_AddRefs(csp));
     NS_ENSURE_SUCCESS(rv, rv);
     if (csp) {
-        bool allowsInline = true;
-        bool reportViolations = false;
-        rv = csp->GetAllowsInlineScript(&reportViolations, &allowsInline);
-        NS_ENSURE_SUCCESS(rv, rv);
-
-        if (reportViolations) {
-            // gather information to log with violation report
-            nsCOMPtr<nsIURI> uri;
-            principal->GetURI(getter_AddRefs(uri));
-            nsAutoCString asciiSpec;
-            uri->GetAsciiSpec(asciiSpec);
-            csp->LogViolationDetails(nsIContentSecurityPolicy::VIOLATION_TYPE_INLINE_SCRIPT,
-                                     NS_ConvertUTF8toUTF16(asciiSpec),
-                                     NS_ConvertUTF8toUTF16(mURL),
-                                     0,
-                                     EmptyString(),
-                                     EmptyString());
-        }
+        bool allowsInlineScript = true;
+        rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_SCRIPT,
+                                  EmptyString(), // aNonce
+                                  EmptyString(), // aContent
+                                  0,             // aLineNumber
+                                  &allowsInlineScript);
 
         //return early if inline scripts are not allowed
-        if (!allowsInline) {
+        if (!allowsInlineScript) {
           return NS_ERROR_DOM_RETVAL_UNDEFINED;
         }
     }
 
     // Get the global object we should be running on.
     nsIScriptGlobalObject* global = GetGlobalObject(aChannel);
     if (!global) {
         return NS_ERROR_FAILURE;
--- a/layout/style/nsStyleUtil.cpp
+++ b/layout/style/nsStyleUtil.cpp
@@ -674,111 +674,22 @@ nsStyleUtil::CSPAllowsInlineStyle(nsICon
     return false;
   }
 
   if (!csp) {
     // No CSP --> the style is allowed
     return true;
   }
 
-  // An inline style can be allowed because all inline styles are allowed,
-  // or else because it is whitelisted by a nonce-source or hash-source. This
-  // is a logical OR between whitelisting methods, so the allowInlineStyle
-  // outparam can be reused for each check as long as we stop checking as soon
-  // as it is set to true. This also optimizes performance by avoiding the
-  // overhead of unnecessary checks.
-  bool allowInlineStyle = true;
-  nsAutoTArray<unsigned short, 3> violations;
-
-  bool reportInlineViolation;
-  rv = csp->GetAllowsInlineStyle(&reportInlineViolation, &allowInlineStyle);
-  if (NS_FAILED(rv)) {
-    if (aRv)
-      *aRv = rv;
-    return false;
-  }
-  if (reportInlineViolation) {
-    violations.AppendElement(static_cast<unsigned short>(
-          nsIContentSecurityPolicy::VIOLATION_TYPE_INLINE_STYLE));
-  }
-
+  // query the nonce
   nsAutoString nonce;
-  if (!allowInlineStyle) {
-    // We can only find a nonce if aContent is provided
-    bool foundNonce = !!aContent &&
-      aContent->GetAttr(kNameSpaceID_None, nsGkAtoms::nonce, nonce);
-    if (foundNonce) {
-      bool reportNonceViolation;
-      rv = csp->GetAllowsNonce(nonce, nsIContentPolicy::TYPE_STYLESHEET,
-                               &reportNonceViolation, &allowInlineStyle);
-      if (NS_FAILED(rv)) {
-        if (aRv)
-          *aRv = rv;
-        return false;
-      }
-
-      if (reportNonceViolation) {
-        violations.AppendElement(static_cast<unsigned short>(
-              nsIContentSecurityPolicy::VIOLATION_TYPE_NONCE_STYLE));
-      }
-    }
+  if (aContent) {
+    aContent->GetAttr(kNameSpaceID_None, nsGkAtoms::nonce, nonce);
   }
 
-  if (!allowInlineStyle) {
-    bool reportHashViolation;
-    rv = csp->GetAllowsHash(aStyleText, nsIContentPolicy::TYPE_STYLESHEET,
-                            &reportHashViolation, &allowInlineStyle);
-    if (NS_FAILED(rv)) {
-      if (aRv)
-        *aRv = rv;
-      return false;
-    }
-    if (reportHashViolation) {
-      violations.AppendElement(static_cast<unsigned short>(
-            nsIContentSecurityPolicy::VIOLATION_TYPE_HASH_STYLE));
-    }
-  }
+  bool allowInlineStyle = true;
+  rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_STYLESHEET,
+                            nonce, aStyleText, aLineNumber,
+                            &allowInlineStyle);
+  NS_ENSURE_SUCCESS(rv, false);
 
-  // What violation(s) should be reported?
-  //
-  // 1. If the style tag has a nonce attribute, and the nonce does not match
-  // the policy, report VIOLATION_TYPE_NONCE_STYLE.
-  // 2. If the policy has at least one hash-source, and the hashed contents of
-  // the style tag did not match any of them, report VIOLATION_TYPE_HASH_STYLE
-  // 3. Otherwise, report VIOLATION_TYPE_INLINE_STYLE if appropriate.
-  //
-  // 1 and 2 may occur together, 3 should only occur by itself. Naturally,
-  // every VIOLATION_TYPE_NONCE_STYLE and VIOLATION_TYPE_HASH_STYLE are also
-  // VIOLATION_TYPE_INLINE_STYLE, but reporting the
-  // VIOLATION_TYPE_INLINE_STYLE is redundant and does not help the developer.
-  if (!violations.IsEmpty()) {
-    MOZ_ASSERT(violations[0] == nsIContentSecurityPolicy::VIOLATION_TYPE_INLINE_STYLE,
-               "How did we get any violations without an initial inline style violation?");
-    // This inline style is not allowed by CSP, so report the violation
-    nsAutoCString asciiSpec;
-    aSourceURI->GetAsciiSpec(asciiSpec);
-    nsAutoString styleSample(aStyleText);
-
-    // cap the length of the style sample at 40 chars.
-    if (styleSample.Length() > 40) {
-      styleSample.Truncate(40);
-      styleSample.AppendLiteral("...");
-    }
-
-    for (uint32_t i = 0; i < violations.Length(); i++) {
-      // Skip reporting the redundant inline style violation if there are
-      // other (nonce and/or hash violations) as well.
-      if (i > 0 || violations.Length() == 1) {
-        csp->LogViolationDetails(violations[i], NS_ConvertUTF8toUTF16(asciiSpec),
-                                 styleSample, aLineNumber, nonce, aStyleText);
-      }
-    }
-  }
-
-  if (!allowInlineStyle) {
-    NS_ASSERTION(!violations.IsEmpty(),
-        "CSP blocked inline style but is not reporting a violation");
-    // The inline style should be blocked.
-    return false;
-  }
-  // CSP allows inline styles.
-  return true;
+  return allowInlineStyle;
 }