Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ssh: Replace cockpit-ssh with cockpit.beiboot #19441

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

martinpitt
Copy link
Member

@martinpitt martinpitt commented Oct 5, 2023

https://issues.redhat.com/browse/COCKPIT-1029

TODO:

  • PR build: Drop ssh manifest.json for pybridge install #19435
  • PR beiboot: feature parity with cockpit-ssh #19401
  • Rethink/reimplement cockpit.conf [Ssh-Login] host = (ignore_hostkey in cockpitsshrelay.c) with 127.0.0.1 default (covered by TestLogin.testServer and TestLoopback.testBasic)
  • The other failures are due to "invalid conversation token", e.g. this one. This sometimes happens, and the next attempt works again. Fixed in beiboot: feature parity with cockpit-ssh #19401
  • Implement COCKPIT_SSH_KNOWN_HOSTS_FILE in beiboot. Should be covered by TestWsBastionContainer (on fcos)
  • Drop src/ssh/
  • docs, selinux policy, comments, etc. -- git grep cockpit-ssh
  • Drop cockpit-ssh code paths from login.js
  • Drop testLoginSshBeiboot, ensure all failure scenarios are included in other tests
  • Figure out a new mechanism to disable beiboot functionality from distro packages (until we devise a new policy), and add a test for that: Make Apps page conditional on /usr/bin/cockpit-bridge? Covered by test/verify/check-loopback TestLoopback.testBasic (removes /usr/bin/cockpit-bridge)
  • Fix initial superuser with ws container

@martinpitt martinpitt added blocked Don't land until something else happens first (see task list) no-test For doc/workflow changes, or experiments which don't need a full CI run, labels Oct 5, 2023
@martinpitt martinpitt force-pushed the beiboot-default branch 2 times, most recently from 88c969d to 3b3581c Compare October 6, 2023 06:06
@martinpitt martinpitt changed the title ssh: Replace cockpit-ssh with cockpit.beiboot with the pybridge ssh: Replace cockpit-ssh with cockpit.beiboot Mar 22, 2024
@martinpitt martinpitt force-pushed the beiboot-default branch 3 times, most recently from c34aa52 to ab45c5a Compare September 3, 2024 12:53
@martinpitt martinpitt force-pushed the beiboot-default branch 3 times, most recently from a5197b7 to 43ee8f1 Compare September 20, 2024 13:11
@martinpitt martinpitt removed the blocked Don't land until something else happens first (see task list) label Sep 23, 2024
`cockpit-ssh` always considers both the global and the user known_hosts
files, and rejects a host if a non-matching key is present in either.
But with `ssh` the key in the user known_hosts file completely overrides
the global one.

In our case, the user known_hosts file is coming from the browser's
localStorage. So reset that before writing a bad key to the global file,
which is compatible with both backends.
`COCKPIT_SSH_CONNECT_TO_UNKNOWN_HOSTS` does *not* mean that cockpit-ssh
auto-accepts all unknown host keys, so this attempt was bogus.

(Not setting it means that cockpit-ssh would not even make a TCP
connection attempt to an unknown host; setting the variable makes
cockpit-ssh work like ssh to always TCP-connect, and then negotiate the
host key).
@martinpitt martinpitt removed the no-test For doc/workflow changes, or experiments which don't need a full CI run, label Sep 23, 2024
@martinpitt martinpitt force-pushed the beiboot-default branch 2 times, most recently from 9a2c89d to 70a2d16 Compare September 23, 2024 11:58
With old Python versions, SIGTERMing cockpit.beiboot on logout sometimes
shows a single

> Traceback (most recent call last):

line in the journal without any further details. This is harmless and
doesn't happen with current Python versions, so just ignore it.
This never happens with cockpit-ssh in the default configuration: That
reads cockpit.conf's [Ssh-Login] host= option (defaulting to 127.0.0.1)
and ignores that host's key. This is fine for 127.0.0.1, but dangerous
and unexpected for external hosts: These should *always* use proper SSH
host key validation and change detection.

That cockpit.conf option is normally meant to set the default remote
host for the login page (evaluated in ws), and either way isn't even
documented in our manpage.

cockpit-beiboot does not read this option, and really shouldn't: Let's
keep it as a regular SSH client, and do the special-casing on the login
page.
We are going to need the inverse of `--always` for the initial
replacement of cockpit-ssh with beiboot, i.e. always run the remote
`cockpit-bridge` and fail if it isn't installed.

Replace `--always` with a `--remote-bridge` choice of "auto", "always",
"never".

Exceptions from beiboot gadgets are not properly propagated, so that
requires some plumbing.
Covered by `TestWsBastionContainer.testKnownHosts`.
When beiboot in bastion mode doesn't get user/password credentials from
the login page, then disable password authentication, similar to what
`cockpit-ssh` does. Without that, ssh will interactively ask about the
password in an auth dialog, which doesn't fit the UI workflow. Moreover,
it breaks the cockpit/ws container in "encrypted SSH key" mode.

Covered by `TestWsBastionContainer.testKeyLogin`
cockpit.beiboot has feature parity with cockpit-ssh, so switch the
default direct remote session program to that. Use
`--remote-bridge=always` mode for the time being in ws and the
container; we are going to support that eventually, but let's not change
everything at once.

Change ws' detection of remote login availability to "cockpit-bridge and
ssh are available". This involves forking a shell now (for running the
`command` shell builtin), add an expected message to
`TestConnection.testHttpsInstanceDoS`.

Drop the `COCKPIT_SSH_BRIDGE_COMMAND` env var documentation,
cockpit-beiboot does not use that.

Adjust some error messages in the tests.

https://issues.redhat.com/browse/COCKPIT-1029
Nothing uses this any more, superseded by cockpit-beiboot. This gets rid
of the libssh build dependency.

Drop the `Provides: cockpit-ssh` from Debian. No package ever related to
that virtual package name, and it's meaningless these days.
With cockpit-beiboot being the only remote command, the login page now
never gets the full host key as part of the initial conversation, only
the placeholder.
Comment on lines +870 to +871
console.error("login: got unexpected host key prompt, expecting login-data placeholder:", key);
fatal(_("Internal protocol error"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 2 added lines are not executed by any test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants