Re: [libvirt] [PATCH 5/7] Convert remote daemon & acl code to use polkit API
On 09/10/2014 10:20 AM, Daniel P. Berrange wrote:
Convert the remote daemon auth check and the access control
code to use the common polkit API for checking auth.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
daemon/remote.c | 235 ++-----------------------------------
src/access/viraccessdriverpolkit.c | 91 ++++++--------
2 files changed, 45 insertions(+), 281 deletions(-)
<...snip...>
Since this was pushed yesterday - my morning Coverity picked up on an
issue this morning...
-static char *
-virAccessDriverPolkitFormatProcess(const char *actionid)
+static int
+virAccessDriverPolkitGetCaller(const char *actionid,
+ pid_t *pid,
+ unsigned long long *startTime,
+ uid_t *uid)
{
virIdentityPtr identity = virIdentityGetCurrent();
- pid_t pid;
- unsigned long long startTime;
- uid_t uid;
- char *ret = NULL;
-#ifndef PKCHECK_SUPPORTS_UID
- static bool polkitInsecureWarned;
-#endif
+ int ret = -1;
if (!identity) {
virAccessError(VIR_ERR_ACCESS_DENIED,
_("Policy kit denied action %s from
<anonymous>"),
actionid);
- return NULL;
+ return -1;
}
- if (virIdentityGetUNIXProcessID(identity, &pid) < 0)
+ if (virIdentityGetUNIXProcessID(identity, pid) < 0)
(1) Event deref_ptr_in_call: Dereferencing pointer "pid". [details]
Also see events: [check_after_deref]
goto cleanup;
- if (virIdentityGetUNIXProcessTime(identity, &startTime) < 0)
+ if (virIdentityGetUNIXProcessTime(identity, startTime) < 0)
goto cleanup;
- if (virIdentityGetUNIXUserID(identity, &uid) < 0)
+ if (virIdentityGetUNIXUserID(identity, uid) < 0)
goto cleanup;
if (!pid) {
(2) Event check_after_deref: Null-checking "pid" suggests that it may
be null, but it has already been dereferenced on all paths leading to
the check.
Also see events: [deref_ptr_in_call]
Should this reference now be "(!*pid)" ?
John
@@ -101,25 +99,14 @@ virAccessDriverPolkitFormatProcess(const char
*actionid)
_("No UNIX process ID available"));
goto cleanup;
}
- if (!startTime) {
- virAccessError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("No UNIX process start time available"));
- goto cleanup;
- }
-#ifdef PKCHECK_SUPPORTS_UID
- if (virAsprintf(&ret, "%llu,%llu,%llu",
- (unsigned long long)pid, startTime, (unsigned long long)uid) <
0)
+ if (virIdentityGetUNIXProcessTime(identity, startTime) < 0)
goto cleanup;
-#else
- if (!polkitInsecureWarned) {
- VIR_WARN("No support for caller UID with pkcheck. This deployment is known
to be insecure.");
- polkitInsecureWarned = true;
- }
- if (virAsprintf(&ret, "%llu,%llu",
- (unsigned long long)pid, startTime) < 0)
+
+ if (virIdentityGetUNIXUserID(identity, uid) < 0)
goto cleanup;
-#endif
+
+ ret = 0;
cleanup:
virObjectUnref(identity);