Support passing unix fds

altdesktop / python-dbus-next

🚌 The next great DBus library for Python with asyncio support

https://python-dbus-next.readthedocs.io/en/latest/

MIT License

191 stars 60 forks source link

Support passing unix fds #54

Closed michallowasrzechonek-silvair closed 3 years ago

michallowasrzechonek-silvair commented 4 years ago

Hi,

We've started working on #33. It's still very much in progress, but I wanted to give you heads up and get some feedback about the general approach.

On the other side, we'll be using https://git.kernel.org/pub/scm/libs/ell/ell.git/

acrisci commented 4 years ago

Yeah looks ok. Just a few notes.

Try to void the monkey patching if possible.
We need to leave room to support this in the high level interface.
The spec seems to say fds can be passed at any time during the message, not just after the first byte and will definitely not be passed if the header is not present (if that affects performance).
You might need to add the NEGOTIATE_UNIX_FDS command to the auth protocol.

michallowasrzechonek-silvair commented 4 years ago

Unfortunately, asyncio doesn't expose sendmsg/recvmsg coroutines :( What we could do is keep these as regular functions, at the cost of slightly ugly invocation.

As for passing fds at any point, I'm pretty sure that all existing implementations do that while sending the initial fixed size header. Allowing fds to be passed along the body is somewhat problematic, because recvmsg is atomic, i.e. it truncates messages if buffer is too small, and has a buffer size limit. And dbus-next parses messages on the fly, so we would need to refactor the whole unmarshalling part to use a in-memory buffer.

High level API and negotiation: ok.

rafalgajda-silvair commented 4 years ago

Hi all, Current version seams to be working and all tests are passing. Could you take a moment to review the code changes and see if I missed something? Much obliged.

michallowasrzechonek-silvair commented 4 years ago

@acrisci Did you have a chance to look at this?

acrisci commented 4 years ago

I'm getting a test failure on travis that I can reproduce locally.

acrisci commented 4 years ago

Use make format to run the formatter.

acrisci commented 4 years ago

Are you guys still working on this? It seems like a good feature. Let me know if I can help.

michallowasrzechonek-silvair commented 4 years ago

Hi,

It turned out that our target platform puts restrictions on SCM_RIGHTS, and we can't use it directly. We ended up with a workaround, passing path to an unix socket istead of a descriptor.

So no, we're not actively working on that feature, sorry :(

At least the PR is functional, we've tested it with ELL acting as a service side.

rafalgajda-silvair commented 4 years ago

Hi, I might get some spare time to finish this pull request. It would be great if You could have a look at it again and let me know if there is anything more to be fixed before merging.

acrisci commented 3 years ago

Sorry for not getting to this sooner. There are just too many subtle issues here to do this in one PR. But what is here is basically good.

I am going to merge this, make sure it's turned off so it doesn't affect any existing code and mark the feature as experimental so we can make breaking changes for awhile. I'll tag you on any PRs I make in the future.

rafalgajda-silvair commented 3 years ago

Sure thing, I'm just glad this code will not get lost :D