Looking for co-mentors for a GSoC Dolphin/KIO project

[Elvis Angelaccio]

On Thu, Jan 26, 2017 at 2:31 AM, Aleix Pol <aleixpol at kde.org> wrote:
> On Wed, Jan 25, 2017 at 11:33 PM, Elvis Angelaccio
> <elvis.angelaccio at kde.org> wrote:
>> On Wed, Jan 25, 2017 at 3:43 AM, Aleix Pol <aleixpol at kde.org> wrote:
>>> On Tue, Jan 24, 2017 at 11:59 PM, Elvis Angelaccio
>>> <elvis.angelaccio at kde.org> wrote:
>>>> On Mon, Jan 23, 2017 at 4:15 AM, Aleix Pol <aleixpol at kde.org> wrote:
>>>>> On Sat, Jan 14, 2017 at 11:58 PM, Elvis Angelaccio
>>>>> <elvis.angelaccio at kde.org> wrote:
>>>>>> Hi,
>>>>>> I played a bit with kauth in the file ioslave [1] and everything seems
>>>>>> to work nicely :)
>>>>>> [2] is a patch for kio that allows to delete file:// URLs upon polkit
>>>>>> authentication.
>>>>>>
>>>>>> The helper just calls unlink(). In the patch I call the helper (from
>>>>>> the ioslave) only if unlink() failes with EACCES or EPERM errors.
>>>>>> This could be seen as code duplication (because we have unlink in two
>>>>>> places), one of the things Aleix was afraid of. On the other hand, why
>>>>>> call an helper process if you don't need it?
>>>>>>
>>>>>> Cheers,
>>>>>> Elvis
>>>>>>
>>>>>> [1]: https://phabricator.kde.org/T5070
>>>>>> [2]: https://phabricator.kde.org/file/data/mcjxwtqj2hijhebldlgb/PHID-FILE-ceznjt2trea7lli6v5n6/kauth.diff
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Sun, Jan 8, 2017 at 8:32 PM, Elvis Angelaccio
>>>>>> <elvis.angelaccio at kde.org> wrote:
>>>>>>> On Sun, Jan 8, 2017 at 8:14 PM, Aleix Pol <aleixpol at kde.org> wrote:
>>>>>>>> On Sun, Jan 8, 2017 at 7:48 PM, Elvis Angelaccio
>>>>>>>> <elvis.angelaccio at kde.org> wrote:
>>>>>>>>> Hi David and everyone,
>>>>>>>>> GSoC 2017 is near and it's time to discuss project ideas.
>>>>>>>>> This year I want to tackle one of the biggest problems in Dolphin:
>>>>>>>>> lack of polkit support. Dolphin users often don't know how to use the
>>>>>>>>> command line, so when they have to move root-owned files around they
>>>>>>>>> are forced to start Dolphin with root permissions, which is bad and
>>>>>>>>> dangerous.
>>>>>>>>>
>>>>>>>>> GSoC is a great opportunity and I'm confident we can find an awesome
>>>>>>>>> student who will fix this. :)
>>>>>>>>>
>>>>>>>>> I started a draft of the project idea in [1] and I'm looking for
>>>>>>>>> co-mentors and feedback.
>>>>>>>>> In particular @David:
>>>>>>>>> 1) Am I right to claim that KAuth support needs to go only in the file
>>>>>>>>> protocol? i.e. basically refactor into KAuth actions the parts of
>>>>>>>>> file.cpp and file_unix.cpp where the actual interaction with the
>>>>>>>>> filesystem happens.
>>>>>>>>> 2) Is this even possible from a techical point of view? Or maybe KAuth
>>>>>>>>> and ioslaves don't play well together?
>>>>>>>>>
>>>>>>>>> You should be able to edit the shared document, I hope. Please let me
>>>>>>>>> know if that's not the case.
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>> Elvis
>>>>>>>>>
>>>>>>>>> [1]: https://share.kde.org/index.php/s/bRBrrkOkuSCbkhT
>>>>>>>>
>>>>>>>> Hi Elvis,
>>>>>>>> This is a good idea and I see tha it can be useful. I see some
>>>>>>>> problems though, that could lead the wrong people to the task.
>>>>>>>>
>>>>>>>> How do we want to implement this? Do we want to be able to run
>>>>>>>> kioslaves as root? Or do we want to Implement kauth/policykit support
>>>>>>>> in the file kioslave?
>>>>>>>
>>>>>>> Yes, kauth in the file kioslave is what I was thinking. Running the
>>>>>>> whole kioslave as root sounds dangerous indeed.
>>>>>>>
>>>>>>>> In either case, no modifications in Dolphin should be required. I'm
>>>>>>>> leaning towards the latter, though.
>>>>>>>>
>>>>>>>> I think it could be interesting to think about the security
>>>>>>>> implications it would have to have a file kio that can escalate to
>>>>>>>> root, note that this will extend to all applications that use KIO, not
>>>>>>>> just dolphin.
>>>>>>>
>>>>>>> Yep indeed. The file slave should only delegate the minimal amount of
>>>>>>> work necessary to the root helper (this probably needs some
>>>>>>> refactoring).
>>>>>>>
>>>>>>>>
>>>>>>>> Also if this is not done properly, we could end up with the whole kio
>>>>>>>> file duplicated into a kauth helper. I'd say it's worth investigating
>>>>>>>> how the student will implement this. It's well-known by now that it's
>>>>>>>> not the best idea to have the students design the approach they'll
>>>>>>>> take.
>>>>>>>
>>>>>>> Right, the approach definitely needs to be clear before the student
>>>>>>> starts. Also because this project involves kio, ioslaves, kdeinit,
>>>>>>> kauth...
>>>>>>> Really easy to get lost in all this stuff for a new contributor :)
>>>>>
>>>>> This is interesting.
>>>>>
>>>>> You'd have 1 helper per call?
>>>>
>>>> Well, one kauth action per call. But yes, that means one helper
>>>> process spawned per call, right?
>>>
>>> I'm pretty sure you can have different actions in the same helper.
>>
>> Yep that's what I meant, one helper with many actions.
>>
>>>
>>>> I don't think this is going to be a problem, I think the average
>>>> Dolphin user just wants to (e.g.) remove one or few files at the same
>>>> time.
>>>
>>> While your motivation is Dolphin, this will be used by many other
>>> applications. I think it's worth taking that into account and giving
>>> it some thought.
>>
>> Sure. I do have some open questions.
>> In particular, let's say someone is using KIO::del() inside a loop:
>>
>> * Does polkit cache the password? Or the user needs to type it again
>> in the next loop iteration?
>> * If kauth spawns a process for every iteration, is this going to be a
>> problem with huge loops?
>
> It's properly documented, there's few settings that can be used. Like
> "ask every time" or "keep the password" and few others.
>
> Problem is essentially that while kauth works nothing is very safe as
> soon as it relies on dbus.
>
>> We could also make the whole thing opt-in, if we want to play safe.
>> Something like:
>>
>> auto job = KIO::del(...);
>> job->addMetaData("kauth", "true");
>>
>> and then the file ioslave only calls the helper if it finds the
>> metadata above set to true.
>
> I don't think making it hard to reach will help anyone. We need to
> adopt a KIO mindset rather than Dolphin needs kio to do this thing,
> just to get the big picture.

Hi Aleix,
after thinking more about this (also because of Martin's blogpost and
kate commit :D), I now agree that the project should be targeted more
as "kio project", rather than a Dolphin one. Basically, what we want
is a kauth-aware file ioslave *first*, and then making sure that the
main apps (Dolphin/Kate) can use it. On the kate side it should just
work, on the Dolphin side it will need some work but surely not enough
to fill half of the 3 months of coding. I'm going to reword the
project idea on the wiki in the next days to better reflect this.

Btw, Emmanuel pushed a commit [1] similar to the kate one, disabling
root in dolphin master :)

Cheers,
Elvis

[1]: https://cgit.kde.org/dolphin.git/commit/?id=0bdd8e0b0516555c6233fdc7901e9b417cf89791

>
>>>
>>>>> Also do we see this as full support of privileged access or just some
>>>>> helpers?
>>>>
>>>> I'm not sure what you mean here, can you expand?
>>>
>>> Well the question was if you'd want this GSoC (remember, ~3 solid
>>> months of work) for a full mapping of the file kioslave for actions to
>>> be ran as root, or just few actions.
>>>
>>> By your reply I understand you're leaning towards supporting whatever
>>> seems necessary. Which can make sense.
>>
>> Ok I see it now. Yes, the latter (hopefully...)
>>
>>>
>>>>> What do you see happening on more complex and delicate things
>>>>> like reading and writing files?
>>>>
>>>> I think we can even skip the reading case? Either the file is
>>>> world-readable, or it's a file in /var or /proc or whatever and we
>>>> probably don't want Dolphin users to touch it...
>>>
>>> If you implement reading, your Dolphin users will be able to do
>>> something with the files when they click on them (as kwrite will be
>>> able to read the contents).
>>
>> Right. Well the reading case is very similar to writing, after all.
>> Should be doable, indeed :)
>>
>>>
>>>> What we need in the helper is just the writing case imho. Given that
>>>> FileProtocol::copy() is quite complex, to avoid code duplication in
>>>> the helper we could refactor and move that code to a private library
>>>> that would be used both in copy() and the helper action.
>>>
>>> HTH,
>>> Aleix
>>
>> Cheers,
>> Elvis