FileDescriptors don't actually get sent

[rm5248]

This is marked as fixed in #42, however after some testing I have found that sending FileDescriptors does not actually work correctly.

I have some sample code here that demonstrates the issue. What I am trying to do is to create a pipe in a client application and send it to a different application in order to write to it. I create the pipe via JNA, and then attempt to convert it to a java.io.FileDescriptor in order to be sent. When I do that, dbus-monitor has the following output:

method call time=1573231344.276953 sender=:1.1897 -> destination=test.filedescriptor serial=3 path=/; interface=fd_rx.FDInterface; member=setFileDescriptor
   file descriptor
method return time=1573231344.298849 sender=:1.1895 -> destination=:1.1897 serial=5 reply_serial=3

On the receiving side, the file descriptor is parsed and is believed to be valid, however when we attempt to write to it we get an IOException:

[2019-11-08 11:42:24] DEBUG fd_rx.FDRX - Got the file descriptor.  Valid? true
[2019-11-08 11:42:24] ERROR fd_rx.FDRX - IOException: 
java.io.IOException: Bad file descriptor
	at java.io.FileOutputStream.write(Native Method) ~[?:1.8.0_171]
	at java.io.FileOutputStream.write(FileOutputStream.java:290) ~[?:1.8.0_171]
	at java.io.DataOutputStream.writeChars(DataOutputStream.java:297) ~[?:1.8.0_171]
	at fd_rx.FDRX.setFileDescriptor(FDRX.java:31) [classes/:?]
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_171]
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_171]
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_171]
	at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_171]
	at org.freedesktop.dbus.connections.AbstractConnection$2.run(AbstractConnection.java:766) [dbus-java-3.2.0.jar:3.2.0]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [?:1.8.0_171]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [?:1.8.0_171]
	at java.lang.Thread.run(Thread.java:748) [?:1.8.0_171]

I've also created a C++ application that uses dbus-cxx that acts the same as the Java application where it tries to send a FileDescriptor to the other process. This fails with an error, and the following shows up in dbus-monitor:

method call time=1573231622.504991 sender=:1.1900 -> destination=test.filedescriptor serial=2 path=/; interface=fd_rx.FDInterface; member=setFiledescriptor
   file descriptor
         inode: 42492471
         type: fifo
error time=1573231622.505026 sender=org.freedesktop.DBus -> destination=:1.1900 error_name=org.freedesktop.DBus.Error.NotSupported reply_serial=2
   string "Tried to send message with Unix file descriptorsto a client that doesn't support that."

It seems that the root of this problem is when a java.io.FileDescriptor is sent, an integer is simply marshaled onto the message, which is not the correct way to send a filedescriptor over a socket.

According to the DBus spec, the client must send a "NEGOTIATE_UNIX_FD Command" to allow FD passing. From my reading of the specification as well, it seems that after the NEGOTIATE_UNIX_FD has been sent, when you are actually sending the FD this needs to be set in the message header, and the FDs are sent as some kind of array(the spec indicates that they are sent out-of-band). The value of the FD set in the message is the index into this array.

[hypfvieh]

file descriptors are really hard ...

The current implementation just sends the number of the file descriptor so the calling client can create a new FileDescriptor object using the given number to access the same file.

Adding 'NEGOTIATE_UNIX_FD' and 'AGREE_UNIX_FD' is not a problem.
My problem is to understand what "index into an out-of-band array of file descriptors" is supposed to mean (why an array of file descriptors?).
The documentation is not very specific on how to deal with file descriptors.

Maybe my knowledge of the english language is not good enough.
Maybe the documentation is just crap at that point.

If you can help provide patches, hints or anything useful, please let me know

[rm5248]

The problem with sending just the number is that there's no way to take that number and turn it back into a file, since the filedescriptors are unique per-process.

I can take a look at this later this week, but some things that I have found:
C#/Mono code that adds file descriptors: mono/dbus-sharp#62
It looks like jnr-unixsocket doesn't support this at the moment, as what needs to happen is that sendmsg needs to be used instead of sendto(which is what is being used at the moment).

See also this StackOverflow post on doing this in C: https://stackoverflow.com/questions/28003921/sending-file-descriptor-by-linux-socket

[rm5248]

After some speluking through the libdbus code, I think that I have found the method which actually does the writing out.
So it seems that when the specification says

Unsigned 32-bit integer representing an index into an out-of-band array of file descriptors, transferred via some platform-specific mechanism

It is talking about setting certain fields in a struct cmsghdr, which is part of struct msghdr. This explains why the actual number of the filedescriptor needs to be an index into this array, and looking at dbus_message_iter_append_basic we see that the filedescriptors are basically copied over to their own separate array which is then sent out as part of the sendmsg call previously seen.

[rm5248]

Question: is it required to continue to use jnr-unixsocket? I came across this other project just now that already has support for FileDescriptors over a Unix socket. This would swap out some dependencies, but I'm not sure if that would be acceptable or not.

[hypfvieh]

The reason for using jnr-unixsocket is that they do not require any native-compiled wrapper library.

As far as I can see junixsocket uses a wrapper as well.
This is like the old libmatthew which also requird native library to be available.

Because linux (and therefore DBus) is running on many platforms with many different architectures it was always a pain providing native libraries for these platforms.
Different platform versions (e.g. ARMv6/v7) also were annoying.
In there cases the developer of the application has to figure out how to compile the native library and how to use it in combination with dbus-java.

I think this is inconvienent, so that's the main reason why using jnr-unixsockets and that's why I don't want to switch back to another library doing the same stuff I want to avoid.

