27/11/13 12:37, intrigeri wrote:
> Hi,
>
> anonym wrote (26 Nov 2013 17:11:30 GMT) :
>> 21/11/13 09:10, intrigeri wrote:
>
>> What do you think about commit c177710?
>
> Much better. Simplified a bit more in commit 3b41123.
>
>>> * Regarding "It would obviously require to drop `set -e`, because
>>> errors are indeed what could cause this to happen." --> err, well,
>>> I kinda disagree that letting errors propagate further, just so we
>>> can enjoy detecting it later, is "obvious". "set -e" detects a given
>>> class of error conditions, great. The proposed failsafe check would
>>> detect another (probably overlapping) class of error conditions.
>>> I think that both should coexist.
>
>> My point is that `set -e` doesn't simply "detect" errors, like you put
>> it; it *terminates* the script upon certain error conditions, which
>> most likely prevents whatever failsafe we have from detecting its class
>> of errors (at least the non-overlapping part) and warn the user etc. Or
>> am I misunderstanding how you want this failsafe to be implemented?
>
> I think I now see what you mean.
>
> My main concern is that removing `set -e' could lead to situations
> when the failure detection code or the "fail close" code itself fails
> at runtime for some reason, and then a MAC spoofing failure goes
> silently undetected.
>
> What I meant was:
>
> 1. Have `set -e' detect its own class of errors and abort the
> script, and let the caller of this script handle this nicely.
> 2. Have failsafe code at the end of tails-spoof-mac handle the cases
> that `set -e' didn't detect.
>
> However, the script we're talking of (tails-spoof-mac) is run by udev
> rules, so my #1 above can't be done that easily.
>
> How about:
>
> 1. Have the udev rule run a very simple, carefully written and
> robust wrapper script that runs tails-spoof-mac.
> 2. Have failsafe code at the end of tails-spoof-mac handle the more
> or less expected failure modes that `set -e' didn't detect (I
> mean, e.g. some firmware that answers macchanger "yeah, OK, I'll
> use this MAC address" but actually doesn't record the change),
> and return 0 if the "fail close" protection can be
> successfully applied.
> 3. Have the wrapper handle the unexpected cases when tails-spoof-mac
> returns with non-zero exit code (presumably due to `set -e'
> aborting it).
>
> Then, the wrapper script is the only part that is not allowed to have
> bugs, and we have a way to sanely handle bugs that tails-spoof-mac or
> any of its dependencies (macchanger, our shell library) might expose
> in corner cases we've not thought of yet.
This would work, although adding a wrapper for the sole purpose of
"working around" `set -e` is a bit awkward.
I think I have simpler and cleaner suggestion: we keep tails-spoof-mac
as it is, so it still has `set -e` and no fail-safe; instead we put the
fail-safe in tails-unblock-network, right after udev has settled (=> we
know the hooks have finished), i.e. something like:
if mac_spoof_is_enabled; then
for nic in get_all_ethernet_nics; then
if ! mac_is_spoofed; then
# Panic mode: unload the NIC's module, etc.
# Show an appropriate warning in a notification to the user
fi
done
fi
There are even more advantages if we move the NM starting code into the
same script as well (as I suggested in another post): we may want to
*not* start NM if we failed to unload a failing NIC's module in "panic
mode". That'd also more explicitly enforce the pre-condition for
actually taking the final steps to enable the network, which is good.
Cheers!
