freedomofpress / securedrop.org

Code for the SecureDrop project website
https://securedrop.org
GNU Affero General Public License v3.0
40 stars 8 forks source link

"Detect Tor" logic is currently broken #1034

Closed eloquence closed 1 year ago

eloquence commented 1 year ago

As reported by @Thorin-Oakenpants in https://github.com/freedomofpress/securedrop/issues/6795, the logic to detect Tor Browser is currently broken - we're displaying the "You are not anonymous while using this browser!" warning message even to Tor Browser users, using latest Tor Browser. Thorin suggested an alternative detection method here; whichever method we use, we'll likely want to stay in sync between the securedrop and securedrop.org repos.

harrislapiroff commented 1 year ago

@eloquence Is this something you want the Web Team to work on or should we wait for a solution from the SecureDrop team?

SaptakS commented 1 year ago

I like the solution proposed by @Thorin-Oakenpants for detecting Tor Browser, since that resource URL will be present only in Tor Browser. There is a slight risk of Tor Project changing the filename, etc., but that risk can be there even with User Agents. Also, browsers have been discussing locking of any information leak through resource:// (which was mentioned in the original post as well) for quite some time. So I am definitely on board with checking the resource URL but I think maybe let's keep the User Agent check for detecting Tor as well. I wonder if the Tor Project has any official way of detecting whether it's a Tor Browser or not.

As of determining the OS itself, I think the solution mention by @Thorin-Oakenpants is used to check OS in chrome browser? Unless I read the code wrong. I agree that mostly android users would be using Chrome, but there might be firefox users too. In that case maybe we can still continue to use the User Agent information to determine if it's a mobile browser?

If everyone agrees, I can go ahead and create PRs. cc @harrislapiroff @eloquence

Thorin-Oakenpants commented 1 year ago

used to check OS in chrome browser?

nope. the source is chrome:// schema but only applies to gecko


So the logic is

SaptakS commented 1 year ago

the source is chrome:// schema but only applies to gecko

Thanks for clarifying.

Thorin-Oakenpants commented 1 year ago

some pseudo code for you

function get_isLatest() {
    if (CanvasRenderingContext2D.prototype.hasOwnProperty("letterSpacing")) return true // FF115+ / TB13
    if (CanvasRenderingContext2D.prototype.hasOwnProperty("direction")) {
        if (Array(1).includes()) return true // FF102+ / TB12
    }
    return false
}

let isLatest = get_isLatest()

when TB13 is released you would drop the 102+ check and simply return false if not based on at least FF115

Thorin-Oakenpants commented 1 year ago

just hold off for a tiny bit - the TB detection is broken in the first public TB13 alpha (it wasn't in my 2 week old test build), so I'm in the process of resolving that upstream

SaptakS commented 1 year ago

@Thorin-Oakenpants any updates on this?

Thorin-Oakenpants commented 1 year ago

I do have a workaround for TB13 (not sure if it works on android, waiting for a TBA13), but it's not going to last forever, as dtd's are basically phased out. Am looking at TB coding something permanent

https://github.com/arkenfox/TZP/blob/710ce461f9d1451f4fac59ca9ca2353c8839208d/js/generic.js#L359-L360

harrislapiroff commented 1 year ago

Let's go ahead and work on fixing this for current versions of Tor Browser (and get on the same page as SecureDrop) now and keep working on future version support with @Thorin-Oakenpants after that.

Thorin-Oakenpants commented 1 year ago

do you need me to do anything, or can you craft your own code and then give me a ping?

Thorin-Oakenpants commented 1 year ago

uugh .. https://github.com/arkenfox/TZP/commit/bec8f63dcd6fa57c91ca6fcee469ed3fcf595197

so in TB13+

nothing to hold you up here though, just letting know the current fixes/changes as we near TB13