[libvirt] [PATCHv2 2/2] security: Add a new func use stat to get process DAC label
When use qemuProcessAttach to attach a qemu process, cannot get a right DAC label. Add a new func to get process label via stat func. Do not remove virDomainDefGetSecurityLabelDef before try to use stat to get process DAC label, because There are some other func call virSecurityDACGetProcessLabel. v2 add support freeBSD. Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/security/security_dac.c | 95 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 2 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 85253af..89cafa3 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -23,6 +23,11 @@ #include <sys/stat.h> #include <fcntl.h> +#ifdef __FreeBSD__ +# include <sys/sysctl.h> +# include <sys/user.h> +#endif + #include "security_dac.h" #include "virerror.h" #include "virfile.h" @@ -1236,18 +1241,104 @@ virSecurityDACReserveLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return 0; } +#ifdef __linux__ +static int +virSecurityDACGetProcessLabelInternal(pid_t pid, + virSecurityLabelPtr seclabel) +{ + struct stat sb; + char *path = NULL; + char *label = NULL; + int ret = -1; + + VIR_INFO("Getting DAC user and group on process '%d'", pid); + + if (virAsprintf(&path, "/proc/%d", (int) pid) < 0) + goto cleanup; + + if (lstat(path, &sb) < 0) + goto cleanup; + + if (virAsprintf(&label, "+%u:+%u", + (unsigned int) sb.st_uid, + (unsigned int) sb.st_gid) < 0) + goto cleanup; + + if (virStrcpy(seclabel->label, label,VIR_SECURITY_LABEL_BUFLEN) == NULL) + goto cleanup; + ret = 0; + +cleanup: + VIR_FREE(path); + VIR_FREE(label); + return ret; +} +#elif defined(__FreeBSD__) +static int +virSecurityDACGetProcessLabelInternal(pid_t pid, + virSecurityLabelPtr seclabel) +{ + struct kinfo_proc p; + int mib[4]; + size_t len = 4; + char *label = NULL; + int ret = -1; + + sysctlnametomib("kern.proc.pid", mib, &len); + + len = sizeof(struct kinfo_proc); + mib[3] = pid; + + if (sysctl(mib, 4, &p, &len, NULL, 0) < 0) + goto cleanup; + + if (virAsprintf(&label, "+%u:+%u", + (unsigned int) p.ki_ruid, + (unsigned int) p.ki_rgid) < 0) + goto cleanup; + + if (virStrcpy(seclabel->label, label,VIR_SECURITY_LABEL_BUFLEN) == NULL) + goto cleanup; + ret = 0; + +cleanup: + VIR_FREE(label); + return ret; +} +#else +static int +virSecurityDACGetProcessLabelInternal(pid_t pid, + virSecurityLabelPtr seclabel) +{ + return -1; +} +#endif + static int virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, - pid_t pid ATTRIBUTE_UNUSED, + pid_t pid, virSecurityLabelPtr seclabel) { virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); - if (!secdef || !seclabel) + if (!seclabel) return -1; + if (secdef == NULL) { + VIR_DEBUG("missing label for DAC security " + "driver in domain %s", def->name); + + if (virSecurityDACGetProcessLabelInternal(pid, seclabel) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot get process %d DAC label"),pid); + return -1; + } + + return 0; + } + if (secdef->label) ignore_value(virStrcpy(seclabel->label, secdef->label, VIR_SECURITY_LABEL_BUFLEN)); -- 1.8.3.1
On 12/03/2014 11:08 AM, Luyao Huang wrote:
When use qemuProcessAttach to attach a qemu process, cannot get a right DAC label. Add a new func to get process label via stat func. Do not remove virDomainDefGetSecurityLabelDef before try to use stat to get process DAC label, because There are some other func call virSecurityDACGetProcessLabel.
v2 add support freeBSD.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/security/security_dac.c | 95 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 2 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 85253af..89cafa3 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -23,6 +23,11 @@ #include <sys/stat.h> #include <fcntl.h>
+#ifdef __FreeBSD__ +# include <sys/sysctl.h> +# include <sys/user.h> +#endif + #include "security_dac.h" #include "virerror.h" #include "virfile.h" @@ -1236,18 +1241,104 @@ virSecurityDACReserveLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return 0; }
+#ifdef __linux__ +static int +virSecurityDACGetProcessLabelInternal(pid_t pid, + virSecurityLabelPtr seclabel) +{ + struct stat sb; + char *path = NULL; + char *label = NULL; + int ret = -1; + + VIR_INFO("Getting DAC user and group on process '%d'", pid); + + if (virAsprintf(&path, "/proc/%d", (int) pid) < 0) + goto cleanup; +
+ if (lstat(path, &sb) < 0) + goto cleanup;
A more specific error should be reported here via virReportSystemError.
+ + if (virAsprintf(&label, "+%u:+%u", + (unsigned int) sb.st_uid, + (unsigned int) sb.st_gid) < 0) + goto cleanup; + + if (virStrcpy(seclabel->label, label,VIR_SECURITY_LABEL_BUFLEN) == NULL) + goto cleanup;
Using snprintf would simplify this code.
+ ret = 0; + +cleanup: + VIR_FREE(path); + VIR_FREE(label); + return ret; +} +#elif defined(__FreeBSD__) +static int +virSecurityDACGetProcessLabelInternal(pid_t pid, + virSecurityLabelPtr seclabel) +{ + struct kinfo_proc p; + int mib[4]; + size_t len = 4; + char *label = NULL; + int ret = -1; + + sysctlnametomib("kern.proc.pid", mib, &len); + + len = sizeof(struct kinfo_proc); + mib[3] = pid; + + if (sysctl(mib, 4, &p, &len, NULL, 0) < 0) + goto cleanup; +
sysctlbyname would remove the need for a separate nametomib call and get rid of a few variables.
+ if (virAsprintf(&label, "+%u:+%u", + (unsigned int) p.ki_ruid, + (unsigned int) p.ki_rgid) < 0) + goto cleanup; + + if (virStrcpy(seclabel->label, label,VIR_SECURITY_LABEL_BUFLEN) == NULL) + goto cleanup;
Same comment as above.
+ ret = 0; + +cleanup: + VIR_FREE(label); + return ret; +} +#else +static int +virSecurityDACGetProcessLabelInternal(pid_t pid, + virSecurityLabelPtr seclabel) +{ + return -1; +} +#endif + static int virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, - pid_t pid ATTRIBUTE_UNUSED, + pid_t pid, virSecurityLabelPtr seclabel) { virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
- if (!secdef || !seclabel) + if (!seclabel) return -1;
The SELinux version of this function does not check if seclabel is non-NULL, I think we can safely remove the check here as well.
+ if (secdef == NULL) { + VIR_DEBUG("missing label for DAC security " + "driver in domain %s", def->name); + + if (virSecurityDACGetProcessLabelInternal(pid, seclabel) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot get process %d DAC label"),pid);
This would overwrite the more-specific error message from the virSecurityDACGetProcessLabelInternal function.
+ return -1; + } + + return 0; + } + if (secdef->label) ignore_value(virStrcpy(seclabel->label, secdef->label, VIR_SECURITY_LABEL_BUFLEN));
Jan
On 12/05/2014 12:58 AM, Ján Tomko wrote:
On 12/03/2014 11:08 AM, Luyao Huang wrote:
When use qemuProcessAttach to attach a qemu process, cannot get a right DAC label. Add a new func to get process label via stat func. Do not remove virDomainDefGetSecurityLabelDef before try to use stat to get process DAC label, because There are some other func call virSecurityDACGetProcessLabel.
v2 add support freeBSD.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/security/security_dac.c | 95 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 2 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 85253af..89cafa3 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -23,6 +23,11 @@ #include <sys/stat.h> #include <fcntl.h>
+#ifdef __FreeBSD__ +# include <sys/sysctl.h> +# include <sys/user.h> +#endif + #include "security_dac.h" #include "virerror.h" #include "virfile.h" @@ -1236,18 +1241,104 @@ virSecurityDACReserveLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return 0; }
+#ifdef __linux__ +static int +virSecurityDACGetProcessLabelInternal(pid_t pid, + virSecurityLabelPtr seclabel) +{ + struct stat sb; + char *path = NULL; + char *label = NULL; + int ret = -1; + + VIR_INFO("Getting DAC user and group on process '%d'", pid); + + if (virAsprintf(&path, "/proc/%d", (int) pid) < 0) + goto cleanup; + + if (lstat(path, &sb) < 0) + goto cleanup; A more specific error should be reported here via virReportSystemError. Thanks, i will move error settings in this func. + + if (virAsprintf(&label, "+%u:+%u", + (unsigned int) sb.st_uid, + (unsigned int) sb.st_gid) < 0) + goto cleanup; + + if (virStrcpy(seclabel->label, label,VIR_SECURITY_LABEL_BUFLEN) == NULL) + goto cleanup; Using snprintf would simplify this code. Oh, nice, i forgot this function, and thanks your pointing out. + ret = 0; + +cleanup: + VIR_FREE(path); + VIR_FREE(label); + return ret; +} +#elif defined(__FreeBSD__) +static int +virSecurityDACGetProcessLabelInternal(pid_t pid, + virSecurityLabelPtr seclabel) +{ + struct kinfo_proc p; + int mib[4]; + size_t len = 4; + char *label = NULL; + int ret = -1; + + sysctlnametomib("kern.proc.pid", mib, &len); + + len = sizeof(struct kinfo_proc); + mib[3] = pid; + + if (sysctl(mib, 4, &p, &len, NULL, 0) < 0) + goto cleanup; + sysctlbyname would remove the need for a separate nametomib call and get rid of a few variables. Actually, i tried to use this func (sysctlbyname) before, but i found i cannot make it work well here...
If you have some good way to use it, please tell me, thanks a lot for your answer:)
+ if (virAsprintf(&label, "+%u:+%u", + (unsigned int) p.ki_ruid, + (unsigned int) p.ki_rgid) < 0) + goto cleanup; + + if (virStrcpy(seclabel->label, label,VIR_SECURITY_LABEL_BUFLEN) == NULL) + goto cleanup; Same comment as above.
Thanks
+ ret = 0; + +cleanup: + VIR_FREE(label); + return ret; +} +#else +static int +virSecurityDACGetProcessLabelInternal(pid_t pid, + virSecurityLabelPtr seclabel) +{ + return -1; +} +#endif + static int virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, - pid_t pid ATTRIBUTE_UNUSED, + pid_t pid, virSecurityLabelPtr seclabel) { virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
- if (!secdef || !seclabel) + if (!seclabel) return -1; The SELinux version of this function does not check if seclabel is non-NULL, I think we can safely remove the check here as well. Okay
+ if (secdef == NULL) { + VIR_DEBUG("missing label for DAC security " + "driver in domain %s", def->name); + + if (virSecurityDACGetProcessLabelInternal(pid, seclabel) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot get process %d DAC label"),pid); This would overwrite the more-specific error message from the virSecurityDACGetProcessLabelInternal function. Yes, got it, thanks for pointing out. + return -1; + } + + return 0; + } + if (secdef->label) ignore_value(virStrcpy(seclabel->label, secdef->label, VIR_SECURITY_LABEL_BUFLEN));
Jan
participants (3)
-
Ján Tomko -
lhuang -
Luyao Huang