03/03/14 14:23, intrigeri wrote: > Hi, > > I've had a look at the rewritten branch, at commit 7d0ea0b. > >> root ALL = (tor-launcher) NOPASSWD: /usr/bin/tor-launcher > > What is this useful for? root can already run any command as any user > without password, no?
Woops, indeed. Removed. >> amnesia ALL = (root) NOPASSWD: /usr/local/sbin/tails-tor-launcher > > It could be worth restricting the arguments that amnesia can pass to > this command. That would be none, or --force-net-config, right? We only need to call it with --force-net-config. Actually, that sudo rule was intended for the GNOME menu entry that we're now not so sure we want to add after all. I left it there with the option forced. > In /usr/local/sbin/tails-tor-launcher, I'd rather see the four > instances of: > > VAR=value > export VAR > > ... written "export VAR=value" instead, but that's purely a matter of > personal taste, and I don't care much. That's a bashism that just happens to work in dash too. It should be noted that the very similar bashism `local VAR=value` does *not* work in dash. As I've been bitten by the latter one quite severely at one point (when working on persistence in live-boot) I've come to prefer the POSIX way at all times. >> touch /etc/authbind/byport/53 >> chgrp debian-tor /etc/authbind/byport/53 >> chmod g=x /etc/authbind/byport/53 > > Nowadays, I would instead write: > > install --group=debian-tor --mode=0710 /dev/null /etc/authbind/byport/53 > > ... but again, purely a matter of personal taste. I agree. However, I've dropped the authbind stuff in favor of a netfilter REDIRECT, as you suggested earlier, so this is now moot. >> $NICE \ >> $AA_EXEC \ >> - --exec $DAEMON -- $AA_EXEC_ARGS $DEFAULT_ARGS $ARGS >> + --exec /usr/bin/authbind -- $DAEMON $AA_EXEC_ARGS >> $DEFAULT_ARGS $ARGS > > Any reason why /usr/bin/authbind is not grouped with the other prefix > commands ($NICE and $AA_EXEC), instead of being part of the arguments > passed to --exec? ... and this is now moot as well for the same reason. >> Don't ever run Vidalia with -bridgeconf. > > So we could update our Vidalia package: > > 1. to drop vidalia-bridgeconf.patch: not needed anymore > 2. to hide bridge settings (either in > tails-remove-useless-controls.patch, or with a new patch, whatever > is more practical) > > I guess #1 is not a blocker, but I'm unsure about #2. What happens if > a user changes bridges settings in Vidalia, after having set it in Tor > Launcher? And after *not* having set it in Tor Launcher? In both situations the bridges are changed, no more no less. Was there any specific potential problem you thought of? The one issue I can think of at the moment is that Tor Launcher (thanks to my modifications to it) will add an appropriate ClientTransportPlugin line to Tor when bridges are input through it, but vidalia won't. > (And if we address #2, implementing #1 as well does not add more than > a few minutes of work.) Agreed, leaving it there should be of no consequence and saves us the packaging overhead. -- I've rebased the branch completely since I've imported a new Tor Launcher. However, you can review again from after commit 7d0ea0b (which corresponds to where your review ended last time). The only thing that's happened before that except the Tor Launcher bump is that I dropped the commit introducing authbind, per you suggestion. From now on I'll fix the Tor Launcher bumps differently to avoid history rewrites. Big thanks for the review! Cheers! _______________________________________________ tails-dev mailing list tails-dev@boum.org https://mailman.boum.org/listinfo/tails-dev To unsubscribe from this list, send an empty email to tails-dev-unsubscr...@boum.org.
