<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/129983/">https://git.reviewboard.kde.org/r/129983/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 22nd, 2017, 10:08 a.m. UTC, <b>Elvis Angelaccio</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/129983/diff/5/?file=492986#file492986line1" style="color: black; font-weight: bold; text-decoration: underline;">src/ioslaves/file/kauth/file.actions</a>
<span style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">[org.kde.kio.file.execute]</pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">Name=Perform action as privileged user.</pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">3</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">Description=Perform action as privileged user.</pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">4</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">Policy=auth_admin</pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">5</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">Persistence=session</pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</blockquote>
<p>On March 22nd, 2017, 5:09 p.m. UTC, <b>Chinmoy Ranjan Pradhan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">we could allow a more fine-grained control</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</blockquote>
<p>On March 25th, 2017, 4:30 p.m. UTC, <b>Elvis Angelaccio</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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?</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Does this involve editing the policy file directly or just writing a config file?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</blockquote>
<p>On March 26th, 2017, 9:15 a.m. UTC, <b>Chinmoy Ranjan Pradhan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">it would be very cool if we could actually support this.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Indeed its very cool and useful,but still, we will have all these problems.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">One possible solution can be using the <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">annotate</strong> tag. Polkit doc mentions its <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">used for annotating an action with a key/value pair</strong>. If the <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">value</em> is <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">org.freedesktop.policykit.imply</strong> then <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">the subject if authorized for an action with this annotation is also authorized for any action specified by the annotation</strong>.
So what I did is added this line <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;"><annotate key="org.freedesktop.policykit.imply">org.kde.kio.file.rmdir</annotate></code> after the defaults tag of <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">del</em> 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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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 <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">annotate</em> tag to kauth policy generator.</p></pre>
</blockquote>
<p>On March 26th, 2017, 1:17 p.m. UTC, <b>Elvis Angelaccio</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">imply</em> annotation sounds very promising, might be exactly what we are looking for. Unfortunately I don't know more details about it, at the moment.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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 <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">org.kde.kio.file.delete</code> action that handles everything about deletion: single file, empty folder, non-empty folder, etc.</p></pre>
</blockquote>
<p>On March 27th, 2017, noon UTC, <b>Chinmoy Ranjan Pradhan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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 <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">copy</em>
symlink, put, mkdir ---> to <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">create_</em><strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;"><em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">something</em></strong>
chown, chmod ---> to <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">change_something</em> or perhaps we can leave them
what do you say?</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Another solution (possibly simpler) could be a middle ground between "only one generic action" and "one action per operation".</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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 <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">imply</em> annotation is what we need.
Maybe its meant for this very purpose only. The use case mentioned in the doc - <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">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</strong>.</p></pre>
</blockquote>
<p>On March 28th, 2017, 9:39 p.m. UTC, <b>Elvis Angelaccio</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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 :)</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I have updated my proposal. Is it possible for you to review it for one last time today?</p></pre>
<br />
<p>- Chinmoy Ranjan</p>
<br />
<p>On March 27th, 2017, noon UTC, Chinmoy Ranjan Pradhan wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks, David Faure and Elvis Angelaccio.</div>
<div>By Chinmoy Ranjan Pradhan.</div>
<p style="color: grey;"><i>Updated March 27, 2017, noon</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kio
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This is regarding the GSOC idea https://community.kde.org/GSoC/2017/Ideas#Project:_Polkit_support_in_KIO.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The approach is as follows;
1. Whenever file ioslave gets access denied error it calls the method <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">execWithRoot</em> with the action that requires priviledge, the path of items
upon which action needs to be performed and a warning ID as arguments.
2. <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">execWithRoot</em> then executes the KAuth::Action <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">org.kde.kio.file.execute</em>.
3. This Kauth::Action has its Persistence set too 'session'. This means that after authentication the restrictions are dropped for a while, for <br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
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 <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">execWithRoot</em> should show the usual authentication dialog.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/ioslaves/file/CMakeLists.txt <span style="color: grey">(b9132ce)</span></li>
<li>src/ioslaves/file/file.h <span style="color: grey">(109ea80)</span></li>
<li>src/ioslaves/file/file.cpp <span style="color: grey">(eaf6c88)</span></li>
<li>src/ioslaves/file/file_unix.cpp <span style="color: grey">(82eb11a)</span></li>
<li>src/ioslaves/file/kauth/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/ioslaves/file/kauth/file.actions <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/ioslaves/file/kauth/helper.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/ioslaves/file/kauth/helper.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/129983/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2017/03/09/d42570e8-aedf-4c02-801e-362a68755c2c__polkit_integration.png">warning dialog</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>