Re: [Tails-dev] Made New Identity work again, please review

このメッセージを削除

このメッセージに返信
著者: intrigeri
日付:  
To: The Tails public development discussion list
題目: Re: [Tails-dev] Made New Identity work again, please review
Hi,

winterfairy@??? wrote (11 Feb 2014 18:14:18 GMT) :
> I believe I have fixed the regression described in ticket #6383. When
> access to Tor's control port was restricted (to prevent GETINFO address),
> Torbutton could no longer do "New Identity". I have created a filtering
> proxy for the control port, that only allows SIGNAL NEWNYM. This is enough
> to make "New Identity" work again as expected.


Congrats! This usability regression was one of the top mentionned ones
in my experience when doing user support in the last few months.

I won't be doing the entire review'n'merge process, but I had a quick
look, and here are a few quick notes:

1. I'm not entirely convinced by the idea of running
tor-controlport-filter as the vidalia user. Granted, it's the
shortest path, but it feels weird, and having a bit more privilege
separation would be good.

2. The use of string.find(expected text) seems a bit fragile.
Couldn't this matching be done in a bit stricter way?

3. It might be overkill, and surely adds some code, but I would pass
the port to listen on, control port socket path and authentication
cookie path as command-line arguments (preferably named, not
positional). Just to make it easier for others to reuse this piece
of code, and increase collaboration between similar projects.

Other than this, I was (once again!) impressed by the quality of this
submission. I've merged it into our "experimental" branch, and pushed
to our master repository + to our Jenkins auto-builder.

anonym: don't consider this as a full review, I only had a quick look,
really, and did not test this at all.

> (I did not use Whonix existing implementation, because I felt very uneasy
> about running a potentially security critical application as a shell
> script.


Fully agreed. I suggest you point the Whonix developers to your own
implementation, once it's merged.

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