SshClient.connect() does not respect username parameter when a default is specified in ssh/config

[frothga]

Version

2.9.2

Bug description

In ssh/config:

Host A
    ...
Host B
    ...
Host *
    User xxx

In Java code:

SshClient client = SshClient.setUpDefaultClient ();
HostConfigEntry entry = client.getHostConfigEntryResolver ().resolveEffectiveHost ("hhh", 22, null, "xxx", null, null);
// "entry" is ignored. Just noting that resolveEffectiveHost() gets called, in case it matters.
ClientSession session = client.connect ("yyy", "hhh", 22).verify (timeout).getSession ();  // Username "yyy", not "xxx".
session.setUserInteraction (I);
session.addPasswordIdentity ("ppp");
session.auth ().verify (120000);

Actual behavior

Based on debug log, username "xxx" is passed to host, not username "yyy". After removing the default entry in ssh/config, the correct username is passed.

Expected behavior

When the programmer explicitly specifies a username+host pair in the connect() call, it should take precedence over all other sources of username for the given host. In particular, it should definitely override a default value in config, much the same way specific host entries in config override the default. Perhaps some special value could indicate to use the default, for example passing null for username.

Relevant log output

No response

Other information

No response

[tomaswolf]

Agreed. The fix should probably be in SshClient.resolveHost().

All three fields (user, host, and port) need to be handled. In JGit we do:

• If a username is given and not empty, use that instead of whatever the config says. If both the parameter and the config give no user name, use the Java system property "user.name".
• If a port is given and >= 0, use that instead of whatever the config says. If the parameter is < 0 and the config also doesn't specify a port, use 22.
• If "HostName" is not set in the config, set it to the given host.

[tomaswolf]

Actually JGit does this in its HostConfigEntryResolver and always returns a host config entry that has user, host name, and port set.

[frothga]

The original issue above may be due to a bug in my code rather than a bug in MINA. I haven't been able to verify this yet because there are other issues with MINA which also break the resolution process. I can create separate issues if that helps to get them addressed. I'm also willing to submit a pull request to fix some these ...

The comment from December 8, 2022 seems to suggest that I add code to always override values in the returned entry before using them. That means that only forms of ClientSessionCreator.connect() that take a HostConfigEntry are usable. The others are unusable because they don't have the expected behavior. An alternative would be for the implementations of those functions in MINA itself to do this task of overriding fields in the entry, given that the user provided values explicitly. For each, there is a clear no-care value (null for strings, non-positive for port).

Best I can tell from the source code, values provided to resolveEffectiveHost() are only used if values in the selected config entry are no-care. IE: the config file values always override the explicitly-provided values. Simply reversing this logic would go a long way to addressing the above problem.

Intercepting HostConfigEntry is fine if this is a direct connection to the host. For jump hosts, the config file processing should produce the same results as ssh command line. Unfortunately, there appear to be some semantic differences between MINA processing and ssh command-line processing. The ssh man pages specifies that each attribute is resolved individually and that the first matching entry (reading top to bottom in the file) that provides the given attribute is used. This allows wildcard entries later in the file to provide defaults, while more specific entries earlier in the file can provide overrides, on a per-attribute basis. This is apparently not how MINA handles config files. MINA selects one single "best" entry and gets all the attributes from it. Also, MINA does not seem to consider position in the config file. Both of these break the semantics of the ssh config file, making MINA behave differently on a given file that otherwise works correctly for ssh command line.

Here's an illustration of the jump host issue:

In ssh/config:

Host A
    ProxyJump B
    ...
Host *
   User bob
   Port 22

That configuration works, because "B" gets user and port from the catch-all entry (and only the catch-all entry). But if we have

Host A
    ProxyJump B
    ...
Host B
    HostName actual.address  (just some attribute, not important which)
    (but no user or port)
Host *
    User bob
    Port 22

This config fails, because user and port are not specified in the explicit entry. This problem also arises if the "Host B" entry has just one of User or Port, but not both. Basically, the connection is broken because of an incomplete set of primary attributes. For completeness

Host A
    ProxyJump B
    ...
Host B
    User bob
    Port 22

This config does work, since "Host B" has complete information.

[frothga]

Another related issue -- I believe that command line ssh will use the given username (and other attributes) as defaults for the configuration of jump hosts. IE: if I have an ssh config file that does not mention username at all, ssh will still use the username from the command line (or the environment) to log in to each of the jump hosts. The current behavior in MINA's normalizeEntry() would be correct for this case (but incorrect for the primary target host).

The problem is that the SshClient connect() and doConnect() functions do not pass username or other attributes on to parseProxyJumps(). Everything has to come from the proxy URL itself. Again, this forces the ssh config file to contain more info than is required by ssh command line.

Not sure if it is your goal that MINA to work correctly with any config file that works correctly with ssh command line. As a naive user, that's what I'd expect. It would help to either fix these issues or create some documentation saying how MINA interprets config files differently.

[tomaswolf]

This is all correct, and is the reason why JGit uses its own SSH config parsing. (The JGit config file parsing is also not 100% openSSH compatible, in particular it doesn't do match and include directives, but otherwise it fixes the problems you mentioned, like wrong or missing defaults, using a "best" match, and some other details.)

The ProxyJump implementation in Apache MINA sshd has problems anyway; see #351 and in particular #318.

For whether command-line options apply to ProxyJumps, see openSSH bug 3490.

[frothga]

Thanks for the links!

openSSH bug 3490 seems to indicate that they don't use any attributes of the destination host for jump hosts. So I'd guess the same rules apply for determining User. IE: if none is given in the config file then they use the login user.

Just a general note, while I am using JGit, I have other use cases for MINA, specifically to transfer files (scp), start and monitor processes on remote systems.

Thought about this a lot overnight and have an idea for a change to the code that would fix most of my use cases and probably not break anyone else's. Will make a PR for it in a few days. The basic idea is to rewrite ConfigFileHostEntryResolver so it collates attributes across all matching entries, giving precedence to ones earlier in the config file. This is basically an enhanced version of normalizeEntry(), but the current code structure isn't amenable to simply fixing that function. We also need a change to parseProxyJumps() so it considers User and Port info specified in config (#351).