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@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);