GNOME Bugzilla – Bug 771285
Polkit prompt is shown from file chooser without user interaction
Last modified: 2017-08-08 12:36:09 UTC
The polkit prompt should not be shown without user interaction with admin:/// filesystem... Steps to reproduce: 1/ $ gedit admin:///file 2/ close gedit 3/ $ gedit 4/ ctrl+o 5/ polkit prompt is shown, because admin:///file is in recent files and queryinfo is called on it from some reason... Password prompt is not shown for other GVfs locations in such case, because GVfs simply returns G_IO_ERROR_NOT_MOUNTED. Unfortunatelly admin backend is automounted and polkit prompt is not under our control. This bug report is rather "tracker bug" for this issue, because we can't do much with it in GVfs probably... can we?
I think it would be sensible for the GTK recent files machinery to just ignore admin:/// URLs, or turn them into the non-admin URL. Or to put it another way, the fact that a file was accessed through the admin backend is a property of that individual interaction, rather than something that should be recorded as a separate URI.
*** Bug 776256 has been marked as a duplicate of this bug. ***
Copying Matthias. Do you think my above proposal is a good way to fix this?
Sorry, I forgot on this bug a bit. I agree with the proposed way to fix, I don't see any easy way how to fix it on gvfs side. Let's change the product...
I think we should turn admin:// into the file://, it is still the same file. Maybe we want always use G_FILE_ATTRIBUTE_STANDARD_TARGET_URI if available instead of the origin uri...
Why would GtkRecentManager have to do this? GtkRecentManager only gives out the URI from the list of recently used files; the component that is trying to access the files is the file chooser. If the URI scheme is admin://, I guess the file chooser should either filter it out, or should not require a privilege escalation. To be fair, I don't think GIO/GVFS should require a privilege escalation for querying the file information either, unless the file is readable only by the super user — in which case it should probably not list the file at all.
(In reply to Cosimo Cecchi from comment #3) > Copying Matthias. Do you think my above proposal is a good way to fix this? Yes, I think that is right. Showing a recent file list should not spontaneously pop up a polkit dialog.
(In reply to Emmanuele Bassi (:ebassi) from comment #6) > Why would GtkRecentManager have to do this? Well, we can make one helper in GtkRecentManager and we are done..., or we have to change all GTK+ applications which use GtkRecentManager otherwise. > GtkRecentManager only gives out the URI from the list of recently used > files; the component that is trying to access the files is the file chooser. > > If the URI scheme is admin://, I guess the file chooser should either filter > it out, or should not require a privilege escalation. Maybe the problem is rather in applications which write there such locations... > To be fair, I don't think GIO/GVFS should require a privilege escalation for > querying the file information either, unless the file is readable only by > the super user — in which case it should probably not list the file at all. It would be nice to allow querying info without the popup, however, we can't fill the info for files, which can't be listed without the permissions...
Hmm, I think that it might be possible to record client's PIDs in the backend and return some error if the client with this pid did not call the mount operation explicitly, however, this would require some infrastructure changes in GIO/GVfs and also some additional client handling... The idea: We can add G_IO_ERROR_NEED_REMOUNT and implement an infrastructure for remounting. Then each client would have to call g_file_mount, or g_mount_remount if they really want to access those files. So the polkit prompt will be shown only if g_file_mount/g_mount_remount was called for that client. This needs additional work for clients also, but it might be partially handled by the file chooser... Alex, don't you have also some ideas here?
Why is the admin backend automounted? We made mounting explicit for a reason in gvfs. It used to be automatic back in the gnome-vfs days, and it caused all sorts of problems like this, because code in random places does i/o on uris, not expecting this to do user interaction. Of course, in gnome-vfs this caused reentrancy in gtk+, which was even worse, but its pretty bad when it happens out-of-process too. We can special case this in recent files, but that will not be enought. This issue is going to pop up in all sort of places.
As far as I can tell it is especially because we want to use the same uri for all clients, but the authorization is based on PID, so the polkit prompt is shown for each client and not per mount. Explicit mounting would not work this way. The side-effect of automounting is that it also prevents access over FUSE mount point.
Hmm, can't we create uri mapper for admin backend and add pid() internally in mountspec? So the same uri will be used for all clients, but it will be handled internally by different mounts? I have to investigate whether this is possible, or not...
Created attachment 351534 [details] [review] admin: Require mounting for each client explicitly The admin backend is pretty special, because it can't use GMountOperation for authorization and polkit prompt is shown for each client. This leads to unwanted behavior because the admin prompt might be shown unexpectedly (e.g. when obtaining info for recently used files). Let's require mounting explicitly for each client. So each client gets G_IO_ERROR_NOT_MOUNTED if it hasn't called g_file_mountable_mount before. It works nicely for most of the apps which I tested (e.g. with Nautilus, GEdit, Totem, Evince, GIMP, LibreOffice). However, this requires changes for some applications, which expects that the file is already mounted (e.g. EOG). Unfortunatelly, it breaks utils like "gio list" because it fails with "The specified location is not mounted" error and "gio mount admin:///" doesn't help, because it has different PID. This isn't ideal, but it is better than the unexpected password prompts...
Created attachment 351615 [details] [review] admin: Require mounting for each client explicitly Add missing client/adminuri.c file.
Created attachment 351872 [details] [review] admin: Require mounting for each client explicitly Fixed problem with mount prefixes.
I've also proposed a patch for EOG, see Bug 555831.
Review of attachment 351872 [details] [review]: I don't think that approach will work, because the pid in the client may not match the pid in the daemon. For instance, if that app is running in flatpak, then it will not match due to pid namespaces. However, if we use the client dbus unique id instead of the pid, things should work fine. I don't think the adminuri stuff is necessary though. I think its better to special-case on the daemon side. Basically, in handle_lookup_mount(), for admin uris, we add g_dbus_method_invocation_get_sender(invocation) to the mount-spec. You might need to special case some more places though. For instalce list_mounts should probably filter out admin mounts for other clients.
Thanks for the review! Ok, I didn't know about different pids. It seems that dbus id works nicely in this case, but it seems that admin backend doesn't work with flatpak anyway, because polkit authorization checks rely on process pid, but it seems that peer credentials can't be obtained from dbus in flatpak probably... see Bug 784893. I used that way, because I wanted to avoid some special-casing in mount.c code, but ok, I will introduce MountPerClient option instead...
Created attachment 355486 [details] [review] admin: Require mounting for each client explicitly Let's handle this on daemon side only...
Attachment 355486 [details] pushed as 8e9439e - admin: Require mounting for each client explicitly Alex is on a long vacation, so he won't give another review these days and we have to do something with, so let's push this before today release finally...
This seems to have made things much worse for me. I have used gedit to open some admin:/// files in the past. Because of that, every time I initially open gedit, I got a single password prompt. With gvfs 1.33.90, when I open gedit, I now get about 4 password prompts. If I then click the Open button, I get another 4. If I close the Open menu and click it again, I get the same 4 password prompts all over again.
Thanks for testing Jeremy, this isn't really intended. Isn't this just some problem when updating without rebooting? Does this happen to you also after restart ("pkill gvfs" should be enough)? If so, can you please provide gvfsd debug log for the beginning ("pkill gvfs; GVFS_DEBUG=1 $(find /usr/lib* -name gvfsd 2>/dev/null) >gvfsd.log 2>&1")?
Ondrej, after restarting my computer, my original bug is fixed: I don't get password prompts just for opening gedit when I've opened admin:// files in the past. Thanks!