From nobody Sun Apr 28 20:48:50 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1496133939342758.4106981017825; Tue, 30 May 2017 01:45:39 -0700 (PDT) Received: from localhost ([::1]:52277 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dFcmX-0007TU-V2 for importer@patchew.org; Tue, 30 May 2017 04:45:37 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50705) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dFcRK-0006VJ-80 for qemu-devel@nongnu.org; Tue, 30 May 2017 04:23:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dFcRH-0001Nt-7q for qemu-devel@nongnu.org; Tue, 30 May 2017 04:23:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44014) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dFcRG-0001M2-VG for qemu-devel@nongnu.org; Tue, 30 May 2017 04:23:39 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 103CE81243 for ; Tue, 30 May 2017 08:23:38 +0000 (UTC) Received: from moe.brq.redhat.com (dhcp129-131.brq.redhat.com [10.34.129.131]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8A55717151 for ; Tue, 30 May 2017 08:23:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 103CE81243 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=mprivozn@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 103CE81243 From: Michal Privoznik To: qemu-devel@nongnu.org Date: Tue, 30 May 2017 10:23:33 +0200 Message-Id: <99971bb1889d4e5a4d603121034b1ed39a4e6912.1496132443.git.mprivozn@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Tue, 30 May 2017 08:23:38 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH 1/3] qemu-bridge-helper: Reverse return value setting logic X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Instead of initializing the return value @ret to a success value and then having to reset it to failure value in every error path, we can do the opposite. Initialize the value to failure value and then set it to success value only after we've succeeded in all we've attempted. Signed-off-by: Michal Privoznik --- qemu-bridge-helper.c | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c index 5396fbfbb6..af6613ea18 100644 --- a/qemu-bridge-helper.c +++ b/qemu-bridge-helper.c @@ -228,7 +228,7 @@ int main(int argc, char **argv) ACLRule *acl_rule; ACLList acl_list; int access_allowed, access_denied; - int ret =3D EXIT_SUCCESS; + int rv, ret =3D EXIT_FAILURE; =20 #ifdef CONFIG_LIBCAP /* if we're run from an suid binary, immediately drop privileges prese= rving @@ -265,7 +265,6 @@ int main(int argc, char **argv) if (parse_acl_file(DEFAULT_ACL_FILE, &acl_list) =3D=3D -1) { fprintf(stderr, "failed to parse default acl file `%s'\n", DEFAULT_ACL_FILE); - ret =3D EXIT_FAILURE; goto cleanup; } =20 @@ -298,7 +297,6 @@ int main(int argc, char **argv) =20 if ((access_allowed =3D=3D 0) || (access_denied =3D=3D 1)) { fprintf(stderr, "access denied by acl file\n"); - ret =3D EXIT_FAILURE; goto cleanup; } =20 @@ -306,7 +304,6 @@ int main(int argc, char **argv) ctlfd =3D socket(AF_INET, SOCK_STREAM, 0); if (ctlfd =3D=3D -1) { fprintf(stderr, "failed to open control socket: %s\n", strerror(er= rno)); - ret =3D EXIT_FAILURE; goto cleanup; } =20 @@ -314,7 +311,6 @@ int main(int argc, char **argv) fd =3D open("/dev/net/tun", O_RDWR); if (fd =3D=3D -1) { fprintf(stderr, "failed to open /dev/net/tun: %s\n", strerror(errn= o)); - ret =3D EXIT_FAILURE; goto cleanup; } =20 @@ -328,7 +324,6 @@ int main(int argc, char **argv) =20 if (ioctl(fd, TUNSETIFF, &ifr) =3D=3D -1) { fprintf(stderr, "failed to create tun device: %s\n", strerror(errn= o)); - ret =3D EXIT_FAILURE; goto cleanup; } =20 @@ -340,7 +335,6 @@ int main(int argc, char **argv) if (ioctl(ctlfd, SIOCGIFMTU, &ifr) =3D=3D -1) { fprintf(stderr, "failed to get mtu of bridge `%s': %s\n", bridge, strerror(errno)); - ret =3D EXIT_FAILURE; goto cleanup; } =20 @@ -353,7 +347,6 @@ int main(int argc, char **argv) if (ioctl(ctlfd, SIOCSIFMTU, &ifr) =3D=3D -1) { fprintf(stderr, "failed to set mtu of device `%s' to %d: %s\n", iface, mtu, strerror(errno)); - ret =3D EXIT_FAILURE; goto cleanup; } =20 @@ -364,14 +357,12 @@ int main(int argc, char **argv) if (ioctl(ctlfd, SIOCGIFHWADDR, &ifr) < 0) { fprintf(stderr, "failed to get MAC address of device `%s': %s\n", iface, strerror(errno)); - ret =3D EXIT_FAILURE; goto cleanup; } ifr.ifr_hwaddr.sa_data[0] =3D 0xFE; if (ioctl(ctlfd, SIOCSIFHWADDR, &ifr) < 0) { fprintf(stderr, "failed to set MAC address of device `%s': %s\n", iface, strerror(errno)); - ret =3D EXIT_FAILURE; goto cleanup; } =20 @@ -384,15 +375,14 @@ int main(int argc, char **argv) ifargs[2] =3D 0; ifargs[3] =3D 0; ifr.ifr_data =3D (void *)ifargs; - ret =3D ioctl(ctlfd, SIOCDEVPRIVATE, &ifr); + rv =3D ioctl(ctlfd, SIOCDEVPRIVATE, &ifr); #else ifr.ifr_ifindex =3D ifindex; - ret =3D ioctl(ctlfd, SIOCBRADDIF, &ifr); + rv =3D ioctl(ctlfd, SIOCBRADDIF, &ifr); #endif - if (ret =3D=3D -1) { + if (rv =3D=3D -1) { fprintf(stderr, "failed to add interface `%s' to bridge `%s': %s\n= ", iface, bridge, strerror(errno)); - ret =3D EXIT_FAILURE; goto cleanup; } =20 @@ -401,7 +391,6 @@ int main(int argc, char **argv) if (ioctl(ctlfd, SIOCGIFFLAGS, &ifr) =3D=3D -1) { fprintf(stderr, "failed to get interface flags for `%s': %s\n", iface, strerror(errno)); - ret =3D EXIT_FAILURE; goto cleanup; } =20 @@ -409,7 +398,6 @@ int main(int argc, char **argv) if (ioctl(ctlfd, SIOCSIFFLAGS, &ifr) =3D=3D -1) { fprintf(stderr, "failed to bring up interface `%s': %s\n", iface, strerror(errno)); - ret =3D EXIT_FAILURE; goto cleanup; } =20 @@ -417,13 +405,13 @@ int main(int argc, char **argv) if (send_fd(unixfd, fd) =3D=3D -1) { fprintf(stderr, "failed to write fd to unix socket: %s\n", strerror(errno)); - ret =3D EXIT_FAILURE; goto cleanup; } =20 /* ... */ =20 /* profit! */ + ret =3D EXIT_SUCCESS; =20 cleanup: if (fd >=3D 0) { --=20 2.13.0 From nobody Sun Apr 28 20:48:50 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1496133872690318.4693902256764; Tue, 30 May 2017 01:44:32 -0700 (PDT) Received: from localhost ([::1]:52269 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dFclT-0006pL-98 for importer@patchew.org; Tue, 30 May 2017 04:44:31 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50698) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dFcRK-0006VB-6K for qemu-devel@nongnu.org; Tue, 30 May 2017 04:23:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dFcRH-0001Oy-V1 for qemu-devel@nongnu.org; Tue, 30 May 2017 04:23:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:8753) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dFcRH-0001Nn-PU for qemu-devel@nongnu.org; Tue, 30 May 2017 04:23:39 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D1DFAC83E for ; Tue, 30 May 2017 08:23:38 +0000 (UTC) Received: from moe.brq.redhat.com (dhcp129-131.brq.redhat.com [10.34.129.131]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5702317151 for ; Tue, 30 May 2017 08:23:38 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com D1DFAC83E Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=mprivozn@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com D1DFAC83E From: Michal Privoznik To: qemu-devel@nongnu.org Date: Tue, 30 May 2017 10:23:34 +0200 Message-Id: In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Tue, 30 May 2017 08:23:39 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH 2/3] qemu-bridge-helper: Reverse return value setting logic in parse_acl_file X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Just like in the previous commit, it's better to have just a single point of exit from a function as we can have cleanup code just once instead of copying it all over the place. Signed-off-by: Michal Privoznik --- qemu-bridge-helper.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c index af6613ea18..a7f9bf06cc 100644 --- a/qemu-bridge-helper.c +++ b/qemu-bridge-helper.c @@ -65,6 +65,7 @@ static int parse_acl_file(const char *filename, ACLList *= acl_list) FILE *f; char line[4096]; ACLRule *acl_rule; + int ret =3D -1; =20 f =3D fopen(filename, "r"); if (f =3D=3D NULL) { @@ -92,9 +93,8 @@ static int parse_acl_file(const char *filename, ACLList *= acl_list) =20 if (arg =3D=3D NULL) { fprintf(stderr, "Invalid config line:\n %s\n", line); - fclose(f); errno =3D EINVAL; - return -1; + goto cleanup; } =20 *arg =3D 0; @@ -132,15 +132,16 @@ static int parse_acl_file(const char *filename, ACLLi= st *acl_list) parse_acl_file(arg, acl_list); } else { fprintf(stderr, "Unknown command `%s'\n", cmd); - fclose(f); errno =3D EINVAL; - return -1; + goto cleanup; } } =20 - fclose(f); + ret =3D 0; =20 - return 0; + cleanup: + fclose(f); + return ret; } =20 static bool has_vnet_hdr(int fd) --=20 2.13.0 From nobody Sun Apr 28 20:48:50 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1496133158321471.5913119058198; Tue, 30 May 2017 01:32:38 -0700 (PDT) Received: from localhost ([::1]:52171 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dFcZw-0005Yq-N2 for importer@patchew.org; Tue, 30 May 2017 04:32:36 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50700) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dFcRK-0006VE-6w for qemu-devel@nongnu.org; Tue, 30 May 2017 04:23:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dFcRI-0001Pt-Qx for qemu-devel@nongnu.org; Tue, 30 May 2017 04:23:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44092) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dFcRI-0001P4-IZ for qemu-devel@nongnu.org; Tue, 30 May 2017 04:23:40 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9F1B78123E for ; Tue, 30 May 2017 08:23:39 +0000 (UTC) Received: from moe.brq.redhat.com (dhcp129-131.brq.redhat.com [10.34.129.131]) by smtp.corp.redhat.com (Postfix) with ESMTP id 23D4C17151 for ; Tue, 30 May 2017 08:23:38 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 9F1B78123E Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=mprivozn@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 9F1B78123E From: Michal Privoznik To: qemu-devel@nongnu.org Date: Tue, 30 May 2017 10:23:35 +0200 Message-Id: In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Tue, 30 May 2017 08:23:39 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH 3/3] qemu-bridge-helper: Take ACL file gid into account X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" There's a problem with the way we currently parse ACL files. The problem is, the bridge helper has usually SUID flag set and thus runs as root in which case all the ACL files are parsed (/etc/qemu/bridge.conf and all other files it includes). Therefore, if there's say bob.conf owned by root:bob and I run the bridge helper, it doesn't really matter whether I'm in the bob group or not. Everything that is allowed to bob group is allowed to me too. The way this problem is fixed is whenever an ACL file is parsed, the group owner of the file is stored among with the rules so that it can be compared when evaluating. There is one exception though. If an ACL file is owned by root group the rules within apply to all groups. This is because that's the usual setup currently (the bridge.conf is usually owned by root:root) and anybody from root group can plug the interface themselves anyway. This idea of groups was introduced in bdef79a2994d6f0383 but got broken by ditros setting SUID onto the binary. Signed-off-by: Michal Privoznik --- qemu-bridge-helper.c | 44 ++++++++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c index a7f9bf06cc..eab9ad5096 100644 --- a/qemu-bridge-helper.c +++ b/qemu-bridge-helper.c @@ -48,6 +48,7 @@ enum { =20 typedef struct ACLRule { int type; + gid_t group; char iface[IFNAMSIZ]; QSIMPLEQ_ENTRY(ACLRule) entry; } ACLRule; @@ -65,8 +66,13 @@ static int parse_acl_file(const char *filename, ACLList = *acl_list) FILE *f; char line[4096]; ACLRule *acl_rule; + struct stat stbuf; int ret =3D -1; =20 + if (stat(filename, &stbuf) < 0) { + return -1; + } + f =3D fopen(filename, "r"); if (f =3D=3D NULL) { return -1; @@ -117,6 +123,7 @@ static int parse_acl_file(const char *filename, ACLList= *acl_list) acl_rule->type =3D ACL_DENY; snprintf(acl_rule->iface, IFNAMSIZ, "%s", arg); } + acl_rule->group =3D stbuf.st_gid; QSIMPLEQ_INSERT_TAIL(acl_list, acl_rule, entry); } else if (strcmp(cmd, "allow") =3D=3D 0) { acl_rule =3D g_malloc(sizeof(*acl_rule)); @@ -126,6 +133,7 @@ static int parse_acl_file(const char *filename, ACLList= *acl_list) acl_rule->type =3D ACL_ALLOW; snprintf(acl_rule->iface, IFNAMSIZ, "%s", arg); } + acl_rule->group =3D stbuf.st_gid; QSIMPLEQ_INSERT_TAIL(acl_list, acl_rule, entry); } else if (strcmp(cmd, "include") =3D=3D 0) { /* ignore errors */ @@ -229,6 +237,7 @@ int main(int argc, char **argv) ACLRule *acl_rule; ACLList acl_list; int access_allowed, access_denied; + gid_t group; int rv, ret =3D EXIT_FAILURE; =20 #ifdef CONFIG_LIBCAP @@ -275,24 +284,31 @@ int main(int argc, char **argv) */ access_allowed =3D 0; access_denied =3D 0; + group =3D getgid(); QSIMPLEQ_FOREACH(acl_rule, &acl_list, entry) { - switch (acl_rule->type) { - case ACL_ALLOW_ALL: - access_allowed =3D 1; - break; - case ACL_ALLOW: - if (strcmp(bridge, acl_rule->iface) =3D=3D 0) { + /* If the acl policy file is owned by root group it + * applies to all groups. Otherwise it applies to that + * specific group. */ + if (!acl_rule->group || + (acl_rule->group && acl_rule->group =3D=3D group)) { + switch (acl_rule->type) { + case ACL_ALLOW_ALL: access_allowed =3D 1; - } - break; - case ACL_DENY_ALL: - access_denied =3D 1; - break; - case ACL_DENY: - if (strcmp(bridge, acl_rule->iface) =3D=3D 0) { + break; + case ACL_ALLOW: + if (strcmp(bridge, acl_rule->iface) =3D=3D 0) { + access_allowed =3D 1; + } + break; + case ACL_DENY_ALL: access_denied =3D 1; + break; + case ACL_DENY: + if (strcmp(bridge, acl_rule->iface) =3D=3D 0) { + access_denied =3D 1; + } + break; } - break; } } =20 --=20 2.13.0