After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 642020 - Disable automounting while screen is locked
Disable automounting while screen is locked
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: plugins
2.91.x
Other Linux
: Normal normal
: ---
Assigned To: Martin Pitt
gnome-settings-daemon-maint
Depends on:
Blocks:
 
 
Reported: 2011-02-10 12:18 UTC by Martin Pitt
Modified: 2011-02-24 18:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
(gsd) Disable automounting while screen is locked (5.27 KB, patch)
2011-02-21 14:55 UTC, Martin Pitt
committed Details | Review
(nautilus) Disable automounting while screen is locked (9.93 KB, patch)
2011-02-22 13:09 UTC, Martin Pitt
committed Details | Review

Description Martin Pitt 2011-02-10 12:18:49 UTC
(Originally filed at https://launchpad.net/bugs/714958)

On the recent Shmoocon there was a presentation by Jon Larimer demonstrating how to abuse vulnerabilities and bugs, or even just creating socially or security compromising thumbnails in mounting and thumbnailing, which happens  on automounting USB drives. This is a particular issue when this happens on a locked box where the attacker doesn't otherwise have access to the user account:

  http://www.net-security.org/secworld.php?id=10544

Yes, it still involves physical access, but there's still a difference between sitting down on another person's laptop, rebooting, rooting, and fiddling with the data, or just quickly pluggin in a malicious USB stick on the go.

I don't think this should be too worrysome in practice, but as a precaution I'd suggest to disable automounting in nautilus while the screen saver is active/locked. In particular, I'd add a "GetActive() == True" (on gnome-screensaver's session bus object) to 
volume_added_callback().

Does that sound reasonable to you? Do you have ideas for a better implementation?

I'm happy to work on a patch, but I'd like to discuss the implementation first.

Thanks!
Comment 1 Matthias Clasen 2011-02-12 02:03:45 UTC
I agree with the general idea that we should not automount when the screen is locked.

But note that the automounting code has moved on to gnome-settings-daemon:
http://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/automount
Comment 2 Martin Pitt 2011-02-21 14:55:02 UTC
Created attachment 181462 [details] [review]
(gsd) Disable automounting while screen is locked

First working patch. It is deliberately written with leniency, to not block automount if the screensaver is not running or fails otherwise.

As the screen saver might start later than gnome-settings-daemon, and be restarted in between, don't keep a permanent proxy object for it, but instead grab that at the time when the automount check happens. At that point the plugin needs to wait for the result anyway, so I used a synchronous D-BUS call for this. Do you see anything wrong with that?

Thanks,

Martin
Comment 3 André Klapper 2011-02-21 15:03:13 UTC
Would this also fix security issue bug 642133 or is that unrelated?
Comment 4 Matthias Clasen 2011-02-21 15:09:05 UTC
No, that is a bit different. We used to disable automounting altogether on the login screen, afair. And we should make sure that that continues to be the case after the automounting moved to gsd.
Comment 5 Cosimo Cecchi 2011-02-21 17:31:44 UTC
Martin, thanks for your patch. I refactored it to use GDBus async patterns and pushed it to master [1] (with the addition of a queue of volumes to be mounted after the screensaver is unlocked).

[1] http://git.gnome.org/browse/gnome-settings-daemon/commit/?id=71deedf732a0fa52a58407c271c764f851cb6f57

Closing as FIXED.
Comment 6 Martin Pitt 2011-02-22 07:38:42 UTC
Thanks Cosimo, looks great!

With that I got an assertion crash when plugging in an USB drive while the screen was locked:

ERROR:gsd-automount-manager.c:188:check_volume_queue: assertion failed: (l == NULL)

This assertion doesn't really seem to make sense here -- after all, we _do_ expect that some volumes have queued up in check_volume_queue() once the screen saver got inactive, and the while loop right after the assertion iterates over it. I think it'd be safer to just return from the function if the screen saver is currently active (as check_volume_queue() is called in both cases).

Fixed in

  http://git.gnome.org/browse/gnome-settings-daemon/commit/?id=90c0f8676b06041c599fd941514e09ba8704f0ef

Unfortunately the release came in between yesterday.
Comment 7 Martin Pitt 2011-02-22 13:09:43 UTC
Created attachment 181582 [details] [review]
(nautilus) Disable automounting while screen is locked

In case someone else needs this for current gnome-2-32 nautilus branch, or you want to have this upstream, too: this patch ports thegnome-settings-daemon trunk commits 71deedf7 and 90c0f8676 to nautilus.
Comment 8 Cosimo Cecchi 2011-02-24 17:57:03 UTC
Comment on attachment 181582 [details] [review]
(nautilus) Disable automounting while screen is locked

Martin, after you remove the g_debug() statements (or replace them with nautilus_debug_log() calls), feel free to commit this to the gnome-2-32 nautilus branch.
Comment 9 Martin Pitt 2011-02-24 18:20:07 UTC
Comment on attachment 181582 [details] [review]
(nautilus) Disable automounting while screen is locked

Changed to nautilus_debug_log(), and pushed to gnome-2-32.