Avoid shadowing variables that are already declared in a previous scope,
and make such occurrences compile-time errors. These are not functional
changes.
In a few places do related code cleanup.
A similar ostree PR is here:
https://github.com/ostreedev/ostree/pull/2195
This fixes compilation with GLib 2.63’s `gdbus-codegen`, which
automatically enables Unix FD support for D-Bus APIs which use the `h`
type, even if the `org.gtk.GDBus.C.UnixFD` annotation is missing.
By adding the annotation, the generated header whether using
`gdbus-codegen` 2.63 or < 2.63.
This is a slight API break in GLib, but it’s questionable about whether
flatpak should have always had the annotation.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
The org.freedesktop.Flatpak user D-Bus service isn't just used by
flatpak(1) or applications running as Flatpaks. It's also used by
toolbox(1) for similar reasons:
* To keep various configuration files inside the container
synchronized with the host
* To let the container request certain commands to be run on the host
The org.freedesktop.Flatpak D-Bus service itself doesn't need much in
the way of dependencies, but inherits a lot of unused shared library
linkages through the libflatpak-common.la convenience library. Removing
these unused shared libraries reduces the footprint of toolbox(1) for
those who care about such things. eg., Fedora CoreOS.
This commit brings down the number of shared libraries to 19 from 62.
Closes: #3052
Approved by: alexlarsson
... by moving the definition of FlatpakHostCommandFlags from the
'common' sub-directory to 'session-helper'. It hasn't been used by
anything else ever since flatpak-builder was moved to a separate
repository in commit 52bd146561.
Closes: #3052
Approved by: alexlarsson
Toolbox [1] containers, like Flatpak containers, need to keep some
configuration files synchronized with the host. eg., /etc/localtime,
/etc/resolv.conf, etc.. The easiest way to keep them updated is to bind
mount the entire /etc from the host at some location inside the
container, and then replace the container's copy of the files with
symbolic links to the bind mounted copies.
However, this doesn't work when the host's copy of a configuration file
is itself a symbolic link to a location outside /etc. eg.,
/etc/localtime, which points somewhere inside /usr/share/zoneinfo. In
that case the host needs to be actively monitored via inotify(7) and
an updated copy of the file needs to maintained in a separate
directory, which can be bind mounted into the container.
This monitoring infrastructure is already present in Flatpak and it
makes sense to leverage it for Toolbox containers too.
However, Flatpak containers have only one user, the current one logged
into the host, whereas Toolbox containers have a longer list of users
like the host itself. Therefore, if the directory getting bind mounted
(ie., $XDG_RUNTIME_DIR/.flatpak-helper/monitor) isn't world readable,
then some users inside the container won't be able to read the
configuration file. eg., this will fail:
$ sudo -u operator cat /run/host/monitor/localtime
Relaxing the directory permissions on the host shouldn't pose a
security risk. They are copies of originals that are world readable
anyway.
[1] https://github.com/debarshiray/toolboxCloses: #2916
Approved by: alexlarsson
Uncrustify has an option "nl_func_var_def_blk" which is supposed to
ensure there's a newline character between the block of variable
definitions and the rest of the function body, but it gets confused and
thinks that the first instance of "g_autoptr" or "g_auto" being used on
a variable is the start of the function body. So this commit removes
those extra newline characters and removes that option in uncrustify.cfg
so they don't get re-added the next time uncrustify is run.
Here's the command I used:
perl -0777 -i -pe 's/\n(\n\s*g_auto\()/\1/g' `git ls-tree --name-only
-r HEAD | grep \\\.[ch]$ | grep -v common/valgrind-private.h |
grep -v app/flatpak-polkit-agent-text-listener\\\.[ch]`
I ran it again with "g_autoptr" in place of "g_auto", and made a few
manual edits to add back the newline when the g_auto* was in the middle
of a function body rather than at the top.
Closes: #2715
Approved by: matthiasclasen
There was a typo here: (gpointer *) pid_data should have been
(gpointer *) &pid_data, so that g_hash_table_iter_next() would make
pid_data a pointer to a PidData struct. Instead, the previous
implementation left pid_data set to NULL, leading to a NULL dereference
and segmentation fault whenever a name fell off the bus while a watched
client existed.
Instead of directly inserting the missing "&", I've used a pattern
that avoids needing the cast, in an attempt to make it more obviously
correct.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Closes: #2417
Approved by: matthiasclasen
Add flag that instructs the session-helper to kill
the spawned command when the caller drops off the bus.
Closes: #2326Closes: #2365
Approved by: alexlarsson
This exposes a /etc/timezone with the current timezone, as per the old
debian spec: https://wiki.debian.org/TimeZoneChanges
In case we're using the session-helper this will be extracted from
the host config and applied whenever that changes.
Normally timezone info is specified by /etc/localtime being a symlink
into the locale data, and you can look at the symlink value itself.
However, in the sandbox we can't update a symlink in /etc at runtime,
nor can we make it of the canonical form as that would point into the
runtime. This is why /etc/timezone is used.
This fixes https://github.com/flatpak/flatpak/issues/2190Closes: #2214
Approved by: alexlarsson
p11-kit server version < 0.23.10 doesn't properly daemonize which
causes g_spawn_sync() to hang forever waiting for the pipe to
close.
We work around this by passing --sh, which is only available in the
working version, and fill fail the old ones.
https://github.com/flatpak/flatpak/issues/1768Closes: #1782
Approved by: alexlarsson
If p11-kit server is installed on the host, we spawn a copy of this, forwarding the access to the
p11-kit trust module in a read-only way.
We then (if the above worked) bind mount the socket as /run/user/$UID/p11-kit/pkcs11 in the sandbox,
which is the default socket path for the p11-kit-client module.
We also add a configuration file in /etc/pkcs11/modules/p11-kit-trust.module that makes the trust
module actually load the client module instead. This means applications automatically switch
to using the host certs for trust if possible, and use the runtime ca-certificates otherwise.
Additionally we add a config file that always disables pkcs user
config merging, because pkcs11 modules on the host are unlikely to work in a random runtime.
Closes: #1757
Approved by: alexlarsson
I think this enhances readability significantly, reduces code duplication
and allows you to follow the rationale behind why monitors are being
added/removed a lot more clearly than the previous nested/ad-hoc logic in
file_changed. Also adds debug printouts in the case that file monitors
are not created successfully.
Closes: #1194
Approved by: pwithnall
The monitor data structures are never replaced in the hash table, so
it's not necessary. Keep MonitorData pointers in scope in main() and
set up g_autoptr instead.
Closes: #1194
Approved by: pwithnall
The GLib logging framework automatically appends a \n to messages, so it
doesn’t need to be added by callers.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
For example, add $(AM_CFLAGS) to mumble_CFLAGS. Since $(WARN_CFLAGS) is
only added to $(AM_CFLAGS), this fixes the lack of inclusion of the
compiler warning flags in the compilation of half of flatpak.
Note that $(AM_*) variables are only used by automake if a more specific
(per-target) special variable is not defined instead. So if you define
mumble_CFLAGS, AM_CFLAGS will not be used for that target unless
explicitly included in mumble_CFLAGS.
See
https://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html.
Do the same for $(AM_LIBADD), $(AM_LDFLAGS), etc. These are not
currently defined, but it’s good practice to include them in
mumble_LIBADD (etc.) just in case they’re defined in future. Hopefully
their inclusions will be cargo-culted to any new targets which are
added, retaining full coverage of the code base.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Just because stdout/stderr happen to be going to a terminal doesn't
necessarily mean that we want the command launched by HostCommand
to take ownership of the terminal - in the case where 'flatpak run'
is launched from a terminal, stderr will typically be left pointing
to the terminal for any command. In that case the terminal already
has a controlling process, and TIOCSCTTY will fail; avoid scary
messages by making the failure only a g_debug()
This was using the "from" fd from the fd_map, which will have already been
closed by time we reach this portion of the child setup.
Tracking the movement of FDs while resolving the remappings is rather
tedious and error prone, so just locate the final fd before calling
the ioctl() in child setup.
This is needed by e.g gnome-builder that want to pass this pid to
a profiler service. Its not really less secure/sandboxed this way,
because the app can launch whatever he wants in the host, which could
anyway talk back and report the real pid.
