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

Delete this message

Reply to this message
Author: intrigeri
Date:  
To: The Tails public development discussion list
Subject: Re: [Tails-dev] Please review feature/separate_Tor_streams
Hi,

Ague Mill wrote (26 Sep 2012 08:48:21 GMT) :
> Great work! I am happy to see how the implementation turns out. :)


Thanks for the review.

>> +++ 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.


Fixed (64ad230e).

>> +++ 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.


Fixed (4863fe7).

>> +++ 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.


Consistent with what? All this paragraph of yours is very unclear to
me, and I'm not sure what you mean. Please rephrase, e.g. point out
a specific potential problem you are envisioning :)

>> +++ 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.


Fixed (62a1e61).

BTW, contribute/design/Tor_enforcement/DNS has another outdated
instance of firewall.conf.

>> +++ 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>


Sure (f30c4f9).

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


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


Are talking of the list (of links to configuration files) that follows
the introduction sentence you are quoting, or what?

If you are, well, right, but this is a general problem with our entire
design document. That would be partly addressed by a check for broken
links, and additional strictness on the design doc front when
reviewing/merging branch.

> No tests done so far. This one deserve to be looked at closely.


Sure. Let's keep in mind it has been in experimental for a few weeks.

I think we should go through the list of networking applications again
(from the good old move to torsocks days), and make sure they all work
as intended. I did it once already when developing this branch, but an
additional pass before merging would be enough. Volunteers?

(If there are any, I've just pushed my latest fixes, and merged into
experimental again, but I think the tests should be conducted, if
possible, on devel + feature/separate_Tor_streams rather than
experimental. Just to be on the safe side. BTW, once this branch is
merged, I think I'll reset --hard experimental to a saner state.)

> Uh... and actually, those changes might require to add some more
> tests to the checklist. What do you think?


I'll think of it later today or tomorrow.

Cheers,
--
intrigeri
| GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc
| OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc