Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reload config on SELinux policy load #379

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/broker/broker.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
#include "util/sockopt.h"
#include "util/user.h"

static int broker_propagate_sighup(Broker *broker) {
return controller_dbus_send_sighup(&broker->controller);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return controller_dbus_send_sighup(&broker->controller);
int r;
r = controller_dbus_send_sighup(&broker->controller);
return error_fold(r);

}

static int broker_dispatch_signals(DispatchFile *file) {
Broker *broker = c_container_of(file, Broker, signals_file);
struct signalfd_siginfo si;
Expand All @@ -37,6 +41,10 @@ static int broker_dispatch_signals(DispatchFile *file) {

c_assert(l == sizeof(si));

if (si.ssi_signo == SIGHUP) {
broker_propagate_sighup(broker);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably want to propagate the return value of this call via error_fold(). You currently fall-through to DISPATCH_EXIT, which will terminate the broker (which likely is what you are seeing in your tests).

}

return DISPATCH_E_EXIT;
}

Expand Down Expand Up @@ -134,6 +142,7 @@ int broker_new(Broker **brokerp, const char *machine_id, int log_fd, int control
sigemptyset(&sigmask);
sigaddset(&sigmask, SIGTERM);
sigaddset(&sigmask, SIGINT);
sigaddset(&sigmask, SIGHUP);

broker->signals_fd = signalfd(-1, &sigmask, SFD_CLOEXEC | SFD_NONBLOCK);
if (broker->signals_fd < 0)
Expand Down Expand Up @@ -210,6 +219,7 @@ int broker_run(Broker *broker) {
sigemptyset(&signew);
sigaddset(&signew, SIGTERM);
sigaddset(&signew, SIGINT);
sigaddset(&signew, SIGHUP);

sigprocmask(SIG_BLOCK, &signew, &sigold);

Expand Down
44 changes: 44 additions & 0 deletions src/broker/controller-dbus.c
Original file line number Diff line number Diff line change
Expand Up @@ -789,3 +789,47 @@ int controller_dbus_send_reload(Controller *controller, User *user, uint32_t ser

return 0;
}

/**
* controller_dbus_send_reload() - Will send a sighup up the chain to reload configs.
*/
int controller_dbus_send_sighup(Controller *controller) {
static const CDVarType type[] = {
C_DVAR_T_INIT(
CONTROLLER_T_MESSAGE(
C_DVAR_T_TUPLE0
)
)
};
_c_cleanup_(c_dvar_deinit) CDVar var = C_DVAR_INIT;
_c_cleanup_(message_unrefp) Message *message = NULL;
_c_cleanup_(c_freep) void *data = NULL;
size_t n_data;
int r;

c_dvar_begin_write(&var, (__BYTE_ORDER == __BIG_ENDIAN), type, 1);
c_dvar_write(&var, "((yyyyuu[(y<o>)(y<s>)(y<s>)])())",
c_dvar_is_big_endian(&var) ? 'B' : 'l', DBUS_MESSAGE_TYPE_SIGNAL, DBUS_HEADER_FLAG_NO_REPLY_EXPECTED, 1, 0, (uint32_t)-1,
DBUS_MESSAGE_FIELD_PATH, c_dvar_type_o, "/org/bus1/DBus/Broker",
DBUS_MESSAGE_FIELD_INTERFACE, c_dvar_type_s, "org.bus1.DBus.Broker",
DBUS_MESSAGE_FIELD_MEMBER, c_dvar_type_s, "Sighup");

r = c_dvar_end_write(&var, &data, &n_data);
if (r)
return error_origin(r);

r = message_new_outgoing(&message, data, n_data);
if (r)
return error_fold(r);
data = NULL;

r = connection_queue(&controller->connection, NULL, message);
if (r) {
if (r == CONNECTION_E_QUOTA)
return CONTROLLER_E_QUOTA;

return error_fold(r);
}

return 0;
}
1 change: 1 addition & 0 deletions src/broker/controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ int controller_dbus_dispatch(Controller *controller, Message *message);
int controller_dbus_send_activation(Controller *controller, const char *path, uint64_t serial);
int controller_dbus_send_reload(Controller *controller, User *user, uint32_t serial);
int controller_dbus_send_environment(Controller *controller, const char * const *env, size_t n_env);
int controller_dbus_send_sighup(Controller *controller);

C_DEFINE_CLEANUP(Controller *, controller_deinit);

Expand Down
11 changes: 11 additions & 0 deletions src/util/selinux.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <c-stdaux.h>
#include <selinux/selinux.h>
#include <selinux/avc.h>
#include <signal.h>
#include <stdlib.h>
#include "util/audit.h"
#include "util/error.h"
Expand Down Expand Up @@ -340,6 +341,15 @@ static int bus_selinux_log(int type, const char *fmt, ...) {
return 0;
}

/**
* On a policy reload we need to reparse the SELinux configuration file, since
* this could have changed. The call back is registered in the broker, but
* we need to reload the configuration in the launcher.
*/
static int policy_reload_callback(int seqno) {
return raise(SIGHUP);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does SELinux do with return values from this callback?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In our case nothing. If the callback returns non-zero selinux_status_updated returns -1, but the dbus-broker code isn't calling selinux_status_updated directly. Its calling it through selinux_check_access which ignores the return.

}

/**
* bus_selinux_init_global() - initialize the global SELinux context
*
Expand Down Expand Up @@ -386,6 +396,7 @@ int bus_selinux_init_global(void) {
}

selinux_set_callback(SELINUX_CB_LOG, (union selinux_callback)bus_selinux_log);
selinux_set_callback(SELINUX_CB_POLICYLOAD, (union selinux_callback)policy_reload_callback);

/* XXX: set audit callback to get more metadata in the audit log? */

Expand Down
Loading