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 763890 - GVFS should sanely handle out of band LUKS unlocks
GVFS should sanely handle out of band LUKS unlocks
Status: RESOLVED FIXED
Product: gvfs
Classification: Core
Component: udisks2 volume monitor
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gvfs-maint
gvfs-maint
Depends on:
Blocks:
 
 
Reported: 2016-03-18 18:00 UTC by Nathaniel McCallum
Modified: 2016-04-18 07:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
udisks2: Lock unlocked volumes on eject/stop (11.30 KB, patch)
2016-03-24 09:35 UTC, Ondrej Holy
committed Details | Review
udisks2: Abort mount operation if volume is unlocked (draft) (4.51 KB, patch)
2016-03-24 11:33 UTC, Ondrej Holy
none Details | Review
udisks2: Abort mount operation if volume is unlocked (5.43 KB, patch)
2016-04-06 14:15 UTC, Ondrej Holy
committed Details | Review

Description Nathaniel McCallum 2016-03-18 18:00:09 UTC
For background, read this thread:
https://lists.freedesktop.org/archives/devkit-devel/2016-March/001754.html

Basically, GVFS presumes it is the only client to UDisks2. So if a disk is unlocked using some automated procedure, GVFS doesn't cope very well.

There are two incorrect behaviors that I have noted.

First, there is a race condition for unlock. If the block device is unlocked while the modal unlock dialog is displayed, it is not dismissed automatically.

Second, if you cancel the model dialog, an error is generated when attempting to eject the disk.
Comment 1 Bastien Nocera 2016-03-18 18:05:03 UTC
Can you explain as simply as possible how to reproduce the bug, possibly without the use of custom software?
Comment 2 Nathaniel McCallum 2016-03-18 18:12:22 UTC
I provided a reproducer in this message to the mailing list:
https://lists.freedesktop.org/archives/devkit-devel/2016-March/001757.html

Namely, create an encrypted usb key with the password
"foo". Then, execute the following in a terminal and immediately insert
the usb key (I am presuming here your usb key is on /dev/sdc):

sleep 10 ; busctl call org.freedesktop.UDisks2
/org/freedesktop/UDisks2/block_devices/sdc
org.freedesktop.UDisks2.Encrypted Unlock "sa{sv}" foo 0

The above command will sleep for enough time for the dialog box to appear and then unlock the disk. Notice that the dialog does not disappear. Then, cancel the dialog and attempt to eject the disk.

None of this requires custom software.
Comment 3 Ondrej Holy 2016-03-22 12:45:59 UTC
(In reply to Nathaniel McCallum from comment #0)
> For background, read this thread:
> https://lists.freedesktop.org/archives/devkit-devel/2016-March/001754.html
> 
> Basically, GVFS presumes it is the only client to UDisks2. So if a disk is
> unlocked using some automated procedure, GVFS doesn't cope very well.
> 
> There are two incorrect behaviors that I have noted.
> 
> First, there is a race condition for unlock. If the block device is unlocked
> while the modal unlock dialog is displayed, it is not dismissed
> automatically.

That's true and I am able to reproduce this using the provided steps from Comment 2 (i.e. without the Tang software).

The volume monitor doesn't expect that something else can unlock the disk and  system modal dialog with password prompt is shown by gnome-shell. 

We might cancel the mount operation if we get some signal from udisks (as we already do when the device is unplugged), but consequently the volume won't be automounted...

> Second, if you cancel the model dialog, an error is generated when
> attempting to eject the disk.

What error? Where do you see the error? How was the disk ejected?
Comment 4 Ondrej Holy 2016-03-22 12:50:09 UTC
So, it would be best probably to add support for Tang project in the udisks2 volume monitor to avoid the race and do proper automount. In do_unlock() is already check for crypttab file and for password in keyring, so we might check also for Tang and handle it there before showing the password prompt...
Comment 5 Ondrej Holy 2016-03-24 09:21:16 UTC
Ok, I realized what is wrong with ejecting and see following error:
"Cannot eject drive in use: Encrypted device /dev/sdb1 is unlocked"

The problem is when you try to eject the device, which is already unlocked, but not mounted. This is because that locking and unlocking is handled by mount/unmount operations, however the volume is not mounted and thus is not locked properly on eject...
Comment 6 Ondrej Holy 2016-03-24 09:35:32 UTC
Created attachment 324661 [details] [review]
udisks2: Lock unlocked volumes on eject/stop

The eject/stop operation fails currently with "Cannot eject drive in use: Encrypted device /dev/sdb1 is unlocked" if any volume is unlocked and isn't mounted. The volume monitor is able to mount already unlocked volumes, so it should be able to eject unlocked volumes also. Lock the unlocked volumes if there are any before eject/stop operation.
Comment 7 Ondrej Holy 2016-03-24 11:30:17 UTC
So, the attached patch should fix the second issue, however I am not really sure what to do with the first one, because it seems Tang doesn't have any public api...

Gnome shell automounter tries to mount the volumes if drive is connected. In the same time Tang tries to unlock the volumes. Volume monitor might cancel current mount operation and close the dialog if something unexpected happens (e.g. the volume has been unlocked), but then the volume won't be automounted (it will be e.g. unlocked only). However I don't think it is reasonable to continue the mount operation if something is happening with the device from outside...
Comment 8 Ondrej Holy 2016-03-24 11:33:46 UTC
Created attachment 324670 [details] [review]
udisks2: Abort mount operation if volume is unlocked (draft)

There is a patch which closes the dialog and continue in mount operation. The patch is a result of my testing, however I am not really convinced that this is right way to do...
Comment 9 Nathaniel McCallum 2016-03-24 15:39:10 UTC
Let's walk through an example here of the proper way to use the UDisks2 API.

GNOME should offer two state transitions:
1. Locked => Unlocked
2. Unmounted => Mounted

It is important to note that there is no relationship between them. Both state transitions are initiated by signals from /org/freedesktop/UDisks2 (org.freedesktop.DBus.ObjectManager).

The transition from unmounted to mounted is triggered by a new org.freedesktop.UDisks2.Block interface on an object which has no org.freedesktop.UDisks2.Encrypted interface. Other policy may apply (including possibly dereferencing CryptoBackingDevice during policy evaluation).

The transition from locked to unlocked is triggered by a new org.freedesktop.UDisks2.Encrypted interface. This process is canceled by either the removal of the org.freedesktop.UDisks2.Encrypted interface or the appearance of a new org.freedesktop.UDisks2.Block interface where CryptoBackingDevice maps to the object of the current state change. Other policy may apply.

As mentioned above, the two state transitions have no logical connection between them. When the state changes from locked to unlocked, UDisks2 will automatically create the proper objects which may, in turn, start the mounting state change.
Comment 10 Ondrej Holy 2016-03-29 12:14:29 UTC
I'd like to just notice, that the volume monitor was written by David Zeuthen, author of UDisks, so I suppose that he knew, how to properly use UDisks2 API. 

GNOME is based on high-level GIO filesystem abstraction (which is part of GLib). GVfs provides a wide range of virtual filesystems, so it is not only for devices represented by UDisks2. GIO API provides high-level mount operation which can ask for a password if needed. The mount procedure is designed to be universal for all possible GVolume objects. So LUKS encrypted device may be mounted in a same way as e.g. SMB shares. So mount operation (in GIO terms) internally handles both - unlocking and mounting (in UDisks terms) - for LUKS encrypted devices and vice versa for unmounting.

Also both - locked and unlocked block - is represented by one GVolume object.

You suggest that we should offer two state transition. I'm not sure what you mean exactly with it and how it can help us...

Volume monitor knows internally that the volume is locked/unlocked, mounted/unmounted. It is able to mount both locked and unlocked. It is also able to eject devices with volumes in all those states (which consists from unmount and/or unlock operations in terms of UDisks) with the first patch. There is just problem, that something unexpected happen with the volume during mount operation (i.e. it is unlocked externally in the meantime)...
Comment 11 Nathaniel McCallum 2016-03-29 13:39:03 UTC
(In reply to Ondrej Holy from comment #10)
> GNOME is based on high-level GIO filesystem abstraction (which is part of
> GLib). GVfs provides a wide range of virtual filesystems, so it is not only
> for devices represented by UDisks2. GIO API provides high-level mount
> operation which can ask for a password if needed.

This is the problem: mounting and passwords are conflated. In our case, we need to cancel password prompting without canceling mounting.

> You suggest that we should offer two state transition. I'm not sure what you
> mean exactly with it and how it can help us...

There are three states:

1. Locked, Unmounted
2. Unlocked, Unmounted
3. Unlocked, Mounted

Thus there are two state changes:
* #1 => #2
* #2 => #3

The password prompt should be offered during the first state change. It should disappear when the state reaches #2 regardless of its own internal state.

> Volume monitor knows internally that the volume is locked/unlocked,
> mounted/unmounted. It is able to mount both locked and unlocked. It is also
> able to eject devices with volumes in all those states (which consists from
> unmount and/or unlock operations in terms of UDisks) with the first patch.
> There is just problem, that something unexpected happen with the volume
> during mount operation (i.e. it is unlocked externally in the meantime)...

It seems to me like we are constrained by the following contracts:
https://developer.gnome.org/gio/stable/GVolume.html#g-volume-mount
https://developer.gnome.org/gio/stable/GMountOperation.html

As I understand it, gvfs needs to detect that the device was already unlocked (via the signal from udisks2), finish the mounting operation and then emit "reply" on GMountOperation. Is that not correct?
Comment 12 Nathaniel McCallum 2016-03-29 13:57:08 UTC
Unfortunately, gnome-shell doesn't listen for the reply signal.

https://git.gnome.org/browse/gnome-shell/tree/js/ui/shellMountOperation.js#n112
Comment 13 Ondrej Holy 2016-03-29 15:21:07 UTC
(In reply to Nathaniel McCallum from comment #11)
> (snap)
> 
> As I understand it, gvfs needs to detect that the device was already
> unlocked (via the signal from udisks2), finish the mounting operation and
> then emit "reply" on GMountOperation. Is that not correct?

This is basically what attachment 324670 [details] [review] tries to do... and unfortunately I don't see a better way, how to handle this though I am not really satisfied with that... 

(In reply to Nathaniel McCallum from comment #12)
> Unfortunately, gnome-shell doesn't listen for the reply signal.

Hmm, yes, but the prompt disappears, however probably after the mount is finished...
Comment 14 Ondrej Holy 2016-03-29 15:26:13 UTC
Can you try the attached patches? Does it fix the issues you mentioned?
Comment 15 Nathaniel McCallum 2016-03-29 16:39:04 UTC
No. I don't see any changes with the patches applied.

1. Unlock succeeds.
2. Mount does not.
3. The dialog never disappears.
Comment 16 Ondrej Holy 2016-03-30 15:44:01 UTC
Hmm, thanks for testing. How did you test it? Using the sleep and busctl call... or using Tang? Are you sure that your newly built udisks2 monitor was executed, not the system one?
Comment 17 Nathaniel McCallum 2016-03-30 15:53:18 UTC
I tested using tang. I patched the rpm of gvfs, rebuilt and installed it. Then I restarted gnome-shell. I hope that was sufficient. If not, I can test again on Friday (I'm OOO until then).
Comment 18 Ondrej Holy 2016-03-31 09:29:00 UTC
Hmm, it is not sufficient if you just restart gnome-shell without logging off. You can kill and spawn gvfs manually instead, e.g.:
pkill gvfs; gvfs-mount -l
Comment 19 Nathaniel McCallum 2016-04-04 16:22:18 UTC
On a clean Fedora 23 install (no tang), I did the following:

1. I downloaded the source RPM (gvfs-1.26.3-1.fc23) and added the patch to the spec file and rebuilt. The patch applied cleanly and the build succeeded. The only other change was to bump the build number.

2. I updated to the new RPMs (gvfs*-1.26.3-2.fc23.x86_64).

3. I rebooted.

4. I did the sleep test from comment 2. The dialog didn't disappear.
Comment 20 Nathaniel McCallum 2016-04-05 16:50:30 UTC
It appears I missed the first patch. After including both patches, it worked for both the sleep test and the tang daemon.

However, the tang-luks-udisks2 daemon unlocks the disk as root. This causes the mount process to prompt the user for sudo permissions in order to mount a disk unlocked by another user. I could run tang-luks-udisks2 daemon as the user, but this would allow the user to obtain the automated unlocking key -- something I'd like to avoid.

Suggestions?
Comment 21 Nathaniel McCallum 2016-04-05 17:02:29 UTC
I think this last problem seems like my problem (we haven't really thought about multi-user since we have been doing early boot up until this point). In short, the patches work. Can they be merged?
Comment 22 Ondrej Holy 2016-04-06 14:15:35 UTC
Created attachment 325480 [details] [review]
udisks2: Abort mount operation if volume is unlocked
Comment 23 Ondrej Holy 2016-04-06 14:20:04 UTC
Thanks for a testing. I pushed the patches as commit dd98c02 and commit beea21e (gvfs is not branched yet, so the patches will be part of gnome-3-20 also).
Comment 24 Ondrej Holy 2016-04-18 07:43:28 UTC
It seems that the attachment 325480 [details] [review] causes Bug 765146...