ws: Conditionally block channel requests to remote hosts

[mvollmer]

When AllowMultiHost is false, cockpit-ws will reject all GET requests that would load from a non-localhost bridge.

[jelly]

From a quick look this looks correct, but we probably want to have a test for this? Do we also show a proper error then if a user navigates to an old bookmark?

The only question I have about the code is, what is filled in host_by_checksum as that is the only path I can't really follow.

[mvollmer]

The only question I have about the code is, what is filled in host_by_checksum as that is the only path I can't really follow.

Good point, I didn't consider checksums urls... I'll check.

(In practice, checksums are only known the the shell once a manifest has been loaded from a host, but this is of course about closing all the doors and not rely on what the shell is doing.)

[mvollmer]

From a quick look this looks correct, but we probably want to have a test for this?

Yeah, via curl.

Do we also show a proper error then if a user navigates to an old bookmark?

They get redirected by the shell. This change here should have no effect with our current shell. It should never construct URLs for remote hosts when AllowMultiHost is false. But the shell is a lot of code, and there might be bugs, so we should also block the requests we don't want to happen in cockpit-ws.

[mvollmer]

The only question I have about the code is, what is filled in host_by_checksum as that is the only path I can't really follow.

Good point, I didn't consider checksums urls... I'll check.

They are handled by parse_host_and_etag and thus a checksum url will also be handled by cockpit_channel_response_serve and is also subject to the new "remote host" check.

[mvollmer]

From a quick look this looks correct, but we probably want to have a test for this?

Yeah, via curl.

Done.

[martinpitt]

Thanks!

[martinpitt]

Thanks! If you have to push again, please remove the redundancy, but not that important.

[martinpitt]

Both of these are already done in setUp(). Should hopefully not hurt, though.

[mvollmer]

Hmm, setup_ssh_auth was necessary, for some reason.

[martinpitt]

testUrlRoot is unhappy 😢

test failures