Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
brett-smith opened this issue Jul 18, 2022 · 10 comments
Closed

Comments

@brett-smith
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor Author

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

@hypfvieh
Copy link
Owner

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
Copy link
Contributor Author

brett-smith commented Jul 20, 2022

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

brett-smith commented Jul 21, 2022

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
Copy link
Owner

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants