security: make it possible to set SELinux label of child process from its binary

Normally when a child process is started by libvirt, the SELinux label
of that process is set to virtd_t (plus an MCS range). In at least one
case (passt) we need for the SELinux label of a child process label to
match the label that the binary would have transitioned to
automatically if it had been run standalone (in the case of passt,
that label is passt_t).

This patch modifies virSecuritySELinuxSetChildProcessLabel() (and all
the functions above it in the call chain) so that the toplevel
function can set a new argument "useBinarySpecificLabel" to true. If
it is true, then virSecuritySELinuxSetChildProcessLabel() will call
the new function virSecuritySELinuxContextSetFromFile(), which uses
the selinux library function security_compute_create() to determine
what would be the label of the new process if it had been run
standalone (rather than being run by libvirt) - the MCS range from the
normally-used label is added to this newly derived label, and that is
what is used for the new process rather than whatever is in the
domain's security label (which will usually be virtd_t).

In order to easily verify that nothing was broken by these changes to
the call chain, all callers currently set useBinarySpecificPath =
false, so all behavior should be completely unchanged. (The next
patch will set it to true only for the case of running passt.)

https://bugzilla.redhat.com/2172267
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This commit is contained in:
Laine Stump 2023-03-01 15:34:32 -05:00
parent 60afe39576
commit 75056f61f1
16 changed files with 99 additions and 11 deletions

View File

@ -217,7 +217,7 @@ qemuDBusStart(virQEMUDriver *driver,
virCommandDaemonize(cmd); virCommandDaemonize(cmd);
virCommandAddArgFormat(cmd, "--config-file=%s", configfile); virCommandAddArgFormat(cmd, "--config-file=%s", configfile);
if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, NULL) < 0) if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, false, NULL) < 0)
goto cleanup; goto cleanup;
if (virPidFileReadPath(pidfile, &cpid) < 0) { if (virPidFileReadPath(pidfile, &cpid) < 0) {

View File

@ -281,7 +281,7 @@ qemuPasstStart(virDomainObj *vm,
if (qemuExtDeviceLogCommand(driver, vm, cmd, "passt") < 0) if (qemuExtDeviceLogCommand(driver, vm, cmd, "passt") < 0)
return -1; return -1;
if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, NULL) < 0) if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, false, NULL) < 0)
goto error; goto error;
return 0; return 0;

View File

