Fix: agent port file is o+w when launching as root
authorJonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Thu, 21 Jul 2022 13:30:27 +0000 (09:30 -0400)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Fri, 19 Aug 2022 16:20:53 +0000 (12:20 -0400)
commitde5abcb02431896a1827dff5d3376e1f2e124cd7
tree97d1c0bcd7251f1a87c4bc163d507d5130676bbb
parent206e6505316df1b86196d6582ef1e632e47d43a5
Fix: agent port file is o+w when launching as root

Observed issue
==============

When starting as root, the following permissions are observed:

[-rw-rw-rw-]  agent.port
[-rw-r--r--]  lttng-sessiond.pid

When starting as user:

[-rw-rw----]  agent.port
[-rw-rw-r--]  lttng-sessiond.pid

Note that despite being created by the same function,
`utils_create_pid_file`, the permissions are not the same.

Cause
=====

`get_wait_shm` manipulates the umask and does not restore it, thus
influencing the outcome of following file creations that don't enforce
specific permissions (using chmod).

Also `fopen` defaults to mode `0666 & ~umask`, thus resulting in
unnecessarily lax permissions when the session daemon is started as a
non-privileged user (umask = 0002, most of the time).

Solution
========

Mimic other call sites of umask(), modify then revert the umask.

Open the pid and agent port files as 0644 letting the umask to do its
job as necessary for those files.

Remove unnecessary umask() usage when chmod is directly used.

Known drawbacks
===============

Use of umask in a multi-threaded process is not recommended. Still our
current usage is limited and mostly happens during the initialization
phase. The usage of umask() is required for the `wait_shm` since on
FreeBSD it is not possible to chmod an shm file descriptor. The default
umask would interfere here.

Discussion
==========

The usage in run-as is valid even when in no-clone mode (valgrind) since
it is the sole user of umask() following the initialization phase. When
spawned as a separate process the clearing of umask is totally valid
even if it is not ideal since we are ignoring any umask set by the user.

It seems like the current usage is the lesser evil here.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ie224d254714fff05f4bced471ebfa8f19eede26a
src/bin/lttng-sessiond/client.cpp
src/bin/lttng-sessiond/register.cpp
src/common/shm.cpp
src/common/utils.cpp
This page took 0.02563 seconds and 4 git commands to generate.