Skip to content

Commit

Permalink
shell: Don't allow connections to remote machines via URLs...
Browse files Browse the repository at this point in the history
...when the host switcher is disabled. Instead, redirect them to
localhost.

The tests that use multiple machines add those machines by navigating
to their URL and then logging into them via the trouble shooting
dialog. Those tests have to explicitly enable the host switcher for
this to continue to work.
  • Loading branch information
mvollmer committed Aug 8, 2024
1 parent ee43bc0 commit 4cb9238
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 12 deletions.
8 changes: 8 additions & 0 deletions pkg/shell/indexes.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,14 @@ function MachinesIndex(index_options, machines, loader) {

if (!state)
state = index.retrieve_state();

// Force a redirect to localhost when the host switcher is
// disabled. That way, people won't accidentally connect to
// remote machines via URL bookmarks or similar that point to
// them.
if (!host_switcher_enabled)
state.host = "localhost";

let machine = machines.lookup(state.host);

/* No such machine */
Expand Down
31 changes: 22 additions & 9 deletions test/verify/check-shell-host-switching
Original file line number Diff line number Diff line change
Expand Up @@ -124,19 +124,32 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers):
m2.execute("hostnamectl set-hostname machine2")
m3.execute("hostnamectl set-hostname machine3")

# Switch the switcher off for the begining of this test.
if self.multihost_enabled:
m1.write("/etc/cockpit/cockpit.conf",
'[WebService]\nAllowMultiHost=no\n')

# This should all work without being admin on machine1
self.login_and_go(superuser=False)

# on recent OSes, switcher is disabled by default
if not self.multihost_enabled:
b.wait_text("#hosts-sel .ct-switcher-localonly", "admin@localhost")
self.assertFalse(b.is_present("#hosts-sel button"))
b.assert_pixels("#hosts-sel", "no-switching", skip_layouts=["mobile"])
b.logout()
b.wait_text("#hosts-sel .ct-switcher-localonly", "admin@localhost")
self.assertFalse(b.is_present("#hosts-sel button"))
b.assert_pixels("#hosts-sel", "no-switching", skip_layouts=["mobile"])

# enable host switcher for this test
self.enable_multihost(m1)
b.login_and_go(superuser=False)
# Check that URLs to remore hosts get redirected to the local
# session.
b.wait_js_cond('window.location.pathname == "/system"')
b.go("/@10.111.113.2/storage")
b.wait_js_cond('window.location.pathname == "/storage"')

# Enable host switcher for the rest of the test
b.logout()
if self.multihost_enabled:
# clean up AllowMultiHost=no from above
m1.execute("rm /etc/cockpit/cockpit.conf")
self.enable_multihost(m1)
m1.restart_cockpit()
b.login_and_go(superuser=False)

b.assert_pixels("#nav-system", "nav-system", skip_layouts=["mobile"])
b.set_layout("mobile")
Expand Down
4 changes: 3 additions & 1 deletion test/verify/check-shell-multi-machine
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ class TestMultiMachineAdd(testlib.MachineCase):
# and failing to load sofware updates breaks pixel tests in release builds
self.setup_provisioned_hosts(disable_preload=True)
self.setup_ssh_auth()
self.enable_multihost(self.machine)

def testBasic(self):
b = self.browser
Expand Down Expand Up @@ -250,6 +251,7 @@ class TestMultiMachine(testlib.MachineCase):
self.allow_journal_messages("sudo: unable to resolve host machine1: .*")

self.setup_provisioned_hosts(disable_preload=True)
self.enable_multihost(self.machine)

def checkDirectLogin(self, root='/', known_host=False):
b = self.browser
Expand Down Expand Up @@ -381,7 +383,7 @@ class TestMultiMachine(testlib.MachineCase):

hostname_selector = "#system_information_hostname_text"

m.write("/etc/cockpit/cockpit.conf", "[WebService]\nUrlRoot = cockpit-new")
m.write("/etc/cockpit/cockpit.conf", "[WebService]\nUrlRoot = cockpit-new\nAllowMultiHost=yes\n")
m.start_cockpit()

# Make sure normal urls don't work.
Expand Down
1 change: 1 addition & 0 deletions test/verify/check-shell-multi-os
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class TestRHEL8(testlib.MachineCase):

stock_m = self.machines['stock']
stock_m.execute("hostnamectl set-hostname stock")
self.enable_multihost(dev_m)

# Wait for connectivity between the two
stock_m.execute("ping -q -w5 -c5 10.111.113.1")
Expand Down
1 change: 1 addition & 0 deletions test/verify/check-superuser
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ class TestSuperuserDashboard(testlib.MachineCase):
def test(self):
b = self.browser
self.setup_provisioned_hosts()
self.enable_multihost(self.machine)

self.login_and_go()
b.go("/@10.111.113.2")
Expand Down
10 changes: 9 additions & 1 deletion test/verify/check-system-realms
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,13 @@ class CommonTests:
do_test(alice_user_pass, ['HTTP/1.1 200 OK', '"csrf-token"'], session_leader='cockpit-session', retry=True)

# enable cert based auth
m.write("/etc/cockpit/cockpit.conf", '[WebService]\nClientCertAuthentication = true\n', append=True)
# FIXME: this is ugly, and we don't currently accept multiple [WebService] sections
if self.multihost_enabled:
# no previous cockpit.conf
m.write("/etc/cockpit/cockpit.conf", '[WebService]\nClientCertAuthentication = true\n', append=True)
else:
# enable_multihost above already wrote [WebService]
m.write("/etc/cockpit/cockpit.conf", 'ClientCertAuthentication = true\n', append=True)
# cert auth should work now
do_test(alice_cert_key, ['HTTP/1.1 200 OK', '"csrf-token"'])
# password auth, too
Expand Down Expand Up @@ -495,6 +501,7 @@ class TestRealms(testlib.MachineCase):
self.op_admin_password = "#realms-op-admin-password"
self.domain_sel = "#system_information_domain_button"
self.machine.execute("hostnamectl set-hostname x0.cockpit.lan")
self.enable_multihost(self.machine)

# realmd times out on inactivity, which occasionally races with the proxy
self.allow_journal_messages("couldn't get all properties of org.freedesktop.realmd.Service.*org.freedesktop.DBus.Error.NoReply: Remote peer disconnected")
Expand Down Expand Up @@ -989,6 +996,7 @@ class TestKerberos(testlib.MachineCase):
def setUp(self):
super().setUp()
maybe_setup_fake_chrony(self.machine)
self.enable_multihost(self.machine)

def configure_kerberos(self, keytab):
self.machines["services"].execute("/root/run-freeipa")
Expand Down
2 changes: 2 additions & 0 deletions test/verify/check-system-shutdown-restart
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ class TestShutdownRestart(testlib.MachineCase):
m2 = self.machines['machine2']
b2 = self.new_browser(m2)

self.enable_multihost(m2)

m.start_cockpit()

self.login_and_go("/system")
Expand Down

0 comments on commit 4cb9238

Please sign in to comment.