Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Use Ajax to send files in receive mode to workaround browser bug with large files #901

Closed
wants to merge 3 commits into from

Conversation

mig5
Copy link
Collaborator

@mig5 mig5 commented Feb 13, 2019

No description provided.

@mig5 mig5 requested a review from micahflee as a code owner February 13, 2019 01:50
@mig5
Copy link
Collaborator Author

mig5 commented Feb 13, 2019

I couldn't get the flashes to appear properly after all. So I leave this as a simpler PoC, at least for you to test that it indeed fixes the upload issue. More work will need to be done

@micahflee
Copy link
Collaborator

screenshot from 2019-02-13 08-42-30

It looks like the Content-Security-Policy is blocking loading the javascript file.

@micahflee
Copy link
Collaborator

I'm actually quite confused why the script is being blocked though. Here are the errors in the Tor Browser console:

Loading failed for the <script> with source “http://cqy7hro5qffhhdjyjtory7ut36why4yrvsup2rekrd6jzcjcpcgbqvid.onion/static/js/receive.js”.
Content Security Policy: The page’s settings blocked the loading of a resource at http://cqy7hro5qffhhdjyjtory7ut36why4yrvsup2rekrd6jzcjcpcgbqvid.onion/static/js/receive.js (“script-src”).

The header looks like this:

Content-Security-Policy: default-src 'self'; style-src 'self'; script-src 'self'; img-src 'self' data:;

And the script being loaded is indeed getting loaded from 'self':

<script src="/static/js/receive.js"></script>

@micahflee
Copy link
Collaborator

The problem was the Tor Browser security slider set to Safer, I think. One of the things it does is "JavaScript is disabled on non-HTTPS sites", and it appears that HTTP sites that are .onions aren't an exception. If I set the security slider to Standard, it works.

@micahflee
Copy link
Collaborator

But nice, I confirmed that uploading via Ajax does indeed resolve #899.

@micahflee
Copy link
Collaborator

How does this warning look? The text is:

Warning: Due to a bug in Tor Browser and Firefox, if you want to upload a file bigger than 50mb, you must set the Tor Browser security slider to Standard. Otherwise your upload will never finish. If you plan on only uploading small files, your current settings are fine.

screenshot from 2019-02-13 09-15-27

@micahflee
Copy link
Collaborator

Ok I've got to work on other stuff now, but FYI my work in progress is in my mig5-send_file_ajax branch. I'll make a PR into your branch when it's closer to being complete.

@holantonela-zz
Copy link

Sad about this situation. I hope we can have more flexibility per tab in the Tor Browser (and make Tor Browser smarter and friendlier for .onion sites), so users don't need to sacrifice the global security level, but enable/disable features temporarily. That said, a link to open Security Slider/Settings could be cool in that line.

@holantonela-zz
Copy link

Also, we have https://trac.torproject.org/projects/tor/ticket/27610 to fix this.

@micahflee
Copy link
Collaborator

I know, me too @holantonela. Per-tab security sliders would be great.

After talking in the keybase chat with @mig5 about how sometimes this bug may still be triggered on smaller uploads, and also after reviewing SecureDrop's current plans for UX around messaging this bug, I changed the text to:

Warning: Due to a bug in Tor Browser and Firefox, uploads sometimes never finish. To upload reliably, either set your Tor Browser security slider to Standard or turn off your Tor Browser's NoScript XSS setting.

screenshot from 2019-02-14 09-23-34

The "security slider" link goes to https://tb-manual.torproject.org/en-US/security-slider/, and the link includes the noreferrer attribute, to avoid leaking the OnionShare address in the referrer header if someone clicks it:

<a rel="noreferrer" target="_blank" href="https://tb-manual.torproject.org/en-US/security-slider/">security slider</a>

The "turn off your Tor Browser's NoScript XSS setting" links to a new static page that I've added to the OnionShare web server (/noscript-xss-instructions, it exists in share and receive mode, and at the same path whether or public mode is enabled or disabled):

screenshot from 2019-02-14 09-23-38

The complete text is:

Disable your Tor Browser's NoScript XSS setting

If your security slider is set to Safest, JavaScript is disabled so XSS vulnerabilities won't affect you, which makes it safe to disable NoScript's XSS protections.

Here is how to disable this setting:

  1. Click the menu icon in the top-right of Tor Browser and open "Add-ons"
  2. Next to the NoScript add-on, click the "Preferences" button
  3. Switch to the "Advanced" tab
  4. Uncheck "Sanitize cross-site suspicious requests"

If you'd like to learn technical details about this issue, check this issue on GitHub.

The link there again uses the noreferrer header.

@micahflee
Copy link
Collaborator

Also, we have https://trac.torproject.org/projects/tor/ticket/27610 to fix this.

Nice. I also just opened a related issue specifically about the <noscript> tag and the security slider set to Safer: https://trac.torproject.org/projects/tor/ticket/29506

@holantonela-zz
Copy link

It's cool that you are sharing patterns. In the short future (TB8.5), you will be able to link directly to about:preferences#security, so users can change the security setting there and also that bug should be solved.
The relevant ticket for the security settings work: https://trac.torproject.org/projects/tor/ticket/25658

@holantonela-zz
Copy link

Also, we have https://trac.torproject.org/projects/tor/ticket/27610 to fix this.

Nice. I also just opened a related issue specifically about the <noscript> tag and the security slider set to Safer: https://trac.torproject.org/projects/tor/ticket/29506

yes, thanks. Tagged it.

@micahflee
Copy link
Collaborator

I'm closing this PR in favor of #904, so that way my code can get reviewed before merged.

@micahflee micahflee closed this Feb 16, 2019
@mig5 mig5 deleted the send_file_ajax branch May 7, 2019 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants