cockpit-project / cockpit

Cockpit is a web-based graphical interface for servers.
http://www.cockpit-project.org/
GNU Lesser General Public License v2.1
11.24k stars 1.11k forks source link

pybridge/ferny: Strict "no connect to host" for unknown hosts #18847

Open martinpitt opened 1 year ago

martinpitt commented 1 year ago

Originally posted by @martinpitt in https://github.com/cockpit-project/cockpit/issues/18713#issuecomment-1562756174

I am trying to understand the reason behind that private special-casing. At first sight this feels quite unrelated to host key checking -- that needs to happen for opening a shared connection as well. Of course we also need to actually fix ferny here -- it needs to ask for a host key when it's needed only.

doc/protocol.md is rather vague about private/shared, it doesn't say anything about when that would be used. We don't use that in any external cockpit project.

Update: @croissanne remembers the/a reason: this is to protect against connecting to malicious machines with direct remote login URLs on some malicious web pages/email/etc. Question is if that is worth all the trouble, or if that shouldn't just ask you for confirming the host key. Up to that point, the communication happens either way, and users should hopefully get suspicious unless that's something that they actually want to do (like clicking on some "web console" button in Foreman)

@stefwalter also confirmed this, that it is protection against malicious URLs. He said that at least back then with cockpit-ssh, it would locally match the server name/IP to the known_hosts files, and thus not even TCP-connect to the remote machine. That would be a much stronger protection than what was suggested above. I checked what SSH does there, and it does not actually do that: Even with StrictHostKeyChecking=yes and CheckHostIP=yes it still connect()s to the remote machine and reads things from it. So our current ferny wrapping of SSH does not really do that.

allisonkarlitskaya commented 1 year ago

imho: there's an argument here that ssh(1) shouldn't attempt to connect to the remote host in the case where StrictHostKeyChecking=yes and there is no hostkey that could match, but I don't think there's place for us to implement that at the ferny level.

For what it's worth, we could do this via ssh-keygen -F. Maybe it makes sense to add an API for that, but to not use it ourselves by default. If cockpit wants to be extra-paranoid, it can be.

edit: read this with "we"="ferny"

martinpitt commented 1 year ago

Notes from meeting: If we care enough, we could implement this with something like ssh -G <given target> to resolve aliases and the configuration, and then try to match the parsed hostname/port in the {user,global}knownhostsfiles (with ssh-keygen -F preferably) before trying to connect.

martinpitt commented 1 year ago

this is to protect against connecting to malicious machines with direct remote login URLs on some malicious web pages/email/etc

That doesn't actually work directly -- from outside the link wouldn't get the cookie, so you would end up at the login page. So it takes at least some social engineering.

Some more discussion with Stef also brought up that this was not primarily meant as a security measure, but to avoid traffic amplification -- i.e. turning short GET requests to a public bastion host to SSH connection attempts.

martinpitt commented 1 year ago

Quote from @stefwalter in https://github.com/cockpit-project/cockpit/issues/18713#issuecomment-1569819777:

I am trying to understand the reason behind that private special-casing. At first sight this feels quite unrelated to host key checking -- that needs to happen for opening a shared connection as well. Of course we also need to actually fix ferny here -- it needs to ask for a host key when it's needed only.

private channels are fundamentally unrelated to unknown hosts (and the related key handling). Private channels are a general capability in the cockpit routing for a transport to be used by just that one private cockpit channel.

An example of this is when a specific "user" is specified on a channel for a specific host. It should not then be automatically used by other channels for that host. Thus it should often be private. We used to use this for syncing users between hosts, and other cross host setup.

It appears that the only remaining use of private channels in the cockpit code is related to loading and verifying host keys. It thus sets the {{{COCKPIT_SSH_CONNECT_TO_UNKNOWN_HOSTS=true}}} environment variable for a new cockpit-ssh process allowing it to connect to a host that was never connected to before. Normally cockpit-ssh will only (TCP) connect to known hosts to prevent bounce or mirroring DDOS attacks.

Hope that helps.