[bug#42380] [PATCH] gnu: Add torbrowser.

[André Batista]

Hi Clément,

ter 12 dez 2023 às 12:21:18 (1702394478), clement@lassieur.org enviou:
> 
> Hi, this is a package for Tor Browser.  I initially wanted to base my work on
> André's but I believe pretty much everything is new now.  André's work helped
> nonetheless, so thank you André.
>

Nice to see someone picking that up. Even though there were quite a lot
of changes both on guix and on Tor Browser which made my package
definition mostly obsolete, I should state beforehand that your code is
both clearer and more concise which also means more maintainable than
mine. Moreover the removal of mozilla's store was a most needed
improvement.

However, I do think that there are things which need to be fixed before
commiting this patch. I've made some intersperced comments bellow, both
to you and other reviewers.

First and foremost:

The noscript addon seems to be missing from the browser. If one goes
to the 'about:addons' tab, it is neither listed nor manageable there.
This makes the security slider almost useless and also implies that
as things stand we would lead guixen to run potentialy harmful and
nonfree javascript code unknowingly and without a warning.

You can check that on https://coveryourtracks.eff.org for the
difference between this browser fingerprint and the upstream one.

Other than that, the current recipe is not deterministic. This is
probably due to the 'BuildID' which is a timestamp.

See: (#$output)/lib/torbrowser/platform.ini

Moreover, both upstream torbrowser and guix' icecat build an
internationalized browser with several locales and the browser as is
offers users on startup to change or set the browser locale even though
we did not provide any other than en-US.

I don't think the current en-US only is a show stopper, but let's make
a note on internationalizing it later.

> A few notes:
>  - HTTPS-everywhere extension is now built-in.

In my understading, the extension got removed as the feature it provided
is now part of firefox itself.

>  - The name is "torbrowser" because it's obvious that we don't bundle anything
>    in Guix, that's how other distros do and it's simpler.

What { is || is not } obvious is highly subjective. Maybe to most people
it is obvious that the distro version of some software is not the
upstream one. On the other hand, maybe it's not obvious to many that,
with regards to TorBrowser's goals, this is a significative difference
as it potentialy implies a reduced anonymity set.

'torbrowser-unbundle' was a pun on the original torbrowser name ("Tor
Browser Bundle") and it was intended as some kind of warning to users
that the guix package cannot live up to a vital upstream goal, namely
that all users are using an identical browser in order to avoid, best
as possible, any leak which could be used to fingerprint/deanonymize
users. It was also kind of an homage to upstream directives if you
will.

However, even if some guix users may be unaware, this is an improvement
to the current situation where people use icecat with tor which
undeniably means a reduced anonymity set. Also, the hint may have been
too weak to convey the intended warning. So I won't strongly oppose
naming it simply 'torbrowser' if I'm the only one who sees a point on
doing otherwise.

>
> diff --git a/gnu/packages/browser-extensions.scm 
> b/gnu/packages/browser-extensions.scm
> index 21c519eda31c..9efa94b77396 100644
> --- a/gnu/packages/browser-extensions.scm
> +++ b/gnu/packages/browser-extensions.scm
> @@ -21,6 +21,7 @@
>  (define-module (gnu packages browser-extensions)
>    #:use-module (guix gexp)
>    #:use-module (guix packages)
> +  #:use-module (guix download)
>    #:use-module (guix git-download)
>    #:use-module (guix build-system copy)
>    #:use-module (guix build-system gnu)
> @@ -221,3 +222,28 @@ (define passff
>  
>  (define-public passff/icecat
>    (make-icecat-extension passff))
> +
> +(define noscript
> +  (package
> +    (name "noscript")
> +    (version "11.4.28")
> +    (source (origin
> +              (method url-fetch/zipbomb)
> +              (uri (string-append
> +                    "https://noscript.net/download/releases/noscript-"; 
> version
> +                    ".xpi"))
> +              (sha256
> +               (base32
> +                "051wawi0yjyramp743yjawqaz59g3m2gcivm24b44ibd4arpdl2l"))))
> +    (build-system copy-build-system)
> +    (properties '((addon-id . "{73a6fe31-595d-460b-a920-fcc0f8843232}")))
> +    (arguments
> +     `(#:install-plan '(("." ,(assq-ref properties 'addon-id)))))
> +    (home-page "https://noscript.net";)
> +    (synopsis "Software providing extra protection for various browsers.")
> +    (description "The NoScript Security Suite is a software providing extra
> +protection for web browsers.")
> +    (license license:gpl3+)))
> +
> +(define-public noscript/icecat
> +  (make-icecat-extension noscript))

As I understand it, we are not building noscript from source, but getting
a previously built which has minified JS. I never got to build it from
source and also don't think this makes it uncommitable (agains FSDG), but
maybe we could have a note to re-work this definition later in order to
have it built from source (the guix way!).

...

> diff --git a/gnu/packages/tor.scm b/gnu/packages/tor.scm
> index 71f32b3f4331..31e9945f5d39 100644
> --- a/gnu/packages/tor.scm
> +++ b/gnu/packages/tor.scm

...

> +(define-public torbrowser
> +  (package
> +    (inherit icecat-minimal)
> +    (name "torbrowser")
> +    ;; To find the last version, browse
> +    ;; 
> https://archive.torproject.org/tor-package-archive/torbrowser/<version>
> +    ;; (<version> is the version of the `torbrowser-assets` package).  There
> +    ;; should be only one archive that starts with 
> "src-firefox-tor-browser-".
> +    (version "115.5.0esr-13.0-1-build4")

Is there any reason why you chose to use the 'src' version, instead of
the TorBrowser release version (aka torbroser-assets one). At first I
think it would be better if our version were the same as upstream as
it would be clearer to both users and maintainers which version guix
is offering without installing it.

Besides, are you sure this src version number is guaranteed to be
progressive towards higher numbers?

Decomposing it:

Firefox version  |   tb build ver |   tb build attempt
115.5.0esr       |   13.0-1       |   build4

FF version: always increases, but not necessarily in the same step as
torbrowser releases;

tb build version: usually remains the same throughout a major torbrowser
release series;

tb build attempt: varies with the release process and sometimes it
decreases.


> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri
> +        (string-append
> +         "https://archive.torproject.org/tor-package-archive/torbrowser/";
> +         (package-version torbrowser-assets)
> +         "/src-firefox-tor-browser-" version ".tar.xz"))
> +       (sha256
> +        (base32
> +         "0p0qsfc2l2bicqjr1kxciiij5qz7n8xqyvyn8f13fvk0wyg94c6v"))))
> +    (build-system mozilla-build-system)
> +    (arguments
> +     (substitute-keyword-arguments (package-arguments icecat-minimal)
> +       ((#:configure-flags flags '())
> +        #~(cons*
> +           "--without-relative-data-dir" ;store is read-only

Shouldn't we also set '--with-user-appdir=.torbrowser' ?

There is a comment on 'src/browser/config/mozconfigs/tor-browser' that
says we need to set this flag when the relative data dir is unset.

> +           "--disable-base-browser-update"
> +           "--enable-update-channel=release"

Does this mean that users get notified when there is a new torbrowser
release upstream? Shouldn't this flag be removed?

> +           "--with-branding=browser/branding/tb-release"
> +           (string-append "--prefix=" #$output)
> +           (string-append "--with-base-browser-version="
> +                          #$(package-version
> +                             (this-package-input "torbrowser-assets")))
> +           #$flags))
> +       ((#:phases phases)
> +        #~(modify-phases #$phases
> +            (add-before 'configure 'setenv
> +              (lambda _
> +                (setenv "CONFIG_SHELL" (which "bash"))
> +                ;; Install location is prefix/lib/$MOZ_APP_NAME.  Also
> +                ;; $MOZ_APP_NAME is the executable name.  Default is
> +                ;; "firefox".
> +                (setenv "MOZ_APP_NAME" "torbrowser")
> +                ;; Profile location (relative to "~/.").  Default is
> +                ;; lower($MOZ_APP_VENDOR/$MOZ_APP_BASENAME), which is:
> +                ;; ~/.tor project/firefox.
> +                (setenv "MOZ_APP_PROFILE" "torbrowser/browser")
> +                ;; WM_CLASS (default is "$MOZ_APP_NAME-$MOZ_UPDATE_CHANNEL").

This comment was unclear for me at first, probably due to my own
ignorance. To the benefit of others, this is in line with instructions
on 'src/browser/config/mozconfigs/tor-browser' as a hint to window
managers on GNU/Linux.

> +                (setenv "MOZ_APP_REMOTINGNAME" "Tor Browser")
> +                ;; Persistent state directory for the build system (default 
> is
> +                ;; $HOME/.mozbuild).
> +                (setenv "MOZBUILD_STATE_PATH"
> +                        (in-vicinity (getcwd) ".mozbuild"))))

...

> +                    (lambda ()
> +                      (format #t "// first line must be a comment~%")
> +                      ;; Locking prevents these values being written to
> +                      ;; prefs.js, avoiding Store path capture.
> +                      (format #t "lockPref(~s, ~s);~%"
> +                              "extensions.torlauncher.torrc-defaults_path"
> +                              (in-vicinity
> +                               lib "TorBrowser/Data/Tor/torrc-defaults"))
> +                      (format #t "lockPref(~s, ~s);~%"
> +                              "extensions.torlauncher.tor_path"
> +                              (search-input-file inputs "bin/tor"))

This has the undesired side-effect of making impossible to run TorBrowser
with a shepherd tor instance. Is it really needed?

Besides the inefficiency of running two tor processes, using a single one
has the benefit of making eventual onion service auth keys available both
on the browser and to other user software on the same location.

> +                      ;; Required for Guix packaged extensions
> +                      ;; SCOPE_PROFILE=1, SCOPE_APPLICATION=4, SCOPE_SYSTEM=8
> +                      ;; Default is 5.

...

> +            (replace 'install-desktop-entry
> +              (lambda _
> +                (let ((apps (in-vicinity #$output "share/applications")))
> +                  (mkdir-p apps)
> +                  (make-desktop-entry-file
> +                   (in-vicinity apps "torbrowser.desktop")
> +                   #:name "Tor Browser"
> +                   #:exec
> +                   (format #f "~a %u" (in-vicinity #$output 
> "bin/torbrowser"))

Why do away with the 'start-tor-browser.sh'? Part of the logic there is
redundant or not necessary on a system install, but not everything.

> +                   #:comment
> +                   "Tor Browser is +1 for privacy and -1 for mass 
> surveillance"
> +                   #:categories '("Network" "WebBrowser" "Security")
> +                   #:startup-w-m-class "Tor Browser"
> +                   #:icon "tor-browser"))))
> +            (replace 'install-icons
> +              (lambda* (#:key inputs #:allow-other-keys)
> +                (for-each
> +                 (lambda (size)
> +                   (let ((oldpath (string-append
> +                                   "browser/branding/tb-release/default"
> +                                   size ".png"))
> +                         (newpath (string-append #$output
> +                                                 "/share/icons/hicolor/"
> +                                                 size "x" size "/apps")))
> +                     (mkdir-p newpath)
> +                     (copy-file oldpath
> +                                (in-vicinity newpath "tor-browser.png"))))
> +                 '("16" "22" "24" "32" "48" "64" "128" "256"))))))))
> +    (inputs
> +     (modify-inputs (package-inputs icecat-minimal)
> +       (append bash-minimal
> +               tor

Why not tor-client instead? I don't see a legitimate use case of running
relays on the torbrowser.

Also, shouldn't this be a propagated input so as to not be garbage
collected?

> +               torbrowser-assets)))
> +    (propagated-inputs
> +     (list noscript/icecat))

This appears to be insufficient. See comments above.

Thanks for your work on guix and cheers!