securedrop icon indicating copy to clipboard operation
securedrop copied to clipboard

Move UA checks to server-side code instead of Javascript

Open zenmonkeykstop opened this issue 2 years ago • 8 comments

Description

In a related issue, #1787, @psivesely mentions the possibility of moving the UA checks for non-TBB browsers and the Android versions of Tor Browser to server-side code instead of Javascript. This would make it more likely that users will see them, as some users may already have JS turned off when they visit a SecureDrop instance.

(This has implications for #6318 as well.)

zenmonkeykstop avatar Aug 11 '22 16:08 zenmonkeykstop

This is probably a good task for a new contributor. Currently the user-agent checks are handled client-side in https://github.com/freedomofpress/securedrop/blob/develop/securedrop/static/js/source.js.

Instead, we want to handle it server-side. Probably we could use an app.before_request handler (e.g. https://github.com/freedomofpress/securedrop/blob/develop/securedrop/source_app/init.py#L85) to check the UA and set a variable on whether a warning needs to be shown. And then have logic in the base.html template to actually display the warning.

legoktm avatar Aug 30 '22 03:08 legoktm

Hi @legoktm, Is it okay if I work on this issue?

noumxn avatar Sep 22 '22 16:09 noumxn

@noumxn go for it!

legoktm avatar Sep 23 '22 20:09 legoktm

Hi @legoktm , could you please tell me how I should go about implementing this? I can see the UA checks navigator.userAgent.match in the functions looksLikeTorBrowser() and looksLikeTorBrowserAndriod() use the regex in source.js and the function ready invokes appropriate html code in index.html. But I'm unable to figure out how I should move forward with this.

I also have another doubt regarding the regex for Tor Browser - TBB_UA_REGEX.

const TBB_UA_REGEX = /Mozilla\/5\.0 \((Windows NT 10\.0|X11; Linux x86_64|Macintosh; Intel Mac OS X 10\.[0-9]{2}|Windows NT 10\.0; Win64; x64|Android; Mobile); rv:[0-9]{2,3}\.0\) Gecko\/20100101 Firefox\/([0-9]{2,3})\.0/

It contains Android; Mobile. And I think it shouldn't be there. Or I've just completely misunderstood the whole thing 😅 in which case I'd just appriciate any guidance regarding how I should be going about the changes I need to make.

noumxn avatar Sep 24 '22 10:09 noumxn

I think it would be something we add to the source app's base template - https://github.com/freedomofpress/securedrop/blob/develop/securedrop/source_templates/base.html. The simplest way to pass information to all templates is probably to set it on flask.g, like we do in https://github.com/freedomofpress/securedrop/blob/develop/securedrop/source_app/init.py#L85.

It contains Android; Mobile. And I think it shouldn't be there. Or I've just completely misunderstood the whole thing 😅...

Nope, you're right, good spot! Mobile devices shouldn't match TBB_UA_REGEX, they should only match TB4A_UA_REGEX. So if you want to fix that too... :)

legoktm avatar Sep 26 '22 16:09 legoktm

Hi @legoktm , I am sorry but I will not be able to work on this issue for a week or so. I have currently unassigned myself from this issue incase someone else comes along and is interested. I will be able to get back on this in a week if no one else has started working on this by then.

Nope, you're right, good spot! Mobile devices shouldn't match TBB_UA_REGEX, they should only match TB4A_UA_REGEX. So if you want to fix that too... :)

I did open a PR that fixes this

noumxn avatar Sep 29 '22 16:09 noumxn

All good, thanks for being upfront and the PR. Even if this task is taken by then, I'm sure we can find another thing for you to work on :-)

legoktm avatar Sep 29 '22 20:09 legoktm

Hello! I was looking into working on this issue but had a question:

I see that SecureDrop currently performs UA checks in Javascript by checking navigator.userAgent, timezone, and screen/window size. https://github.com/freedomofpress/securedrop/blob/968b8c14658490c9dd561df2cba041d10e9a3475/securedrop/static/js/source.js#L14-L25

From my understanding, moving the UA checks to the server-side would mean primarily relying on the User-Agent header string for detection since timezone and screen/window size are not sent to the server. The problem with this approach is that this would also match any regular Firefox browser.

The only other way I could think of would be checking whether the user traffic originates from a Tor exit node IP, which would be more complex and require maintaining a list of node IPs.

Any thoughts on how to proceed?

sea-kelp avatar Jan 29 '23 10:01 sea-kelp