`cockpit-ws` has never protected hostnames from being interpreted as cli
options when passing them to the auth commands (`cockpit-session`,
`cockpit-ssh`, `cockpit.beiboot`). There have been a couple of relevant
changes over the years:
- our move to using cockpit-session via unix socket has removed
exposure to this problem for `cockpit-session`
- our move from `cockpit-ssh` (glib argument parser) to
`cockpit.beiboot` (Python argparse) has unfortunately exposed us to
https://github.com/python/cpython/issues/66623 which means (due to a
strange heuristic) that arguments starting with '-' can be
interpreted as positionals if they also have spaces in them
This gives a way to get a hostname starting with a `-` to ssh (where it
*will* be interpreted as an option) and the following argument (the
python invocation on the remote) will be interpreted as the hostname.
Fortunately, new versions of ssh will reject this hostname. In any
case, we should firm up the code here and add `--` to ensure that it's
definitely interpreted as a hostname by ssh.
For a similar reason add a `--` to the ssh command in `cockpit-ws`.
CVE-2026-4631
The shell currently accidentally forwards the "init" message from each
iframe to the shell, meaning an extra init message gets sent for every
page.
Based on the comments in this function about "Only control messages with
a channel are forwardable" and "Everything else gets forwarded", it
seems unintentional that init messages (which are explicitly handled and
don't have a channel) were ever intended to be forwarded.
Unfortunately, the "does it have a channel?" check is an "else if" in
the same chain of conditions as the check for `command == "init"` and
(unlike all of the other commands that are checked for) the `"init"`
case doesn't have a `return;` statement at the bottom.
As a result, we accidentally fall through to the "Everything else gets
forwarded" case and send the init message from the iframe along to the
ws.
This bug dates all the way back to the original version of the shell
added in bf2f46943b ("shell: Implement new multiple dashboards
navigation"). It went unnoticed because the websocket handling code
doesn't check if an init message was already received.
Unfortunately, because the shell is served by the bridge, we need to
support old versions of the shell indefinitely. In order to avoid
regressions, add a test to ensure that cockpit-ws will always accept
these extra init messages.
This is done as a separate commit to make it easier to review each half.
Otherwise there's too much churn in the previous patch for git to even
be able to follow the rename.
`test-stream` has a pair of tests which connect to the webserver itself
and speak HTTP directly in order to test if the `stream` channel is
working with TCP connections.
Unfortunately, after they send the request, they send a `"done"` control
message. This is Cockpit's half-close indicator and the channel
translates it to `shutdown(SHUT_WR)` which causes the webserver to read
EOF.
At that point, cockpit-ws will continue sending the response and then
close the connection. This behaviour in response to a half-closed
connection is non-standard though. The webserver may interpret
half-close to mean that the request is finished or it may interpret it
to mean that the client wishes to abort. After being discussed for
years, the httpwg seemed to have decided that the standard says nothing
on this point and should continue to say nothing on this point, adding
the following text:
Note that a TCP connection that is half-closed by the client does not
delimit a request message, nor does it imply that the client is no longer
interested in a response. In general, transport signals cannot be relied
upon to signal edge cases, since HTTP/1.1 is independent of transport.
- https://github.com/httpwg/http-core/issues/22
- https://github.com/httpwg/http-core/pull/431
Hypercorn behaves the same way as the existing cockpit-ws code. There
is an open issue to change the behaviour in uvicorn, but it doesn't seem
to be under active consideration:
- https://github.com/Kludex/uvicorn/issues/1264
Since this test is not really interested in testing the exact edge-case
behaviour of our HTTP server implementation, let's just remove the
half-close to avoid the issue.
...and use the new Channel class at the same time.
Also: replace the character-by-character encoding with `TextEncoder` to
match the `TextDecoder` already in use on the receiving side.
At some point we either changed the System button to a different
component or PatternFly started using a different CSS variable. Fixing
this is simple as we can also zero the button gap for the CSS grid.
Signed-off-by: Freya Gustavsson <freya@venefilyn.se>
This has been skipped since this file existed and was copied at that
point from the `pyTESTS_HANGING` list in the old automake test setup,
but even going back to that commit, it seems to be working. It's
possible that something with libraries changed or that it was even
already working at that time, but in any case, it's working now. Let's
run it and make sure it keeps working.
The only failing test is now the websocket test. Move the single
exception into the body of the function and add a message about why.
Our "framed" tests create an iframe and implement a yolo version of the
router in the shell inside of a single function for forwarding messages
back and forth between the websocket and the iframe. The instruct the
iframe to use a hardcoded `channel-seed` (ie: the prefix used on all
channel names) of `"2:"`. That's not OK because the webserver told *us*
(the parent window) to use a particular channel seed and now we're just
making things up, which means that the channels created in the iframe
won't start with the channel seed that all messages on the websocket are
supposed to use.
This hasn't been a problem so far because nothing in the existing
`cockpit-ws` ever actually enforces the restriction that a particular
websocket only communicates using its own `channel-seed`. The incoming
Python webserver is more strict about this point, however, which would
cause these tests to fail.
Let's derive the `channel-seed` for the iframe in a similar way to how
the shell does it.
It's a bit of a stdlib quirk that `socket.bind()` doesn't accept a
`Path` even when many other calls like `os.unlink()` do. Paper it over
so our own callers don't have to deal with it.
Removing the hardcoded package name lets us move the files in this
directory around and use them with different names. It's also just more
consistent with everything else in the directory.
Also fix a formatting issue that was introduced on a previous rebase.
Turn this into a proper package by adding an empty `__init__.py`.
When run from the tests, `test-server` outputs its URL to stdout without
the trailing slash. That currently works because `yarl.URL()` picks up
the slack and adds it implicitly, but it's technically wrong and we want
to slowly move away from yarl. Add the `/`..
We used to create and bind the webserver listener socket for ourselves
in order to know what port number we'd get, and pass it into cockpit-ws
via the `LISTENER_FDS` mechanism.
We removed the dynamic port number in 6d6ad3602f ("c-client: Keep
localstorage") which cleared the way for the simplification of moving
the socket creation back to cockpit-ws in 8d32150aa7 ("client:
Simplify cockpit-ws startup").
Unfortunately the "simplify" commit forgot to remove the bit that
actually passed the fd, so since then we've been passing a random fd
through to beiboot. Let's clean that up.
Flaky pixel tests can occur where memory still shows on a disabled
service. Waiting on the memory to disappear before taking pixel tests
will ensure pixel tests match.
Fixes: https://github.com/cockpit-project/cockpit/issues/23030
Signed-off-by: Freya Gustavsson <freya@venefilyn.se>
Having this as a separate function is completely ridiculous in Python,
so let's get rid of it. It's used from a single place (which
base64-encodes it) so we can just use the base64 version of the API from
the start.
Note also: 128 *bytes* is complete overkill here, so let's use 128bits
(16 bytes) instead.
Drop two extremely trivial testcases.
This emulates the effect of `cockpit-session` being run as a template
unit by systemd with an associated `Accept=true` socket. It has support
for basic authentication and 2FA and can be used as a library in an
async task.
We implement this using the authorize helpers we added in the last
commit.
Port `src/common/cockpitauthorize.c` to Python as literally as
reasonably possible. Copy all of the test cases as well.
We put this new code in the `test/pytest/ws/` subdirectory so that we
can have different ruff configuration: this code is meant to run with
Python 3.12+ and uses `ruff format` with the default options.
We write this code to be compatible with Python 3.12 (since we'll never
use it from the bridge and never send it via beiboot). Make sure we
don't try to load it when running our older Python versions via tox. We
can do that via an ignore list in conftest.py but need to adjust our
existing ignore hook so that the two can work together.
Assisted-by: Claude Opus 4.5 <noreply@anthropic.com>
If you start talking to the bridge by immediately opening a channel
you'll get a confusing assertion failure deep down from the guts of the
bridge in the HostRoutingRule wondering why the `init_host` hasn't been
set yet on the `Router`.
Check this explicitly: `init` needs to happen before any channel control
or data messages.
The `<%= title %>` business in the tests has gone unexpanded forever.
Change it to `%title%` which is what the plugin actually expands, and
change the plugin to expand more than once instance of the metavariables
it encounters (since the `%title%` is in the `<title>` and `<h1>` both).
Anaconda wants to allow users to remotely log in Cockpit's WebUI
using a PIN for authentication. Instead of adding an Anaconda mode to
`cockpit.conf` allow Anaconda to bring its own login.html, JavaScript
and CSS. This gives Anaconda full flexibility over the login process
combined with implementing their own handler for basic auth.
We previously used `plymouth` package to get the Ubuntu logo, but it
didn't include variants for both light and dark mode which was a bit of
an issue. In Ubuntu 24.04 and higher `base-files` package includes the
normal branding files we need, but we now fail tests as we didn't verify
these files exist on Ubuntu 22.04.
Reverting the change isn't great as we run into the light and dark mode
issues again, but we can conditionally fetch the new logos if they exist
and fallback to `plymouth` for the logo.
Signed-off-by: Freya Gustavsson <freya@venefilyn.se>
Recently the Cockpit logo was updated within the website to use a better
font while also moving the Cockpit icon to reduce whitespace issues and
visually center Cockpit's plane within the circle.
This updates the branding section to include the updated logo.
Related-to: https://github.com/cockpit-project/cockpit-project.github.io/pull/472
Signed-off-by: Freya Gustavsson <freya@venefilyn.se>
Addresses the following AVC denial introduced by systemd 260:
avc: denied { getattr } for pid=16530 comm="cockpit-ws" name="/" dev="pidfs" ino=1 scontext=system_u:system_r:cockpit_ws_t:s0 tcontext=system_u:object_r:pidfs_t:s0 tclass=filesystem permissive=0
systemd 260 refactored internal code to use pidfs in most places which
is the likely cause of the new violation.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2449605
Changed console.log to console.error when logging errors from
cockpit.user() promise rejection. This provides better visibility
of errors in the browser console and follows the convention of
using console.error for error conditions.
Co-authored-by: hobostay <hobostay@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Packit doesn't support downloading all sources and will instead only
download the last Source listed in a specfile. We need to download the
`Source0` manually to get `cockpit.tar.xz` as `Source1` currently points
to `cockpit-node.tar.xz`.
Related-to: https://github.com/cockpit-project/cockpit/issues/22883
Related-to: https://github.com/packit/packit-service/issues/1505
Assisted-by: Claude
Signed-off-by: Freya Gustavsson <freya@venefilyn.se>
Co-authored-by: Maja Massarini <mmassari@redhat.com>
This code seems to be expecting a header where:
- the header has 3 parts; and
- the first part is 'X-Conversation'
but that's not what the check is doing. It only returns if both checks
fail, which means that the function will continue where:
- the header has 3 parts; OR
- the first part is 'X-Conversation'
This means that any three-part header will be accepted (even if it
doesn't start with "X-Conversation") and any header starting with
"X-Conversation" will be accepted, even if it doesn't have three parts.
That last one is a particular problem because the very next thing we do
is try to access the 2nd and 3rd parts, which may not even exist.
This bug goes back at least as far as 74e58fc8c1 ("ws: Support
conversations in authentication") before "X-Conversation" was even the
name of the challenge (it was "X-Login-Reply" at the time).
We can fix it by changing the `&&` here to a `||`: we want to return if
*either* check fails. Also `!==`, because.
In C23 strchr is now implemented different from the POSIX definition and
has two forms:
char *strchr(char *s, int c);
const char *strchr(const char *s, int c);
Base64 is passed as const, so the return value should also be const.
Supports dark style now as taken from our codebase elsewhere. File is
still entirely self-contained and needs no external imports apart from
font.
Changes made:
- Font updated to RH Display Variable Font
- Info icon updated to style in PF6
- Info icon changed to SVG from PNG
- Supports dark theme
Signed-off-by: Freya Gustavsson <freya@venefilyn.se>
Since 2023 CSS Nesting is baseline supported. [1] Initially it required
one to write `.foo { & .bar { } }` but that restriction has been lifted
for class / id selectors since 2025. To keep maximum compatibility with
older browsers use the & everywhere. Note that for type and pseudo class
selectors an & is always required otherwise it would result into
`.parent-rule *:hover` instead of `.parent-rule:hover`. [2]
The main motivation to move the login page to plain CSS is an idea to
let Anaconda bring their own login.css and not requiring a SASS
compilation step makes that much easier. Secondly for theming it is
easier to reason with a plain CSS file.
[1] https://caniuse.com/css-nesting
[2] https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Selectors/Nesting_selector
