summaryrefslogtreecommitdiff
path: root/common
Commit message (Collapse)AuthorAgeFilesLines
* common: Fix an uninitialized variableHEADmainSimon McVittie2023-05-171-1/+1
| | | | | | | | | This could be uninitialized if the app has --socket=fallback-x11 but not --socket=wayland, although that's a combination that doesn't make a great deal of sense. Regression introduced during recent refactoring. Fixes: db7a8bb5 "common: Split up socket setup from flatpak-run into multiple files" Signed-off-by: Simon McVittie <smcv@collabora.com>
* common: Move json-glib backports to their own fileSimon McVittie2023-05-174-12/+37
| | | | | | There is currently no source for this one, only a header. Signed-off-by: Simon McVittie <smcv@collabora.com>
* glib-backports: Use g_ascii_string_to_unsigned if GLib is new enoughSimon McVittie2023-05-173-34/+39
| | | | | | | | | | | | Use the real GLib function if we can, and resync the backport with the version in GLib 2.76.2: use a compatibility replacement for G_NUMBER_PARSER_ERROR so that it can be textually identical to the version in GLib, and revert Flatpak changes to the whitespace. The only functional change is that if the function fails, we'll raise G_NUMBER_PARSER_ERROR_INVALID if GLib is new enough. Signed-off-by: Simon McVittie <smcv@collabora.com>
* glib-backports: Move flatpak_utils_ascii_string_to_unsigned to hereSimon McVittie2023-05-174-92/+91
| | | | | | | | Currently this is used unconditionally, even if GLib is new enough. That will be changed in a subsequent commit; no functional change intended in this one. Signed-off-by: Simon McVittie <smcv@collabora.com>
* glib-backports: Resync g_get_language_names_with_category with GLib 2.76.2Simon McVittie2023-05-171-2/+4
| | | | | | | | - 0e7bf99e "Use "e" mode flag in fopen () calls for race-free setting of the close-on-exec flag" - Add #ifndef G_OS_WIN32, which is irrelevant for Flatpak but keeps the code textually equivalent to GLib's Signed-off-by: Simon McVittie <smcv@collabora.com>
* glib-backports: Resync ISO8601 parsing with GLib 2.76.2Simon McVittie2023-05-171-19/+39
| | | | | | | | | | | | - 3384ed3f "Fixing signedness warnings in glib/gdatetime.c" - faa1d63c "glib: Fix various compiler warnings when compiling with G_DISABLE_ASSERT" - 4ddabfc6 "gdatetime: Avoid an assertion failure when parsing some ISO 8601 dates" - b4eaac58 "gdatetime: Handle leap seconds in ISO8601 dates" - f9d0135a "gdatetime: Port to use new g_time_zone_new_identifier() constructor" (partially reverted here) - c3805d74 "gdatetime: Disallow NAN as a number of seconds in a GDateTime" - 5d7f4b8f "gdatetime: Remove floating point from seconds parsing code" - d5580edf "Fix non-initialized variable in glib/gdatetime.c"
* glib-backports: Document which version we backportedSimon McVittie2023-05-171-1/+1
| | | | Signed-off-by: Simon McVittie <smcv@collabora.com>
* glib-backports: Add a note that a couple of functions aren't backportsSimon McVittie2023-05-172-0/+5
| | | | | | | | | We can't backport GLib's implementations of these, because they make use of GHashTable/GPtrArray internals. Instead, we have a reimplementation of the same API, which accepts the cost of some redundant work as a reasonable price to pay for backwards-compatibility. Signed-off-by: Simon McVittie <smcv@collabora.com>
* glib-backports: Make g_key_file_save_to_file match the GLib implementationSimon McVittie2023-05-171-0/+6
| | | | | | | This just adds some assertions, no functional changes (assuming we're calling it correctly). Signed-off-by: Simon McVittie <smcv@collabora.com>
* common: Move direct backports from GLib to a new translation unitSimon McVittie2023-05-176-857/+916
| | | | | | | | | | | | | | | | | flatpak-utils.c is getting quite large, and is a mixture of code with and without non-GLib dependencies, making it inconvenient to reuse in other projects (like Steam's pressure-vessel) or link into Flatpak services that don't need all of our dependencies (json-glib, appstream, ostree). One obvious piece of low-hanging fruit for reducing the size of this file is to move the GLib backports into their own translation unit. Sort them by GLib version, so that when we increase our GLib dependency it's easy to delete the ones that are no longer applicable. No functional changes intended in this commit. Signed-off-by: Simon McVittie <smcv@collabora.com>
* common: Split up socket setup from flatpak-run into multiple filesSimon McVittie2023-05-1514-1372/+1743
| | | | | | | | | | flatpak-run is large enough to be getting unwieldy, so separate it out into various smaller modules. A side benefit of these is that they'll be easier to reuse in other projects, like Steam's pressure-vessel tool. Signed-off-by: Simon McVittie <smcv@collabora.com>
* transaction: Add new flatpak_transaction_add_rebase_and_uninstall() APIPhilip Withnall2023-03-303-7/+126
| | | | | | | | | | | | | | | | This mostly replaces `flatpak_transaction_add_rebase()`. It’s necessary because the uninstall op for an eol-rebased app needs to be linked to the install/update op for the rebased app, otherwise one op can proceed after the other has failed (or they can be run in the wrong order) and result in the old app being uninstalled but the new one not installed. The following commit will port the internal flatpak `FlatpakTransaction` subclasses to use it. Other consumers of `FlatpakTransaction` (such as gnome-software) will have to be ported as well. Signed-off-by: Philip Withnall <pwithnall@endlessos.org> Fixes: #3991
* transaction: Add an out_op argument to flatpak_transaction_add_ref()Philip Withnall2023-03-301-6/+14
| | | | | | | | | This will be used in the next commit to simplify some new code. Currently, this introduces no functional changes. Signed-off-by: Philip Withnall <pwithnall@endlessos.org> Helps: #3991
* Use new --disable-userns bubblewrap feature when possibleAlexander Larsson2023-03-241-0/+32
| | | | | | | | | | | | | | This feature (added in https://github.com/containers/bubblewrap/pull/488) allows us to improve the guarantees of disallowing the sandbox to use recursive user namespaces (which is a security risk) compared to the existing limits that use seccomp. [smcv: Move this to flatpak_run_setup_base_argv() so it will apply equally in apply_extra_data() and `flatpak build`; make the compile-time check for a setuid bwrap into a runtime check] Co-authored-by: Simon McVittie <smcv@collabora.com> Signed-off-by: Simon McVittie <smcv@collabora.com>
* dir: Consistently initialize g_autofree variablesSimon McVittie2023-03-201-1/+1
| | | | | | | | | | | | | | | All g_autofree and g_autoptr variables should be initialized, either with a value or NULL, and some gcc versions warn if they are not. This particular instance was harmless, because we initialized regexp to either NULL or a valid non-NULL value as the first statement of the function, so there was no way to return before regexp was initialized; but if we introduced an early-return such as a g_return_val_if_fail before the current first statement, then that early return would have caused uninitialized stack contents to be freed, likely resulting in a crash. Signed-off-by: Simon McVittie <smcv@collabora.com>
* dir: Fix a minor leak of an OSTree checksumPhilip Withnall2023-03-201-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The checksum here can leak if `flatpak_dir_remote_load_cached_summary()` returns false at least once. Spotted by asan while running gnome-software: ``` Direct leak of 2925 byte(s) in 45 object(s) allocated from: #0 0x7f44774ba6af in __interceptor_malloc (/lib64/libasan.so.8+0xba6af) #1 0x7f44764c941a in g_malloc ../../source/glib/glib/gmem.c:130 #2 0x7f445bc860e7 in ostree_checksum_from_bytes src/libostree/ostree-core.c:1599 #3 0x7f445bdbea82 in flatpak_dir_remote_fetch_indexed_summary /opt/gnome/source/flatpak/common/flatpak-dir.c:12563 #4 0x7f445bd9932e in flatpak_remote_state_ensure_subsummary /opt/gnome/source/flatpak/common/flatpak-dir.c:577 #5 0x7f445bdbfd42 in _flatpak_dir_get_remote_state /opt/gnome/source/flatpak/common/flatpak-dir.c:12872 #6 0x7f445bdc006c in flatpak_dir_get_remote_state_optional /opt/gnome/source/flatpak/common/flatpak-dir.c:12953 #7 0x7f445be07886 in flatpak_transaction_ensure_remote_state /opt/gnome/source/flatpak/common/flatpak-transaction.c:2057 #8 0x7f445be095c7 in flatpak_transaction_add_ref /opt/gnome/source/flatpak/common/flatpak-transaction.c:2732 #9 0x7f445be09c37 in flatpak_transaction_add_update /opt/gnome/source/flatpak/common/flatpak-transaction.c:2940 #10 0x7f445bdd202c in flatpak_installation_list_installed_refs_for_update /opt/gnome/source/flatpak/common/flatpak-installation.c:1103 #11 0x7f445bf07824 in gs_flatpak_add_updates ../../source/gnome-software/plugins/flatpak/gs-flatpak.c:2082 #12 0x7f445bf2e2b9 in gs_plugin_add_updates ../../source/gnome-software/plugins/flatpak/gs-plugin-flatpak.c:484 #13 0x7f44770533b2 in gs_plugin_loader_call_vfunc ../../source/gnome-software/lib/gs-plugin-loader.c:620 #14 0x7f447705430f in gs_plugin_loader_run_results ../../source/gnome-software/lib/gs-plugin-loader.c:748 #15 0x7f447706cb03 in gs_plugin_loader_process_thread_cb ../../source/gnome-software/lib/gs-plugin-loader.c:3110 #16 0x7f44769967ed in g_task_thread_pool_thread ../../source/glib/gio/gtask.c:1531 #17 0x7f447650e760 in g_thread_pool_thread_proxy ../../source/glib/glib/gthreadpool.c:350 #18 0x7f447650dd02 in g_thread_proxy ../../source/glib/glib/gthread.c:831 ``` Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
* Set size of file info for symlinks to 0Jan Alexander Steffens (heftig)2023-03-171-0/+1
| | | | | `ostree_raw_file_to_content_stream` will try to read it, causing a critical warning with GLib 2.76 causing tests to fail.
* run: Prevent TIOCLINUX ioctl, the same as TIOCSTISimon McVittie2023-03-161-0/+4
| | | | | | | | | | | | | | | | The TIOCLINUX ioctl is only available on Linux virtual consoles such as /dev/tty1. It has several Linux-specific functions, one of which is a copy/paste operation which can be used for attacks similar to TIOCSTI. This vulnerability does not affect typical graphical terminal emulators such as xterm, gnome-terminal and Konsole, and Flatpak is primarily designed to be run from a Wayland or X11 graphical environment, so this is relatively unlikely to be a practical problem. CVE-2023-28100, GHSA-7qpw-3vjv-xrqp Resolves: https://github.com/flatpak/flatpak/security/advisories/GHSA-7qpw-3vjv-xrqp Signed-off-by: Simon McVittie <smcv@debian.org>
* Reject paths given to --filesystem/--persist with special charactersRyan Gonzalez2023-03-163-7/+71
| | | | | | | | | | There isn't much in the way of legit reasons for this, but it's a potential security footgun when displaying the text. CVE-2023-28101, GHSA-h43h-fwqx-mpp8 Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com> Co-authored-by: Simon McVittie <smcv@collabora.com>
* Ensure special characters in permissions and metadata are escapedRyan Gonzalez2023-03-162-1/+92
| | | | | | | | | This prevents someone from placing special characters in order to manipulate the appearance of the permissions list. CVE-2023-28101, GHSA-h43h-fwqx-mpp8 Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
* transaction: Remove unused error argument from flatpak_transaction_add_op()Philip Withnall2023-02-231-22/+8
| | | | | | Introduces no functional changes, but simplifies the code a little. Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
* transaction: Fix a minor typo in a documentation commentPhilip Withnall2023-02-211-1/+1
| | | | Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
* transaction: Ignore uninstall operations for no deployDan Nicholson2023-02-181-1/+23
| | | | | | | | | | | | If `no_deploy` has been set to `TRUE` in a transaction, then the intention is that no changes will be made to the installed flatpaks. Currently that's not the case for explicitly or implicitly added uninstall operations. That's particularly bad for eol-rebase flatpaks since they old version will be automatically removed without the new version being installed. To address this, prevent uninstall operations from being added for no deploy transactions. Closes: #5172
* flatpak-run: Unset GDK_BACKENDDan Nicholson2023-02-131-0/+1
| | | | | | | | | | | If the `GDK_BACKEND` environment variable is present and it's value does not match the Wayland and X11 socket configuration, then a GTK app will fail to run since it will only consider the display backend from the environment variable. This should probably be extended to cover other display environment variables such as `QT_QPA_PLATFORM` for Qt and `SDL_VIDEODRIVER` for SDL. However, I've only tested this with GTK applications.
* dir: If metadata is syntactically invalid, say which file is the problemSimon McVittie2023-02-081-2/+2
| | | | | | Similar to the previous commit, but for metadata. Signed-off-by: Simon McVittie <smcv@collabora.com>
* dir: If overrides are syntactically invalid, include path in error messageSimon McVittie2023-02-083-3/+9
| | | | | | | | It's unhelpful to say something like "Key file contains line “x” which is not a key-value pair, group, or comment" without specifying which file we are talking about. Signed-off-by: Simon McVittie <smcv@collabora.com>
* utils-http: Avoid deprecation warning with curl >= 7.85.0Simon McVittie2023-02-071-0/+4
| | | | | | | CURLOPT_PROTOCOLS_STR is documented as the replacement for CURLOPT_PROTOCOLS. Signed-off-by: Simon McVittie <smcv@collabora.com>
* run: Avoid double-free of gpgconf stdout streamSimon McVittie2023-02-071-1/+1
| | | | | | | | | g_subprocess_get_stdout_pipe() does not transfer ownership, so the stream still belongs to the GSubprocess and we must not unref it. Fixes: 764e5a4d "Add --socket=gpg-agent" Resolves: https://github.com/flatpak/flatpak/issues/5095 Signed-off-by: Simon McVittie <smcv@collabora.com>
* dir: Add more debug output for unused refsPhaedrus Leeds2023-02-061-12/+65
| | | | | | | | | | Inspired by the slight mystery of https://github.com/flatpak/flatpak/issues/5192, this commit adds more debug output to the functions that analyze installations for unused refs. Some of the output is g_debug() (enabled by -vv) and some is g_info() (enabled by -v) since otherwise the output would be too verbose. This should hopefully help debug problems, not only related to the unused refs code but anything involving dependencies.
* Revert "Add support for files generated by appstreamcli compose (#5277)"Patrick Griffis2023-02-044-49/+12
| | | | This reverts commit 88f7ecd000f84b75bd055ad6b6f92d7881e5e377.
* Add support for files generated by appstreamcli compose (#5277)Luna2023-02-044-12/+49
| | | | Co-authored-by: Bartłomiej Piotrowski <b@bpiotrowski.pl> Co-authored-by: Jamie Murphy <hello@itsjamie.dev>
* utils: Unmap the old summary.idx file before trying to replace itForest2023-01-221-0/+4
| | | | | | | | | | | | | | | | | | | Exporting to an existing repo on a Samba filesystem failed with EACCES when libglnx called renameat() to replace the old summary.idx file. error: renameat: Permission denied This occurred even when the user had appropriate permissions to the file and its ancestor directories. The problem was that flatpak had mapped the old file into memory for reading, and still held a reference to that mapping when attempting to replace the underlying file. Apparently this works on some filesystems, but not on cifs. We therefore release the memory mapping before replacing the underlying file. Fixes #5257 Co-authored-by: Patrick <tingping@tingping.se>
* exports: Don't export parent or ancestor of reserved directoriesSimon McVittie2023-01-121-0/+13
| | | | | | | | | | | | | | | | | Previously, --filesystem=/run would prevent apps from starting by breaking our ability to set up /run/flatpak and /run/host. Now it is ignored, with a diagnostic message, resolving #5205 and #5207. Similarly, --filesystem=/symlink-to-root (or --filesystem=host) would have prevented apps from starting if a symlink like `/symlink-to-root -> /` or `/symlink-to-root -> .` exists, and refusing to export the target of that symlink avoids that failure mode, resolving #1357. Resolves: https://github.com/flatpak/flatpak/issues/1357 Resolves: https://github.com/flatpak/flatpak/issues/5205 Resolves: https://github.com/flatpak/flatpak/issues/5207 Signed-off-by: Simon McVittie <smcv@collabora.com>
* context: Show a warning if we cannot provide any $HOMESimon McVittie2023-01-121-4/+8
| | | | | | | | | | | | | | | | | | | | If $HOME is below a reserved path (for example `/usr/home/thompson` for Unix traditionalists) or otherwise cannot be shared, or is a symbolic link to somewhere that cannot be shared, then we will end up running the app with $HOME not existing. This is unexpected, so we should make more noise about it. There are two situations here, both of which get a warning: if we have --filesystem=home or --filesystem=host then we are trying to share the real $HOME with the application, and if we do not, then we are trying to create a directory at the location of the real $HOME and replicate the chain of symlinks (if any) leading from $HOME to that location. Unlike the previous commit, this is not expected to happen during unit testing, so we do not use a g_warning() for this. Diagnoses: https://github.com/flatpak/flatpak/issues/5035 Signed-off-by: Simon McVittie <smcv@collabora.com>
* context: Show a warning when --filesystem exists but can't be sharedSimon McVittie2023-01-121-4/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | If the user gives us a override or command-line argument that we cannot obey, like --filesystem=/usr/share/whatever or --filesystem=/run/flatpak/whatever, then it's confusing that we silently ignore it. We should give them an opportunity to see that their override was ineffective. However, there are a few situations where we still want to keep quiet. If there is a --filesystem argument for something that simply doesn't exist, we don't diagnose the failure to share it: that avoids creating unnecessary noise for apps that opportunistically share locations that might or might not exist, like the way the Steam app on Flathub asks for access to $XDG_RUNTIME_DIR/app/com.discordapp.Discord. Similarly, if we have been asked for --filesystem=host, the root directory is very likely to contain symlinks into a reserved path, like /lib -> usr/lib. We don't need a user-visible warning for that. We actually use the equivalent of g_message() rather than g_warning(), to avoid this being fatal during unit testing (in particular when we do a `flatpak info` on an app that has never been run, which will be unable to share its `.var/app` subdirectory). `app/flatpak-main.c` currently displays them as equivalent to each other anyway. Signed-off-by: Simon McVittie <smcv@collabora.com>
* exports: Move error handling up into callerSimon McVittie2023-01-123-61/+141
| | | | | | | | | | | | | This lets flatpak_context_export() or other callers decide how they want to handle failure to export each path. For now, the callers in FlatpakExports are still using g_debug() unconditionally, but we can now have somewhat better test coverage. Helps: https://github.com/flatpak/flatpak/issues/1357 Helps: https://github.com/flatpak/flatpak/issues/5035 Helps: https://github.com/flatpak/flatpak/issues/5205 Helps: https://github.com/flatpak/flatpak/issues/5207 Signed-off-by: Simon McVittie <smcv@collabora.com>
* exports: Make _exports_path_expose produce a GError on failureSimon McVittie2023-01-121-31/+64
| | | | | | | | This is a step towards allowing its direct and indirect callers to decide how serious the failure is, and debug or warn accordingly. Helps: https://github.com/flatpak/flatpak/issues/5205 Signed-off-by: Simon McVittie <smcv@collabora.com>
* exports: Never try to export /.flatpak-infoSimon McVittie2023-01-121-0/+1
| | | | | | | Just for completeness, in practice the host system will not have this. Helps: https://github.com/flatpak/flatpak/issues/5205 Signed-off-by: Simon McVittie <smcv@collabora.com>
* exports: Never try to export paths below /run/flatpak or /run/hostSimon McVittie2023-01-121-0/+2
| | | | | | | These directories are reserved for Flatpak's own use. Helps: https://github.com/flatpak/flatpak/issues/5205 Signed-off-by: Simon McVittie <smcv@collabora.com>
* exports, context: List unexported paths one per line in sorted orderSimon McVittie2023-01-122-3/+26
| | | | | | | This will reduce conflicts when new entries are added. Helps: https://github.com/flatpak/flatpak/issues/5205 Signed-off-by: Simon McVittie <smcv@collabora.com>
* dir: Use consistent FlatpakRunFlags everywhereSimon McVittie2022-12-191-9/+12
| | | | | | In principle either of these functions might read any of the flags. Signed-off-by: Simon McVittie <smcv@collabora.com>
* portal: Factor out flatpak_bwrap_is_unprivileged()Simon McVittie2022-12-192-0/+14
| | | | | | | We can use this for other features that rely on having a non-setuid version of bubblewrap. Signed-off-by: Simon McVittie <smcv@collabora.com>
* common: Replace all flatpak_debug2() with g_debug()Simon McVittie2022-12-156-50/+48
| | | | | | | They are now equivalent. Resolves: https://github.com/flatpak/flatpak/issues/5001 Signed-off-by: Simon McVittie <smcv@collabora.com>
* common: Make flatpak_debug2() use the same domain as g_debug()Simon McVittie2022-12-152-14/+1
| | | | | | | | | | | | | | | | | | | | | | Now that we are logging `flatpak -v` messages with log level INFO, and printing INFO messages in the same way as DEBUG, we can reserve log level DEBUG for `flatpak -v -v` messages. This means we no longer need a weird secondary debug domain. There is a very small behaviour change here: G_MESSAGES_DEBUG=flatpak is now similar to `flatpak -v -v` (previously `flatpak -v`), and G_MESSAGES_DEBUG=flatpak2 no longer has any effect. This seems more in line with what would be expected from a GLib-based application. In flatpak(1) and the system helper, this does not change behaviour other than that: the same messages are logged by `-v` and by `-v -v` as before. In daemons that do not implement `-v -v` (the OCI authenticator, portal and session helper), it continues to be necessary to use G_MESSAGES_DEBUG to see flatpak_debug2() messages. Signed-off-by: Simon McVittie <smcv@collabora.com>
* common: Use g_info() for messages that will be shown by flatpak -vSimon McVittie2022-12-1514-259/+259
| | | | | | | This brings us one step closer to being able to stop using the flatpak2 log domain for messages that are exclusive to `flatpak -v -v`. Signed-off-by: Simon McVittie <smcv@collabora.com>
* Replace calls to g_memdup() with g_memdup2()Simon McVittie2022-12-122-2/+2
| | | | | | | | | | | | g_memdup() is subject to an integer overflow on 64-bit machines if the object being copied is larger than UINT_MAX bytes. I suspect none of these objects can actually be that large in practice, but it's easier to replace all the calls than it is to assess whether we need to replace them. A backport in libglnx is used on systems where GLib is older than 2.68.x. Signed-off-by: Simon McVittie <smcv@collabora.com>
* flatpak-run: unset GIO_EXTRA_MODULESLeorize2022-12-071-0/+1
| | | | | | | | | | | | | This variable contains paths to load GIO modules from. For the most part, they refer to paths outside of the sandbox or if they happen to be in the sandbox, would contain modules that are incompatible with the sandbox runtime (ie. different libc). While I've not found programs that would crash outright, it may cause unexpected behaviors (eg. Apostrophe not being able to render math in preview panel). This variable is set by NixOS for its dependency boxing.
* flatpak-run: unset XKB_CONFIG_ROOTLeorize2022-12-011-0/+1
| | | | | | | | | | | | | This variable is typically used to configure the use of a custom set of XKB definitions. In those cases, it's mostly meant for the X11 server or Wayland compositor. NixOS is known to employ this variable for their custom XKB layout implementation. When the path it points to is unreachable (due to the sandbox), most GTK+/Qt applications will crash on Wayland. Unsetting this does not seem to negatively impact the use of custom XKB layouts with Flatpak applications.
* flatpak-dir: Clean up old leaked deploy dirs at start of deployPhilip Withnall2022-11-161-0/+66
| | | | | | | | | | Following on from commit 85a83a06f95, add some code to clean up old leaked deploy tmpdirs when we next try to deploy the same app (successfully or not). This should free up disk space leaked by failed deploys pre-85a83a06f95. Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
* flatpak-dir: Add a debug message to remove_old_appstream_tmpdirs()Philip Withnall2022-11-161-0/+1
| | | | | | | This should make it a bit clearer when `rm -rf` is being used in the debug logs. Signed-off-by: Philip Withnall <pwithnall@endlessos.org>