security: replace uses of label and VIR_FREE by g_autofree

Moving towards full adoption of GLib APIs in the AppArmor code.

Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
This commit is contained in:
Georgia Garcia 2025-01-07 12:23:37 -03:00 committed by Jim Fehlig
parent 76b9227eea
commit a751d30220
2 changed files with 46 additions and 96 deletions

View File

@ -115,37 +115,28 @@ profile_loaded(const char *str)
static int static int
profile_status_file(const char *str) profile_status_file(const char *str)
{ {
char *profile = NULL; g_autofree char *profile = NULL;
char *content = NULL; g_autofree char *content = NULL;
char *tmp = NULL; g_autofree char *tmp = NULL;
int rc = -1;
int len; int len;
profile = g_strdup_printf("%s/%s", APPARMOR_DIR "/libvirt", str); profile = g_strdup_printf("%s/%s", APPARMOR_DIR "/libvirt", str);
if (!virFileExists(profile)) if (!virFileExists(profile))
goto failed; return -1;
if ((len = virFileReadAll(profile, MAX_FILE_LEN, &content)) < 0) { if ((len = virFileReadAll(profile, MAX_FILE_LEN, &content)) < 0) {
virReportSystemError(errno, virReportSystemError(errno,
_("Failed to read \'%1$s\'"), profile); _("Failed to read \'%1$s\'"), profile);
goto failed; return -1;
} }
/* create string that is ' <str> flags=(complain)\0' */ /* create string that is ' <str> flags=(complain)\0' */
tmp = g_strdup_printf(" %s flags=(complain)", str); tmp = g_strdup_printf(" %s flags=(complain)", str);
if (strstr(content, tmp) != NULL) if (strstr(content, tmp) != NULL)
rc = 0; return 0;
else return 1;
rc = 1;
failed:
VIR_FREE(tmp);
VIR_FREE(profile);
VIR_FREE(content);
return rc;
} }
/* /*
@ -218,7 +209,7 @@ static int
use_apparmor(void) use_apparmor(void)
{ {
int rc = -1; int rc = -1;
char *libvirt_daemon = NULL; g_autofree char *libvirt_daemon = NULL;
if (virFileResolveLink("/proc/self/exe", &libvirt_daemon) < 0) { if (virFileResolveLink("/proc/self/exe", &libvirt_daemon) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, virReportError(VIR_ERR_INTERNAL_ERROR,
@ -232,7 +223,7 @@ use_apparmor(void)
return 1; return 1;
if (access(APPARMOR_PROFILES_PATH, R_OK) != 0) if (access(APPARMOR_PROFILES_PATH, R_OK) != 0)
goto cleanup; return rc;
/* First check profile status using full binary path. If that fails /* First check profile status using full binary path. If that fails
* check using profile name. * check using profile name.
@ -247,8 +238,6 @@ use_apparmor(void)
rc = -1; rc = -1;
} }
cleanup:
VIR_FREE(libvirt_daemon);
return rc; return rc;
} }
@ -950,7 +939,8 @@ AppArmorSetChardevLabel(virSecurityManager *mgr,
virDomainChrSourceDef *dev_source, virDomainChrSourceDef *dev_source,
bool chardevStdioLogd G_GNUC_UNUSED) bool chardevStdioLogd G_GNUC_UNUSED)
{ {
char *in = NULL, *out = NULL; g_autofree char *in = NULL;
g_autofree char *out = NULL;
int ret = -1; int ret = -1;
virSecurityLabelDef *secdef; virSecurityLabelDef *secdef;
@ -971,11 +961,11 @@ AppArmorSetChardevLabel(virSecurityManager *mgr,
out = g_strdup_printf("%s.out", dev_source->data.file.path); out = g_strdup_printf("%s.out", dev_source->data.file.path);
if (virFileExists(in)) { if (virFileExists(in)) {
if (reload_profile(mgr, def, in, true) < 0) if (reload_profile(mgr, def, in, true) < 0)
goto done; return ret;
} }
if (virFileExists(out)) { if (virFileExists(out)) {
if (reload_profile(mgr, def, out, true) < 0) if (reload_profile(mgr, def, out, true) < 0)
goto done; return ret;
} }
ret = reload_profile(mgr, def, dev_source->data.file.path, true); ret = reload_profile(mgr, def, dev_source->data.file.path, true);
break; break;
@ -995,9 +985,6 @@ AppArmorSetChardevLabel(virSecurityManager *mgr,
break; break;
} }
done:
VIR_FREE(in);
VIR_FREE(out);
return ret; return ret;
} }
@ -1083,12 +1070,11 @@ AppArmorSetPathLabel(virSecurityManager *mgr,
bool allowSubtree) bool allowSubtree)
{ {
int rc = -1; int rc = -1;
char *full_path = NULL; g_autofree char *full_path = NULL;
if (allowSubtree) { if (allowSubtree) {
full_path = g_strdup_printf("%s/{,**}", path); full_path = g_strdup_printf("%s/{,**}", path);
rc = reload_profile(mgr, def, full_path, true); rc = reload_profile(mgr, def, full_path, true);
VIR_FREE(full_path);
} else { } else {
rc = reload_profile(mgr, def, path, true); rc = reload_profile(mgr, def, path, true);
} }

View File

@ -146,9 +146,8 @@ vah_info(const char *str)
static int static int
parserCommand(const char *profile_name, const char cmd) parserCommand(const char *profile_name, const char cmd)
{ {
int result = -1;
char flag[3]; char flag[3];
char *profile; g_autofree char *profile = NULL;
int status; int status;
int ret; int ret;
@ -163,7 +162,7 @@ parserCommand(const char *profile_name, const char cmd)
if (!virFileExists(profile)) { if (!virFileExists(profile)) {
vah_error(NULL, 0, _("profile does not exist")); vah_error(NULL, 0, _("profile does not exist"));
goto cleanup; return -1;
} else { } else {
const char * const argv[] = { const char * const argv[] = {
"/sbin/apparmor_parser", flag, profile, NULL "/sbin/apparmor_parser", flag, profile, NULL
@ -175,23 +174,18 @@ parserCommand(const char *profile_name, const char cmd)
(WIFEXITED(status) && WEXITSTATUS(status) != 0)) { (WIFEXITED(status) && WEXITSTATUS(status) != 0)) {
if (ret != 0) { if (ret != 0) {
vah_error(NULL, 0, _("failed to run apparmor_parser")); vah_error(NULL, 0, _("failed to run apparmor_parser"));
goto cleanup; return -1;
} else if (cmd == 'R' && WIFEXITED(status) && } else if (cmd == 'R' && WIFEXITED(status) &&
WEXITSTATUS(status) == 234) { WEXITSTATUS(status) == 234) {
vah_warning(_("unable to unload already unloaded profile")); vah_warning(_("unable to unload already unloaded profile"));
} else { } else {
vah_error(NULL, 0, _("apparmor_parser exited with error")); vah_error(NULL, 0, _("apparmor_parser exited with error"));
goto cleanup; return -1;
} }
} }
} }
result = 0; return 0;
cleanup:
VIR_FREE(profile);
return result;
} }
/* /*
@ -201,18 +195,17 @@ static int
update_include_file(const char *include_file, const char *included_files, update_include_file(const char *include_file, const char *included_files,
bool append) bool append)
{ {
int rc = -1;
int plen, flen = 0; int plen, flen = 0;
int fd; int fd;
char *pcontent = NULL; g_autofree char *pcontent = NULL;
char *existing = NULL; g_autofree char *existing = NULL;
const char *warning = const char *warning =
"# DO NOT EDIT THIS FILE DIRECTLY. IT IS MANAGED BY LIBVIRT.\n"; "# DO NOT EDIT THIS FILE DIRECTLY. IT IS MANAGED BY LIBVIRT.\n";
if (virFileExists(include_file)) { if (virFileExists(include_file)) {
flen = virFileReadAll(include_file, MAX_FILE_LEN, &existing); flen = virFileReadAll(include_file, MAX_FILE_LEN, &existing);
if (flen < 0) if (flen < 0)
return rc; return -1;
} }
if (append && virFileExists(include_file)) if (append && virFileExists(include_file))
@ -223,38 +216,31 @@ update_include_file(const char *include_file, const char *included_files,
plen = strlen(pcontent); plen = strlen(pcontent);
if (plen > MAX_FILE_LEN) { if (plen > MAX_FILE_LEN) {
vah_error(NULL, 0, _("invalid length for new profile")); vah_error(NULL, 0, _("invalid length for new profile"));
goto cleanup; return -1;
} }
/* only update the disk profile if it is different */ /* only update the disk profile if it is different */
if (flen > 0 && flen == plen && STREQLEN(existing, pcontent, plen)) { if (flen > 0 && flen == plen && STREQLEN(existing, pcontent, plen)) {
rc = 0; return 0;
goto cleanup;
} }
/* write the file */ /* write the file */
if ((fd = open(include_file, O_CREAT | O_TRUNC | O_WRONLY, 0644)) == -1) { if ((fd = open(include_file, O_CREAT | O_TRUNC | O_WRONLY, 0644)) == -1) {
vah_error(NULL, 0, _("failed to create include file")); vah_error(NULL, 0, _("failed to create include file"));
goto cleanup; return -1;
} }
if (safewrite(fd, pcontent, plen) < 0) { /* don't write the '\0' */ if (safewrite(fd, pcontent, plen) < 0) { /* don't write the '\0' */
VIR_FORCE_CLOSE(fd); VIR_FORCE_CLOSE(fd);
vah_error(NULL, 0, _("failed to write to profile")); vah_error(NULL, 0, _("failed to write to profile"));
goto cleanup; return -1;
} }
if (VIR_CLOSE(fd) != 0) { if (VIR_CLOSE(fd) != 0) {
vah_error(NULL, 0, _("failed to close or write to profile")); vah_error(NULL, 0, _("failed to close or write to profile"));
goto cleanup; return -1;
} }
rc = 0; return 0;
cleanup:
VIR_FREE(pcontent);
VIR_FREE(existing);
return rc;
} }
/* /*
@ -574,7 +560,7 @@ caps_mockup(vahControl * ctl, const char *xmlStr)
{ {
g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlDoc) xml = NULL;
g_autoptr(xmlXPathContext) ctxt = NULL; g_autoptr(xmlXPathContext) ctxt = NULL;
char *arch; g_autofree char *arch = NULL;
if (!(xml = virXMLParse(NULL, xmlStr, _("(domain_definition)"), if (!(xml = virXMLParse(NULL, xmlStr, _("(domain_definition)"),
"domain", &ctxt, NULL, false))) { "domain", &ctxt, NULL, false))) {
@ -600,7 +586,6 @@ caps_mockup(vahControl * ctl, const char *xmlStr)
ctl->arch = virArchFromHost(); ctl->arch = virArchFromHost();
} else { } else {
ctl->arch = virArchFromString(arch); ctl->arch = virArchFromString(arch);
VIR_FREE(arch);
} }
return 0; return 0;
@ -685,15 +670,15 @@ get_definition(vahControl * ctl, const char *xmlStr)
static int static int
vah_add_path(virBuffer *buf, const char *path, const char *perms, bool recursive) vah_add_path(virBuffer *buf, const char *path, const char *perms, bool recursive)
{ {
char *tmp = NULL;
int rc = -1; int rc = -1;
bool readonly = true; bool readonly = true;
bool explicit_deny_rule = true; bool explicit_deny_rule = true;
char *sub = NULL; char *sub = NULL;
char *perms_new = NULL; g_autofree char *tmp = NULL;
char *pathdir = NULL; g_autofree char *perms_new = NULL;
char *pathtmp = NULL; g_autofree char *pathdir = NULL;
char *pathreal = NULL; g_autofree char *pathtmp = NULL;
g_autofree char *pathreal = NULL;
if (path == NULL) if (path == NULL)
return rc; return rc;
@ -730,7 +715,7 @@ vah_add_path(virBuffer *buf, const char *path, const char *perms, bool recursive
if ((pathreal = realpath(pathdir, NULL)) == NULL) { if ((pathreal = realpath(pathdir, NULL)) == NULL) {
vah_error(NULL, 0, pathdir); vah_error(NULL, 0, pathdir);
vah_error(NULL, 0, _("could not find realpath")); vah_error(NULL, 0, _("could not find realpath"));
goto cleanup; return rc;
} }
tmp = g_strdup_printf("%s%s", pathreal, pathtmp); tmp = g_strdup_printf("%s%s", pathreal, pathtmp);
} }
@ -754,7 +739,7 @@ vah_add_path(virBuffer *buf, const char *path, const char *perms, bool recursive
vah_error(NULL, 0, path); vah_error(NULL, 0, path);
vah_error(NULL, 0, _("skipped restricted file")); vah_error(NULL, 0, _("skipped restricted file"));
} }
goto cleanup; return rc;
} }
if (tmp[strlen(tmp) - 1] == '/') if (tmp[strlen(tmp) - 1] == '/')
@ -771,13 +756,6 @@ vah_add_path(virBuffer *buf, const char *path, const char *perms, bool recursive
virBufferAsprintf(buf, " \"%s/\" r,\n", tmp); virBufferAsprintf(buf, " \"%s/\" r,\n", tmp);
} }
cleanup:
VIR_FREE(pathdir);
VIR_FREE(pathtmp);
VIR_FREE(pathreal);
VIR_FREE(perms_new);
VIR_FREE(tmp);
return rc; return rc;
} }
@ -793,36 +771,28 @@ vah_add_file_chardev(virBuffer *buf,
const char *perms, const char *perms,
const int type) const int type)
{ {
char *pipe_in; g_autofree char *pipe_in = NULL;
char *pipe_out; g_autofree char *pipe_out = NULL;
int rc = -1;
if (type == VIR_DOMAIN_CHR_TYPE_PIPE) { if (type == VIR_DOMAIN_CHR_TYPE_PIPE) {
/* add the pipe input */ /* add the pipe input */
pipe_in = g_strdup_printf("%s.in", path); pipe_in = g_strdup_printf("%s.in", path);
if (vah_add_file(buf, pipe_in, perms) != 0) if (vah_add_file(buf, pipe_in, perms) != 0)
goto clean_pipe_in; return -1;
/* add the pipe output */ /* add the pipe output */
pipe_out = g_strdup_printf("%s.out", path); pipe_out = g_strdup_printf("%s.out", path);
if (vah_add_file(buf, pipe_out, perms) != 0) if (vah_add_file(buf, pipe_out, perms) != 0)
goto clean_pipe_out; return -1;
rc = 0;
clean_pipe_out:
VIR_FREE(pipe_out);
clean_pipe_in:
VIR_FREE(pipe_in);
} else { } else {
/* add the file */ /* add the file */
if (vah_add_file(buf, path, perms) != 0) if (vah_add_file(buf, path, perms) != 0)
return -1; return -1;
rc = 0;
} }
return rc; return 0;
} }
static int static int
@ -1473,8 +1443,8 @@ main(int argc, char **argv)
vahControl _ctl = { 0 }; vahControl _ctl = { 0 };
vahControl *ctl = &_ctl; vahControl *ctl = &_ctl;
int rc = -1; int rc = -1;
char *profile = NULL; g_autofree char *profile = NULL;
char *include_file = NULL; g_autofree char *include_file = NULL;
off_t size; off_t size;
bool purged = 0; bool purged = 0;
@ -1517,7 +1487,7 @@ main(int argc, char **argv)
if (ctl->cmd == 'D') if (ctl->cmd == 'D')
unlink(include_file); unlink(include_file);
} else if (ctl->cmd == 'c' || ctl->cmd == 'r') { } else if (ctl->cmd == 'c' || ctl->cmd == 'r') {
char *included_files = NULL; g_autofree char *included_files = NULL;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
if (ctl->cmd == 'c' && virFileExists(profile)) if (ctl->cmd == 'c' && virFileExists(profile))
@ -1579,7 +1549,7 @@ main(int argc, char **argv)
/* create the profile from TEMPLATE */ /* create the profile from TEMPLATE */
if (ctl->cmd == 'c' || purged) { if (ctl->cmd == 'c' || purged) {
char *tmp = NULL; g_autofree char *tmp = NULL;
#if defined(WITH_APPARMOR_3) #if defined(WITH_APPARMOR_3)
const char *ifexists = "if exists "; const char *ifexists = "if exists ";
#else #else
@ -1597,7 +1567,6 @@ main(int argc, char **argv)
vah_error(ctl, 0, _("could not create profile")); vah_error(ctl, 0, _("could not create profile"));
unlink(include_file); unlink(include_file);
} }
VIR_FREE(tmp);
} }
if (rc == 0 && !ctl->dryrun) { if (rc == 0 && !ctl->dryrun) {
@ -1613,14 +1582,9 @@ main(int argc, char **argv)
unlink(profile); unlink(profile);
} }
} }
cleanup:
VIR_FREE(included_files);
} }
cleanup:
vahDeinit(ctl); vahDeinit(ctl);
VIR_FREE(profile);
VIR_FREE(include_file);
exit(rc == 0 ? EXIT_SUCCESS : EXIT_FAILURE); exit(rc == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
} }