Closed Bug 1500061 Opened 6 years ago Closed 5 years ago

TIghten CSP for activity stream

Categories

(Firefox :: Messaging System, enhancement, P1)

Product:
Firefox
Component:
Messaging System
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 68
Iteration:
68.3 - Apr 15 - 28
Tracking Flags:
Tracking Status
firefox68 --- fixed

People

(Reporter: ckerschb, Assigned: k88hudson)

References

(Blocks 1 open bug)

Details

(Keywords: github-merged)

Attachments

(3 files, 2 obsolete files)

Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4778
(deleted), text/x-github-pull-request
freddy
: feedback+
Details
Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4847
(deleted), text/x-github-pull-request
Details
Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4916
(deleted), text/x-github-pull-request
Details 
Within Bug 1448359 [1] we discussed tightening the CSP for activity stream to remove the 'unsafe-inline' bits [2].

Filing this bug to see if we might be able to lock things down gradually.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1448359#c9
[2] https://searchfox.org/mozilla-central/source/browser/components/newtab/bin/render-activity-stream-html.js#100
Eventually we wanna turn the assertion within AssertAboutPageHasCSP() to also cover that a CSP does not include 'unsafe-inline'.
Blocks: 1448359
Iteration: --- → 65.2 (Nov 16)
Priority: -- → P2
Iteration: 65.2 (Nov 16) → 65.3 (Nov 30)
Iteration: 65.3 (Nov 30) → 66.1 - Dec 10-23
Iteration: 66.1 - Dec 10-23 → 67.1 - Jan 28 - Feb 10
Blocks: 1513279
Assignee: nobody → khudson
Priority: P2 → P1

  

  



    
  

        Attachment #9042574 -
        Flags: review?(fbraun)

    
    

    
  
Drive by note: Removing 'unsafe-inline' from script-src is definitely the way to go and I am really happy to see that happening. Ultimately it would be great if we could also remove unsafe-inline from style-src and load all stylesheets from a file instead of inlining it - but we could do that in a follow up.


For this revision however - would it be possible to add object-src 'none'?



  

  



    
    

    
  
Comment on attachment 9042574 [details]
Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4778

@Christoph: I think unsafe-inline style is required due to bug 1513311
+1 to adding `object-src 'none'` as I'm sure we do not want to add flash resources :-)

I also commented on GitHub:
> Looks good! +1
> The adjacent `connect-src` surprises me a bit. Do you actually use fetch, XHR, EventSource, sendBeacon, `<a ping>` or Websockets?
> To clarify, I found it just surprising. Not concerning :)

  

  



        Attachment #9042574 -
        Flags: review?(fbraun) → feedback+

    
    

    
  
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #3)




Ultimately it would be great if we could also remove unsafe-inline from style-src and load all stylesheets from a file instead of inlining it - but we could do that in a follow up.




Bug 1513311 uses CSSOM to dynamically add a sheet and rules to sanitize css-ish json from the server, so that approach would probably need to change to work without inline styles.



  

  


See Also:  → https://bugzilla.mozilla.org/show_bug.cgi?id=1513311

    
  
Keywords: github-merged

    
  
Blocks: 1527504

    
    

    
  
freddyb, we're running into mochitest failures:


Console message: [JavaScript Error: "Content Security Policy: The page’s settings blocked the loading of a resource at inline (“script-src”)." {file: "about:newtab" line: 16}]

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=228060887&repo=try&lineNumber=1980-1987


Have you seen those before related to testing? My current guess is ContentTask.spawn ends up stringifying a function to send as a message to then eval in the content side. Is there a way to disable CSP when testing?



  

  


Flags: needinfo?(fbraun)

    
    

    
  
Reverted with https://github.com/mozilla/activity-stream/commit/a3fe11f0f32415c98fa7a31b996735b4d580bfb1



  

  


No longer blocks: 1527504
Keywords: github-merged
Iteration: 67.1 - Jan 28 - Feb 10 → 67.3 - Feb 25 - Mar 10

    
    

    
  
(In reply to Ed Lee :Mardak from comment #6)




freddyb, we're running into mochitest failures:


Console message: [JavaScript Error: "Content Security Policy: The page’s settings blocked the loading of a resource at inline (“script-src”)." {file: "about:newtab" line: 16}]

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=228060887&repo=try&lineNumber=1980-1987


Have you seen those before related to testing? My current guess is ContentTask.spawn ends up stringifying a function to send as a message to then eval in the content side. Is there a way to disable CSP when testing?




This error message complains about inline script, so it must be either a <script>...</script> tag or an on.. attribute event handler.


eval related errors get a different message (An attempt to call JavaScript from a string (by calling a function like eval) has been blocked).



  

  


Flags: needinfo?(fbraun)
Iteration: 67.3 - Feb 25 - Mar 10 → 67.4 - Mar 11 - 17

    
    

    
  


  

  

  



    
    

    
  

      
      
      
      

        Attached file
          
        Bug 1500061 - Tighten CSP for Activity Stream (obsolete) (deleted)
        — 
      

    


  

  

  



    
    

    
  


  

  

  



    
  

        Attachment #9050747 -
        Attachment is obsolete: true

    
  
Iteration: 67.4 - Mar 11 - 17 → 68.1 - Mar 18 - 31
Iteration: 68.1 - Mar 18 - 31 → 68.2 - Apr 1 - 14

    
    

    
  


  

  

  


Iteration: 68.2 - Apr 1 - 14 → ---
Priority: P1 → P2

    
  
Blocks: 1546535

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/01c3e1ae707d



  

  


Status: NEW → RESOLVED
Iteration: --- → 68.3 - Apr 15 - 28
Closed: 5 years ago

          status-firefox68:
          --- → fixed
Keywords: github-merged
Priority: P2 → P1
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68

        Attachment #9050774 -
        Attachment is obsolete: true

    
  
Component: Activity Streams: Newtab → Messaging System
Blocks: 1567867

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: