D10437: Limit the use of file.so for privilege operation to one application

[Chinmoy Ranjan Pradhan]

chinmoyr updated this revision to Diff 27482.
chinmoyr added a comment.


  Changed approach. Now temporary authorization is revoked.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10437?vs=26905&id=27482

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10437

AFFECTED FILES
  src/ioslaves/file/file_unix.cpp

To: chinmoyr, #frameworks, dfaure, fvogt
Cc: markg, anthonyfieroni, michaelh

D10437: Limit the use of file.so for privilege operation to one application

[Chinmoy Ranjan Pradhan]

chinmoyr added a comment.


  @dfaure I was thinking about revoking authorization just before the idle 
slave is reassigned by klauncher. polkit-qt-1 provides 
`revokeTemporaryAuthorization` just for this purpose. We only need to implement 
it on KAuth's side. Then on slave's side we can do something like 
`KAuth::Action("org.kde.kio.file.exec").revoke()`. How does this sound?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10437

To: chinmoyr, #frameworks, dfaure, fvogt
Cc: markg, anthonyfieroni, michaelh

D10437: Limit the use of file.so for privilege operation to one application

[David Faure]

dfaure added a comment.


  Sounds good, but the tricky part will be finding how to run some code in the 
(idle) slave at the right time. Maybe in SlaveBase::disconnectSlave() ?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10437

To: chinmoyr, #frameworks, dfaure, fvogt
Cc: markg, anthonyfieroni, michaelh

D10437: Limit the use of file.so for privilege operation to one application

[David Faure]

