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

firefoxPackages.tor-browser*, tor-browser-bundle: remove #77452

Merged
merged 1 commit into from Jan 10, 2020

Conversation

@flokli
Copy link
Contributor

@flokli flokli commented Jan 10, 2020

These are all based on firefox versions with known vulnerabilities
exploited in the wild.

We seriously shouldn't ship this in nixpkgs, especially not for
sensitive applications as the Tor Browser.

tor-browser-bundle is just a wrapper around
firefoxPackages.tor-browser, so let's remove it too.

tor-browser-bundle-bin is the much safer bet, which is individually
downloaded from dist.torproject.org and just patchelf-ed locally to
work on NixOS.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@flokli flokli requested review from grahamc, andir, vcunat and alyssais Jan 10, 2020
Copy link
Member

@alyssais alyssais left a comment

I do think it would be nice to build TBB ourselves at some point, especially since it’s reproducible, but it’s clearly not something we’re up to right now, so I agree with removing.

@flokli
Copy link
Contributor Author

@flokli flokli commented Jan 10, 2020

It's currently causing more harm than good.

pkgs/top-level/aliases.nix Outdated Show resolved Hide resolved
@flokli flokli force-pushed the flokli:tor-remove-insecure branch from 75ff861 to 9e475d5 Jan 10, 2020
These are all based on firefox versions with known vulnerabilities
exploited in the wild.

We seriously shouldn't ship this in nixpkgs, especially not for
sensitive applications as the Tor Browser.

`tor-browser-bundle` is just a wrapper around
`firefoxPackages.tor-browser`, so let's remove it too.

`tor-browser-bundle-bin` is the much safer bet, which is individually
downloaded from `dist.torproject.org` and just `patchelf`-ed locally to
work on NixOS.

Co-Authored-By: Alyssa Ross <hi@alyssa.is>
Co-Authored-By: Andreas Rammhold <andreas@rammhold.de>
Co-Authored-By: Graham Christensen <graham@grahamc.com>
@flokli flokli force-pushed the flokli:tor-remove-insecure branch from 9e475d5 to 1efaa03 Jan 10, 2020
@flokli flokli merged commit 39f9b46 into NixOS:master Jan 10, 2020
12 checks passed
12 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@flokli flokli deleted the flokli:tor-remove-insecure branch Jan 10, 2020
@flokli flokli mentioned this pull request Jan 10, 2020
0 of 10 tasks complete
@emilazy emilazy mentioned this pull request Jan 10, 2020
5 of 10 tasks complete
@vcunat
Copy link
Member

@vcunat vcunat commented Jan 10, 2020

@oxij might be interested, even if it's merged already.

dtzWill added a commit to dtzWill/nixpkgs that referenced this pull request Jan 10, 2020
firefoxPackages.tor-browser*, tor-browser-bundle: remove

(cherry picked from commit 39f9b46)
@oxij
Copy link
Contributor

@oxij oxij commented Jan 11, 2020

@alyssais
Copy link
Member

@alyssais alyssais commented Jan 11, 2020

@oxij
Copy link
Contributor

@oxij oxij commented Jan 11, 2020

@flokli
Copy link
Contributor Author

@flokli flokli commented Jan 11, 2020

@oxij I don't think there's any pressure to revert immediately. For 19.09, derivations are marked as insecure, and users are directed to tor-browser-bundle-bin.

tor-browser-bundle currently is vulnerable to at least CVE-2019-17026, and based on an unsupported firefox version. I wouldn't advertise it as "the only sane web browser setup there currently is" (because of the vulnerabilities), advise against using it for online browsing, and would feel much more comfortable if people would use tor-browser-bundle-bin.

I appreciate the effort you and the SLNOS community took into packaging tor-browser from source, but at least for nixpkgs, maintaining all the different flavours in the firefox expression has become a huge maintenance burden, especially if we're talking about keeping extra switches for versions already out of support of firefox by itself.
If people really need to use these old unsupported versions for some obscure reasons (like needing support for some never-updated plugins) we should defer to an older nixpkgs channel.

As far as Tor Browser is concerned, I'd also like if we'd be able to provide a self-built Tor Browser. However, apart from the fact this needs to be done with a lot of caution, it should be based on a currently supported firefox version, maybe simply a set of patches on top. As you said by yourself, given the current SLNOS tor-browser maintainer kinda resigned, I don't feel like this should be introduced before these issues have been adressed.

@oxij
Copy link
Contributor

@oxij oxij commented Jan 11, 2020

@flokli
Copy link
Contributor Author

@flokli flokli commented Jan 11, 2020

Yes, this is not only about tor-browser, but about keeping code for unsupported firefox versions in the tree.

@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Feb 5, 2020

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/january-2020-in-nixos/5771/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.