The same assertion appears closer to the top of the function, and there
is no opportunity for it to have become false here.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
Found by source code inspection while trying to debug an unrelated
leak.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
This was added around 12½ years ago, in a commented-out state, and has
remained commented out ever since. It turns out these test vectors
do pass, although perhaps they didn't at the time.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
The deleted lines used to be a test for _dbus_validate_signature(),
until I deleted that function. We also had a completely separate
test for _dbus_validate_signature_with_reason() which remains present.
Some of the test vectors were tested in both places.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
This lets _dbus_warn() and _dbus_warn_check_failed() fall through
to flushing stderr and calling _dbus_abort(), meaning that failed
checks and warnings can result in a core dump as intended.
By renaming the FATAL severity to ERROR, we ensure that any code
contributions that assumed the old semantics will fail to compile.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
All callers should use _dbus_validate_signature_with_reason() directly.
The only remaining callers were this function's own tests.
As a side benefit, this commit removes a TODO pointing out that this
function did not follow normal DBusString conventions, by considering
a length outside the bounds of the DBusString to be an ordinary
lack of validity rather than a fatal programming error.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
As noted in the previous commit, it's a trap.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
This function looks appealing, but it is a trap, particularly in
_dbus_return_val_if_fail() checks. It returns a boolean result, which
cannot distinguish between "failed because we ran out of memory" and
"failed because the string is actually invalid"; but
_dbus_validate_signature_with_reason() allocates memory. Use the
over-complicated version directly, so libdbus can continue to
bend over backwards to support the (possibly mythical) operating systems
that limit memory consumption and do not overcommit, such that malloc()
can genuinely return NULL.
Bug detected by running the DBusVariant unit test (fd.o #101568) under
dbus' failing-malloc() instrumentation.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
If this is reinstated it will need some checks. In particular, it
was using _dbus_check_is_valid_signature() in an unsafe way:
_dbus_check_is_valid_signature() cannot be used in a
_dbus_return_val_if_fail() check because it does not distinguish
between error by the caller, and out-of-memory conditions.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
By default ${runstatedir} is the same as ${localstatedir}/run, but many
Linux distributions configure it to be /run and mount a tmpfs in that
location. All other factors being equal, it is preferable to use /run
where available because it is guaranteed to be local, whereas traversing
/var might involve automounting a networked filesystem (even though
/var/run itself is very likely to be a tmpfs).
/run or /var/run is currently only used in a few places in dbus, but
I plan to make more use of it during the development of
<https://bugs.freedesktop.org/show_bug.cgi?id=100344>.
The pid file is not part of the API between dbus and other software
(other than distribution init scripts for dbus itself), so we do not
need to keep it strictly compatible; so it is OK to move it.
We do not yet use /run for the system bus socket, because that is
part of the API between D-Bus clients and servers, and has always been
"officially" /var/run/dbus/system_bus_socket.
<https://bugs.freedesktop.org/show_bug.cgi?id=101628> tracks the
possibility of changing that.
Similarly, we do not replace /var/run/console with /run/console, because
that path is part of the API between dbus-daemon and the obsolete PAM
modules pam_console and pam_foreground that used /var/run/console.
<https://bugs.freedesktop.org/show_bug.cgi?id=101629> tracks the possible
future removal of that code path.
In the CMake build system, the equivalent of ${runstatedir} remains
hard-coded to the equivalent of ${localstatedir}/run for simplicity. For
the sort of system-wide installations that would consider redefining
${runstatedir} to /run, the Autotools build system is strongly
recommended: in particular this is what Linux distributions are expected
to use.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101569
The wording and formatting used here is consistent with other
semi-recently-added match keys.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
Reviewed-by: Philip Withnall <withnall@endlessm.com>
[smcv: Wrap BecomeMonitor in <literal> as per Philip's review]
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
This is no longer true, and it seems less misleading to raise an
error than to obey the letter of the spec by quietly ignoring calls
from an inappropriate caller.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
Now that we're starting to implement methods in more places, it makes
sense to share this code. The Stats interface can already benefit.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
Eavesdropping on unicast messages to other processes is not something
that should be done by processes in containers, or on the system bus
by users other than root or the bus owner. bus/system.conf.in
does not enable eavesdropping, but adding inadvisable configuration
could. This brings it into line with Monitoring.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
When we listen on a tcp: address we should get a connectable tcp:
address, and so on.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
dbus_server_get_address() returns a copy. It isn't clear why.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
These are like unix:tmpdir=/something, except that the resulting
socket is always path-based, never abstract.
This is desirable for two reasons:
* If a Linux container manager wants to expose a path-based socket
into the container, it can do so by bind-mounting it in the
container's filesystem namespace. That cannot work for abstract
sockets because they are not files.
* Conversely, if a Linux container manager does not want to expose
a path-based socket in the container, it can avoid bind-mounting it,
or bind-mount some harmless object like /dev/null over it.
That cannot work for abstract sockets because access to abstract
sockets is part of the network namespace, which is all-or-nothing.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
On systemd systems, /etc/machine-id is guaranteed to exist and has
the same format as the D-Bus machine ID. The major D-Bus implementations
read /etc/machine-id if it exists, but some less up-to-date
implementations still only read /var/lib/dbus/machine-id. We can be
nice to those implementations by ensuring /var/lib/dbus/machine-id
is a symlink; this way, the two files can never get out of sync.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101570
If we somehow get an autolaunch address with multiple
semicolon-separated components, and one of them fails, then we will
hit an assertion failure when we try the next one.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101257
(cherry picked from commit ecdcb86bff)
The build timestamp is not particularly useful (the version number of
the package is already present in the HTML), and it prevents the build
from being reproducible. See <https://reproducible-builds.org/> for more
information.
Signed-off-by: Simon McVittie <smcv@debian.org>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=100692
(cherry picked from commit 0310ead002)
The error message was leaked when blocking on a pending call after
the connection was disconnected.
Reviewed-by: Philip Withnall <withnall@endlessm.com>
[smcv: re-word commit message]
Reviewed-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101481
The error message was leaked when blocking on a pending call after
the connection was disconnected.
Reviewed-by: Philip Withnall <withnall@endlessm.com>
[smcv: re-word commit message]
Reviewed-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101481
According to git history, this test was written in 2006 by Red Hat
employee John Palmieri and has received only trivial changes since
then. Red Hat gave permission in 2007 for their contributions to
be relicensed under the MIT/X11 license. We cannot take advantage
of that permission to relicense the core library or the dbus-daemon
from GPL-2+|AFL-2.0 to MIT/X11, because one early copyright holder
(CodeFactory AB) could not be traced, but we might as well use a
permissive license for simple test code that has not had
CodeFactory AB contributions.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101481
Currently when asked the SELinux context of the owner of
org.freedesktop.DBus, the dbus-daemon is returning an error.
In the same situation when asked about the Unix user or the PID, the
daemon would return its own user or pid. Do the same for the SELinux
context by returning the daemon one.
In particular this avoids an issue seen with systemd --user, where
dbus-daemon responds to UpdateActivationEnvironment() by passing on the
new environment to systemd with o.fd.systemd1.Manager.SetEnvironment(),
but systemd cannot get the caller's SELinux context and so rejects the
SetEnvironment() call.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101315
[smcv: Extend commit message to describe the symptom this fixes]
Reviewed-by: Simon McVittie <smcv@collabora.com>
Now that g_test_trap_fork() has gone, we no longer have any calls to
GLib functions deprecated in or before 2.40.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101362
This re-executes the same binary with special command-line options
instead of forking. As a result, it can work on Windows, and is less
dependent on Unix libraries continuing to work across a fork().
(This has been confirmed to work in Windows binaries running under Wine.)
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101362
This is quite old (it's the version in Ubuntu 14.04 LTS, and older
than the version in Debian 8) but gives us g_test_skip(),
g_test_trap_subprocess() and GVariantDict, all of which will be
useful in the regression tests.
Remove workarounds for old versions.
After this commit we are still using the deprecated g_test_trap_fork(),
which will be removed in a subsequent commit. Don't opt-in to the new
deprecation warnings from 2.38 and 2.40 yet, because under our recommended
settings for dbus developers (-Werror) they would break the build.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101362
This function already raised an error, and all callers handled that
error as gracefully as they could (because _dbus_generate_uuid() is
failable, since 2015). Given that, it seems unnecessarily hostile
to do a _dbus_warn_check_failed() unless we have no better alternative:
yes, it indicates that dbus has not been installed correctly, but
during build-time tests it's entirely reasonable that dbus has not
yet been installed.
Callers are:
* DBusConnection, to implement Peer.GetMachineId()
* The bus driver, to implement Peer.GetMachineId()
* X11 autolaunching
* dbus_get_local_machine_id()
Of those, only the last one is not in a position to return an error
gracefully, so move the _dbus_warn_check_failed() to there.
Migrate the text about the D-Bus library being incorrectly set up
into the error emitted by the Unix implementation, and to make it
less misleading, include separate error messages for both the
files we try to read:
$ bwrap --ro-bind / / --dev /dev --tmpfs /etc --tmpfs /var \
./tools/dbus-uuidgen --get
D-Bus library appears to be incorrectly set up: see the manual
page for dbus-uuidgen to correct this issue. (Failed to open
"/var/lib/dbus/machine-id": No such file or directory; Failed to open
"/etc/machine-id": No such file or directory)
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=13194
