Bug #10160
MAC spoofing panic mode is broken
100%
Description
It’s really stupid. Look at config/chroot_local-includes/usr/local/sbin/tails-unblock-network
. It will wait for config/chroot_local-includes/usr/local/sbin/tails-spoof-mac
(via the udevadm settle
) to do its thing. If the thing is panic mode, i.e. to stop NetworkManager, well tails-unblock-network
will start NetworkManager any way.
In other words, if MAC spoofing is enabled, and for whatever reason some NIC cannot be spoofed and its module cannot be unloaded, then the MAC spoofing panic mode is broken => the error notification will be shown but networking will be enabled from the failing device.
(For the record, I discovered this while working on Feature #6302. Automated testing (or testing at all) FTW :))
Subtasks
History
#1 Updated by anonym 2015-09-04 02:39:21
- Status changed from Confirmed to In Progress
Applied in changeset commit:5f49252608666fa2fd9a4014f6209a1537be6d5e.
#2 Updated by anonym 2015-09-04 02:39:43
- Assignee deleted (
anonym) - % Done changed from 0 to 50
- QA Check set to Ready for QA
#3 Updated by anonym 2015-09-07 09:31:10
- blocks
Feature #6302: Write MAC spoofing tests added
#4 Updated by anonym 2015-09-07 13:53:06
- Feature Branch set to bugfix/10160-mac-spoofing-panic
#5 Updated by bertagaz 2015-09-12 08:16:17
- Assignee set to bertagaz
#6 Updated by bertagaz 2015-09-13 02:01:04
- Assignee changed from bertagaz to anonym
- % Done changed from 50 to 60
- QA Check changed from Ready for QA to Info Needed
Tested it with the mac spoofing feature implemented in the test/6302-mac-spoofing
with and without an ISO containing this patch.
This bugfix perfectly makes sense to me. Nice to see the automated test suite found an issue! :)
I’d be ready to merge it, but I have one remark: in disable_network()
in the config/chroot_local-includes/usr/local/sbin/tails-spoof-mac
why not mv the network manager files to something like .disabled files rather than deleting them? This way people could still put them back if they have set the sudo password to go on with the session. I wonder if implementing this idea would require to document it though. Maybe if you agree our Documentation Master (sajolida) could give a hint about that.
#7 Updated by anonym 2015-09-13 07:21:26
- Assignee changed from anonym to bertagaz
- QA Check changed from Info Needed to Ready for QA
bertagaz wrote:
> Tested it with the mac spoofing feature implemented in the test/6302-mac-spoofing
with and without an ISO containing this patch.
>
> This bugfix perfectly makes sense to me.
\o/
> Nice to see the automated test suite found an issue! :)
Isn’t it? :)
> I’d be ready to merge it, but I have one remark: in disable_network()
in the config/chroot_local-includes/usr/local/sbin/tails-spoof-mac
why not mv the network manager files to something like .disabled files rather than deleting them?
Sure, why not. Done in commit commit:5a74f29.
> This way people could still put them back if they have set the sudo password to go on with the session. I wonder if implementing this idea would require to document it though. Maybe if you agree our Documentation Master (sajolida) could give a hint about that.
This I’m a bit less sure about. The way we’ve been thinking about the MAC spoofing feature so far has been: “if MAC spoofing fails catastrophically but you absolutely need networking, then reboot and disable that feature, but read the docs first to understand the consequences of leaking the MAC address”. Documenting this will just make those docs even longer. Let’s not do this.
#8 Updated by bertagaz 2015-09-13 08:20:46
- Status changed from In Progress to Fix committed
- % Done changed from 60 to 100
Applied in changeset commit:659aedeb75c3c1860e4531b77b6d996478954a46.
#9 Updated by bertagaz 2015-09-13 08:24:29
- Assignee deleted (
bertagaz) - QA Check changed from Ready for QA to Pass
anonym wrote:
> bertagaz wrote:
> > I’d be ready to merge it, but I have one remark: in disable_network()
in the config/chroot_local-includes/usr/local/sbin/tails-spoof-mac
why not mv the network manager files to something like .disabled files rather than deleting them?
>
> Sure, why not. Done in commit commit:5a74f29.
Nice sounds ok to me. Didn’t test again the whole branch but just that last commit and it seems to work well, so merged in stable!
> > This way people could still put them back if they have set the sudo password to go on with the session. I wonder if implementing this idea would require to document it though. Maybe if you agree our Documentation Master (sajolida) could give a hint about that.
>
> This I’m a bit less sure about. The way we’ve been thinking about the MAC spoofing feature so far has been: “if MAC spoofing fails catastrophically but you absolutely need networking, then reboot and disable that feature, but read the docs first to understand the consequences of leaking the MAC address”. Documenting this will just make those docs even longer. Let’s not do this.
Ack, agree it may not need to bloat the documentation more than that. Let’s keep that as a hidden feature. ;)
#10 Updated by bertagaz 2015-09-22 14:43:12
- Status changed from Fix committed to Resolved
- QA Check deleted (
Pass)