Bug #8158

Unsafe Browser does not start

Added by goupille 2014-10-19 15:06:38 . Updated 2014-12-03 19:39:56 .

Status:
Resolved
Priority:
Elevated
Assignee:
Category:
Target version:
Tails_1.2.1
Start date:
2014-10-19
Due date:
% Done:

100%

Feature Branch:
bugfix/8158-stacked-rootfs-vs-chroot-browsers
Type of work:
Research
Blueprint:

Starter:
Affected tool:
Unsafe Browser
Deliverable for:

Description

when lauching the usafe browser, the warning show up but multiple users waited and the unsafe browser didn’t start. I found that in the logs (.xsession-errors):

unzip: cannot find or open /var/lib/unsafe-browser/chroot//usr/local/lib/tor-browser/browser/omni.ja, /var/lib/unsafe-browser/chroot//usr/local/lib/tor-browser/browser/omni.ja.zip or /var/lib/unsafe-browser/chroot//usr/local/lib/tor-browser/browser/omni.ja.ZIP.
sed: can’t read /tmp/tmp.wAjfBOdkDP/chrome/en-US/locale/branding/brand.dtd: No such file or directory

this issue looks a lot like https://labs.riseup.net/code/issues/8152, I couldn’t reproduce it either

Files

fix_unsafe_browser_chroot2.patch (704 B) sanic, 2014-10-20 09:16:40
fix_unsafe_browser_chroot3.patch (905 B) sanic, 2014-10-20 11:49:39

Subtasks

Related issues

Related to Tails - Bug #8152: I2P browser doesn't start after incremental upgrade to Tails 1.2 Resolved 2014-10-17

History

#1 Updated by ioerror 2014-10-19 18:05:27

goupille wrote:
> when lauching the usafe browser, the warning show up but multiple users waited and the unsafe browser didn’t start. I found that in the logs (.xsession-errors):
>
> unzip: cannot find or open /var/lib/unsafe-browser/chroot//usr/local/lib/tor-browser/browser/omni.ja, /var/lib/unsafe-browser/chroot//usr/local/lib/tor-browser/browser/omni.ja.zip or /var/lib/unsafe-browser/chroot//usr/local/lib/tor-browser/browser/omni.ja.ZIP.
> sed: can’t read /tmp/tmp.wAjfBOdkDP/chrome/en-US/locale/branding/brand.dtd: No such file or directory
>
> this issue looks a lot like https://labs.riseup.net/code/issues/8152, I couldn’t reproduce it either

I had the following issue:

amnesia@amnesia:~$ sudo unsafe-browser
* Setting up chroot
* Configuring chroot
(Reading database ... 129226 files and directories currently installed.)
Removing xul-ext-adblock-plus ...
Removing xul-ext-torbutton ...
unzip:  cannot find or open /var/lib/unsafe-browser/chroot//usr/local/lib/tor-browser/browser/omni.ja, /var/lib/unsafe-browser/chroot//usr/local/lib/tor-browser/browser/omni.ja.zip or /var/lib/unsafe-browser/chroot//usr/local/lib/tor-browser/browser/omni.ja.ZIP.
sed: can't read /tmp/tmp.lesFEJUXhH/chrome/en-US/locale/branding/brand.dtd: No such file or directory
amnesia@amnesia:~$

#2 Updated by sanic 2014-10-20 06:59:35

  • File <del>missing: fix_unsafe_browser_chroot.patch</del> added

It turns out the chroot created in /usr/local/sbin/unsafe-browser comes only from filesystem.squashfs, and ignores any incremental upgrades. This is also why the bug cannot be reproduced by people who have a full, non-incremental install.

Attached is a patch to fix it.

#3 Updated by intrigeri 2014-10-20 07:33:50

  • Category set to 183
  • Assignee set to anonym
  • Priority changed from Normal to Elevated
  • Target version set to Tails_1.2.1
  • QA Check set to Ready for QA

#4 Updated by intrigeri 2014-10-20 07:34:10

  • related to Bug #8152: I2P browser doesn't start after incremental upgrade to Tails 1.2 added

#5 Updated by sanic 2014-10-20 09:16:40

Oops. I accidentally uploaded a non-working version, but I don’t know how to edit my previous post. Here is the confirmed working version.

#6 Updated by anonym 2014-10-20 09:22:15

  • Assignee changed from anonym to sanic
  • QA Check changed from Ready for QA to Dev Needed

sanic wrote:
> It turns out the chroot created in /usr/local/sbin/unsafe-browser comes only from filesystem.squashfs, and ignores any incremental upgrades. This is also why the bug cannot be reproduced by people who have a full, non-incremental install.

Great catch! This means that the Unsafe Browser always has been running the oldest version of the browser available. Unsafe indeed.

> Attached is a patch to fix it.

It looks good, except:

