On 01/03/2018 06:00 PM, Christian Ehrhardt wrote:
Since 1b4f66e "security: introduce virSecurityManager (Set|Restore)ChardevLabel" this is a public API of security manager.
Implementing this in apparmor avoids miss any rules that should be added for devices labeled via these calls.
I believe s/miss/missing/, s/labeled/labelled/
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/security_apparmor.c | 74 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 60a8e08..e91f157 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -946,6 +946,77 @@ AppArmorRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, }
static int +AppArmorSetChardevLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainChrSourceDefPtr dev_source, + bool chardevStdioLogd ATTRIBUTE_UNUSED) +{ + char *in = NULL, *out = NULL; + int ret = -1; + + virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def, + SECURITY_APPARMOR_NAME); + if (!secdef) + return 0;
There shouldn't be empty line in variable declaration. However, there should be one after the block. BTW: here and in the function below - if we need to call a function with a long name to initialize a variable we usually do that on a separate line. virSecurityLabelDefPtr secdef; secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); if (!secdef) return 0;
+ + switch ((virDomainChrType) dev_source->type) { + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_FILE: + case VIR_DOMAIN_CHR_TYPE_UNIX: + case VIR_DOMAIN_CHR_TYPE_PTY: + ret = reload_profile(mgr, def, dev_source->data.file.path, true); + break; + + case VIR_DOMAIN_CHR_TYPE_PIPE: + if (virAsprintf(&in, "%s.in", dev_source->data.file.path) < 0 || + virAsprintf(&out, "%s.out", dev_source->data.file.path) < 0) + goto done; + if (virFileExists(in)) { + if (reload_profile(mgr, def, in, true) < 0) + goto done; + } + if (virFileExists(out)) { + if (reload_profile(mgr, def, out, true) < 0) + goto done; + } + ret = reload_profile(mgr, def, dev_source->data.file.path, true); + break; + + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_UDP: + case VIR_DOMAIN_CHR_TYPE_TCP: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_LAST: + ret = 0; + break; + } + + done: + VIR_FREE(in); + VIR_FREE(out); + return ret; +} + +static int +AppArmorRestoreChardevLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainChrSourceDefPtr dev_source ATTRIBUTE_UNUSED, + bool chardevStdioLogd ATTRIBUTE_UNUSED) +{ + virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return 0; + + return reload_profile(mgr, def, NULL, false); +} + +static int AppArmorSetSavedStateLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, const char *savefile) @@ -1067,6 +1138,9 @@ virSecurityDriver virAppArmorSecurityDriver = {
.domainSetPathLabel = AppArmorSetPathLabel,
+ .domainSetSecurityChardevLabel = AppArmorSetChardevLabel, + .domainRestoreSecurityChardevLabel = AppArmorRestoreChardevLabel, + .domainSetSecurityImageFDLabel = AppArmorSetFDLabel, .domainSetSecurityTapFDLabel = AppArmorSetFDLabel,
Michal