[tor-commits] [sandboxed-tor-browser/master] Bug 21764: Use bubblewrap's `--die-with-parent` when supported.

[yawning at torproject.org]

commit 66aaf6fea4293fdb0f9f233ea612e3b53efc4172
Author: Yawning Angel <yawning at schwanenlied.me>
Date:   Thu Apr 13 07:58:01 2017 +0000

    Bug 21764: Use bubblewrap's `--die-with-parent` when supported.
    
    Tor Browser nightly (ESR52, e10s) doesn't exit cleanly when SIGINT is
    sent to `sandboxed-tor-browser` without this.  Unfortunately the option
    requires bubblewrap >= 0.1.8, but at least exiting firefox normally
    works fine with older bubblewrap.
---
 ChangeLog                                          |  1 +
 .../internal/sandbox/hugbox.go                     | 86 +++++++++++++---------
 2 files changed, 51 insertions(+), 36 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 92ce6c0..d634264 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,5 @@
 Changes in version 0.0.5 - UNRELEASED:
+ * Bug 21764: Use bubblewrap's `--die-with-parent` when supported.
  * Fix e10s Web Content crash on systems with grsec kernels.
  * Add `prlimit64` to the firefox system call whitelist.
 
diff --git a/src/cmd/sandboxed-tor-browser/internal/sandbox/hugbox.go b/src/cmd/sandboxed-tor-browser/internal/sandbox/hugbox.go
index 118b7a7..dd2e51a 100644
--- a/src/cmd/sandboxed-tor-browser/internal/sandbox/hugbox.go
+++ b/src/cmd/sandboxed-tor-browser/internal/sandbox/hugbox.go
@@ -91,9 +91,10 @@ type hugbox struct {
 
 	// Internal options, not to be *modified* except via helpers, unless you
 	// know what you are doing.
-	bwrapPath string
-	args      []string
-	fileData  [][]byte
+	bwrapPath    string
+	bwrapVersion *bwrapVersion
+	args         []string
+	fileData     [][]byte
 
 	runtimeDir string // Set at creation time.
 }
@@ -244,6 +245,12 @@ func (h *hugbox) run() (*Process, error) {
 	h.file("/etc/passwd", []byte(passwdBody))
 	h.file("/etc/group", []byte(groupBody))
 
+	dieWithParent := h.bwrapVersion.atLeast(0, 1, 8)
+	if dieWithParent {
+		Debugf("sandbox: bubblewrap supports `--die-with-parent`.")
+		fdArgs = append(fdArgs, "--die-with-parent")
+	}
+
 	if h.fakeDbus {
 		h.setupDbus()
 	}
@@ -421,23 +428,47 @@ func newHugbox() (*hugbox, error) {
 		return nil, fmt.Errorf("sandbox: unable to find bubblewrap binary")
 	}
 
-	// Bubblewrap <= 0.1.2-2 (in Debian terms, 0.1.3 for the rest of us), is
-	// a really bad idea because I'm a retard, and didn't expect bubblewrap
-	// to be ptrace-able when I contributed support for the hostname.
-	//
-	// There is a CVE for it.  Sensible people have made 0.1.3 available,
-	// including jessie-backports.  Ubuntu is still shipping an old version.
-	// Sucks to be them.
-	if ok, err := bubblewrapAtLeast(h.bwrapPath, 0, 1, 3); err != nil {
+	// Query and cache the bubblewrap version.
+	var err error
+	if h.bwrapVersion, err = getBwrapVersion(h.bwrapPath); err != nil {
 		return nil, err
-	} else if !ok {
-		return nil, fmt.Errorf("sandbox: bubblewrap appears to be older than 0.1.3, you MUST upgrade.")
+	} else {
+		Debugf("sandbox: bubblewrap '%v' detected.", h.bwrapVersion)
+
+		// Bubblewrap <= 0.1.2-2 (in Debian terms, 0.1.3 for the rest of us),
+		// is a really bad idea because I'm a retard, and didn't expect
+		// bubblewrap to be ptrace-able when I contributed support for setting
+		// the hostname.
+		if !h.bwrapVersion.atLeast(0, 1, 3) {
+			return nil, fmt.Errorf("sandbox: bubblewrap appears to be older than 0.1.3, you MUST upgrade.")
+		}
 	}
 
 	return h, nil
 }
 
-func getBubblewrapVersion(f string) (int, int, int, error) {
+type bwrapVersion struct {
+	maj, min, pl int
+}
+
+func (v *bwrapVersion) atLeast(maj, min, pl int) bool {
+	if v.maj > maj {
+		return true
+	}
+	if v.maj == maj && v.min > min {
+		return true
+	}
+	if v.maj == maj && v.min == min && v.pl >= pl {
+		return true
+	}
+	return false
+}
+
+func (v *bwrapVersion) String() string {
+	return fmt.Sprintf("%d.%d.%d", v.maj, v.min, v.pl)
+}
+
+func getBwrapVersion(f string) (*bwrapVersion, error) {
 	cmd := &exec.Cmd{
 		Path: f,
 		Args: []string{f, "--version"},
@@ -448,7 +479,7 @@ func getBubblewrapVersion(f string) (int, int, int, error) {
 	}
 	out, err := cmd.CombinedOutput()
 	if err != nil {
-		return 0, 0, 0, fmt.Errorf("sandbox: failed to query bubblewrap version: %v", string(out))
+		return nil, fmt.Errorf("sandbox: failed to query bubblewrap version: %v", string(out))
 	}
 	vStr := strings.TrimPrefix(string(out), "bubblewrap ")
 	vStr = strings.TrimSpace(vStr)
@@ -456,37 +487,20 @@ func getBubblewrapVersion(f string) (int, int, int, error) {
 	// Split into major/minor/pl.
 	v := strings.Split(vStr, ".")
 	if len(v) < 3 {
-		return 0, 0, 0, fmt.Errorf("unable to determine bubblewrap version")
+		return nil, fmt.Errorf("unable to determine bubblewrap version")
 	}
 
+	// Parse the version.
 	var iVers [3]int
 	for i := 0; i < 3; i++ {
 		iv, err := strconv.Atoi(v[i])
 		if err != nil {
-			return 0, 0, 0, fmt.Errorf("unable to determine bubblewrap version: %v", err)
+			return nil, fmt.Errorf("unable to parse bubblewrap version: %v", err)
 		}
 		iVers[i] = iv
 	}
 
-	return iVers[0], iVers[1], iVers[2], nil
-}
-
-func bubblewrapAtLeast(f string, maj, min, pl int) (bool, error) {
-	iMaj, iMin, iPl, err := getBubblewrapVersion(f)
-	if err != nil {
-		return false, err
-	}
-
-	if iMaj > maj {
-		return true, nil
-	}
-	if iMaj == maj && iMin > min {
-		return true, nil
-	}
-	if iMaj == maj && iMin == min && iPl >= pl {
-		return true, nil
-	}
-	return false, nil
+	return &bwrapVersion{maj: iVers[0], min: iVers[1], pl: iVers[2]}, nil
 }
 
 func writeBuffer(w io.WriteCloser, contents []byte) error {