+    for rootfs_dir in /lib/live/mount/rootfs/*; do

It’s incorrect to rely on the lexical ordering of file names (which is the result of the glob’s shell substitution). For instance when we release 1.2.1 and you install the 1.2_to_1.2.1 IUK on top of 1.1.2_to_1.2, your patch will produce the order: 1.2, 1.2.1, filesystem.squashfs, so 1.2’s filesystem will stack on top of 1.2.1’s. Mix in some release candidates and it gets even worse, and imagine if we used it in the pre-1.0 days, when we had releases like 0.9 and 0.10.

I think that instead of a glob we can do ls -t or (clearer) ls --sort=time --time=ctime, but relying on the modification/creation date feels a bit fragile. What about the reverse of ls’s “natural sort of (version) numbers within text”, i.e. ls -v --reverse? In either case, I’d like a code comment too.

sanic, are you committing to fix your patch? I “promoted” you to Developer status so I could assign this ticket to you. Otherwise, please reassign it to me.

#7 Updated by sanic 2014-10-20 09:30:08

  • File deleted (fix_unsafe_browser_chroot.patch)

#8 Updated by intrigeri 2014-10-20 09:55:15

> It’s incorrect to rely on the lexical ordering of file names (which is the result of the glob’s shell substitution).

Can’t we use the ordering from Tails.module (or whatever it’s called)?

#9 Updated by sanic 2014-10-20 11:51:41

intrigeri wrote:
> > It’s incorrect to rely on the lexical ordering of file names (which is the result of the glob’s shell substitution).
>
> Can’t we use the ordering from Tails.module (or whatever it’s called)?

That’s what we ended up doing.

I attached the patch, it’s confirmed to work on both my and anonym’s computer.

#10 Updated by sanic 2014-10-20 11:56:47

  • Status changed from New to In Progress
  • Assignee changed from sanic to anonym
  • % Done changed from 0 to 50
  • QA Check changed from Dev Needed to Ready for QA

intrigeri wrote:
> > It’s incorrect to rely on the lexical ordering of file names (which is the result of the glob’s shell substitution).
>
> Can’t we use the ordering from Tails.module (or whatever it’s called)?

That’s what we ended up doing.

I attached the patch, it’s confirmed to work on both my and anonym’s computer.

#11 Updated by anonym 2014-10-20 17:54:39

sanic wrote:
> intrigeri wrote:
> > > It’s incorrect to rely on the lexical ordering of file names (which is the result of the glob’s shell substitution).
> >
> > Can’t we use the ordering from Tails.module (or whatever it’s called)?
>
> That’s what we ended up doing.
>
> I attached the patch, it’s confirmed to work on both my and anonym’s computer.

I believe it’s just a coincidence that it worked. Look at this:

+    while read rootfs_dir; do
+        rootfs_dir="$rootfs_dir_path/$rootfs_dir"
+        mountpoint -q "$rootfs_dir" && \
+        aufs_opts="${aufs_opts}:${rootfs_dir}=rr+wh"
+    done < "$tails_module_path"

The problem I see is that Tails.module is ordered with the oldest squashfs first, so we will potentially end up using a previous Tails version’s Tor Browser (Firefox). The reason it worked now and gave the correct Tor Browser version (at least for me) is that Tor Browser didn’t exist in the older Tails version’s (1.1.2) filesystem.squashfs, so whatever was in the later (lower) stacked 1.2.squashfs got through. Sorry for not catching this when we discussed this on IRC.

So, what I think you should do is:

    tac "$tails_module_path" | while read rootfs_dir; do
        rootfs_dir="$rootfs_dir_path/$rootfs_dir"
        mountpoint -q "$rootfs_dir" && \
        aufs_opts="${aufs_opts}:${rootfs_dir}=rr+wh"
    done

(Note that trying to reverse the order by assigning aufs_opts="${rootfs_dir}=rr+wh:${aufs_opts}" won’t work since aufs_opts is initialized with stuff that has to be in the beginning of the option string.

Does this make sense?

#12 Updated by anonym 2014-10-21 00:30:29

  • QA Check changed from Ready for QA to Dev Needed
  • Feature Branch set to bugfix/8158-stacked-rootfs-vs-chroot-browsers

anonym wrote:
> So, what I think you should do is:
>
> […]
>
> Does this make sense?

Actually it doesn’t. The problem with the cat | while read pattern is that thanks to the pipe, the while-loop is executed in a sub-shell, so any assignments done within it are lost. Duh. And process substitution (<( command )) isn’t POSIX, and the fifo workarounds aren’t very pretty. Oh well…

I’m pushing an adaption of the patch into bugfix/8158-stacked-rootfs-vs-chroot-browsers, but I haven’t tested it yet in any scenario where there are stacked rootfs:es.

#13 Updated by anonym 2014-10-21 13:16:20

  • Assignee deleted (anonym)
  • QA Check changed from Dev Needed to Ready for QA

anonym wrote:
> I’m pushing an adaption of the patch into bugfix/8158-stacked-rootfs-vs-chroot-browsers, but I haven’t tested it yet in any scenario where there are stacked rootfs:es.

Worked fine with Tails 1.1.2 upgraded to 1.2, and then upgraded to 1.3~experimental-61a22a3 (from an IUK I prepared from commit 61a22a3 of experiemental, which contains this branch).

Since I had to re-write the patch, I suppose it should be review’n’merged by someone else, so please review’n’merge!

#14 Updated by intrigeri 2014-10-30 15:44:36

  • Assignee set to intrigeri

#15 Updated by intrigeri 2014-10-30 17:05:21

  • Status changed from In Progress to Fix committed
  • Assignee deleted (intrigeri)
  • % Done changed from 50 to 100
  • QA Check changed from Ready for QA to Pass

#16 Updated by BitingBird 2014-12-03 19:39:56

  • Status changed from Fix committed to Resolved