Due to frequent timeouts during testing lets increase the timeout to
make sure it has time to update before the timeout.
Signed-off-by: Freya Gustavsson <freya@venefilyn.se>
When we look for a specific language for an element in AppStream, such
as Summary or Description, we first try to find the requested language
otherwise we fallback to the first element we found.
As the first item might not be the default language in some cases we
could get a format like this
<summary xml:lang="fr">Gestionnaire d'abonnement dans Cockpit</summary>
<summary>Subscription Manager in Cockpit</summary>
<summary xml:lang="it">Gestore abbonamenti nel Cockpit</summary>
<summary xml:lang="ja">Cockpit のサブスクリプションマネージャー</summary>
<summary xml:lang="ka">გამოწერების მმართველი Cockpit-ში</summary>
<summary xml:lang="ko">Cockpit 서브스크립션 관리자</summary>
<summary xml:lang="ru">Менеджер подписок в Cockpit</summary>
<summary xml:lang="zh-Hans-CN">Cockpit 中的订阅管理器</summary>
In this case we'd choose the first summary found which would be French
instead of English.
Instead, we can first look for the language we want, and if we found an
element without language attribute we use that as a fallback. If both
fail we still fallback to the first element found.
Signed-off-by: Freya Gustavsson <freya@venefilyn.se>
So that we don't rely on the name appearing at the same time as the
systemd unit becoming active. This is no longer the case since
firewalld 2.4.1, see https://github.com/firewalld/firewalld/pull/1528
The tests run into the same issue, and now needs to wait for firewalld
to be officially running before trying to manage it via firewall-cmd.
The recv_all() function can fail for two different reasons when the
loop at line 145 exits:
- Buffer filled (count >= size) without receiving EOF
- Timeout from wait_for_io()
Print the correct error message.
As per poll(2), all errors except `EINTR` (which we already retry)
are not runtime errors, but programming errors that ought to be fatal.
The only edge case is maybe `EINVAL`, but if our `n_fds == 1` is already
exceeding RLIMIT_NOFILE, we are in deep trouble anyway, and exiting is
still the right thing to do.
With that, change `wait_for_io()` to return a bool, so that error
handling in the caller becomes simpler and more robust.
In connection_create_metadata(), the port numbers extracted from
sockaddr_in.sin_port and sockaddr_in6.sin6_port are in network byte
order (big-endian). These values were being used directly without
conversion to host byte order, causing incorrect port numbers to be
logged and reported in the metadata JSON. This didn't break anything as
nothing uses the `origin-port` property right now.
For example, on little-endian systems, port 443 (0x01BB in network
order) would be reported as 47873 (0xBB01 when interpreted as host
order).
Fixed by applying ntohs() to convert the port values to host byte order
before use. This ensures correct port numbers are logged and passed to
cockpit-ws in the metadata.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
HTTP/1.1 clients expect persistent connections by default. Send an
explicit "Connection: close" for errors and redirects so that browsers
show errors reliably.
Also drop the bogus Content-Type for a redirect.
Check for overflow when parsing LISTEN_PID and LISTEN_FDS environment
variables, as documented in strtol(3). However, don't explicitly check
for `ULONG_MAX` and friends, as that is both brittle and also slightly
incorrect (as ULONG_MAX may actually be a valid fd). So
explicitly zero errno and then check for any error (which can really
just be ERANGE, unless the string is really broken and specifies a
base).
systemd controls these environment variables, so this isn't practically
relevant, just defense in depth and helping security scanners.
If pthread_create() fails, connection_count was incremented but the
thread was never created. This could cause the idle timeout to never
trigger even when there are no actual connections.
Creating the thread first would introduce a race condition (the thread
could run to completion and decrement it before we increment). Hence,
explicitly revert the increment on failure.
Add explicit validation that host and path values don't contain CR or
LF characters before using them in the redirect response.
Add an additional defense-in-depth check to ensure header injection
remains impossible. Currently, this validation cannot be triggered
because read_line() already ensures that lines can't contain `\n` and
validates that lines end with either "\r\n" or "\n". Hence we also can't
unit test this.
c-tls' httpredirect.c parses HTTP requests with read_line() which
ensures that `\n` cannot appear in the middle, and lines end with
`\r\n`. This should prevent attempts of header injection or HTTP split
responses. Ensure that this is the case.
Previously, only the Shell would participate in implementing session
idle timeouts. This means that a session was not guaranteed to time
out when the browser decided to suspend JavaScript execution.
Now the main actor for session idle timeout is the bridge: When
JavaScript starts, it will instruct the bridge to start the session
idle timeout timer. When it expires, the bridge will exit and end the
session even if JavaScript has been stopped.
While waiting for the timers to elapse, the bridge will send events
out over a new "session-control" channel type that inform the pages
when the final countdown has started, and when it is time to logout.
User activity is also reported over such a channel.
Previously, we tried to survive unexpected alerts by dismissing those
that existed at the start of the test. But new ones might unexpectedly
appear also while the test is running.
So let's give up on the initial cleanup and make the test robust
against unexpected alerts. The row-id attribute helps greatly and was
indeed already used in part of the test.
- When stalling the download artificially, we never make it to the
progressbar.
- "Cancel" remains allowed during setup.
- It provides perfect description and bug summaries.
Assert that the memory allocation calculation does not overflow. For
large allocations (which are not practical, but a bug or attack) this
gives a clear and defined error, instead of undefined behaviour.
GnuTLS works fine with the modern PKCS#8 format. I just got confused by
test failures as I previously forgot to update the fingerprints in
testing.h previously.
This reverts commit 670d811f94.
Regenerate test certificates with modern PKCS#8 format.
The previous code in exit_pam_init_problem() only escaped ' and /. But
PAM modules can return arbitrary messages (including potential
user-defined content), which could contain control characters like
newlines, tabs, or other special characters that would break JSON
encoding.
Re-use cockpit_json_print_string_property() instead, which does escaping
correctly.
pam_ssh_add_load() checked `result.si_code == 1` to distinguish partial
key loading failures from other errors. This is incorrect and fragile.
The `si_code` field in siginfo_t indicates *how* the child process
terminated (e.g., CLD_EXITED for normal exit, CLD_KILLED for signal),
not the exit status. The exit status is in `si_status`.
The value 1 happens to equal CLD_EXITED on Linux, so this worked by
accident. But it's not portable and semantically wrong (comparing a
termination reason to a magic number).
The intent is to check if ssh-add exited with status 1, which indicates
partial failure (some keys failed to load).
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Follow the strtoul(3) manpage pattern for proper error detection.
Without proper error checks, invalid PID strings could be partially
parsed or overflow could go undetected, potentially storing an incorrect
PID value that could later be used to kill the wrong process.
By default the printer now outputs compact JSON which is more aligned
with what is expected of Cockpit. When using `control` it will now print
it compact by default.
This can be changed by using `--pretty-json` argument.
Signed-off-by: Freya Gustavsson <freya@venefilyn.se>
The only major change is that `exactOptionalPropertyTypes` is enabled in
our TypeScript configuration disallowing us to pass optional properties
as undefined.
Assisted-by: Claude Opus 4.6
The cockpit-wsinstance static user was replaced with DynamicUser in
version 330. Cleaning up the obsolete user on upgrade is no longer
necessary, as Debian stable already ships version 337. Drop both the
workaround and the adduser dependency.
It has stopped doing that in Fedora 44 and elsewhere.
The documented value for "never" is -1, and we recognize this now as
well. We also use "passwd -x -1" when setting password expiry to
"never".
We had another failure with Packit release infra where Packit workflow
failed due to type incorrectly being `None` where it needs to be either
string or list.
Checking with Packit team again and they apologized and proposed we just
use empty string. This took place in ther Matrix.
Signed-off-by: Freya Gustavsson <freya@venefilyn.se>
OpenSSL 3.x changed the default private key format from traditional RSA
(BEGIN RSA PRIVATE KEY) to PKCS#8 (BEGIN PRIVATE KEY). The GnuTLS-based
tests fail with PKCS#8 format keys, producing
GNUTLS_E_PREMATURE_TERMINATION errors during TLS handshake.
Use the -traditional flag with openssl genrsa to generate keys in the
legacy RSA format that GnuTLS can handle properly.
This wasn't yet the case in 2020 when we refreshed the certificates the
last time.
Freya Gustavsson