[tor-commits] [sandboxed-tor-browser/master] Bug 22910: Deprecate the volatile extension dir option.

yawning at torproject.org yawning at torproject.org
Fri Jul 14 17:22:16 UTC 2017


commit 701c0656be16440203147eb0ea6104486bb77096
Author: Yawning Angel <yawning at schwanenlied.me>
Date:   Fri Jul 14 17:18:24 2017 +0000

    Bug 22910: Deprecate the volatile extension dir option.
    
    Foot + gun options are bad, the "Get Addons" pane is force disabled via
    a locked pref by virtue of being Mozilla + Google botnet bullshit, and
    this commit also switches to an extension whitelist.
---
 ChangeLog                                          |  1 +
 README.md                                          |  2 +-
 data/installer/mozilla.cfg                         |  5 +--
 data/ui/gtkui.ui                                   | 42 ++--------------------
 .../internal/sandbox/application.go                | 22 +++++++++---
 .../internal/sandbox/hugbox.go                     |  4 +++
 .../internal/tor/surrogate.go                      | 16 ---------
 .../internal/ui/config/config.go                   | 14 --------
 .../internal/ui/gtk/config.go                      | 25 ++++++-------
 9 files changed, 39 insertions(+), 92 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c281169..2d7ee1a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,5 @@
 Changes in version 0.0.11 - UNRELEASED:
+ * Bug 22910: Deprecate the volatile extension dir option.
 
 Changes in version 0.0.10 - 2017-07-12:
  * Bug 22829: Remove default obfs4 bridge riemann.
diff --git a/README.md b/README.md
index 6cad572..f5013d0 100644
--- a/README.md
+++ b/README.md
@@ -36,7 +36,7 @@ Things that the sandbox breaks:
  * Audio (Unless allowed via the config)
  * DRI
  * X11 input methods (IBus requires access to the host D-Bus)
- * Installing addons via the browser UI (Unless allowed via the config)
+ * Installing addons (Addons are whitelisted)
  * Tor Browser's updater (launcher handles keeping the bundle up to date)
 
 Places where the sandbox could be better:
diff --git a/data/installer/mozilla.cfg b/data/installer/mozilla.cfg
index 3aeed7a..64f0e60 100644
--- a/data/installer/mozilla.cfg
+++ b/data/installer/mozilla.cfg
@@ -26,6 +26,7 @@ lockPref("toolkit.startup.max_resumed_crashes", -1);
 // directive worked as advertised.  But it doesn't appear to actually take.
 // defaultPref("extensions.torbutton.security_slider", 1);
 
-// Disable the `about:addons`->"Get Addons" pane because it is unsafe, and
-// Mozilla gets "telemetery" using Google Analytics.
+// Disable the `about:addons`->"Get Addons" pane because it is unsafe,  Mozilla
+// gets "telemetery" using Google Analytics, and extensions are individually
+// read-only bind mounted in, so it will not work at all.
 lockPref("extensions.getAddons.showPane", false);
diff --git a/data/ui/gtkui.ui b/data/ui/gtkui.ui
index c6cefb7..728fccb 100644
--- a/data/ui/gtkui.ui
+++ b/data/ui/gtkui.ui
@@ -628,42 +628,6 @@
                   </packing>
                 </child>
                 <child>
-                  <object class="GtkBox">
-                    <property name="visible">True</property>
-                    <property name="can_focus">False</property>
-                    <property name="margin_bottom">6</property>
-                    <child>
-                      <object class="GtkLabel">
-                        <property name="visible">True</property>
-                        <property name="can_focus">False</property>
-                        <property name="halign">start</property>
-                        <property name="label" translatable="yes">Modifiable Extensions (UNSAFE: Security, Anonymity)</property>
-                      </object>
-                      <packing>
-                        <property name="expand">True</property>
-                        <property name="fill">True</property>
-                        <property name="position">0</property>
-                      </packing>
-                    </child>
-                    <child>
-                      <object class="GtkSwitch" id="volatileExtensionsSwitch">
-                        <property name="visible">True</property>
-                        <property name="can_focus">True</property>
-                      </object>
-                      <packing>
-                        <property name="expand">False</property>
-                        <property name="fill">True</property>
-                        <property name="position">1</property>
-                      </packing>
-                    </child>
-                  </object>
-                  <packing>
-                    <property name="expand">False</property>
-                    <property name="fill">True</property>
-                    <property name="position">3</property>
-                  </packing>
-                </child>
-                <child>
                   <object class="GtkBox" id="downloadsDirBox">
                     <property name="visible">True</property>
                     <property name="can_focus">False</property>