@ -7747,7 +7747,7 @@ qemuProcessLaunch(virConnectPtr conn,
VIR_DEBUG("Setting up security labelling"); VIR_DEBUG("Setting up security labelling");
if (qemuSecuritySetChildProcessLabel(driver->securityManager, if (qemuSecuritySetChildProcessLabel(driver->securityManager,
vm->def, cmd) < 0) vm->def, false, cmd) < 0)
goto cleanup; goto cleanup;
virCommandSetOutputFD(cmd, &logfile); virCommandSetOutputFD(cmd, &logfile);

View File

@ -636,6 +636,7 @@ qemuSecurityCommandRun(virQEMUDriver *driver,
virCommand *cmd, virCommand *cmd,
uid_t uid, uid_t uid,
gid_t gid, gid_t gid,
bool useBinarySpecificLabel,
int *exitstatus) int *exitstatus)
{ {
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
@ -643,8 +644,10 @@ qemuSecurityCommandRun(virQEMUDriver *driver,
int ret = -1; int ret = -1;
if (virSecurityManagerSetChildProcessLabel(driver->securityManager, if (virSecurityManagerSetChildProcessLabel(driver->securityManager,
vm->def, cmd) < 0) vm->def, useBinarySpecificLabel,
cmd) < 0) {
return -1; return -1;
}
if (uid != (uid_t) -1) if (uid != (uid_t) -1)
virCommandSetUID(cmd, uid); virCommandSetUID(cmd, uid);

View File

@ -115,6 +115,7 @@ int qemuSecurityCommandRun(virQEMUDriver *driver,
virCommand *cmd, virCommand *cmd,
uid_t uid, uid_t uid,
gid_t gid, gid_t gid,
bool useBinarySpecificLabel,
int *exitstatus); int *exitstatus);
/* Please note that for these APIs there is no wrapper yet. Do NOT blindly add /* Please note that for these APIs there is no wrapper yet. Do NOT blindly add

View File

@ -325,7 +325,7 @@ qemuSlirpStart(virDomainObj *vm,
if (qemuExtDeviceLogCommand(driver, vm, cmd, "slirp") < 0) if (qemuExtDeviceLogCommand(driver, vm, cmd, "slirp") < 0)
goto error; goto error;
if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, NULL) < 0) if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, false, NULL) < 0)
goto error; goto error;
rc = virPidFileReadPath(pidfile, &pid); rc = virPidFileReadPath(pidfile, &pid);

View File

@ -963,8 +963,9 @@ qemuTPMEmulatorStart(virQEMUDriver *driver,
return -1; return -1;
if (qemuSecurityCommandRun(driver, vm, cmd, cfg->swtpm_user, if (qemuSecurityCommandRun(driver, vm, cmd, cfg->swtpm_user,
cfg->swtpm_group, NULL) < 0) cfg->swtpm_group, false, NULL) < 0) {
goto error; goto error;
}
if (virPidFileReadPath(pidfile, &pid) < 0) { if (virPidFileReadPath(pidfile, &pid) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

View File

@ -152,7 +152,7 @@ int qemuExtVhostUserGPUStart(virQEMUDriver *driver,
virCommandAddArgFormat(cmd, "--render-node=%s", video->accel->rendernode); virCommandAddArgFormat(cmd, "--render-node=%s", video->accel->rendernode);
} }
if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, NULL) < 0) if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, false, NULL) < 0)
goto error; goto error;
rc = virPidFileReadPath(pidfile, &pid); rc = virPidFileReadPath(pidfile, &pid);

View File

@ -570,6 +570,7 @@ AppArmorSetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
static int static int
AppArmorSetSecurityChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED, AppArmorSetSecurityChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
virDomainDef *def, virDomainDef *def,
bool useBinarySpecificLabel G_GNUC_UNUSED,
virCommand *cmd) virCommand *cmd)
{ {
g_autofree char *profile_name = NULL; g_autofree char *profile_name = NULL;

View File

@ -2273,6 +2273,7 @@ virSecurityDACSetProcessLabel(virSecurityManager *mgr,
static int static int
virSecurityDACSetChildProcessLabel(virSecurityManager *mgr, virSecurityDACSetChildProcessLabel(virSecurityManager *mgr,
virDomainDef *def, virDomainDef *def,
bool useBinarySpecificLabel G_GNUC_UNUSED,
virCommand *cmd) virCommand *cmd)
{ {
virSecurityDACData *priv = virSecurityManagerGetPrivateData(mgr); virSecurityDACData *priv = virSecurityManagerGetPrivateData(mgr);

View File

@ -96,6 +96,7 @@ typedef int (*virSecurityDomainSetProcessLabel) (virSecurityManager *mgr,
virDomainDef *def); virDomainDef *def);
typedef int (*virSecurityDomainSetChildProcessLabel) (virSecurityManager *mgr, typedef int (*virSecurityDomainSetChildProcessLabel) (virSecurityManager *mgr,
virDomainDef *def, virDomainDef *def,
bool useBinarySpecificLabel,
virCommand *cmd); virCommand *cmd);
typedef int (*virSecurityDomainSecurityVerify) (virSecurityManager *mgr, typedef int (*virSecurityDomainSecurityVerify) (virSecurityManager *mgr,
virDomainDef *def); virDomainDef *def);

View File

@ -885,10 +885,14 @@ virSecurityManagerSetProcessLabel(virSecurityManager *mgr,
int int
virSecurityManagerSetChildProcessLabel(virSecurityManager *mgr, virSecurityManagerSetChildProcessLabel(virSecurityManager *mgr,
virDomainDef *vm, virDomainDef *vm,
bool useBinarySpecificLabel,
virCommand *cmd) virCommand *cmd)
{ {
if (mgr->drv->domainSetSecurityChildProcessLabel) if (mgr->drv->domainSetSecurityChildProcessLabel) {
return mgr->drv->domainSetSecurityChildProcessLabel(mgr, vm, cmd); return mgr->drv->domainSetSecurityChildProcessLabel(mgr, vm,
useBinarySpecificLabel,
cmd);
}
virReportUnsupportedError(); virReportUnsupportedError();
return -1; return -1;

View File

@ -145,6 +145,7 @@ int virSecurityManagerSetProcessLabel(virSecurityManager *mgr,
virDomainDef *def); virDomainDef *def);
int virSecurityManagerSetChildProcessLabel(virSecurityManager *mgr, int virSecurityManagerSetChildProcessLabel(virSecurityManager *mgr,
virDomainDef *def, virDomainDef *def,
bool useBinarySpecificLabel,
virCommand *cmd); virCommand *cmd);
int virSecurityManagerVerify(virSecurityManager *mgr, int virSecurityManagerVerify(virSecurityManager *mgr,
virDomainDef *def); virDomainDef *def);

View File

@ -152,6 +152,7 @@ virSecurityDomainSetProcessLabelNop(virSecurityManager *mgr G_GNUC_UNUSED,
static int static int
virSecurityDomainSetChildProcessLabelNop(virSecurityManager *mgr G_GNUC_UNUSED, virSecurityDomainSetChildProcessLabelNop(virSecurityManager *mgr G_GNUC_UNUSED,
virDomainDef *vm G_GNUC_UNUSED, virDomainDef *vm G_GNUC_UNUSED,
bool useBinarySpecificLabel G_GNUC_UNUSED,
virCommand *cmd G_GNUC_UNUSED) virCommand *cmd G_GNUC_UNUSED)
{ {
return 0; return 0;

View File

@ -560,6 +560,52 @@ virSecuritySELinuxContextAddRange(const char *src,
return ret; return ret;
} }
static char *
virSecuritySELinuxContextSetFromFile(const char *origLabel,
const char *binaryPath)
{
g_autofree char *currentCon = NULL;
g_autofree char *binaryCon = NULL;
g_autofree char *naturalLabel = NULL;
g_autofree char *updatedLabel = NULL;
/* First learn what would be the context set
* if binaryPath was exec'ed from this process.
*/
if (getcon(&currentCon) < 0) {
virReportSystemError(errno, "%s",
_("unable to get SELinux context for current process"));
return NULL;
}
if (getfilecon(binaryPath, &binaryCon) < 0) {
virReportSystemError(errno, _("unable to get SELinux context for '%s'"),
binaryPath);
return NULL;
}
if (security_compute_create(currentCon, binaryCon,
string_to_security_class("process"),
&naturalLabel) < 0) {
virReportSystemError(errno,
_("unable create new SELinux label based on label '%s' and file '%s'"),
origLabel, binaryPath);
return NULL;
}
/* now get the type from the original label
* (which already has proper MCS set) and add it to
* the new label
*/
updatedLabel = virSecuritySELinuxContextAddRange(origLabel, naturalLabel);
VIR_DEBUG("original label: '%s' binary: '%s' binary-specific label: '%s'",
origLabel, binaryPath, NULLSTR(updatedLabel));
return g_steal_pointer(&updatedLabel);
}
static char * static char *
virSecuritySELinuxGenNewContext(const char *basecontext, virSecuritySELinuxGenNewContext(const char *basecontext,
const char *mcs, const char *mcs,
@ -2986,10 +3032,13 @@ virSecuritySELinuxSetProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
static int static int
virSecuritySELinuxSetChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED, virSecuritySELinuxSetChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
virDomainDef *def, virDomainDef *def,
bool useBinarySpecificLabel G_GNUC_UNUSED,
virCommand *cmd) virCommand *cmd)
{ {
/* TODO: verify DOI */ /* TODO: verify DOI */
virSecurityLabelDef *secdef; virSecurityLabelDef *secdef;
g_autofree char *tmpLabel = NULL;
const char *label = NULL;
secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
if (!secdef || !secdef->label) if (!secdef || !secdef->label)
@ -3006,8 +3055,30 @@ virSecuritySELinuxSetChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
return -1; return -1;
} }
/* pick either the common label used by most binaries exec'ed by
* libvirt, or the specific label of this binary.
*/
if (useBinarySpecificLabel) {
const char *binaryPath = virCommandGetBinaryPath(cmd);
if (!binaryPath)
return -1; /* error was already logged */
tmpLabel = virSecuritySELinuxContextSetFromFile(secdef->label,
binaryPath);
if (!tmpLabel)
return -1;
label = tmpLabel;
} else {
label = secdef->label;
}
/* save in cmd to be set after fork/before child process is exec'ed */ /* save in cmd to be set after fork/before child process is exec'ed */
virCommandSetSELinuxLabel(cmd, secdef->label); virCommandSetSELinuxLabel(cmd, label);
return 0; return 0;
} }

View File

@ -458,6 +458,7 @@ virSecurityStackSetProcessLabel(virSecurityManager *mgr,
static int static int
virSecurityStackSetChildProcessLabel(virSecurityManager *mgr, virSecurityStackSetChildProcessLabel(virSecurityManager *mgr,
virDomainDef *vm, virDomainDef *vm,
bool useBinarySpecificLabel,
virCommand *cmd) virCommand *cmd)
{ {
virSecurityStackData *priv = virSecurityManagerGetPrivateData(mgr); virSecurityStackData *priv = virSecurityManagerGetPrivateData(mgr);
@ -465,9 +466,11 @@ virSecurityStackSetChildProcessLabel(virSecurityManager *mgr,
int rc = 0; int rc = 0;
for (; item; item = item->next) { for (; item; item = item->next) {
if (virSecurityManagerSetChildProcessLabel(item->securityManager, vm, cmd) < 0) if (virSecurityManagerSetChildProcessLabel(item->securityManager, vm,
useBinarySpecificLabel, cmd) < 0) {
rc = -1; rc = -1;
} }
}
return rc; return rc;
} }