hypfvieh / dbus-java

Improved version of java DBus library provided by freedesktop.org (https://dbus.freedesktop.org/doc/dbus-java/)
https://hypfvieh.github.io/dbus-java/
MIT License
185 stars 73 forks source link

Question, how can I implement a custom transport with custom parameters? #174

Closed brett-smith closed 2 years ago

brett-smith commented 2 years ago

I intend to implement a custom transport (an SSH one), and believe I have come across a design choice that makes this harder.

I would like to be able to use custom schemes and parameters in the address string. For example, the kind of string I envisage being used would look something like ..

ssh:host=dbustest.acme.com,port=22,path=/some/path/to/dbus.socket,username=joeb,privatekey=/home/joeb/.ssh/id_rsa

The above example would use Unix Domain Socket forwarding (supported by OpenSSH servers for example).

However, I do not see how I can get at any parameters from a BusAddress that are not well known. At first glance, it looks like a simple getParameter() and hasParameter() would suffice, but I would appreciate any further suggestions you have before I dive in and make a PR.

hypfvieh commented 2 years ago

BusAddress definitely needs some attention. I just started cleaning up that mess I currently have with BusAddress a few days ago.

Before you start doing anything please wait until I've finished my work at that point. These changes may also affect the Transports because I want to get rid of the address-String usage internal and want to use BusAddress or subclasses of it to have a clean separation between special things the transport needs to know and things which dbus-java needs to know.

brett-smith commented 2 years ago

Ok, yeh I saw some commits. Will await the go ahead.

hypfvieh commented 2 years ago

I just pushed my changes. This change includes the possibility to add/remove/get parameters from the BusAddress object.

Also BusAddress is used internally instead of Strings.

brett-smith commented 2 years ago

Thanks for the fast response. This works good, I now have a working SSH transport.

However, there is one further restriction I cannot see an obvious way to overcome.

Take for example the bus address ssh:path=/run/dbus/system_bus_socket,via=sshtestserver,viaPort=22,username=brett,password=mysupersecret.

I would like to avoid to providing the password as part of the address string. Apart from the obvious security implications, address string parsing does not appear to support any kind of escaping, so there is a danger with any password that has as :, = or ,.

So ideally, passing a callback interface to the transport would be nice. E.g.

public void setAuthenticationCallback(Callable<char[]> callback) {
// ...
}

This is then called when the password is needed, and the user is prompted for the password, or it's retrieved from some other secure place.

The address string appears to be the only way to communicate configuration to a transport before the connection is made. TransportBuilder has a withAutoConnect(), but how do I get to the TransportBuilder? This appears to be used, and the transport built and connected before any user code can get to it.

I can work around this for the moment by using static methods to set the callback on my SshTransport class, but this is not ideal.

hypfvieh commented 2 years ago

The term 'authentication' would be confusing because DBus authentication is done by SASL and is completely independent of what your transport does.

TransportBuilder will use java service loader to load a specific transport and after that connect() is called on that instance. The abstract class AbstractTransport will then call connectImpl() and I guess that you already need the authentication details at that point.

I added a callback (a simple runnable) called preConnectCallback which can be set on the transport using TransportBuilder.

The auto-connect option is indeed only accessible using the TransportBuilder. Usually you use something like DBusConnectionBuilder.forSession().build() in a try-with-resources block, so you would expect that the connection is established instantly instead of calling any additional methods.

Disabling auto-connect would therefore only make sense when you want to create a listening socket (your own DBusDaemon (like shown in EmbeddedDBusDaemon)) where you have to deal with TransportBuilder anyway.

brett-smith commented 2 years ago

Thanks again, I see the new method. However, I still cannot see how I can actually get to TransportBuilder from user code. It is created in AbstractConnection, with no opportunity to set anything on it before the connection is made.

Am I missing something?

hypfvieh commented 2 years ago

I updated the TransportBuilder and all that stuff around it. Now it is possible to access transport configuration from DBus/DirectConnectionBuilder.

I also updated ITransportProvider so you will receive the TransportConfig object and can do additional configuration. For this, TransportConfig provides a Map<String,Object> where you can place additional arbitary information which will be available when createTransport is called. I updated the preConnectCallback as well, which now is a Consumer which receives the current transport for additional actions.

brett-smith commented 2 years ago

Absolutely perfect, thanks.

I have used static utility functions on the transport to help set these. E.g.

var builder = DBusConnectionBuilder
        .forAddress("ssh:path=/run/dbus/system_bus_socket,username=admin,via=blue,viaPort=2222");

SshTransport.setAuthenticationConfigurator(
            (auths) -> Arrays.asList(new PasswordAuthenticator(() -> "admin")),
        builder.transportConfig());

try (var connection = builder.build()) {
    System.out.println(String.join(System.lineSeparator(), connection.getNames()));
}

I'll be publishing this transport as open source at some point. The SSH support is provided by Maverick Synergy which is LGPL. However, it is using bleeding edge as-yet-unreleased support for Unix Domain Sockets, and the open source branch of Synergy lags behind the commercial branch a little.

Thanks again, feel free to close this issue.

Edit: The source for this transport can now be found at https://github.com/sshtools/dbus-java-transport-ssh

hypfvieh commented 2 years ago

Interesting, didn't know Maverick Synergy. I always used jsch or similar when trying to do ssh-client stuff with Java. Did they implement unix sockets themselves or are they using the JDK (like dbus-transport-native-unixsockets)? Unix Socket support was finalized with JDK 16, so I don't consider it as "bleeding edge" ;-)

brett-smith commented 2 years ago

Ah heh, "they" is actually me. I am involved with JAdaptive as well as LogonBox, and have been for a very long time. This is our 3rd (or maybe 4th depending on how you look at it) SSH API.

Unix Domain Sockets is bleeding edge in terms of support for it in Maverick Synergy, because I only added it last week :). I am not aware of any other Java SSH APIs that support it yet.

And yeh, it is using JDK16. Very much like dbus-java, the next release of Maverick will be Java 11, with optional support for unix domain sockets via a Java 16+ add on.