Hi,
First of all, not that I've fixed a few bugs and updated the automated
test suite so it deals with the TBB migration.
21/09/14 00:51, intrigeri wrote:
> Hi,
>
> anonym wrote (20 Sep 2014 01:20:00 GMT) :
>>>> +Explanation: Block installation of iceweasel until it has been removed from our APT repo
>>>> +Package: iceweasel
>>>> +Pin: origin deb.tails.boum.org
>>>> +Pin-Priority: -1
>>>
>>> I'm curious why we need that at all, and the explanation isn't very
>>> convincing: even once we remove it from our own APT repo, it will be
>>> available in the Debian ones, so I don't get it.
[...]
> Something like:
>
> Explanation: keep our fake equivs-generated iceweasel package
> Package: iceweasel
> Pin: origin ""
> Pin-Priority: 1020
>
> might be just enough to express exactly what we want to say
> (untested), in a way that would work in more situations.
I tested it, and it didn't work:
[...]
The following NEW packages will be installed:
libmozjs24d xulrunner-24.0
The following packages will be DOWNGRADED:
iceweasel
0 upgraded, 2 newly installed, 1 downgraded, 0 to remove and 0 not
upgraded.
Need to get 21.8 MB of archives.
After this operation, 56.0 MB of additional disk space will be used.
E: There are problems and -y was used without --force-yes
P: Begin unmounting filesystems...
[...]
>>> Regarding config/chroot_local-hooks/10-tbb, a lot of the code could
>>> enjoy some refactoring. Currently, configuration, low-level processing
>>> and the high-level flow are too strongly intermingled for my taste.
>
>> Could you please elaborate?
>
> I would suggest *naming* operations that are being done, [...]
> But oh well, it feels strange to pretend I can teach you anything wrt.
> software design and refactoring, you already know all this :)
I misunderstood the scope of what you meant. I have started some work in
this direction and will push it later just so that work doesn't block.
>>>> +TBB_EXT="${TBB_INSTALL}/extensions"
>>>
>>> I'm curious why we need to put extensions in a custom place, instead
>>> of letting them live in the place as in the TBB.
>
>> I.e. directly in the browser profile skeleton at /etc/icewease/profile?
>> Well, I'd like to just be able to copy the profile skeleton when
>> creating a new profile without wasting space (well, RAM because tmpfs)
>> on duplicating every extension.
>
> OK, makes a lot of sense. Make it clear in a comment? (I think we
> should aim at the smallest possible delta with the TB here, so
> documenting *why* this and that bit of our delta is needed will help
> whenever we try to make the delta smaller in the future, and someone
> will be asking exactly this kind of questions :)
Ok, I think I've improved it a bit now.
>>>> +# We don't want tor-launcher to be part of the browser, and we need our
>>>> +# patched stand-alone version any way.
>>>
>>> s/and/as/ ? Also, I suggest pointing to the parent ticket that tracks
>>> upstreaming our changes.
>
>> Well, I meant it as two separate reasons for doing that. I now realize
>> the "stand-alone" in the second part creates some overlap with the first
>> part, which may cause confusion. Would removing "stand-alone" make it
>> clearer?
>
> Ah, I got it. How about: "We don't want tor-launcher to be part of the
> regular browser profile. Moreover, for the stand-alone tor-launcher we
> use, we need our patched version. So, the version shipped in the TB
> really is not useful for us."
Looks better, indeed. Applied.
>>> In config/chroot_local-hooks/12-remove_unwanted_browser_searchplugins:
>>>
>>>> +PLUGIN_DIR=/usr/local/lib/tor-browser/Browser/browser/searchplugins
>>>
>>> It seems that we're hard-coding the same path information in different
>>> places. How about setting TBB_INSTALL and friends in a common place,
>>> that can be sourced by all scripts that need it?
>
>> Sure. I couldn't come up with a place where we already do this. Do you
>> have any suggestion for a good location? /etc/live/config.d?
>
> I'm unsure whether we really want to export all these variables as
> part of the global system-wide environment. I would instead store them
> somewhere that can be sourced when needed.
>
> We already do similar things in auto/config (saving stuff to
> /etc/amnesia/) and auto/build (saving stuff to
> /usr/share/amnesia/build/). I think that /usr makes more sense than
> /etc, as what we want to save here is really static information about
> how/where vendor-provided software is setup in the ISO, rather than
> configuration => /usr/local/lib/tails-shell-library/tor-browser.sh,
> maybe? It might be that we need more than variables in there at some
> point, so bootstrapping a mini-shell-library with these doesn't seem
> too crazy.
Agreed. This is now done.
>>>> - daddr 127.0.0.1 proto tcp syn mod multiport destination-ports (9050 9061 9062 9151) {
>>>> + daddr 127.0.0.1 proto tcp syn mod multiport destination-ports (9050 9061 9062 9150) {
>>>
>>> It would be nice for anyone who has custom configuration that depends
>>> on the SOCKS port to keep the 9151 we've been using until now. OTOH,
>>> maybe people are installing scripts from the Internet that rely on the
>>> default TBB port, so I'm unsure what's best. Is there a good reason to
>>> change it?
>
>> I thought this would be a good opportunity to lessen the configuration
>> delta with the TBB. In this instance it means that we don't have to look
>> out for upstream changes that depends the SOCKSPort value they use,
>> which IMHO is valuable.
>
> OK, I agree. The contribute/design/stream_isolation page needs an
> update, then.
It was in the very first commit of this branch.
>>>> +cp -r /etc/tor-browser/profile "${USER_PROFILE}"/profile.default
>>>
>>> I would use rsync and preserve more inode information.
>
>> I switched to `rsync -a`, but for whatever reason the destination didn't
>> get the "extensions" sub directory. Therefore I just switched to juse
>> `cp -a`. Any ideas?
>
> I would need to know exactly what you did to be able to help.
I can't remember exactly, but I think I just s/cp -r/rsync -a/. What
command would you expect to work like you want?
>>>> + mkdir -p /usr/Data/Browser/profile.default/preferences
>>>
>>> /usr/Data/, really?
>
>> Yes, really! :S
>
>>> Is that the "hardcoded relative-patch only default
>>> profile dir" mentioned above?
>
>> Exactly.
>
>>> Maybe a symlink would allow to put the
>>> actual files in a more relevant place, while keeping compatibility
>>> with the TBB's code?
>
>> Symlink how? Since -profile doesn't work together with -app,
>> /usr/Data/... will be used for the profile no matter what. Well, the
>> profile directory is relative to the application.ini file passed via
>> -app, so we can control where it ends up by putting the .ini file in an
>> appropriate place. One of the first things I tried was to create the
>> following symlink:
>
>> /home/tor-launcher/.tor-launcher/application/tor-launcher-standalone ->
>> /usr/share/tor-launcher-standalone
>
>> but the Tor Browser isn't fooled by symlinks and finds the real path.
>
> OK, got it.
>
>> This would work if we'd copy all of /usr/share/tor-launcher-standalone
>> into the place of the symlink instead... which isn't so bad since it's
>> only 325 KB in size (and so increased RAM usage because tmpfs). Do you
>> think commit f33296b improves the situation?
>
> At least it makes things a bit clearer. But then, why don't we:
>
> * directly install the torbrowser-launcher code in the directory (in
> tor-launcher's $HOME directory) where it will have to be run from
> at ISO build time? once chroot_local-hooks/06-adduser_tor-launcher
> has been run, that $HOME directory exists for us to put stuff in;
> * do the profiles.ini etc. mangling at ISO build time too
>
> ?
Well, despite the workaround, it feels better to have it in /usr than a
speficic user's home. The implication (~300 KiB more tmpfs space) isn't
that bad. Also, it's my hope that this is just a temporary workaround.
See #7943.
> Here are more comments.
>
> You'll want to:
>
> * add /usr/local/bin/tor-browser to $SHELL_PROGS in the
> refresh-translations script
> * chmod +x config/chroot_local-includes/usr/local/bin/{tor-browser,generate-tor-browser-profile}
Fixed!
> Regarding commit e554c1437:
>
> * please use set -eu
> * please use the shell pipefail option
> * please check that the output is non-empty before returning it,
> and exit 1 otherwise
>
> ... else, we're losing quite some of the robustness of the previous
> Perl script.
Fixed!
Cheers!
