Re: [Tails-dev] Please review feature/separate_Tor_streams

Delete this message

Reply to this message
Autor: Ague Mill
Data:  
Dla: The Tails public development discussion list
Temat: Re: [Tails-dev] Please review feature/separate_Tor_streams
On Mon, Sep 24, 2012 at 07:55:23PM +0200, intrigeri wrote:
> Please review feature/separate_Tor_streams. The bulk of the changes
> has been in experimental for a few weeks.


Great work! I am happy to see how the implementation turns out. :)

> +++ b/config/chroot_local-includes/etc/iceweasel/pref/iceweasel.js
> +pref("network.security.ports.banned", "8118,8123,9050,9051,9061,9062,9063,9051");


9051 appears twice.

> +++ b/config/chroot_local-includes/etc/tor/tor-tsocks-mua.conf
> +# This is the configuration for libtsocks (transparent socks) for use
> +# with tor, which is providing a socks server on port 9050 by default.
> +server_port = 9061


The header is confusing. I think it should be more specific to the MUA
case.

> +++ b/config/chroot_local-includes/usr/local/sbin/htpdate
> +        [ 'proxy|p:s', "what to pass to curl's --socks5-hostname" ],


According to its manpage, curl will use the following environment
variable: `http_proxy`, `HTTPS_PROXY`, `FTP_PROXY`, `ALL_PROXY`.
I wonder if it would not make the behaviour of htpdate more consistent
to manually unset those variables before running `curl`. Otherwise,
htpdate could use a proxy with '-p' specified on the command-line.

I am not very strong on this, but this could lead to some (bad?)
surprises.

> +++ b/wiki/src/contribute/design/Tor_enforcement/Network_filter.mdwn
> +[[!tails_gitweb config/chroot_local-includes/etc/firewall.conf]]


This needs to be updated now that we are using ferm.

> +++ b/wiki/src/contribute/design/stream_isolation.mdwn


I think this page deserve a link to proposal 171:
<https://gitweb.torproject.org/torspec.git/blob/HEAD:/proposals/171-separate-streams.txt>

> +Applications are configured to use the right SOCKS port:


I have a tickling feeling that this list will get outdated...


No tests done so far. This one deserve to be looked at closely.
Uh... and actually, those changes might require to add some more tests
to the checklist. What do you think?

--
Ague