D14467: Auth Support: Drop privileges if target is not owned by root

bcooksley added a comment. Thanks REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14467 To: chinmoyr, dfaure, ngraham, elvisangelaccio, #frameworks, #dolphin, maltek Cc: bcooksley, maltek, mreeves, mgerstner, fvogt, kde-frameworks-devel, LeGast00n, michaelh, ngraham, br

D14467: Auth Support: Drop privileges if target is not owned by root

chinmoyr added a comment. In D14467#483848 , @bcooksley wrote: > It would appear that the commit of this change disturbed somethng with FreeBSD builds - see https://build.kde.org/view/Failing/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.12/1

D14467: Auth Support: Drop privileges if target is not owned by root

bcooksley added a comment. It would appear that the commit of this change disturbed somethng with FreeBSD builds - see https://build.kde.org/view/Failing/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.12/135/ Could someone take a look into that please? REPOSITORY R241 KIO REVISION D

D14467: Auth Support: Drop privileges if target is not owned by root

chinmoyr edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14467 To: chinmoyr, dfaure, ngraham, elvisangelaccio, #frameworks, #dolphin, maltek Cc: maltek, mreeves, mgerstner, fvogt, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D14467: Auth Support: Drop privileges if target is not owned by root

This revision was automatically updated to reflect the committed changes. Closed by commit R241:efeede07c8eb: Auth Support: Drop privileges if target is not owned by root (authored by chinmoyr). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D14467?vs=60249&id=60255#toc REPOSITORY R241

D14467: Auth Support: Drop privileges if target is not owned by root

chinmoyr added a comment. Thanks for your help! REPOSITORY R241 KIO BRANCH arcpatch-D14467 REVISION DETAIL https://phabricator.kde.org/D14467 To: chinmoyr, dfaure, ngraham, elvisangelaccio, #frameworks, #dolphin, maltek Cc: maltek, mreeves, mgerstner, fvogt, kde-frameworks-devel, LeGa

D14467: Auth Support: Drop privileges if target is not owned by root

maltek accepted this revision. maltek added a comment. This revision is now accepted and ready to land. Looks good to me now! REPOSITORY R241 KIO BRANCH arcpatch-D14467 REVISION DETAIL https://phabricator.kde.org/D14467 To: chinmoyr, dfaure, ngraham, elvisangelaccio, #frameworks, #dol

D14467: Auth Support: Drop privileges if target is not owned by root

chinmoyr updated this revision to Diff 60249. chinmoyr added a comment. Separated (l)chown since it's to be done with elavated privileges. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14467?vs=60215&id=60249 BRANCH arcpatch-D14467 REVISION DETAIL https:

D14467: Auth Support: Drop privileges if target is not owned by root

maltek added inline comments. INLINE COMMENTS > chinmoyr wrote in filehelper.cpp:133 > Ah! Since I was testing inside /opt I didn't notice. I think the order here > should be: drop privilege -> change grp -> gain privilege -> change user. IMO, it's fine (and less complicated) to just do both in

D14467: Auth Support: Drop privileges if target is not owned by root

chinmoyr added inline comments. INLINE COMMENTS > maltek wrote in filehelper.cpp:133 > For `chown`, dropping privileges here means that the `chown` later can't > succeed - it's not possible to 'gift' a file to another user. I think it > should be handled more like `DEL/RMDIR/MKDIR` etc. Ah! Si

D14467: Auth Support: Drop privileges if target is not owned by root

chinmoyr updated this revision to Diff 60215. chinmoyr marked 6 inline comments as done. chinmoyr added a comment. Addresed the issues. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14467?vs=60167&id=60215 BRANCH arcpatch-D14467 REVISION DETAIL https://p

D14467: Auth Support: Drop privileges if target is not owned by root

maltek requested changes to this revision. maltek added a comment. This revision now requires changes to proceed. I noticed a few more things on the second read. INLINE COMMENTS > filehelper.cpp:123 > +const QByteArray baseName = basename(tempPath2.data()); > +int parent_fd = open(par

D14467: Auth Support: Drop privileges if target is not owned by root

chinmoyr updated this revision to Diff 60167. chinmoyr marked 5 inline comments as done. chinmoyr added a comment. Thanks @maltek. I've fixed all the issues. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14467?vs=59319&id=60167 BRANCH arcpatch-D14467 REVIS

D14467: Auth Support: Drop privileges if target is not owned by root

maltek requested changes to this revision. maltek added a comment. This revision now requires changes to proceed. I've gone over the code and found some issues. I haven't fully thought through the design on a conceptual level, because I assume Matthias already did. INLINE COMMENTS > filehelp

D14467: Auth Support: Drop privileges if target is not owned by root

chinmoyr added a comment. In D14467#479223 , @ngraham wrote: > @mgerstner How is this looking now? Yea/nay? He is not available for the next couple of weeks. However he did ask one of his colleagues to have a look at it. REPOSITORY R24

D14467: Auth Support: Drop privileges if target is not owned by root

ngraham added a comment. @mgerstner How is this looking now? Yea/nay? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14467 To: chinmoyr, dfaure, ngraham, elvisangelaccio, #frameworks, #dolphin Cc: mgerstner, fvogt, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bru

D14467: Auth Support: Drop privileges if target is not owned by root

chinmoyr updated this revision to Diff 59319. chinmoyr added a comment. use seteuid to make the logic for rename work REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14467?vs=59290&id=59319 BRANCH arcpatch-D14467 REVISION DETAIL https://phabricator.kde.org

D14467: Auth Support: Drop privileges if target is not owned by root

chinmoyr updated this revision to Diff 59290. chinmoyr added a comment. - int -> ActionType - separated the logic in dropPrivileges() to two parts - accepting mode argument in mkdir - used *at() functions - minor cosmetic changes @mgerstner In case of rename, when owners are differ

D14467: Auth Support: Drop privileges if target is not owned by root

mgerstner added a comment. chinmoyr asked me to review this patch since I was involved with A CVE in similar code in kate / ktexteditor a while ago. Back then the logic was special purpose to replace a file in the file system with content provided via D-Bus. This here is a way more gener

D14467: Auth Support: Drop privileges if target is not owned by root

dfaure added a comment. +1, the idea seems sane to me, but I'm no expert with this kind of API so I'd like someone else to check it. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14467 To: chinmoyr, dfaure, ngraham, elvisangelaccio, #frameworks, #dolphin Cc: kde-frame

D14467: Auth Support: Drop privileges if target is not owned by root

ngraham added reviewers: elvisangelaccio, Frameworks, Dolphin. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14467 To: chinmoyr, dfaure, ngraham, elvisangelaccio, #frameworks, #dolphin Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14467: Auth Support: Drop privileges if target is not owned by root

chinmoyr created this revision. chinmoyr added reviewers: dfaure, ngraham. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. chinmoyr requested review of this revision. REVISION SUMMARY For actions chown, chmod and utime, process