winfsp / sshfs-win

SSHFS For Windows
https://winfsp.dev
Other
5.07k stars 254 forks source link

Remove default ssh port #380

Closed LeadroyaL closed 1 year ago

LeadroyaL commented 1 year ago

Port is not a must option for ssfhs.exe.

If -oPort=1234 is passed, sshfs.exe will pass -Port=1234 to ssh.exe.
If nothing is passed, sshfs.exe will pass nothing to ssh.exe.

The bug is: current code always force use -oPort=22 even if user doesn't assign any port.

    port = "22";

When port is assigned in alias and the port is not 22, ssh.exe should connect to custom port in alias rather than 22.

billziss-gh commented 1 year ago

I see what you are saying and I agree that it would be convenient to be able to specify the port in an alias.

Your PR brings up another problem: the UNC namespace (i.e. all names that look like \\Server\Share\Path) is global and visible to all in Windows. Therefore if one account creates an alias mybank then the same \\sshfs\mybank is visible to all accounts. This should not be a problem in terms of access control (the creator of the \\sshfs\mybank prefix can apply appropriate access control to restrict others from accessing their file system), but it can be a problem for squatting (no one else can create a prefix \\sshfs\mybank).

I believe that this problem already exists in today's code (i.e. without your patch). Your PR simply brought this to my attention. If we attempted to fix this problem, we might have to disallow aliases (which I assume is not what you want) and enforce that the syntax user@host is always used.

I am not sure if the aforementioned problem is a serious one, given that most Windows systems are single user systems anyway (I believe).

billziss-gh commented 1 year ago

@LeadroyaL I am attempting to merge your PR, but I am unable to do so either via the GitHub web interface or via the command line. It is not clear why this is so!

billziss-gh commented 1 year ago

I have merge this PR. Thank you for your contribution.