Insecure and very expensive search for SSH executable

[pavelsher]

It seems plugin does quite expensive search of an SSH executable on the agent. Seems it scans all agent files for presence of it and then uses the first found SSH executable. Not only this is a very expensive in terms of agent resources, but it also imposes a big security risk. Imagine there was a malicious build on this agent which left some SSH executable in checkout directory. If this SSH will be found, then it can steal private key of another build which uses SSH tunner build feature.

I'd recommend to avoid full scan searches altoghether and rely on either configuration or search for SSH in a well known places where it could be installed by a user with sysadmin permissions.

[Vampire]

Do you really think this is much of an issue?
This extensive search over all files is only done as very last resort if nothing else worked.

First explicit config in agent properties is checked,
then explicit config in system properties is checked,
then explicit config in environment variables is checked,
then ssh in the path is checked,
then /usr/local/bin/ssh, /usr/sbin/ssh, /sbin/ssh, /usr/bin/ssh and /bin/ssh are checked.

Only if all those failed, the extensive filesystem search is done to have as much out-of-the-box works as possible.
And this not without first logging a warning that this is done now and what the options are to prevent it.
This extensive search can easily be disabled by either having ssh in the path or in one of the hard-coded places, or by setting one of the explicit configuration options.
This is also additionally documented in the readme file.

So if this plugin is used in an unsafe environment, like building PRs from arbitrary people,
the filesystem search can easily be disabled by setting a configuration option.
While I think that in most cases a build agent actually has the ssh executable in the path anyway, especially if one wants to use this plugin.

[pavelsher]

Do you really think this is much of an issue?

Yes it is. Both security and performance are an issue here.

Only if all those failed, the extensive filesystem search is done to have as much out-of-the-box works as possible.

So basically it will be done on any Windows or not Unix like platform. Also it will be done each time in case of cloud agents on such platforms and it will slow down the agent startup dramatically.

And this not without first logging a warning that this is done now and what the options are to prevent it.

Unfortunately users only look into the logs if something does not work, and even in this case they prefer to send logs to us, JetBrains.

So if this plugin is used in an unsafe environment, like building PRs from arbitrary people, the filesystem search can easily be disabled by setting a configuration option.

You assume that users know and understand consequences of the file system scan. If all of us were so precautios security vulnerabilities would not be so important. Usually nobody thinks that some malicious code can be executed on their agents and can steal their private SSH keys.

So yes, security concern is quite big here. We probably will need to add a noticeable security warning on the plugin page description to warn users about it until the issue is fixed.

[Vampire]

Yes it is. Both security and performance are an issue here.

Imho not in the usual case, but ok.

Only if all those failed, the extensive filesystem search is done to have as much out-of-the-box works as possible.

So basically it will be done on any Windows or not Unix like platform. Also it will be done each time in case of cloud agents on such platforms and it will slow down the agent startup dramatically.

I don't think so, because I expect the users of this plugin actually knowing what they do. And on any Windows or not Unix like platform, the search will most probably not find any ssh executable. And assuming the user wants to actually use the plugin, he will most likely set the configuration property to make it work.

So yes, security concern is quite big here. We probably will need to add a noticeable security warning on the plugin page description to warn users about it until the issue is fixed.

There is no need to threaten me with consequences. :-/ I didn't say I don't do something, I just asked a question.

As I have the code already, what would you think if the behavior could be enabled by an agent configuration property and in the documentation for that property of course a prominent hint about security implications. Would that be acceptable? I doubt anyone will activate it, but I have the code in place already, so it wouldn't need much effort to keep it optionally.

[pavelsher]

I don't think so, because I expect the users of this plugin actually knowing what they do. And on any Windows or not Unix like platform, the search will most probably not find any ssh executable.

These days it may not be like this for Windows. Windows now allows installing SSH server, also ssh client comes with Git. So some SSH executable can be found, but it can take a lot of time to locate it.

There is no need to threaten me with consequences. :-/ I didn't say I don't do something, I just asked a question.

I am sorry it sounded like this, I did not know how to formulate it better as English is not my first language. I just wanted to say that we have to put a warning if we think there can be security issues with the plugin. Then it's up to a user to decide whether they want to use it or not. I am not talking about a warning like: we think this plugin is insecure, don't use it. There'd be an explanation under what circumstances there can be problems.

As I have the code already, what would you think if the behavior could be enabled by an agent configuration property and in the documentation for that property of course a prominent hint about security implications. Would that be acceptable?

This would be perfectly fine. With our options we often have so called "auto" mode, if property is set to some "auto" value, then we perform automatic computations, otherwise we treat it as is.

[Vampire]

@pavelsher can you tell me how I edit the update notes in the plugin portal?