Fix security issues with KAuth support in KIO
Open, HighPublic

Description

KAuth support in KIO is currently disabled due to following security issues:

  • 1. The privilege is persistent for the entire session.
  • 2. The confirmation prompt for the kauth action use does not tell what is going to happen. So you might open a file dialog and then instead of opening a file, write to /bin/sh.
  • 3. Trivial stack-based buffer overflow in the kauth helper: https://cgit.kde.org/kio.git/tree/src/ioslaves/file/sharefd_p.h#n57
  • 4. The socket used to send and receive file descriptors does not have any kind of permission check.
  • 5. Having KIO::Job show a prompt achieves nothing. An application can easily bypass it.

Possible solutions:
Issue 1
Try to revoke authorization of slave and if unsuccessful delete the slave in klauncher.
D10818 D10822 D10437: Store authorization status
D10820 : Send authorization status
D10641 : Revoke authorization
D10824 : Delete slave if revoking wasn't done or unsuccessful

Issue 2
Maybe a "Details" button in the prompt will solve it. Still need more info.

Issue 3
Don't use strcpy.
D10273 : Create proper socket address structure

Issue 4
Create socket in user's runtime directory and accept file descriptor only form root process.
D10410 : Secure socket to app connection
D10409 D10411 : Secures app to socket connection

Issue 5
Show prompt from slave's side
D10567 D10568 : Handle prompt in KIO slave

Edit:
Some problems with ktexteditor's privilege escalation, and possible improvements are mentioned here:
https://bugzilla.suse.com/show_bug.cgi?id=1033055#c13

Listed below are some of those improvements that (I think) should be made in KIO as well:

  • 6. Improve message in polkit prompt (issue no. 2). BTW how do I fix this?
  • 7. Don't elevate privilege if the directory is read-only and user is the owner.
  • 8. If owner of target directory is not root then drop privileges (rejecting the whole operation might be impractical).
chinmoyr triaged this task as High priority.
ngraham updated the task description. (Show Details)Apr 4 2018, 8:49 PM
fvogt added a comment.Apr 5 2018, 6:55 AM

KAuth support in KIO is currently disabled due to following security issues:

I just noticed that the KAuth integration in kio isn't actually disabled at all - kio itself doesn't use it anymore but anything else still can. Intentional?

Not at all intentional. It just didn't crossed my mind. I suppose the cmake file needs to be patched to not build the helper?

fvogt added a comment.Apr 5 2018, 7:51 AM

Not at all intentional. It just didn't crossed my mind. I suppose the cmake file needs to be patched to not build the helper?

Yup, or just not install it.

The security review by the SUSE security team of an analogous change in ktexteditor highlighted some problems there. @chinmoyr, you may want to look the review up for reference:

https://bugzilla.suse.com/show_bug.cgi?id=1033055#c13

chinmoyr updated the task description. (Show Details)Jun 1 2018, 12:20 PM
chinmoyr updated the task description. (Show Details)Jun 1 2018, 12:44 PM

+ @sharvey, who has been interested in improving the authorization dialog.

mati865 added a subscriber: mati865.Jun 3 2018, 1:52 PM

So what more is left to do here? Just improve the UI of the authorization dialog?

mreeves added a subscriber: mreeves.Jun 4 2018, 3:49 AM

So what more is left to do here? Just improve the UI of the authorization dialog?

Yes and the warning message shown prior to this dialog is quite generic so that needs improvement as well.

Thanks. If you need any design help for that last task, please feel free to reach out to VDG. We'd be happy to work with you on it!

cfeck added a subscriber: cfeck.Jun 8 2018, 9:32 PM

What is status of that? ;)

ngraham added a comment.EditedJan 29 2019, 4:57 AM

What needs to be done to push this forward? Do you need any help?

What needs to be done to push this forward? Do you need any help?

Descriptive warning and privilege drop or job cancellation based on target's ownership. The latter part is up for review while some help is required for the warning stuff.

  1. Improve message in polkit prompt (issue no. 2). BTW how do I fix this?

You can put placeholders in the message and pass them as annotations to the description.
I'm not sure this exists in Kauth which adds some layers on top. If it doesn't it needs adding