From nobody Sat Feb 7 10:44:38 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1567684727; cv=none; d=zoho.com; s=zohoarc; b=HI/S1JHyoDMuQzko/4cWpEJWlVn+vJzPXi+rKgl6Li7fDqRAfqJICaXsSlg5qMZjmvgRy+GySeJCW8/fIamp1pWkPZQNCu8olZgKkVpCoDj0TbF3zn26hnC0esI4uPy2YHuwi+ikemXKszMc1OU7lR83fFI0KFII0nZ+496Dq8o= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1567684727; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=M2MymHBj99PV2eNaU0Kb7yFX9FgcyFk93LM2UMWbU/E=; b=bxcFD7mWhXIxeJil5XxZYoYOvZRoSSMqIV2H9MPcyrko9t5SFw+QpuPESejzkmB2C7M1aFgfmAsfg1yMnK+UxYDjB6M8z1eqvSlSMp28+uSKTqydH3q5yCv1tjmHkypYxSXXih5g6eqzdnDFj8PHUyn0hVBQLjgKn2mgYKVaRv0= ARC-Authentication-Results: i=1; mx.zoho.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1567684727871485.68514496140426; Thu, 5 Sep 2019 04:58:47 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A8FC58830E; Thu, 5 Sep 2019 11:58:46 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7877E60623; Thu, 5 Sep 2019 11:58:46 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 18464180B536; Thu, 5 Sep 2019 11:58:46 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x85Bubs2030957 for ; Thu, 5 Sep 2019 07:56:37 -0400 Received: by smtp.corp.redhat.com (Postfix) id 7BB6F60C5E; Thu, 5 Sep 2019 11:56:37 +0000 (UTC) Received: from dhcp-17-64.lcy.redhat.com (unknown [10.42.17.64]) by smtp.corp.redhat.com (Postfix) with ESMTP id E8EE860C18; Thu, 5 Sep 2019 11:56:36 +0000 (UTC) From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= To: libvir-list@redhat.com Date: Thu, 5 Sep 2019 12:56:24 +0100 Message-Id: <20190905115627.11493-7-berrange@redhat.com> In-Reply-To: <20190905115627.11493-1-berrange@redhat.com> References: <20190905115627.11493-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 6/9] util: sanitize return values for virIdentity getters X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Thu, 05 Sep 2019 11:58:47 +0000 (UTC) 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. Signed-off-by: Daniel P. Berrang=C3=A9 --- src/access/viraccessdriverpolkit.c | 18 +++- src/admin/admin_server.c | 52 ++++++---- src/util/viridentity.c | 156 ++++++++++++++++++++--------- tests/viridentitytest.c | 31 ++++-- tests/virnetserverclienttest.c | 10 +- 5 files changed, 180 insertions(+), 87 deletions(-) diff --git a/src/access/viraccessdriverpolkit.c b/src/access/viraccessdrive= rpolkit.c index 75dbf8a0fa..e61ac6fa19 100644 --- a/src/access/viraccessdriverpolkit.c +++ b/src/access/viraccessdriverpolkit.c @@ -80,6 +80,7 @@ virAccessDriverPolkitGetCaller(const char *actionid, { virIdentityPtr identity =3D virIdentityGetCurrent(); int ret =3D -1; + int rc; =20 if (!identity) { virAccessError(VIR_ERR_ACCESS_DENIED, @@ -88,17 +89,28 @@ virAccessDriverPolkitGetCaller(const char *actionid, return -1; } =20 - if (virIdentityGetProcessID(identity, pid) < 0) { + if ((rc =3D virIdentityGetProcessID(identity, pid)) < 0) + goto cleanup; + + if (rc =3D=3D 0) { virAccessError(VIR_ERR_INTERNAL_ERROR, "%s", _("No process ID available")); goto cleanup; } - if (virIdentityGetProcessTime(identity, startTime) < 0) { + + if ((rc =3D virIdentityGetProcessTime(identity, startTime)) < 0) + goto cleanup; + + if (rc =3D=3D 0) { virAccessError(VIR_ERR_INTERNAL_ERROR, "%s", _("No process start time available")); goto cleanup; } - if (virIdentityGetUNIXUserID(identity, uid) < 0) { + + if ((rc =3D virIdentityGetUNIXUserID(identity, uid)) < 0) + goto cleanup; + + if (rc =3D=3D 0) { virAccessError(VIR_ERR_INTERNAL_ERROR, "%s", _("No UNIX caller UID available")); goto cleanup; diff --git a/src/admin/admin_server.c b/src/admin/admin_server.c index 80e5a36679..248df3f795 100644 --- a/src/admin/admin_server.c +++ b/src/admin/admin_server.c @@ -222,6 +222,7 @@ adminClientGetInfo(virNetServerClientPtr client, const char *attr =3D NULL; virTypedParameterPtr tmpparams =3D NULL; virIdentityPtr identity =3D NULL; + int rc; =20 virCheckFlags(0, -1); =20 @@ -234,11 +235,12 @@ adminClientGetInfo(virNetServerClientPtr client, readonly) < 0) goto cleanup; =20 - if (virIdentityGetSASLUserName(identity, &attr) < 0 || - (attr && - virTypedParamsAddString(&tmpparams, nparams, &maxparams, - VIR_CLIENT_INFO_SASL_USER_NAME, - attr) < 0)) + if ((rc =3D virIdentityGetSASLUserName(identity, &attr)) < 0) + goto cleanup; + if (rc =3D=3D 1 && + virTypedParamsAddString(&tmpparams, nparams, &maxparams, + VIR_CLIENT_INFO_SASL_USER_NAME, + attr) < 0) goto cleanup; =20 if (!virNetServerClientIsLocal(client)) { @@ -247,48 +249,60 @@ adminClientGetInfo(virNetServerClientPtr client, sock_addr) < 0) goto cleanup; =20 - if (virIdentityGetX509DName(identity, &attr) < 0 || - (attr && - virTypedParamsAddString(&tmpparams, nparams, &maxparams, - VIR_CLIENT_INFO_X509_DISTINGUISHED_NA= ME, - attr) < 0)) + if ((rc =3D virIdentityGetX509DName(identity, &attr)) < 0) + goto cleanup; + if (rc =3D=3D 1 && + virTypedParamsAddString(&tmpparams, nparams, &maxparams, + VIR_CLIENT_INFO_X509_DISTINGUISHED_NAM= E, + attr) < 0) goto cleanup; } else { pid_t pid; uid_t uid; gid_t gid; - if (virIdentityGetUNIXUserID(identity, &uid) < 0 || + if ((rc =3D virIdentityGetUNIXUserID(identity, &uid)) < 0) + goto cleanup; + if (rc =3D=3D 1 && virTypedParamsAddInt(&tmpparams, nparams, &maxparams, VIR_CLIENT_INFO_UNIX_USER_ID, uid) < 0) goto cleanup; =20 - if (virIdentityGetUserName(identity, &attr) < 0 || + if ((rc =3D virIdentityGetUserName(identity, &attr)) < 0) + goto cleanup; + if (rc =3D=3D 1 && virTypedParamsAddString(&tmpparams, nparams, &maxparams, VIR_CLIENT_INFO_UNIX_USER_NAME, attr) < 0) goto cleanup; =20 - if (virIdentityGetUNIXGroupID(identity, &gid) < 0 || + if ((rc =3D virIdentityGetUNIXGroupID(identity, &gid)) < 0) + goto cleanup; + if (rc =3D=3D 1 && virTypedParamsAddInt(&tmpparams, nparams, &maxparams, VIR_CLIENT_INFO_UNIX_GROUP_ID, gid) < 0) goto cleanup; =20 - if (virIdentityGetGroupName(identity, &attr) < 0 || + if ((rc =3D virIdentityGetGroupName(identity, &attr)) < 0) + goto cleanup; + if (rc =3D=3D 1 && virTypedParamsAddString(&tmpparams, nparams, &maxparams, VIR_CLIENT_INFO_UNIX_GROUP_NAME, attr) < 0) goto cleanup; =20 - if (virIdentityGetProcessID(identity, &pid) < 0 || + if ((rc =3D virIdentityGetProcessID(identity, &pid)) < 0) + goto cleanup; + if (rc =3D=3D 1 && virTypedParamsAddInt(&tmpparams, nparams, &maxparams, VIR_CLIENT_INFO_UNIX_PROCESS_ID, pid) < 0) goto cleanup; } =20 - if (virIdentityGetSELinuxContext(identity, &attr) < 0 || - (attr && - virTypedParamsAddString(&tmpparams, nparams, &maxparams, - VIR_CLIENT_INFO_SELINUX_CONTEXT, attr) < 0= )) + if ((rc =3D virIdentityGetSELinuxContext(identity, &attr)) < 0) + goto cleanup; + if (rc =3D=3D 1 && + virTypedParamsAddString(&tmpparams, nparams, &maxparams, + VIR_CLIENT_INFO_SELINUX_CONTEXT, attr) < 0) goto cleanup; =20 *params =3D tmpparams; diff --git a/src/util/viridentity.c b/src/util/viridentity.c index 55312fc0a0..964a33d339 100644 --- a/src/util/viridentity.c +++ b/src/util/viridentity.c @@ -281,10 +281,8 @@ virIdentitySetAttr(virIdentityPtr ident, * with the identifying attribute @attr in @ident. If * @attr is not set, then it will simply be initialized * to NULL and considered as a successful read - * - * Returns 0 on success, -1 on error */ -static int +static void virIdentityGetAttr(virIdentityPtr ident, unsigned int attr, const char **value) @@ -292,20 +290,29 @@ virIdentityGetAttr(virIdentityPtr ident, VIR_DEBUG("ident=3D%p attribute=3D%d value=3D%p", ident, attr, value); =20 *value =3D ident->attrs[attr]; - - return 0; } =20 =20 +/* + * Returns: 0 if not present, 1 if present, -1 on error + */ int virIdentityGetUserName(virIdentityPtr ident, const char **username) { - return virIdentityGetAttr(ident, - VIR_IDENTITY_ATTR_USER_NAME, - username); + virIdentityGetAttr(ident, + VIR_IDENTITY_ATTR_USER_NAME, + username); + + if (!*username) + return 0; + + return 1; } =20 =20 +/* + * Returns: 0 if not present, 1 if present, -1 on error + */ int virIdentityGetUNIXUserID(virIdentityPtr ident, uid_t *uid) { @@ -313,31 +320,44 @@ int virIdentityGetUNIXUserID(virIdentityPtr ident, const char *userid; =20 *uid =3D -1; - if (virIdentityGetAttr(ident, - VIR_IDENTITY_ATTR_UNIX_USER_ID, - &userid) < 0) - return -1; - + virIdentityGetAttr(ident, + VIR_IDENTITY_ATTR_UNIX_USER_ID, + &userid); if (!userid) - return -1; + return 0; =20 - 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; + } =20 *uid =3D (uid_t)val; =20 - return 0; + return 1; } =20 + +/* + * Returns: 0 if not present, 1 if present, -1 on error + */ int virIdentityGetGroupName(virIdentityPtr ident, const char **groupname) { - return virIdentityGetAttr(ident, - VIR_IDENTITY_ATTR_GROUP_NAME, - groupname); + virIdentityGetAttr(ident, + VIR_IDENTITY_ATTR_GROUP_NAME, + groupname); + + if (!*groupname) + return 0; + + return 1; } =20 =20 +/* + * Returns: 0 if not present, 1 if present, -1 on error + */ int virIdentityGetUNIXGroupID(virIdentityPtr ident, gid_t *gid) { @@ -345,23 +365,28 @@ int virIdentityGetUNIXGroupID(virIdentityPtr ident, const char *groupid; =20 *gid =3D -1; - if (virIdentityGetAttr(ident, - VIR_IDENTITY_ATTR_UNIX_GROUP_ID, - &groupid) < 0) - return -1; + virIdentityGetAttr(ident, + VIR_IDENTITY_ATTR_UNIX_GROUP_ID, + &groupid); =20 if (!groupid) - return -1; + return 0; =20 - 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; + } =20 *gid =3D (gid_t)val; =20 - return 0; + return 1; } =20 =20 +/* + * Returns: 0 if not present, 1 if present, -1 on error + */ int virIdentityGetProcessID(virIdentityPtr ident, pid_t *pid) { @@ -369,66 +394,99 @@ int virIdentityGetProcessID(virIdentityPtr ident, const char *processid; =20 *pid =3D 0; - if (virIdentityGetAttr(ident, - VIR_IDENTITY_ATTR_PROCESS_ID, - &processid) < 0) - return -1; + virIdentityGetAttr(ident, + VIR_IDENTITY_ATTR_PROCESS_ID, + &processid); =20 if (!processid) - return -1; + return 0; =20 - 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; + } =20 *pid =3D (pid_t)val; =20 - return 0; + return 1; } =20 =20 +/* + * Returns: 0 if not present, 1 if present, -1 on error + */ int virIdentityGetProcessTime(virIdentityPtr ident, unsigned long long *timestamp) { const char *processtime; - if (virIdentityGetAttr(ident, - VIR_IDENTITY_ATTR_PROCESS_TIME, - &processtime) < 0) - return -1; + + *timestamp =3D 0; + virIdentityGetAttr(ident, + VIR_IDENTITY_ATTR_PROCESS_TIME, + &processtime); =20 if (!processtime) - return -1; + return 0; =20 - 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; + } =20 - return 0; + return 1; } =20 =20 +/* + * Returns: 0 if not present, 1 if present, -1 on error + */ int virIdentityGetSASLUserName(virIdentityPtr ident, const char **username) { - return virIdentityGetAttr(ident, - VIR_IDENTITY_ATTR_SASL_USER_NAME, - username); + virIdentityGetAttr(ident, + VIR_IDENTITY_ATTR_SASL_USER_NAME, + username); + + if (!*username) + return 0; + + return 1; } =20 =20 +/* + * Returns: 0 if not present, 1 if present, -1 on error + */ int virIdentityGetX509DName(virIdentityPtr ident, const char **dname) { - return virIdentityGetAttr(ident, - VIR_IDENTITY_ATTR_X509_DISTINGUISHED_NAME, - dname); + virIdentityGetAttr(ident, + VIR_IDENTITY_ATTR_X509_DISTINGUISHED_NAME, + dname); + + if (!*dname) + return 0; + + return 1; } =20 =20 +/* + * Returns: 0 if not present, 1 if present, -1 on error + */ int virIdentityGetSELinuxContext(virIdentityPtr ident, const char **context) { - return virIdentityGetAttr(ident, - VIR_IDENTITY_ATTR_SELINUX_CONTEXT, - context); + virIdentityGetAttr(ident, + VIR_IDENTITY_ATTR_SELINUX_CONTEXT, + context); + + if (!*context) + return 0; + + return 1; } =20 =20 diff --git a/tests/viridentitytest.c b/tests/viridentitytest.c index d76c779dd5..1eadd6173a 100644 --- a/tests/viridentitytest.c +++ b/tests/viridentitytest.c @@ -41,6 +41,7 @@ static int testIdentityAttrs(const void *data ATTRIBUTE_U= NUSED) int ret =3D -1; virIdentityPtr ident; const char *val; + int rc; =20 if (!(ident =3D virIdentityNew())) goto cleanup; @@ -48,18 +49,18 @@ static int testIdentityAttrs(const void *data ATTRIBUTE= _UNUSED) if (virIdentitySetUserName(ident, "fred") < 0) goto cleanup; =20 - if (virIdentityGetUserName(ident, &val) < 0) + if ((rc =3D virIdentityGetUserName(ident, &val)) < 0) goto cleanup; =20 - if (STRNEQ_NULLABLE(val, "fred")) { + if (STRNEQ_NULLABLE(val, "fred") || rc !=3D 1) { VIR_DEBUG("Expected 'fred' got '%s'", NULLSTR(val)); goto cleanup; } =20 - if (virIdentityGetGroupName(ident, &val) < 0) + if ((rc =3D virIdentityGetGroupName(ident, &val)) < 0) goto cleanup; =20 - if (val !=3D NULL) { + if (val !=3D NULL || rc !=3D 0) { VIR_DEBUG("Unexpected groupname attribute"); goto cleanup; } @@ -69,10 +70,10 @@ static int testIdentityAttrs(const void *data ATTRIBUTE= _UNUSED) goto cleanup; } =20 - if (virIdentityGetUserName(ident, &val) < 0) + if ((rc =3D virIdentityGetUserName(ident, &val)) < 0) goto cleanup; =20 - if (STRNEQ_NULLABLE(val, "fred")) { + if (STRNEQ_NULLABLE(val, "fred") || rc !=3D 1) { VIR_DEBUG("Expected 'fred' got '%s'", NULLSTR(val)); goto cleanup; } @@ -90,6 +91,7 @@ static int testIdentityGetSystem(const void *data) int ret =3D -1; virIdentityPtr ident =3D NULL; const char *val; + int rc; =20 #if !WITH_SELINUX if (context) { @@ -104,13 +106,20 @@ static int testIdentityGetSystem(const void *data) goto cleanup; } =20 - if (virIdentityGetSELinuxContext(ident, &val) < 0) + if ((rc =3D virIdentityGetSELinuxContext(ident, &val)) < 0) goto cleanup; =20 - if (STRNEQ_NULLABLE(val, context)) { - VIR_DEBUG("Want SELinux context '%s' got '%s'", - context, val); - goto cleanup; + if (context =3D=3D NULL) { + if (val !=3D NULL || rc !=3D 0) { + VIR_DEBUG("Unexpected SELinux context %s", NULLSTR(val)); + goto cleanup; + } + } else { + if (STRNEQ_NULLABLE(val, context) || rc !=3D 1) { + VIR_DEBUG("Want SELinux context '%s' got '%s'", + context, val); + goto cleanup; + } } =20 ret =3D 0; diff --git a/tests/virnetserverclienttest.c b/tests/virnetserverclienttest.c index 3cd76f42ff..d094de9840 100644 --- a/tests/virnetserverclienttest.c +++ b/tests/virnetserverclienttest.c @@ -85,7 +85,7 @@ static int testIdentity(const void *opaque ATTRIBUTE_UNUS= ED) goto cleanup; } =20 - if (virIdentityGetUserName(ident, &gotUsername) < 0) { + if (virIdentityGetUserName(ident, &gotUsername) <=3D 0) { fprintf(stderr, "Missing username in identity\n"); goto cleanup; } @@ -95,7 +95,7 @@ static int testIdentity(const void *opaque ATTRIBUTE_UNUS= ED) goto cleanup; } =20 - if (virIdentityGetUNIXUserID(ident, &gotUserID) < 0) { + if (virIdentityGetUNIXUserID(ident, &gotUserID) <=3D 0) { fprintf(stderr, "Missing user ID in identity\n"); goto cleanup; } @@ -105,7 +105,7 @@ static int testIdentity(const void *opaque ATTRIBUTE_UN= USED) goto cleanup; } =20 - if (virIdentityGetGroupName(ident, &gotGroupname) < 0) { + if (virIdentityGetGroupName(ident, &gotGroupname) <=3D 0) { fprintf(stderr, "Missing groupname in identity\n"); goto cleanup; } @@ -115,7 +115,7 @@ static int testIdentity(const void *opaque ATTRIBUTE_UN= USED) goto cleanup; } =20 - if (virIdentityGetUNIXGroupID(ident, &gotGroupID) < 0) { + if (virIdentityGetUNIXGroupID(ident, &gotGroupID) <=3D 0) { fprintf(stderr, "Missing group ID in identity\n"); goto cleanup; } @@ -125,7 +125,7 @@ static int testIdentity(const void *opaque ATTRIBUTE_UN= USED) goto cleanup; } =20 - if (virIdentityGetSELinuxContext(ident, &gotSELinuxContext) < 0) { + if (virIdentityGetSELinuxContext(ident, &gotSELinuxContext) <=3D 0) { fprintf(stderr, "Missing SELinux context in identity\n"); goto cleanup; } --=20 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list