Review Request 129983: [RFC] PoC patch for polkit support in kio.
Chinmoy Ranjan Pradhan
chinmoyrp65 at gmail.com
Thu Mar 30 13:59:11 UTC 2017
> On March 22, 2017, 10:08 a.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/kauth/file.actions, lines 1-5
> > <https://git.reviewboard.kde.org/r/129983/diff/5/?file=492986#file492986line1>
> >
> > I don't see the advantage of using a single kauth action. This way you are generating only one "generic" polkit action that you can either enable or disable. Instead if we used one polkit action per operation (del, rmdir, etc.) we could allow a more fine-grained control. For example, a sysadmin could decide that the users can create files in write protected locations, but they cannot delete existing files.
>
> Chinmoy Ranjan Pradhan wrote:
> Some operation may use more than one polkit action like Delete. If we use one polkit action per operation then we will have to provide credentials for every polkit action that operation might use. Though we can add necessary code for this but it will only complicate the matter.
>
> > we could allow a more fine-grained control
>
> Does this involve editing the policy file directly or just writing a config file? In later case we can provide control over execution of actions within the ioslave. 'execWithRoot' can perform a check prior to execution of the polkit action.
>
> Elvis Angelaccio wrote:
> > Some operation may use more than one polkit action like Delete. If we use one polkit action per operation then we will have to provide credentials for every polkit action that operation might use.
>
> But only if the actions are different, right? Otherwise we should be fine within the 5 minutes threshold after the first authentication. Can you show some concrete examples where this issue would happen?
>
> > Does this involve editing the policy file directly or just writing a config file?
>
> Have a look at polkit rules. For example, one could write a rule that says "org.kde.kio.file.copy is allowed, org.kde.kio.file.del is not." and it would be very cool if we could actually support this.
>
> Chinmoy Ranjan Pradhan wrote:
> >But only if the actions are different, right? Otherwise we should be fine within the 5 minutes threshold after the first authentication. Can you show some concrete examples where this issue would happen?
>
> Delteing a non-empty directory
> In this operation 'del' and 'rmdir' actions are used. In this case we will have to authenticate for both the actions. And after the first authentication, doing a similar operation will show the warning dialog twice.
>
> Copy operation can also be considered.
> This operation will consist of six actions which are file_open, sendfile, read, write, file_close, chown. Although all the actions will not be used at once usage of atleast two can be expected. Here also we will have to authenticate ouself atleast twice just for copying a single file which is very annoying.
>
> >it would be very cool if we could actually support this.
>
> Indeed its very cool and useful,but still, we will have all these problems.
>
> One possible solution can be using the **annotate** tag. Polkit doc mentions its **used for annotating an action with a key/value pair**. If the _value_ is **org.freedesktop.policykit.imply** then **the subject if authorized for an action with this annotation is also authorized for any action specified by the annotation**.
> So what I did is added this line ```<annotate key="org.freedesktop.policykit.imply">org.kde.kio.file.rmdir</annotate>``` after the defaults tag of _del_ action. After this when I tried deleting a non-empty folder I only got one auth dialog. One more thing I noticed is, with this line in place, when I deleted a single file deleting an empty folder afterwards only showed one warning even though the action was different and was executing for this first time.
>
> I'm still not sure about this so I might be completely wrong as well. If you have any knowledge of this please let me know if I got everything or anything right. And if what I have mentioned is indeed correct then we will have to add the support for _annotate_ tag to kauth policy generator.
>
> Elvis Angelaccio wrote:
> The *imply* annotation sounds very promising, might be exactly what we are looking for. Unfortunately I don't know more details about it, at the moment.
>
> Another solution (possibly simpler) could be a middle ground between "only one generic action" and "one action per operation". For example, we could have a `org.kde.kio.file.delete` action that handles everything about deletion: single file, empty folder, non-empty folder, etc.
>
> Chinmoy Ranjan Pradhan wrote:
> Merging actions sounds good. I have updated the patch as per your suggestion. We can merge some other action as well, like,
> file_open, sendfile, read, write, file_close --> to _copy_
> symlink, put, mkdir ---> to _create__**_something_**
> chown, chmod ---> to _change_something_ or perhaps we can leave them
> what do you say?
>
> > Another solution (possibly simpler) could be a middle ground between "only one generic action" and "one action per operation".
>
> It is only a part of solution. Say, if a user wants to perform a privileged copy after a privileged delete then he must be shown a warning instead of authentication dialog no? For this to happen we need some way to authorize the copy action without any interaction as soon as delete is authorized and I now strongly believe _imply_ annotation is what we need.
> Maybe its meant for this very purpose only. The use case mentioned in the doc - **A typical use of this annotation is when defining an UI shell with a single lock button that should unlock multiple actions from distinct mechanisms**.
>
> Elvis Angelaccio wrote:
> Right, the two things are not mutually exclusive. We could define meta actions like "delete" or "create" but also use the imply annotation if there is a specific need for it. The latest diff looks good to me now, please update the proposal accordingly :)
I have updated my proposal. Is it possible for you to review it for one last time today?
- Chinmoy Ranjan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102932
-----------------------------------------------------------
On March 27, 2017, noon, Chinmoy Ranjan Pradhan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129983/
> -----------------------------------------------------------
>
> (Updated March 27, 2017, noon)
>
>
> Review request for KDE Frameworks, David Faure and Elvis Angelaccio.
>
>
> Repository: kio
>
>
> Description
> -------
>
> This is regarding the GSOC idea https://community.kde.org/GSoC/2017/Ideas#Project:_Polkit_support_in_KIO.
>
> This patch intends to demonstrate one possible approach to provide polkit support in kio. Here its only for the delete operation. This is based on the patch in task https://phabricator.kde.org/T5070.
>
> The approach is as follows;
> 1. Whenever file ioslave gets access denied error it calls the method *execWithRoot* with the action that requires priviledge, the path of items
> upon which action needs to be performed and a warning ID as arguments.
> 2. *execWithRoot* then executes the KAuth::Action *org.kde.kio.file.execute*.
> 3. This Kauth::Action has its Persistence set too 'session'. This means that after authentication the restrictions are dropped for a while, for
> about 5 minutes. This is similar to the behaviour of sudo command.
> 4. During this time we can perform any action as a privileged user without any authentication. So to prevent any mishap i added a warning box which
> would popup before performing any action(only during this period).
> 5. After the said time interval the root privileges are droped and calling *execWithRoot* should show the usual authentication dialog.
>
>
> Diffs
> -----
>
> src/ioslaves/file/CMakeLists.txt b9132ce
> src/ioslaves/file/file.h 109ea80
> src/ioslaves/file/file.cpp eaf6c88
> src/ioslaves/file/file_unix.cpp 82eb11a
> src/ioslaves/file/kauth/CMakeLists.txt PRE-CREATION
> src/ioslaves/file/kauth/file.actions PRE-CREATION
> src/ioslaves/file/kauth/helper.h PRE-CREATION
> src/ioslaves/file/kauth/helper.cpp PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/129983/diff/
>
>
> Testing
> -------
>
>
> File Attachments
> ----------------
>
> warning dialog
> https://git.reviewboard.kde.org/media/uploaded/files/2017/03/09/d42570e8-aedf-4c02-801e-362a68755c2c__polkit_integration.png
>
>
> Thanks,
>
> Chinmoy Ranjan Pradhan
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170330/6cc7cdd8/attachment-0001.html>
More information about the Kde-frameworks-devel
mailing list