D24245: Add support for passing Unix file descriptors

volkov created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. volkov requested review of this revision. REVISION SUMMARY Introduce UnixFileDescriptor class, which holds a copy of a file descriptor. Actually it's a wrapper around QDBusUnixFil

D24245: Add support for passing Unix file descriptors

ngraham added reviewers: fvogt, chinmoyr, cfeck, Frameworks. REPOSITORY R283 KAuth REVISION DETAIL https://phabricator.kde.org/D24245 To: volkov, fvogt, chinmoyr, cfeck, #frameworks Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24245: Add support for passing Unix file descriptors

fvogt requested changes to this revision. fvogt added a comment. This revision now requires changes to proceed. Unfortunately, this breaks public API and ABI by modifying KAuth::ActionReply. I'm not sure whether there has to be any compatibility for the DBus API, but I guess not (logging

D24245: Add support for passing Unix file descriptors

aacid added a reviewer: security-team. REPOSITORY R283 KAuth REVISION DETAIL https://phabricator.kde.org/D24245 To: volkov, fvogt, chinmoyr, cfeck, #frameworks, security-team Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24245: Add support for passing Unix file descriptors

volkov added a comment. DBus is an internal dependency for KAuth. With the wrapper users don't have to link with QtDBus. REPOSITORY R283 KAuth REVISION DETAIL https://phabricator.kde.org/D24245 To: volkov, fvogt, chinmoyr, cfeck, #frameworks, security-team Cc: kde-frameworks-devel, LeGa

D24245: Add support for passing Unix file descriptors

volkov updated this revision to Diff 66946. volkov added a comment. fix breaking API and ABI of KAuth::ActionReply REPOSITORY R283 KAuth CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24245?vs=66905&id=66946 BRANCH master REVISION DETAIL https://phabricator.kde.org/D24245 A

D24245: Add support for passing Unix file descriptors

volkov added a dependent revision: D24259: WIP: Save file directly if possible when root privileges are required. REPOSITORY R283 KAuth REVISION DETAIL https://phabricator.kde.org/D24245 To: volkov, fvogt, chinmoyr, cfeck, #frameworks, security-team Cc: kde-frameworks-devel, LeGast00n, GB_2

D24245: Add support for passing Unix file descriptors

aacid added inline comments. INLINE COMMENTS > HelperProxy.cpp:25 > + > +#include > +#include i don't think we should have dbus stuff in HelperProxy, dbus stuff should be in DBusHelperProxy, no? REPOSITORY R283 KAuth REVISION DETAIL https://phabricator.kde.org/D24245 To: volkov, fvogt,

D24245: Add support for passing Unix file descriptors

volkov added a comment. yes, thanks REPOSITORY R283 KAuth REVISION DETAIL https://phabricator.kde.org/D24245 To: volkov, fvogt, chinmoyr, cfeck, #frameworks, security-team Cc: aacid, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24245: Add support for passing Unix file descriptors

volkov added inline comments. INLINE COMMENTS > aacid wrote in HelperProxy.cpp:25 > i don't think we should have dbus stuff in HelperProxy, dbus stuff should be > in DBusHelperProxy, no? yes, thanks REPOSITORY R283 KAuth REVISION DETAIL https://phabricator.kde.org/D24245 To: volkov, fvog

D24245: Add support for passing Unix file descriptors

volkov updated this revision to Diff 66968. volkov added a comment. moved dbus stuff to DBusHelperProxy REPOSITORY R283 KAuth CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24245?vs=66946&id=66968 BRANCH master REVISION DETAIL https://phabricator.kde.org/D24245 AFFECTED FIL

D24245: Add support for passing Unix file descriptors

volkov edited the summary of this revision. REPOSITORY R283 KAuth REVISION DETAIL https://phabricator.kde.org/D24245 To: volkov, fvogt, chinmoyr, cfeck, #frameworks, security-team Cc: aacid, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24245: Add support for passing Unix file descriptors

aacid added inline comments. INLINE COMMENTS > kauthunixfiledescriptor.h:33 > + > +class KAUTH_EXPORT UnixFileDescriptor > +{ Does this class really need to be exported and its header installed? Seems like this is implementation detail? Who would use this header? REPOSITORY R283 KAuth REVIS

D24245: Add support for passing Unix file descriptors

volkov added inline comments. INLINE COMMENTS > aacid wrote in kauthunixfiledescriptor.h:33 > Does this class really need to be exported and its header installed? > > Seems like this is implementation detail? Who would use this header? It's needed to mark file descriptors in QVariantMap. Usage

D24245: Add support for passing Unix file descriptors

aacid added a comment. Applications are supposed to do that? REPOSITORY R283 KAuth REVISION DETAIL https://phabricator.kde.org/D24245 To: volkov, fvogt, chinmoyr, cfeck, #frameworks, security-team Cc: aacid, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24245: Add support for passing Unix file descriptors

volkov added a comment. Yes, UnixFileDescriptor allows to use existing methods Action::setArguments() and ActionReply::setData(). REPOSITORY R283 KAuth REVISION DETAIL https://phabricator.kde.org/D24245 To: volkov, fvogt, chinmoyr, cfeck, #frameworks, security-team Cc: aacid, kde-framew

D24245: Add support for passing Unix file descriptors

volkov added a comment. Without UnixFileDescriptor you could try to write int fd = ... Action action(...); action.addArgument(QStringLiteral("fd"), fd); but then KAuth won't be able to detect that it'a file descriptor and will pass it to a helper as int. REPOSITORY R283

D24245: Add support for passing Unix file descriptors

aacid added a comment. In D24245#539508 , @volkov wrote: > Without UnixFileDescriptor you could try to write > > int fd = ... > Action action(...); > action.addArgument(QStringLiteral("fd"), fd); > > > but then KAuth won't be a

D24245: Add support for passing Unix file descriptors

volkov added a comment. It will require adding the following methods: Action::setFileDescriptors() Action::addFileDescriptor() Action::fileDescriptors() ActionReply::setFileDescriptors() ActionReply::addFileDescriptor() ActionReply::fileDescriptors() and E

D24245: Add support for passing Unix file descriptors

aacid added a comment. ok, fair enough. Am i correct in understanding this breaks the wire-protocol for the dbus messages? Are we sure that's fine? REPOSITORY R283 KAuth REVISION DETAIL https://phabricator.kde.org/D24245 To: volkov, fvogt, chinmoyr, cfeck, #frameworks, securit