dfaure added a comment.


  Indeed the sender could definitely fake the PID.
  One could generate and send a sha1 and store it in the slave (and send it as 
metadata with every command), but this can still be sniffed.
  I assume the KAuth security principle is that an intruder (who would have 
access to your session, and therefore can do a lot of things already, including 
installing a keylogger), shouldn't be able to get root access?
  In that case, either kio_file should lose priviledges immediately (sounds 
annoying for the user, but maybe that's the price of this feature?), or the app 
(libkio) should perform the file operations directly.
  
  I'm surprised you didn't hit that yet, btw. E.g. deleting local files, will 
not involve kio_file. DeleteJobPrivate::deleteNextFile calls QFile::remove() 
directly. Of course this isn't the case for all file operations (otherwise your 
patch wouldn't work at all), and it might not even be a good idea to generalize 
this (it's already visible that deleting a 6 GB local file will freeze the app, 
because of this - which was written under the assumption that deleting is fast).
  
  Brainstorming further: the other possibility is that kio_file processes that 
gained root auth, cannot be reused by another app later on. This could be done 
somehow in the KIO scheduler or in klauncher, if they can be told that this 
slave should be killed rather than reused once idle. The design of that stuff 
isn't fully clear in my mind (I didn't write that part), but make sure not to 
get confused by "idle slave which is associated to my process" 
(KIO::Scheduler's IdleJob, kills the slave after 3 minutes), and "idle slave 
that has been returned to klauncher" (in frameworks/kinit), for use by another 
process (or killed after 30s). But I can't find the code that returns an idle 
slave to klauncher (only a "slave that has been put on hold", which is a 
different use case (documented in kio/docs/krun-passing-slaves.txt).

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10437

To: chinmoyr, #frameworks, dfaure, fvogt
Cc: markg, anthonyfieroni, michaelh

D10437: Limit the use of file.so for privilege operation to one application

[Fabian Vogt]

fvogt added a comment.


  In D10437#204377 , @chinmoyr wrote:
  
  > The whole work is being done inside KIO::Job. If the application uses 
regular Jobs then I can't see how it can fake it.
  
  
  By not using KIO or using a modified KIO. Never assume you can trust anything 
you get sent.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10437

To: chinmoyr, #frameworks, dfaure, fvogt
Cc: anthonyfieroni, michaelh

D10437: Limit the use of file.so for privilege operation to one application

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> slavebase.h:957
> + */
> +bool mAppChanged;
>  

Will break ABI, add new member variables only in SlaveBasePrivate

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10437

To: chinmoyr, #frameworks, dfaure, fvogt
Cc: anthonyfieroni, michaelh

D10437: Limit the use of file.so for privilege operation to one application

[Chinmoy Ranjan Pradhan]

chinmoyr added a comment.


  In D10437#204417 , @markg wrote:
  
  > Could you provide steps to reproduce what you try to fix?
  
  
  Steps:
  create files /opt/{a,b}
  open kioslavetest
  delete /opt/a
  open ksysguard and send interrupt signal to kioslavetest
  again open kioslavetest
  delete /opt/b
  
  Without this patch "/opt/b" will be deleted but password won't be asked.
  With this patch an error will be shown.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10437

To: chinmoyr, #frameworks, dfaure, fvogt
Cc: markg, anthonyfieroni, michaelh

D10437: Limit the use of file.so for privilege operation to one application

[Mark Gaiser]

markg added a comment.


  In D10437#204416 , @chinmoyr wrote:
  
  > In D10437#204402 , @markg wrote:
  >
  > > I just tried this:
  > >  kdesu gwenview (type root password and go to the root homefolder).
  > >  In gwenview i can now go to that folder.
  > >  In dolphin (started as user, not root) i can't get into that folder.
  > >
  > > I don't really see what you try to fix here as it seems to be working 
just fine here.
  > >  Or i don't understand it.. In that case, please provide reproducible 
steps.
  >
  >
  > Currently file ioslave does not support reading contents of a locked(is 
this the right word?) folder. So no file operations are possible.
  
  
  Could you provide steps to reproduce what you try to fix?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10437

To: chinmoyr, #frameworks, dfaure, fvogt
Cc: markg, anthonyfieroni, michaelh

D10437: Limit the use of file.so for privilege operation to one application

[Chinmoy Ranjan Pradhan]

chinmoyr added a comment.


  In D10437#204402 , @markg wrote:
  
  > I just tried this:
  >  kdesu gwenview (type root password and go to the root homefolder).
  >  In gwenview i can now go to that folder.
  >  In dolphin (started as user, not root) i can't get into that folder.
  >
  > I don't really see what you try to fix here as it seems to be working just 
fine here.
  >  Or i don't understand it.. In that case, please provide reproducible steps.
  
  
  Currently file ioslave does not support reading contents of a locked(is this 
the right word?) folder. So no file operations are possible.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10437

To: chinmoyr, #frameworks, dfaure, fvogt
Cc: markg, anthonyfieroni, michaelh

D10437: Limit the use of file.so for privilege operation to one application

[Fabian Vogt]

fvogt added a comment.


  In D10437#204413 , @chinmoyr wrote:
  
  > In D10437#204382 , @fvogt wrote:
  >
  > > In D10437#204377 , @chinmoyr 
wrote:
  > >
  > > > The whole work is being done inside KIO::Job. If the application uses 
regular Jobs then I can't see how it can fake it.
  > >
  > >
  > > By not using KIO or using a modified KIO. Never assume you can trust 
anything you get sent.
  >
  >
  > Going by this logic, it seems any attempt at providing security from job's 
side is pointless.
  
  
  It is.
  
  > So how about moving the handling of prompts to slave's side? At least we 
can be sure a prompt will be shown all the time.
  
  Sounds good. Once polkit granted file.so authorized access to the helper, it 
needs to be treated as privilege boundary so it needs to prompt.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10437

To: chinmoyr, #frameworks, dfaure, fvogt
Cc: markg, anthonyfieroni, michaelh

D10437: Limit the use of file.so for privilege operation to one application

[Chinmoy Ranjan Pradhan]

chinmoyr added a comment.


  In D10437#204382 , @fvogt wrote:
  
  > In D10437#204377 , @chinmoyr 
wrote:
  >
  > > The whole work is being done inside KIO::Job. If the application uses 
regular Jobs then I can't see how it can fake it.
  >
  >
  > By not using KIO or using a modified KIO. Never assume you can trust 
anything you get sent.
  
  
  Going by this logic, it seems any attempt at providing security from job's 
side is pointless. So how about moving the handling of prompts to slave's side? 
At least we can be sure a prompt will be shown all the time.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10437

To: chinmoyr, #frameworks, dfaure, fvogt
Cc: markg, anthonyfieroni, michaelh

D10437: Limit the use of file.so for privilege operation to one application

[Mark Gaiser]

markg added a comment.


  I just tried this:
  kdesu gwenview (type root password and go to the root homefolder).
  In gwenview i can now go to that folder.
  In dolphin (started as user, not root) i can't get into that folder.
  
  I don't really see what you try to fix here as it seems to be working just 
fine here.
  Or i don't understand it.. In that case, please provide reproducible steps.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10437

To: chinmoyr, #frameworks, dfaure, fvogt
Cc: markg, anthonyfieroni, michaelh

D10437: Limit the use of file.so for privilege operation to one application

[Chinmoy Ranjan Pradhan]

chinmoyr added a comment.


  The whole work is being done inside KIO::Job. If the application uses regular 
Jobs then I can't see how it can fake it.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10437

To: chinmoyr, #frameworks, dfaure, fvogt
Cc: michaelh

D10437: Limit the use of file.so for privilege operation to one application

[Fabian Vogt]

fvogt added a comment.


  If I'm not mistaken, the application sends its own pid to the slave.
  This means it could just fake it.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10437

To: chinmoyr, #frameworks, dfaure, fvogt
Cc: michaelh

D10437: Limit the use of file.so for privilege operation to one application

[Chinmoy Ranjan Pradhan]

chinmoyr created this revision.
chinmoyr added reviewers: Frameworks, dfaure, fvogt.
Restricted Application added a project: Frameworks.
chinmoyr requested review of this revision.

REVISION SUMMARY
  After successful authorization for privilege execution the whole session gets 
full root-level access via file.so.
  This patch changes file ioslave to not perform a privilege operation if the 
application which is requesting the
  operation is different from the one that initially requested the operation. 
However, an exception is made if
  the request came after the expiration of temporary authorization.

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10437

AFFECTED FILES
  src/core/simplejob.cpp
  src/core/slavebase.cpp
  src/core/slavebase.h
  src/ioslaves/file/file_unix.cpp

To: chinmoyr, #frameworks, dfaure, fvogt
Cc: michaelh