But maybe we can provide a patch to the jnr-unixsocket maintainer to support the changes we need.
I did this a while ago (litte patch to support the PASS_CRED feature) when implementing jnr-unixsocket in dbus-java.

I'll try to take a look into the implementation of this stuff in junixsocket next week, maybe I can write that patch.

Alternativly we could change dbus-java to support different 'transport' libraries, so if you need Filedescriptors (rare case in my opinion), you have to add the dbus-java-junixsocket transport, otherwise you can use the dbus-java-jnr-unixsocket transport ...
But lets wait until we are really sure that this kind of feature is really required.

[rm5248]

For using JNR, I've started on some work to see if it is possible. This is difficult because there's not much documentation on how to use JNR, JNA is much more prevalent. Branch is here: https://github.com/rm5248/jnr-unixsocket/tree/rm-testing

The problem that I have just run into when attempting to write a test for this is that there are certain macros that must be used in order for the cmsghdr to be set properly. According to the man page though:

Ancillary data is a sequence of struct cmsghdr structures with appended data. This sequence should only be accessed using the macros described in this manual page and never directly.

If that is true, then it seems likely that there's no way to do this from pure Java and we may need to make some JNI shim library to handle it.

[hypfvieh]

Maybe we get the job done, if we include/use jnr-posix. They already added the missing sendmsg/recvmsg and also some of the macros you mentioned.

[hypfvieh]

I pushed my changes to filedescriptors branch.
I did not implement the stuff required for sendmsg/recvmsg, but already included jnr-posix as dependency.

I tried to add the required handshake-stuff so DBus knows that file descriptor passing is possible (NEGOTIATE_UNIX_FD).

[rm5248]

I have at least a partial implementation at the moment. I think the basic code is correct, but it's not talking with the bus at the moment. My branch is here: https://github.com/rm5248/dbus-java/tree/filedescriptor-passing

The problem that I have at the moment is that the iovec is null(check FDMessageWriter):

19:22:32.236 [DBus Sender Thread-1] TRACE org.freedesktop.dbus.FDMessageWriter - msghdr to send: msghdr {
  msg_name=null,
  msg_namelen=0,
  msg_iov=[
    iovec {
      iov_base=null,
      iov_len=128,
    }
  ],
  msg_control=[
  ],
  msg_controllen=0
  msg_iovlen=1,
  msg_flags=0,
}

Since no data is ever written, we can never talk with the bus(the Hello message doesn't go out). I've tracked it down to what JNR-posix is doing with Pointer.wrap with a ByteBuffer. Just before that there's this line:

logger.debug( "pointer is {}", 
                Pointer.wrap( jnr.ffi.Runtime.getSystemRuntime(), dataSend ) );

Which is giving me a null pointer:

19:22:32.234 [DBus Sender Thread-1] DEBUG org.freedesktop.dbus.FDMessageWriter - pointer is jnr.ffi.provider.jffi.ByteBufferMemoryIO[address=0x0 size=128]

So this looks like a bug in JNR, but it seems weird that it wouldn't work correctly. Am I missing something?

[hypfvieh]

I'm still struggling with a proper test setup. How did you do the testing?

I noticed something in FDMessageWriter class.

On line 63 you use msghdr.allocateControl with the size of the list (e.g. 1 if only 1 FD is in the list).
As far as I understand the datalength is needed here, and I don't think that one FD is only one byte in length.
When comparing it to a test case of jnr-posix, I see that they are using a length 4 (4 bytes) which seems more appropriate to me (see here: jnr-posix IOTest).

Also there is already a constant for SOL_SOCKET in jnr-posix, so you may switch to that one instead of using your own (jnr.constants.platform.SocketLevel.SOL_SOCKET.intValue())

[rm5248]

At the moment, I'm just trying to run the currently-existing tests, and those are failing because it can't connect to the bus correctly. The repo that I linked before also has an FDTX and FDRX classes; currently I'm focused only on sending, receiving requires more work still.

That IOTest from JNR looks useful; there's not much information on how to use JNR so I've been going around somewhat blindly.

[rm5248]

I updated my branch with the fixes required to send messages at least, and all of the tests except for the file descriptor are working at the moment. I started work on the receiving end, but that requires more work still on me understanding what the spec says and what the code is doing with the InputStream in order to read the data.

[hypfvieh]

Nice! Keep up the great work!
I'll try to help if I find some spare time to do so (and if I can finally understand the specs ...)

[rm5248]

Okay, so the good news at this point: sendmsg and recvmsg are both implemented for all the current tests. I've run into a few issues at this point though.

• There's a bug in jnr-posix(fixed on my branch here) that caused an infinite loop.
• The tests(not including the filedescriptor test) occasionally fail with a ConcurrentModificationException in the DBusConnectionTest(test_busnames_should_be_auto_released_on_close_of_non_shared_connection)
• Sometimes it also fails with the TestEmptyCollections test.
• The FileDescriptor test is failing to extract properly; the toString is failing

Some of these seem like race conditions, but I'm not sure why they wouldn't have come up before.

At this point, it seems like I need to update Message.java, as simply adding a new header(UNIX_FDS) doesn't automatically add this to the marshalled message. This may be the cause of the toString failure on the Message object.

If you are able to take a look, that would be helpful.

[hypfvieh]

I'll take a look at that concurrent modification exception.
There are some test cases which were not stable, it's about time to get this fixed.

About your toString issue:
I'm not sure if the code is correct yet.

The exception is caused by trying to read an arraylist without content (e.g. filedescriptors.get(0) when list is empty).
When 'fixing' this issue by checking the list of being empty first, the unit test will fail because no file descriptor is returned but null.

So my guess is, that whatever should be in the filedescriptor list is never written to it.
Did not have the time to debug this, yet.