External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2025-04-26 15:14:42 +00:00
Code

util: sanitize return values for virIdentity getters

Browse Source 
The virIdentity getters are unusual in that they return -1 to indicate
"not found" and don't report any error. Change them to return -1 for
real errors, 0 for not found, and 1 for success.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrangé 2019-08-07 16:30:57 +01:00
parent f3fa662353
commit 45b273d981
5 changed files with 180 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
src/access/viraccessdriverpolkit.c
View File

@ -80,6 +80,7 @@ virAccessDriverPolkitGetCaller(const char *actionid,
{ {
virIdentityPtr identity = virIdentityGetCurrent(); virIdentityPtr identity = virIdentityGetCurrent();
int ret = -1; int ret = -1;
int rc;
if (!identity) { if (!identity) {
virAccessError(VIR_ERR_ACCESS_DENIED, virAccessError(VIR_ERR_ACCESS_DENIED,
@ -88,17 +89,28 @@ virAccessDriverPolkitGetCaller(const char *actionid,
return -1; return -1;
} }
if (virIdentityGetProcessID(identity, pid) < 0) { if ((rc = virIdentityGetProcessID(identity, pid)) < 0)
goto cleanup;
if (rc == 0) {
virAccessError(VIR_ERR_INTERNAL_ERROR, "%s", virAccessError(VIR_ERR_INTERNAL_ERROR, "%s",
_("No process ID available")); _("No process ID available"));
goto cleanup; goto cleanup;
} }
if (virIdentityGetProcessTime(identity, startTime) < 0) {
if ((rc = virIdentityGetProcessTime(identity, startTime)) < 0)
goto cleanup;
if (rc == 0) {
virAccessError(VIR_ERR_INTERNAL_ERROR, "%s", virAccessError(VIR_ERR_INTERNAL_ERROR, "%s",
_("No process start time available")); _("No process start time available"));
goto cleanup; goto cleanup;
} }
if (virIdentityGetUNIXUserID(identity, uid) < 0) {
if ((rc = virIdentityGetUNIXUserID(identity, uid)) < 0)
goto cleanup;
if (rc == 0) {
virAccessError(VIR_ERR_INTERNAL_ERROR, "%s", virAccessError(VIR_ERR_INTERNAL_ERROR, "%s",
_("No UNIX caller UID available")); _("No UNIX caller UID available"));
goto cleanup; goto cleanup;

52
src/admin/admin_server.c
View File

@ -222,6 +222,7 @@ adminClientGetInfo(virNetServerClientPtr client,
const char *attr = NULL; const char *attr = NULL;
virTypedParameterPtr tmpparams = NULL; virTypedParameterPtr tmpparams = NULL;
virIdentityPtr identity = NULL; virIdentityPtr identity = NULL;
int rc;
virCheckFlags(0, -1); virCheckFlags(0, -1);
@ -234,11 +235,12 @@ adminClientGetInfo(virNetServerClientPtr client,
readonly) < 0) readonly) < 0)
goto cleanup; goto cleanup;
if (virIdentityGetSASLUserName(identity, &attr) < 0 || if ((rc = virIdentityGetSASLUserName(identity, &attr)) < 0)
(attr && goto cleanup;
virTypedParamsAddString(&tmpparams, nparams, &maxparams, if (rc == 1 &&
VIR_CLIENT_INFO_SASL_USER_NAME, virTypedParamsAddString(&tmpparams, nparams, &maxparams,
attr) < 0)) VIR_CLIENT_INFO_SASL_USER_NAME,
attr) < 0)
goto cleanup; goto cleanup;
if (!virNetServerClientIsLocal(client)) { if (!virNetServerClientIsLocal(client)) {
@ -247,48 +249,60 @@ adminClientGetInfo(virNetServerClientPtr client,
sock_addr) < 0) sock_addr) < 0)
goto cleanup; goto cleanup;
if (virIdentityGetX509DName(identity, &attr) < 0 || if ((rc = virIdentityGetX509DName(identity, &attr)) < 0)
(attr && goto cleanup;
virTypedParamsAddString(&tmpparams, nparams, &maxparams, if (rc == 1 &&
VIR_CLIENT_INFO_X509_DISTINGUISHED_NAME, virTypedParamsAddString(&tmpparams, nparams, &maxparams,
attr) < 0)) VIR_CLIENT_INFO_X509_DISTINGUISHED_NAME,
attr) < 0)
goto cleanup; goto cleanup;
} else { } else {
pid_t pid; pid_t pid;
uid_t uid; uid_t uid;
gid_t gid; gid_t gid;
if (virIdentityGetUNIXUserID(identity, &uid) < 0 || if ((rc = virIdentityGetUNIXUserID(identity, &uid)) < 0)
goto cleanup;
if (rc == 1 &&
virTypedParamsAddInt(&tmpparams, nparams, &maxparams, virTypedParamsAddInt(&tmpparams, nparams, &maxparams,
VIR_CLIENT_INFO_UNIX_USER_ID, uid) < 0) VIR_CLIENT_INFO_UNIX_USER_ID, uid) < 0)
goto cleanup; goto cleanup;
if (virIdentityGetUserName(identity, &attr) < 0 || if ((rc = virIdentityGetUserName(identity, &attr)) < 0)
goto cleanup;
if (rc == 1 &&
virTypedParamsAddString(&tmpparams, nparams, &maxparams, virTypedParamsAddString(&tmpparams, nparams, &maxparams,
VIR_CLIENT_INFO_UNIX_USER_NAME, VIR_CLIENT_INFO_UNIX_USER_NAME,
attr) < 0) attr) < 0)
goto cleanup; goto cleanup;
if (virIdentityGetUNIXGroupID(identity, &gid) < 0 || if ((rc = virIdentityGetUNIXGroupID(identity, &gid)) < 0)
goto cleanup;
if (rc == 1 &&
virTypedParamsAddInt(&tmpparams, nparams, &maxparams, virTypedParamsAddInt(&tmpparams, nparams, &maxparams,
VIR_CLIENT_INFO_UNIX_GROUP_ID, gid) < 0) VIR_CLIENT_INFO_UNIX_GROUP_ID, gid) < 0)
goto cleanup; goto cleanup;
if (virIdentityGetGroupName(identity, &attr) < 0 || if ((rc = virIdentityGetGroupName(identity, &attr)) < 0)
goto cleanup;
if (rc == 1 &&
virTypedParamsAddString(&tmpparams, nparams, &maxparams, virTypedParamsAddString(&tmpparams, nparams, &maxparams,
VIR_CLIENT_INFO_UNIX_GROUP_NAME, VIR_CLIENT_INFO_UNIX_GROUP_NAME,
attr) < 0) attr) < 0)
goto cleanup; goto cleanup;
if (virIdentityGetProcessID(identity, &pid) < 0 || if ((rc = virIdentityGetProcessID(identity, &pid)) < 0)
goto cleanup;
if (rc == 1 &&
virTypedParamsAddInt(&tmpparams, nparams, &maxparams, virTypedParamsAddInt(&tmpparams, nparams, &maxparams,
VIR_CLIENT_INFO_UNIX_PROCESS_ID, pid) < 0) VIR_CLIENT_INFO_UNIX_PROCESS_ID, pid) < 0)
goto cleanup; goto cleanup;
} }
if (virIdentityGetSELinuxContext(identity, &attr) < 0 || if ((rc = virIdentityGetSELinuxContext(identity, &attr)) < 0)
(attr && goto cleanup;
virTypedParamsAddString(&tmpparams, nparams, &maxparams, if (rc == 1 &&
VIR_CLIENT_INFO_SELINUX_CONTEXT, attr) < 0)) virTypedParamsAddString(&tmpparams, nparams, &maxparams,
VIR_CLIENT_INFO_SELINUX_CONTEXT, attr) < 0)
goto cleanup; goto cleanup;
*params = tmpparams; *params = tmpparams;

156
src/util/viridentity.c
View File

@ -281,10 +281,8 @@ virIdentitySetAttr(virIdentityPtr ident,
* with the identifying attribute @attr in @ident. If * with the identifying attribute @attr in @ident. If
* @attr is not set, then it will simply be initialized * @attr is not set, then it will simply be initialized
* to NULL and considered as a successful read * to NULL and considered as a successful read
*
* Returns 0 on success, -1 on error
*/ */
static int static void
virIdentityGetAttr(virIdentityPtr ident, virIdentityGetAttr(virIdentityPtr ident,
unsigned int attr, unsigned int attr,
const char **value) const char **value)
@ -292,20 +290,29 @@ virIdentityGetAttr(virIdentityPtr ident,
VIR_DEBUG("ident=%p attribute=%d value=%p", ident, attr, value); VIR_DEBUG("ident=%p attribute=%d value=%p", ident, attr, value);
*value = ident->attrs[attr]; *value = ident->attrs[attr];
return 0;
} }
/*
* Returns: 0 if not present, 1 if present, -1 on error
*/
int virIdentityGetUserName(virIdentityPtr ident, int virIdentityGetUserName(virIdentityPtr ident,
const char **username) const char **username)
{ {
return virIdentityGetAttr(ident, virIdentityGetAttr(ident,
VIR_IDENTITY_ATTR_USER_NAME, VIR_IDENTITY_ATTR_USER_NAME,
username); username);
if (!*username)
return 0;
return 1;
} }
/*
* Returns: 0 if not present, 1 if present, -1 on error
*/
int virIdentityGetUNIXUserID(virIdentityPtr ident, int virIdentityGetUNIXUserID(virIdentityPtr ident,
uid_t *uid) uid_t *uid)
{ {
@ -313,31 +320,44 @@ int virIdentityGetUNIXUserID(virIdentityPtr ident,
const char *userid; const char *userid;
*uid = -1; *uid = -1;
if (virIdentityGetAttr(ident, virIdentityGetAttr(ident,
VIR_IDENTITY_ATTR_UNIX_USER_ID, VIR_IDENTITY_ATTR_UNIX_USER_ID,
&userid) < 0) &userid);
return -1;
if (!userid) if (!userid)
return -1; return 0;
if (virStrToLong_i(userid, NULL, 10, &val) < 0) if (virStrToLong_i(userid, NULL, 10, &val) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot parse user ID '%s'"), userid);
return -1; return -1;
}
*uid = (uid_t)val; *uid = (uid_t)val;
return 0; return 1;
} }
/*
* Returns: 0 if not present, 1 if present, -1 on error
*/
int virIdentityGetGroupName(virIdentityPtr ident, int virIdentityGetGroupName(virIdentityPtr ident,
const char **groupname) const char **groupname)
{ {
return virIdentityGetAttr(ident, virIdentityGetAttr(ident,
VIR_IDENTITY_ATTR_GROUP_NAME, VIR_IDENTITY_ATTR_GROUP_NAME,
groupname); groupname);
if (!*groupname)
return 0;
return 1;
} }
/*
* Returns: 0 if not present, 1 if present, -1 on error
*/
int virIdentityGetUNIXGroupID(virIdentityPtr ident, int virIdentityGetUNIXGroupID(virIdentityPtr ident,
gid_t *gid) gid_t *gid)
{ {
@ -345,23 +365,28 @@ int virIdentityGetUNIXGroupID(virIdentityPtr ident,
const char *groupid; const char *groupid;
*gid = -1; *gid = -1;
if (virIdentityGetAttr(ident, virIdentityGetAttr(ident,
VIR_IDENTITY_ATTR_UNIX_GROUP_ID, VIR_IDENTITY_ATTR_UNIX_GROUP_ID,
&groupid) < 0) &groupid);
return -1;
if (!groupid) if (!groupid)
return -1; return 0;
if (virStrToLong_i(groupid, NULL, 10, &val) < 0) if (virStrToLong_i(groupid, NULL, 10, &val) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot parse group ID '%s'"), groupid);
return -1; return -1;
}
*gid = (gid_t)val; *gid = (gid_t)val;
return 0; return 1;
} }
/*
* Returns: 0 if not present, 1 if present, -1 on error
*/
int virIdentityGetProcessID(virIdentityPtr ident, int virIdentityGetProcessID(virIdentityPtr ident,
pid_t *pid) pid_t *pid)
{ {
@ -369,66 +394,99 @@ int virIdentityGetProcessID(virIdentityPtr ident,
const char *processid; const char *processid;
*pid = 0; *pid = 0;
if (virIdentityGetAttr(ident, virIdentityGetAttr(ident,
VIR_IDENTITY_ATTR_PROCESS_ID, VIR_IDENTITY_ATTR_PROCESS_ID,
&processid) < 0) &processid);
return -1;
if (!processid) if (!processid)
return -1; return 0;
if (virStrToLong_ull(processid, NULL, 10, &val) < 0) if (virStrToLong_ull(processid, NULL, 10, &val) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot parse process ID '%s'"), processid);
return -1; return -1;
}
*pid = (pid_t)val; *pid = (pid_t)val;
return 0; return 1;
} }
/*
* Returns: 0 if not present, 1 if present, -1 on error
*/
int virIdentityGetProcessTime(virIdentityPtr ident, int virIdentityGetProcessTime(virIdentityPtr ident,
unsigned long long *timestamp) unsigned long long *timestamp)
{ {
const char *processtime; const char *processtime;
if (virIdentityGetAttr(ident,
VIR_IDENTITY_ATTR_PROCESS_TIME, *timestamp = 0;
&processtime) < 0) virIdentityGetAttr(ident,
return -1; VIR_IDENTITY_ATTR_PROCESS_TIME,
&processtime);
if (!processtime) if (!processtime)
return -1; return 0;
if (virStrToLong_ull(processtime, NULL, 10, timestamp) < 0) if (virStrToLong_ull(processtime, NULL, 10, timestamp) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot parse process time '%s'"), processtime);
return -1; return -1;
}
return 0; return 1;
} }
/*
* Returns: 0 if not present, 1 if present, -1 on error
*/
int virIdentityGetSASLUserName(virIdentityPtr ident, int virIdentityGetSASLUserName(virIdentityPtr ident,
const char **username) const char **username)
{ {
return virIdentityGetAttr(ident, virIdentityGetAttr(ident,
VIR_IDENTITY_ATTR_SASL_USER_NAME, VIR_IDENTITY_ATTR_SASL_USER_NAME,
username); username);
if (!*username)
return 0;
return 1;
} }
/*
* Returns: 0 if not present, 1 if present, -1 on error
*/
int virIdentityGetX509DName(virIdentityPtr ident, int virIdentityGetX509DName(virIdentityPtr ident,
const char **dname) const char **dname)
{ {
return virIdentityGetAttr(ident, virIdentityGetAttr(ident,
VIR_IDENTITY_ATTR_X509_DISTINGUISHED_NAME, VIR_IDENTITY_ATTR_X509_DISTINGUISHED_NAME,
dname); dname);
if (!*dname)
return 0;
return 1;
} }
/*
* Returns: 0 if not present, 1 if present, -1 on error
*/
int virIdentityGetSELinuxContext(virIdentityPtr ident, int virIdentityGetSELinuxContext(virIdentityPtr ident,
const char **context) const char **context)
{ {
return virIdentityGetAttr(ident, virIdentityGetAttr(ident,
VIR_IDENTITY_ATTR_SELINUX_CONTEXT, VIR_IDENTITY_ATTR_SELINUX_CONTEXT,
context); context);
if (!*context)
return 0;
return 1;
} }

31
tests/viridentitytest.c
View File

@ -41,6 +41,7 @@ static int testIdentityAttrs(const void *data ATTRIBUTE_UNUSED)
int ret = -1; int ret = -1;
virIdentityPtr ident; virIdentityPtr ident;
const char *val; const char *val;
int rc;
if (!(ident = virIdentityNew())) if (!(ident = virIdentityNew()))
goto cleanup; goto cleanup;
@ -48,18 +49,18 @@ static int testIdentityAttrs(const void *data ATTRIBUTE_UNUSED)
if (virIdentitySetUserName(ident, "fred") < 0) if (virIdentitySetUserName(ident, "fred") < 0)
goto cleanup; goto cleanup;
if (virIdentityGetUserName(ident, &val) < 0) if ((rc = virIdentityGetUserName(ident, &val)) < 0)
goto cleanup; goto cleanup;
if (STRNEQ_NULLABLE(val, "fred")) { if (STRNEQ_NULLABLE(val, "fred") || rc != 1) {
VIR_DEBUG("Expected 'fred' got '%s'", NULLSTR(val)); VIR_DEBUG("Expected 'fred' got '%s'", NULLSTR(val));
goto cleanup; goto cleanup;
} }
if (virIdentityGetGroupName(ident, &val) < 0) if ((rc = virIdentityGetGroupName(ident, &val)) < 0)
goto cleanup; goto cleanup;
if (val != NULL) { if (val != NULL || rc != 0) {
VIR_DEBUG("Unexpected groupname attribute"); VIR_DEBUG("Unexpected groupname attribute");
goto cleanup; goto cleanup;
} }
@ -69,10 +70,10 @@ static int testIdentityAttrs(const void *data ATTRIBUTE_UNUSED)
goto cleanup; goto cleanup;
} }
if (virIdentityGetUserName(ident, &val) < 0) if ((rc = virIdentityGetUserName(ident, &val)) < 0)
goto cleanup; goto cleanup;
if (STRNEQ_NULLABLE(val, "fred")) { if (STRNEQ_NULLABLE(val, "fred") || rc != 1) {
VIR_DEBUG("Expected 'fred' got '%s'", NULLSTR(val)); VIR_DEBUG("Expected 'fred' got '%s'", NULLSTR(val));
goto cleanup; goto cleanup;
} }
@ -90,6 +91,7 @@ static int testIdentityGetSystem(const void *data)
int ret = -1; int ret = -1;
virIdentityPtr ident = NULL; virIdentityPtr ident = NULL;
const char *val; const char *val;
int rc;
#if !WITH_SELINUX #if !WITH_SELINUX
if (context) { if (context) {
@ -104,13 +106,20 @@ static int testIdentityGetSystem(const void *data)
goto cleanup; goto cleanup;
} }
if (virIdentityGetSELinuxContext(ident, &val) < 0) if ((rc = virIdentityGetSELinuxContext(ident, &val)) < 0)
goto cleanup; goto cleanup;
if (STRNEQ_NULLABLE(val, context)) { if (context == NULL) {
VIR_DEBUG("Want SELinux context '%s' got '%s'", if (val != NULL || rc != 0) {
context, val); VIR_DEBUG("Unexpected SELinux context %s", NULLSTR(val));
goto cleanup; goto cleanup;
}
} else {
if (STRNEQ_NULLABLE(val, context) || rc != 1) {
VIR_DEBUG("Want SELinux context '%s' got '%s'",
context, val);
goto cleanup;
}
} }
ret = 0; ret = 0;

10
tests/virnetserverclienttest.c
View File

@ -85,7 +85,7 @@ static int testIdentity(const void *opaque ATTRIBUTE_UNUSED)
goto cleanup; goto cleanup;
} }
if (virIdentityGetUserName(ident, &gotUsername) < 0) { if (virIdentityGetUserName(ident, &gotUsername) <= 0) {
fprintf(stderr, "Missing username in identity\n"); fprintf(stderr, "Missing username in identity\n");
goto cleanup; goto cleanup;
} }
@ -95,7 +95,7 @@ static int testIdentity(const void *opaque ATTRIBUTE_UNUSED)
goto cleanup; goto cleanup;
} }
if (virIdentityGetUNIXUserID(ident, &gotUserID) < 0) { if (virIdentityGetUNIXUserID(ident, &gotUserID) <= 0) {
fprintf(stderr, "Missing user ID in identity\n"); fprintf(stderr, "Missing user ID in identity\n");
goto cleanup; goto cleanup;
} }
@ -105,7 +105,7 @@ static int testIdentity(const void *opaque ATTRIBUTE_UNUSED)
goto cleanup; goto cleanup;
} }
if (virIdentityGetGroupName(ident, &gotGroupname) < 0) { if (virIdentityGetGroupName(ident, &gotGroupname) <= 0) {
fprintf(stderr, "Missing groupname in identity\n"); fprintf(stderr, "Missing groupname in identity\n");
goto cleanup; goto cleanup;
} }
@ -115,7 +115,7 @@ static int testIdentity(const void *opaque ATTRIBUTE_UNUSED)
goto cleanup; goto cleanup;
} }
if (virIdentityGetUNIXGroupID(ident, &gotGroupID) < 0) { if (virIdentityGetUNIXGroupID(ident, &gotGroupID) <= 0) {
fprintf(stderr, "Missing group ID in identity\n"); fprintf(stderr, "Missing group ID in identity\n");
goto cleanup; goto cleanup;
} }
@ -125,7 +125,7 @@ static int testIdentity(const void *opaque ATTRIBUTE_UNUSED)
goto cleanup; goto cleanup;
} }
if (virIdentityGetSELinuxContext(ident, &gotSELinuxContext) < 0) { if (virIdentityGetSELinuxContext(ident, &gotSELinuxContext) <= 0) {
fprintf(stderr, "Missing SELinux context in identity\n"); fprintf(stderr, "Missing SELinux context in identity\n");
goto cleanup; goto cleanup;
} }