From debbugs-submit-bounces@debbugs.gnu.org Thu Jun 03 17:07:46 2021 Received: (at 42380) by debbugs.gnu.org; 3 Jun 2021 21:07:46 +0000 Received: from localhost ([127.0.0.1]:44859 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1louZ7-0005iB-JY for submit@debbugs.gnu.org; Thu, 03 Jun 2021 17:07:46 -0400 Received: from laurent.telenet-ops.be ([195.130.137.89]:56836) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1louZ4-0005hs-7z for 42380@debbugs.gnu.org; Thu, 03 Jun 2021 17:07:43 -0400 Received: from ptr-bvsjgyjmffd7q9timvx.18120a2.ip6.access.telenet.be ([IPv6:2a02:1811:8c09:9d00:aaf1:9810:a0b8:a55d]) by laurent.telenet-ops.be with bizsmtp id Cl7g2500G0mfAB401l7gYt; Thu, 03 Jun 2021 23:07:41 +0200 Message-ID: Subject: Re: [bug#42380] [PATCH v5 9/9] gnu: Add torbrowser-unbundle. From: Maxime Devos To: =?ISO-8859-1?Q?Andr=E9?= Batista , 42380@debbugs.gnu.org Date: Thu, 03 Jun 2021 23:07:23 +0200 In-Reply-To: <347e91d0ce7f96882f70a081b208c388153cd6b8.1622693271.git.nandre@riseup.net> References: <88c9104dbc255f94f73b0dae2dad41ae23cc3448.1622687706.git.nandre@riseup.net> <347e91d0ce7f96882f70a081b208c388153cd6b8.1622693271.git.nandre@riseup.net> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-jSRqUbLxRYGZMDBwQjuL" User-Agent: Evolution 3.34.2 MIME-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=telenet.be; s=r21; t=1622754461; bh=V86JgEB1d1ygnprnYKFCZfiVwV+Ih7tYzXRLP3U81jE=; h=Subject:From:To:Date:In-Reply-To:References; b=RIF9Xi192qX1hdlGWzScSSdCRMIBRQfbL19O4e1x4ZyIlO+2eWsyANFXzYntxGhdv PW+dGJMx5igm3WOujCpQEJyhCCzUFh1INphCCrVQ83BlLXEp4eVXuZpytrg6eAZUeu Q7tsJI8sN2P4rMNwvxIxfv0/CSt16FKgl3rnR9wPi/X9l1Qn8v5oSzOmZzYDB1hQgL uBWwEf14XfL/UZwNhtgpErzPsNKPoNivulAztmoApw/Vz0QYDz6s36Gld+6zdZXh3F NLaVE/MP+tzOie1NGDyXqfjQQy8zYepRK/VvdNKVVmZyBcg7rF457sHk6hJiYceYJ0 6EP7j6PLyrI2A== X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 42380 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) --=-jSRqUbLxRYGZMDBwQjuL Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Some comments, maybe the have already been addressed previously: > +++ b/gnu/packages/patches/torbrowser-start-desktop.patch > @@ -0,0 +1,22 @@ > +Change TorBrowser desktop file in order for it to be agnostic to the > +path when invoked. > + > +--- torbrowser-68.10.0esr-9.5-1/tbb-scripts/RelativeLink/start-tor-brows= er.desktop.orig 2020-07-05 18:47:40.689484877 -0300 > ++++ torbrowser-68.10.0esr-9.5-1/tbb-scripts/RelativeLink/start-tor-brows= er.desktop 2020-07-24 14:36:37.720579884 -0300 > +@@ -1,4 +1,4 @@ > +-#!/usr/bin/env ./Browser/execdesktop > ++#!/usr/bin/env bash > + # > + # This file is a self-modifying .desktop file that can be run from the = shell. > + # It preserves arguments and environment for the start-tor-browser scri= pt. > +@@ -28,7 +28,7 @@ > + GenericName=3DWeb Browser > + Comment=3DTor Browser is +1 for privacy and =E2=88=921 for mass surveil= lance > + Categories=3DNetwork;WebBrowser;Security; > +-Exec=3Dsh -c '"$(dirname "$*")"/Browser/start-tor-browser --detach || (= [ ! -x "$(dirname "$*")"/Browser/start-tor-browser ] && "$(dirname "$*")"/s= tart-tor-browser --detach)' dummy %k > +-X-TorBrowser-ExecShell=3D./Browser/start-tor-browser --detach > +-Icon=3Dweb-browser > ++Exec=3Dsh -c start-tor-browser > ++X-TorBrowser-ExecShell=3Dstart-tor-browser --detach > ++Icon=3Dtorbrowser What's the reason for switching the icon from web-browser to torbrowser? Also, the guixy way would be to simply replace "$(dirname "$*")/STUFF" by /gnu/store/[...]/MORE-STUF/STUFF. Otherwise, you're assuming "sh" is in the profile. It would also be possible to replace "sh" with (string-append (assoc-ref inputs "bash") "= /bin/sh") I guess. > + StartupWMClass=3DTor Browser > diff --git a/gnu/packages/patches/torbrowser-start-script.patch b/gnu/pac= kages/patches/torbrowser-start-script.patch > new file mode 100644 > index 0000000000..b8c8d9a26a > --- /dev/null > +++ b/gnu/packages/patches/torbrowser-start-script.patch > @@ -0,0 +1,181 @@ > +Change TorBrowser startup script in order for it to setup needed files > +outside guix store. Remove tests which are not needed on guix system. > + > +--- torbrowser-68.10.0esr-9.5-1/tbb-scripts/RelativeLink/start-tor-brows= er.orig 2020-07-05 18:47:40.685485004 -0300 > ++++ torbrowser-68.10.0esr-9.5-1/tbb-scripts/RelativeLink/start-tor-brows= er 2020-07-23 18:13:32.426282743 -0300 > +@@ -5,6 +5,14 @@ > + # > + # Copyright 2017 The Tor Project. See LICENSE for licensing informatio= n. > +=20 > ++TBB_HOME=3D"${HOME}/.local/share/torbrowser" > ++TBB_LOGFILE=3D"${TBB_HOME}/torbrowser.log" > ++TBB_DATA=3D"${TBB_HOME}/Data" > ++TBB_PROFILE=3D"${TBB_DATA}/Browser/profile.default" > ++TBB_STORE_PATH=3D$(dirname $(realpath "$0")) > ++TBB_STORE_DATA=3D"${TBB_STORE_PATH}/TorBrowser/Data" > ++TORRC=3D"${TBB_DATA}/Tor/torrc-defaults" > ++ > + complain_dialog_title=3D"Tor Browser" > +=20 > + # First, make sure DISPLAY is set. If it isn't, we're hosed; scream > +@@ -134,8 +142,8 @@ > + ;; > + -l | --log) > + if [ -z "$2" -o "${2:0:1}" =3D=3D "-" ]; then > +- printf "Logging Tor Browser debug information to tor-brows= er.log\n" > +- logfile=3D"../tor-browser.log" > ++ printf "Logging Tor Browser debug information to torbrowse= r.log\n" Why rename tor-browser.log to torbrowser.log? > + [...] > ++# Try to be agnostic to where we're being started from, check if files = are on its > ++# default paths and chdir to TBB_HOME > ++if [ -e "${TORRC}" ]; then > ++ cd "${TBB_HOME}" > ++else > ++ mkdir -p "${TBB_HOME}" > ++ cp -R "${TBB_STORE_DATA}" "${TBB_HOME}" > ++ chmod -R 700 "${TBB_HOME}" > ++ mkdir -p "${TBB_PROFILE}" > ++ echo "user_pref(\"extensions.torlauncher.torrc-defaults_path\", \"${= TORRC}\");"\ > ++ > "${TBB_PROFILE}/user.js" > ++ echo "ClientTransportPlugin meek_lite,obfs2,obfs3,obfs4,scramblesuit= exec ${TBB_STORE_PATH}/TorBrowser/Tor/PluggableTransports/obfs4proxy"\ > ++ >> "${TORRC}" > ++ cd "${TBB_HOME}" > + fi "mkdir" and "cp" are from coreutils, which are not guaranteed to be in the profile. I'd suggest: (1) (preferred) use substitute* in a build phase to replace 'mkdir' and 'cp' & co with the absolute store path (2) or add coreutils to propagated-inputs Likewise for sed.=20 > [...] >=20 > + if [ "$register_desktop_app" -eq 1 ]; then > + mkdir -p "$HOME/.local/share/applications/" > +- cp ../start-tor-browser.desktop "$HOME/.local/share/applications/" > ++ cp "${TBB_STORE_PATH}/start-tor-browser.desktop" "$HOME/.local/share/a= pplications/" > + update-desktop-database "$HOME/.local/share/applications/" > + printf "Tor Browser has been registered as a desktop app for this user= in ~/.local/share/applications/\n" > + exit 0 Is this required on Guix and would it work well on Guix? Copying .desktop f= iles around seems counter to =E2=80=98Guix suppots transactional upgrades and roll-back= s, [...]. [...] reproducible operating systems=E2=80=99 and not very functional. Shouldn't "guix install torbrowser-unbundle" be sufficient? noscript seems an useful extension for IceCat as well. Maybe move it to gnuzilla.scm? Maybe move https-everywhere there as well? (Separate issue: https-everywhere seems to be bundled in IceCat ...) > +(define https-everywhere-lib-wasm > + (let ((commit "45b1622f1240659aca4762fa336aad1322d6d50f")) > + (package > + (name "https-everywhere-lib-wasm") > + (version "2021.4.15") > + (source > + (origin > + (method git-fetch) > + (uri (git-reference > + (url "https://github.com/EFForg/https-everywhere-lib-wasm= ") > + (commit commit))) > + (file-name (git-file-name name version)) > + (sha256 > + (base32 > + "1lq62rzypdzmnnzvfns3ccvv1g7p7g9s8jx788zzigr3gnmkpffx")))) > + (build-system trivial-build-system) > + (arguments > + `(#:modules ((guix build utils)) > + #:builder (begin > + (use-modules (guix build utils)) > + (format #t "Copying source ...~%") > + (copy-recursively (assoc-ref %build-inputs "source"= ) > + %output > + #:log (%make-void-port "w"))))) Why are you copying the source code to somewhere else? This doesn't seem to accomplish anything. I would suggest something like: ;; Source code of =E2=80=98HTTPS Everywhere WASM Library=E2=80=99, ;; licensed as license:lgpl2.1+, used in 'https-everywhere' ;; as an input. (define https-everywhere-lib-wasm/source-code (origin (method git-fetch) [...])) Note that you can use 'origin' objects in 'inputs' and 'native-inputs'. > + (home-page "https://github.com/EFForg/https-everywhere-lib-wasm") > + (synopsis "Browser extension for protection against known attacks"= ) > + (description "Browser extension that protects users from a range o= f > +known attacks on web browsing activity such as Cross-site scripting, cli= ckjack and > +makes possible for the users to block or choose on a per site basis whic= h remote > +javascript to run while browsing the web.") > + (license license:gpl2+)))) The license file seems to tell something different: LGPL2.1+ instead of GPL= 2+: https://github.com/EFForg/https-everywhere-lib-wasm/blob/master/LICENSE > [...] > + (native-inputs > + `(("https-everywhere" ,https-everywhere) > + ("noscript" ,noscript) noscript and https-everywhere seem more like 'inputs' than 'native-inputs' to me, but IIUC they are source-code only and not compiled, so it doesn't really matter here I guess. > [...] > + (add-after 'unpack 'make-bundle > + (lambda* (#:key inputs native-inputs #:allow-other-keys) > + (let ((tor-launcher (assoc-ref inputs "tor-launcher")) > + (tor-launcher-dir "browser/extensions/tor-launche= r") > + (tbb (assoc-ref inputs "tor-browser-build")) > + (tbb-scripts-dir "tbb-scripts")) > + > + (format #t "Copying tor-launcher ...~%") > + (copy-recursively tor-launcher tor-launcher-dir > + #:log (%make-void-port "w")) > + (format #t "Copying tor-browser-build ...~%") > + (mkdir tbb-scripts-dir) > + (copy-recursively tbb tbb-scripts-dir > + #:log (%make-void-port "w")) > + (make-file-writable "browser/app/profile/000-tor-browser.js") > + (make-file-writable (string-append tbb-scripts-dir > + "/RelativeLink/start-tor-browser")) > + (make-file-writable (string-append tbb-scripts-dir > + "/RelativeLink/start-tor-browser.desktop"))) > + #t)) Returning #t at the end of a phase is not required anymore. The warning will disappear when core-updates is merged. > + (replace 'configure > + (lambda* (#:key inputs outputs configure-flags #:allow-ot= her-keys) > + (let* ((out (assoc-ref outputs "out")) > + (bash (which "bash")) > + (flags `(,(string-append "--prefix=3D" out) > + ,@configure-flags))) > + (setenv "SHELL" bash) > + (setenv "AUTOCONF" (string-append > + (assoc-ref %build-inputs "autocon= f") > + "/bin/autoconf")) In build phases, use 'inputs' or 'native-inputs' instead of %build-inputs. It's more explicit, maybe there are other reasons as well. (Here it should be native-inputs I guess). > + (setenv "CONFIG_SHELL" bash) > + (setenv "PYTHON" (string-append (assoc-ref inputs "py= thon-2") > + "/bin/python")) This most likely should be (assoc-ref (or native-inputs inputs) "python-2") instead of (assoc-ref inputs "python-2"). > + (setenv "CC" "gcc") ; needed when Stylo is enabled This most likely should be ,(cc-for-target) instead of "gcc". (The native-inputs/inputs and "gcc" / (cc-for-target) distinction is import= ant when cross-compiling (though not all dependencies are cross-compilable curr= ently, so it's a bit moot for now.)) > + (add-after 'install-extensions 'link-binaries > + (lambda* (#:key inputs native-inputs outputs #:allow-othe= r-keys) You're not using 'native-inputs' in this build phase so you can remove it from the arguments list. > + [...] > + (add-after 'link-binaries 'copy-bundle-data > + (lambda* (#:key inputs native-inputs outputs #:allow-othe= r-keys) Likewise. > + (description > + "Tor Browser is the Tor Project version of Firefox browser. It i= s the only > +recommended way to anonymously browse the web that is supported by the p= roject. > +It modifies Firefox in order to avoid many know application level attack= s on > +the privacy of Tor users. > + > +WARNING: This is not the official Tor Browser and is currently on testin= g. Use > +at your own risk and please report back on guix channels if you find any > +issues.") This seems unnecessarily scary. All packages in guix are =E2=80=98at your o= wn risk=E2=80=99 and every new package is =E2=80=98in testing=E2=80=99 for a while, whatever= that means. What about "Warning: this is not the official built of Tor Browser from upstream. As such, the Guix version of Tor Browser may have small differences that might allow a malicious actor to identify you as a Guix user." ? Greetings, Maxime. --=-jSRqUbLxRYGZMDBwQjuL Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYLlEjRccbWF4aW1lZGV2 b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7rahAP95zsFdn7ynYLFREsPlHz5Ek7X9 gbKPWsJJu3LI4gs/pwEAjG14OWE9tujVF69MJrFTW1uPdoMY8Zxuj3s4bYEUPA4= =hvTf -----END PGP SIGNATURE----- --=-jSRqUbLxRYGZMDBwQjuL--