@@ -698,7 +662,7 @@
                   <packing>
                     <property name="expand">False</property>
                     <property name="fill">True</property>
-                    <property name="position">4</property>
+                    <property name="position">3</property>
                   </packing>
                 </child>
                 <child>
@@ -736,7 +700,7 @@
                   <packing>
                     <property name="expand">False</property>
                     <property name="fill">True</property>
-                    <property name="position">5</property>
+                    <property name="position">4</property>
                   </packing>
                 </child>
                 <child>
@@ -773,7 +737,7 @@
                   <packing>
                     <property name="expand">False</property>
                     <property name="fill">True</property>
-                    <property name="position">6</property>
+                    <property name="position">5</property>
                   </packing>
                 </child>
               </object>
diff --git a/src/cmd/sandboxed-tor-browser/internal/sandbox/application.go b/src/cmd/sandboxed-tor-browser/internal/sandbox/application.go
index 969c6af..4057b21 100644
--- a/src/cmd/sandboxed-tor-browser/internal/sandbox/application.go
+++ b/src/cmd/sandboxed-tor-browser/internal/sandbox/application.go
@@ -102,6 +102,7 @@ func RunTorBrowser(cfg *config.Config, manif *config.Manifest, tor *tor.Tor) (pr
 	realCachesDir := filepath.Join(realBrowserHome, cachesSubDir)
 	realDesktopDir := filepath.Join(realBrowserHome, "Desktop")
 	realDownloadsDir := filepath.Join(realBrowserHome, "Downloads")
+	realExtensionsDir := filepath.Join(realProfileDir, "extensions")
 
 	// Ensure that the `Caches`, `Downloads` and `Desktop` mount points exist.
 	if err = os.MkdirAll(realCachesDir, DirMode); err != nil {
@@ -126,6 +127,7 @@ func RunTorBrowser(cfg *config.Config, manif *config.Manifest, tor *tor.Tor) (pr
 	cachesDir := filepath.Join(browserHome, cachesSubDir)
 	downloadsDir := filepath.Join(browserHome, "Downloads")
 	desktopDir := filepath.Join(browserHome, "Desktop")
+	extensionsDir := filepath.Join(profileDir, "extensions")
 
 	// Filesystem stuff.
 	h.roBind(cfg.BundleInstallDir, filepath.Join(h.homeDir, "sandboxed-tor-browser", "tor-browser"), false)
@@ -135,10 +137,20 @@ func RunTorBrowser(cfg *config.Config, manif *config.Manifest, tor *tor.Tor) (pr
 	h.bind(realCachesDir, cachesDir, false) // XXX: Do I need this?
 	h.roBind(filepath.Join(realProfileDir, "preferences"), filepath.Join(profileDir, "preferences"), false)
 	h.chdir = browserHome
-	if !cfg.Sandbox.VolatileExtensionsDir {
-		// Unless overridden, the extensions directory should be mounted
-		// read-only.
-		h.roBind(filepath.Join(realProfileDir, "extensions"), filepath.Join(profileDir, "extensions"), false)
+
+	// Explicitly bind mount the expected extensions in.
+	//
+	// If the Tor Browser developers ever decide to do something sensible like
+	// sign their XPI files, then the whitelist could be public key based, till
+	// then this may be somewhat fragile.
+	h.tmpfs(extensionsDir)
+	for _, extName := range []string{
+		"{73a6fe31-595d-460b-a920-fcc0f8843232}.xpi", // NoScript
+		"torbutton at torproject.org.xpi",
+		"https-everywhere-eff at eff.org.xpi",
+		"tor-launcher at torproject.org.xpi",
+	} {
+		h.roBind(filepath.Join(realExtensionsDir, extName), filepath.Join(extensionsDir, extName), false)
 	}
 
 	// Env vars taken from start-tor-browser.
@@ -787,7 +799,7 @@ func (h *hugbox) appendRestrictedGtk2(hasAdwaita bool) ([]string, string, error)
 	// and will warn if said subsystem is inaccessible.  As the host D-Bus
 	// is not, and likely will never be accesible from within the container,
 	// attempt to suppress the warnings.
-	h.setenv("NO_AT_BRIDGE", "yes");
+	h.setenv("NO_AT_BRIDGE", "yes")
 
 	return gtkLibs, gtkLibPath, nil
 }
diff --git a/src/cmd/sandboxed-tor-browser/internal/sandbox/hugbox.go b/src/cmd/sandboxed-tor-browser/internal/sandbox/hugbox.go
index dd2e51a..127af77 100644
--- a/src/cmd/sandboxed-tor-browser/internal/sandbox/hugbox.go
+++ b/src/cmd/sandboxed-tor-browser/internal/sandbox/hugbox.go
@@ -157,6 +157,10 @@ func (h *hugbox) assetFile(dest, asset string) {
 	h.file(dest, b)
 }
 
+func (h *hugbox) tmpfs(dest string) {
+	h.args = append(h.args, "--tmpfs", dest)
+}
+
 func (h *hugbox) run() (*Process, error) {
 	// Create the command struct for the sandbox.
 	cmd := &exec.Cmd{
diff --git a/src/cmd/sandboxed-tor-browser/internal/tor/surrogate.go b/src/cmd/sandboxed-tor-browser/internal/tor/surrogate.go
index 1d99431..546e8ab 100644
--- a/src/cmd/sandboxed-tor-browser/internal/tor/surrogate.go
+++ b/src/cmd/sandboxed-tor-browser/internal/tor/surrogate.go
@@ -35,7 +35,6 @@ import (
 
 	"cmd/sandboxed-tor-browser/internal/socks5"
 	"cmd/sandboxed-tor-browser/internal/ui/config"
-	. "cmd/sandboxed-tor-browser/internal/utils"
 )
 
 const (
@@ -140,8 +139,6 @@ type socksProxy struct {
 	sNet, sAddr string
 	tag         string
 
-	allowAboutAddons bool
-
 	l net.Listener
 }
 
@@ -193,18 +190,6 @@ func (p *socksProxy) handleConn(conn net.Conn) {
 		return
 	}
 
-	// Disallow `about:addons` unless the extensions directory is volatile,
-	// because regardless of what Mozilla PR says about respecting privacy,
-	// loading Google Analytics in a page that gets loaded as an IFRAME as
-	// part of an `about:` internal page, is anything but.
-	if host, _ := req.Addr.HostPort(); strings.ToLower(host) == aboutAddonsUnsafeHost {
-		if !p.allowAboutAddons {
-			Debugf("sandbox: Rejecting request to `%s`", aboutAddonsUnsafeHost)
-			req.Reply(socks5.ReplyConnectionNotAllowed)
-			return
-		}
-	}
-
 	// Append our isolation tag.
 	if err := p.rewriteTag(conn, req); err != nil {
 		req.Reply(socks5.ReplyGeneralFailure)
@@ -262,7 +247,6 @@ func launchSocksProxy(cfg *config.Config, tor *Tor) (*socksProxy, error) {
 	if err != nil {
 		return nil, err
 	}
-	p.allowAboutAddons = cfg.Sandbox.VolatileExtensionsDir
 
 	go p.acceptLoop()
 
diff --git a/src/cmd/sandboxed-tor-browser/internal/ui/config/config.go b/src/cmd/sandboxed-tor-browser/internal/ui/config/config.go
index 332c4b5..6ab7749 100644
--- a/src/cmd/sandboxed-tor-browser/internal/ui/config/config.go
+++ b/src/cmd/sandboxed-tor-browser/internal/ui/config/config.go
@@ -200,11 +200,6 @@ type Sandbox struct {
 	// host system DISPLAY from the env var will be used.
 	Display string `json:"display,omitEmpty"`
 
-	// VolatileExtensionsDir mounts the extensions directorey read/write to
-	// allow the installation of addons.  The addon auto-update mechanism is
-	// still left disabled.
-	VolatileExtensionsDir bool `json:"volatileExtensionsDir"`
-
 	// EnablePulseAudio enables access to the host PulseAudio daemon inside the
 	// sandbox.
 	EnablePulseAudio bool `json:"enablePulseAudio"`
@@ -260,15 +255,6 @@ func (sb *Sandbox) SetEnableCircuitDisplay(b bool) {
 	}
 }
 
-// SetVolatileExtensionsDir sets the sandbox extension directory write enable
-// and marks the config dirty.
-func (sb *Sandbox) SetVolatileExtensionsDir(b bool) {
-	if sb.VolatileExtensionsDir != b {
-		sb.VolatileExtensionsDir = b
-		sb.cfg.isDirty = true
-	}
-}
-
 // SetDownloadsDir sets the sandbox `~/Downloads` bind mount source and marks
 // the config dirty.
 func (sb *Sandbox) SetDownloadsDir(s string) {
diff --git a/src/cmd/sandboxed-tor-browser/internal/ui/gtk/config.go b/src/cmd/sandboxed-tor-browser/internal/ui/gtk/config.go
index 36799e8..8a66706 100644
--- a/src/cmd/sandboxed-tor-browser/internal/ui/gtk/config.go
+++ b/src/cmd/sandboxed-tor-browser/internal/ui/gtk/config.go
@@ -60,16 +60,15 @@ type configDialog struct {
 	torSystemIndicator *gtk3.Box
 
 	// Sandbox config elements.
-	pulseAudioSwitch         *gtk3.Switch
-	avCodecSwitch            *gtk3.Switch
-	circuitDisplaySwitch     *gtk3.Switch
-	volatileExtensionsSwitch *gtk3.Switch
-	displayBox               *gtk3.Box
-	displayEntry             *gtk3.Entry
-	downloadsDirBox          *gtk3.Box
-	downloadsDirChooser      *gtk3.FileChooserButton
-	desktopDirBox            *gtk3.Box
-	desktopDirChooser        *gtk3.FileChooserButton
+	pulseAudioSwitch     *gtk3.Switch
+	avCodecSwitch        *gtk3.Switch
+	circuitDisplaySwitch *gtk3.Switch
+	displayBox           *gtk3.Box
+	displayEntry         *gtk3.Entry
+	downloadsDirBox      *gtk3.Box
+	downloadsDirChooser  *gtk3.FileChooserButton
+	desktopDirBox        *gtk3.Box
+	desktopDirChooser    *gtk3.FileChooserButton
 }
 
 const proxySOCKS4 = "SOCKS 4"
@@ -112,9 +111,9 @@ func (d *configDialog) loadFromConfig() {
 	d.pulseAudioSwitch.SetActive(d.ui.Cfg.Sandbox.EnablePulseAudio)
 	d.avCodecSwitch.SetActive(d.ui.Cfg.Sandbox.EnableAVCodec)
 	d.circuitDisplaySwitch.SetActive(d.ui.Cfg.Sandbox.EnableCircuitDisplay)
-	d.volatileExtensionsSwitch.SetActive(d.ui.Cfg.Sandbox.VolatileExtensionsDir)
 	if d.ui.Cfg.Sandbox.Display != "" {
 		d.displayEntry.SetText(d.ui.Cfg.Sandbox.Display)
+		forceAdv = true
 	}
 	if d.ui.Cfg.Sandbox.DownloadsDir != "" {
 		d.downloadsDirChooser.SetCurrentFolder(d.ui.Cfg.Sandbox.DownloadsDir)
@@ -194,7 +193,6 @@ func (d *configDialog) onOk() error {
 	d.ui.Cfg.Sandbox.SetEnablePulseAudio(d.pulseAudioSwitch.GetActive())
 	d.ui.Cfg.Sandbox.SetEnableAVCodec(d.avCodecSwitch.GetActive())
 	d.ui.Cfg.Sandbox.SetEnableCircuitDisplay(d.circuitDisplaySwitch.GetActive())
-	d.ui.Cfg.Sandbox.SetVolatileExtensionsDir(d.volatileExtensionsSwitch.GetActive())
 	if s, err := d.displayEntry.GetText(); err != nil {
 		return err
 	} else {
@@ -382,9 +380,6 @@ func (ui *gtkUI) initConfigDialog(b *gtk3.Builder) error {
 	if d.circuitDisplaySwitch, err = getSwitch(b, "circuitDisplaySwitch"); err != nil {
 		return err
 	}
-	if d.volatileExtensionsSwitch, err = getSwitch(b, "volatileExtensionsSwitch"); err != nil {
-		return err
-	}
 	if d.displayBox, err = getBox(b, "displayBox"); err != nil {
 		return err
 	}



More information about the tor-commits mailing list