From patchwork Mon Mar 6 05:33:45 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Selva Nair X-Patchwork-Id: 3104 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7300:2310:b0:9f:bfa4:120f with SMTP id r16csp2186377dye; Sun, 5 Mar 2023 21:34:57 -0800 (PST) X-Google-Smtp-Source: AK7set+VM4EIK1YW3c2zJHVvj+yOMIT0VvDhC7v6NA2B3R8PgXz30rUg/XfOcouXA+SHvsSBc/fe X-Received: by 2002:a17:90b:1e42:b0:237:f018:6433 with SMTP id pi2-20020a17090b1e4200b00237f0186433mr10344061pjb.27.1678080897506; Sun, 05 Mar 2023 21:34:57 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1678080897; cv=none; d=google.com; s=arc-20160816; b=EGq25ImV+J1B5D+hS2OtGjqsuDv6bHDcZDnEOvG7y2NeUcVvNpchco6O4uynWqRojv kCUH198fkhSzcBT6tsGe9guqTqZYr8Xe0l1vgv1KenkPL6zRxT9kCRf5AbEXqsWxVVze w6AfF7IHSEk+r8guyG09VOdbRR8B+HWIeX9v8u1U+jDBPX77RUEuPUoK+bHm9lLfcSxu g50+xd1dtzu9oZM7VPL3XCj9XvjF4r5AR3FVpetde76gRxcu6Q+Z93xKZBwA2i47elQZ xffpq/Wx0yv6XKptfkvbMu54RtRokjhsuF/GzssfsD5QagEG+K8Y8ktT7GDR/MQ2mu4g VG6w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:content-transfer-encoding:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :mime-version:message-id:date:to:from:dkim-signature:dkim-signature :dkim-signature; bh=WX3g7XzB0tunAr7vp1y1L2BDfJsx4JLisi7L1jgZSBI=; b=CRtrR5uzE9Uke66qihSL9u4UsdJUbIqyeTvsVeKQya4m/6uX04Rtc8HQ0SUCTM1OAt ca3jSvHC0+unL9HW681sO/yGIP8nLxmZzE234C25rMwBw0ahshXOra5HiIOKuXfdd/PW oa/3aDVDmsp65j/N+sEQex7op03IAjjPScYPotZUsu0trj5GmQhhzlnAlRgwHP4psyDQ eauREp/cELwJ5ykNXOnZfA+G2AOE32qDUWS36Ds48ZWCseVc2Tt+MiiK6jhSecgwAOzZ tUICUCWD/l4qhKFwiRRLT/qxkFygDNvVSCK9tt6DQQ8her6QoBbfumsRdaNGfUwlOGuE 1B3w== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=XzX4k2HB; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=BB+KWp54; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20210112 header.b=XUO3M2ly; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id m14-20020a17090ade0e00b00234ac93d720si8694417pjv.30.2023.03.05.21.34.56 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 05 Mar 2023 21:34:57 -0800 (PST) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=XzX4k2HB; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=BB+KWp54; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20210112 header.b=XUO3M2ly; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from [127.0.0.1] (helo=sfs-ml-1.v29.lw.sourceforge.com) by sfs-ml-1.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1pZ3U2-00040z-5V; Mon, 06 Mar 2023 05:34:01 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1pZ3U1-00040o-Cj for openvpn-devel@lists.sourceforge.net; Mon, 06 Mar 2023 05:34:00 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:Message-Id: Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=jO7PM7x8A1iRUcyO8LkNDdHa6aMB6/7TCIlcwNpDLdc=; b=XzX4k2HBvfyVbCmkKbMS/PS8OR N76QbRYl/AWCTsyoMbN3TwbRBp7xqr7xTPPyqTq237ZEHN2AaIvovYHPTPwFN0gEyBp0722e+K9Ju o1taoEGvrGnBeKZj2mH6+w/B1Y01wEYJePiQ05vBgHNVC04b3RnHxWQCQnWSgRvoTfYI=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:Message-Id:Date:Subject:Cc:To:From :Sender:Reply-To:Content-Type:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To: References:List-Id:List-Help:List-Unsubscribe:List-Subscribe:List-Post: List-Owner:List-Archive; bh=jO7PM7x8A1iRUcyO8LkNDdHa6aMB6/7TCIlcwNpDLdc=; b=B B+KWp5458Yjib5aXEzTPo23MtHq0QSlWuDuxiPZ+Mcv30zixgo1bH8hcXQtxlyCWbl6eytDXjSeM0 t8Z846w/hMuGzhAiIYjvScaRexQJaGdHTadY8RQUm43MOdoo7FKr1Vp0Fs7wcUKlnNeaq7BFeZd7V b546MAWmuFTL9XRw=; Received: from mail-il1-f182.google.com ([209.85.166.182]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1pZ3Tv-0006BI-JA for openvpn-devel@lists.sourceforge.net; Mon, 06 Mar 2023 05:34:00 +0000 Received: by mail-il1-f182.google.com with SMTP id r4so5789123ila.2 for ; Sun, 05 Mar 2023 21:33:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1678080830; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=jO7PM7x8A1iRUcyO8LkNDdHa6aMB6/7TCIlcwNpDLdc=; b=XUO3M2lyjL4Umxv7KGhweILeoZDYTuDNWQAl24hLIdN/L7kGndiVTuVr1/LrVczut1 NGLkSQhbleqVhwnPM1Ri1SRL70iCPHiCNQtkLx3A3MOzbCf/2W8v9G0PxWmogUBw5EOX 3vqXBz1MAVB6pMwXtMl70rbZQoOpGIUxTCTt18+HnP/LmLTDpi9Fgags8fcK7J2ZEGze PEslv0eBAg6M6XZFimsoOYulba/zkjIppw/fWdayAGMUaI2l58v6leVxSzgBulSpuncw 3DA0NPUUUKsz9Awykq/YooufxvIMf6/ikVbEkM5J6nKTfryRov7pg10k0YDuLSFdLQUf PzJA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678080830; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=jO7PM7x8A1iRUcyO8LkNDdHa6aMB6/7TCIlcwNpDLdc=; b=cWFh8EkA6AQHK6vRJs7lqTznUf73NhxPYEYZA7BMmP5ANqUicIttSaediAxNku0VSD jkrjbhWTXk1IDC29R5RWasEc58hO8zQAGb/eYI9bZxN7fGplSzJRny5ntvzrnN3KPzjk iHOHtgj3NFBautFPryJx5Xd03cKv9o+/tEmwbv9L40g47dwQ6t1ajudP2BM/0ZbgKyeW TQl51+05Md82AB+zjpEUok7u8mmsRFchdhXmEtTIQ2pSoyt5j9vhiKENvwDZOXS8iAT5 MfFHvqDgzs14rdlVj0rhLtqzYt0c+2j+rQ+B9MPWPyMwm4NuAeohPqXIpEfShlfpBxgw IJEA== X-Gm-Message-State: AO0yUKW9++1AgoRBeUMfTk1K6qZjtH962n7+Fq7g8rv2pMooOYeR4OI8 vED+wMnPGhjytil7YGGxXICRkFUV3N0= X-Received: by 2002:a92:194c:0:b0:317:943c:2280 with SMTP id e12-20020a92194c000000b00317943c2280mr4436280ilm.0.1678080830527; Sun, 05 Mar 2023 21:33:50 -0800 (PST) Received: from uranus.sansel.ca (bras-vprn-tnhlon4053w-lp130-01-70-51-222-66.dsl.bell.ca. [70.51.222.66]) by smtp.gmail.com with ESMTPSA id y28-20020a02ce9c000000b003c2bb30f97esm2940695jaq.43.2023.03.05.21.33.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Mar 2023 21:33:50 -0800 (PST) From: selva.nair@gmail.com To: openvpn-devel@lists.sourceforge.net Date: Mon, 6 Mar 2023 00:33:45 -0500 Message-Id: <20230306053346.796992-1-selva.nair@gmail.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Spam-Score: -0.2 (/) X-Spam-Report: Spam detection software, running on the system "util-spamd-1.v13.lw.sourceforge.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: From: Selva Nair - This pointer is to a static area which can change on further calls to getpwnam, getpwuid etc. Same with struct group returned by getgrnam. As the only field later referred to is uid or gid, fix by saving them instead. Content analysis details: (-0.2 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider [selva.nair[at]gmail.com] -0.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.166.182 listed in wl.mailspike.net] -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [209.85.166.182 listed in list.dnswl.org] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid X-Headers-End: 1pZ3Tv-0006BI-JA Subject: [Openvpn-devel] [PATCH 1/2] Do not save pointer to 'struct passwd' returned by getpwnam etc. X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1759595355053698189?= X-GMAIL-MSGID: =?utf-8?q?1759595355053698189?= From: Selva Nair - This pointer is to a static area which can change on further calls to getpwnam, getpwuid etc. Same with struct group returned by getgrnam. As the only field later referred to is uid or gid, fix by saving them instead. Signed-off-by: Selva Nair Acked-by: Gert Doering --- Though we call getpwnam()/getgrnam() more than once with potentially different user/group names, this has not yet led to a bug because only in one place the state is persisted, and that call happens later. But looks like a bug waiting to happen. src/openvpn/platform.c | 36 +++++++++++++++++++++++------------- src/openvpn/platform.h | 14 ++++---------- src/openvpn/tun.c | 4 ++-- 3 files changed, 29 insertions(+), 25 deletions(-) diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c index 580c4cb8..f6b856f3 100644 --- a/src/openvpn/platform.c +++ b/src/openvpn/platform.c @@ -85,11 +85,16 @@ platform_user_get(const char *username, struct platform_state_user *state) if (username) { #if defined(HAVE_GETPWNAM) && defined(HAVE_SETUID) - state->pw = getpwnam(username); - if (!state->pw) + state->uid = -1; + const struct passwd *pw = getpwnam(username); + if (!pw) { msg(M_ERR, "failed to find UID for user %s", username); } + else + { + state->uid = pw->pw_uid; + } state->username = username; ret = true; #else /* if defined(HAVE_GETPWNAM) && defined(HAVE_SETUID) */ @@ -103,9 +108,9 @@ static void platform_user_set(const struct platform_state_user *state) { #if defined(HAVE_GETPWNAM) && defined(HAVE_SETUID) - if (state->username && state->pw) + if (state->username && state->uid >= 0) { - if (setuid(state->pw->pw_uid)) + if (setuid(state->uid)) { msg(M_ERR, "setuid('%s') failed", state->username); } @@ -124,11 +129,16 @@ platform_group_get(const char *groupname, struct platform_state_group *state) if (groupname) { #if defined(HAVE_GETGRNAM) && defined(HAVE_SETGID) - state->gr = getgrnam(groupname); - if (!state->gr) + state->gid = -1; + const struct group *gr = getgrnam(groupname); + if (!gr) { msg(M_ERR, "failed to find GID for group %s", groupname); } + else + { + state->gid = gr->gr_gid; + } state->groupname = groupname; ret = true; #else /* if defined(HAVE_GETGRNAM) && defined(HAVE_SETGID) */ @@ -142,9 +152,9 @@ static void platform_group_set(const struct platform_state_group *state) { #if defined(HAVE_GETGRNAM) && defined(HAVE_SETGID) - if (state->groupname && state->gr) + if (state->groupname && state->gid >= 0) { - if (setgid(state->gr->gr_gid)) + if (setgid(state->gid)) { msg(M_ERR, "setgid('%s') failed", state->groupname); } @@ -152,7 +162,7 @@ platform_group_set(const struct platform_state_group *state) #ifdef HAVE_SETGROUPS { gid_t gr_list[1]; - gr_list[0] = state->gr->gr_gid; + gr_list[0] = state->gid; if (setgroups(1, gr_list)) { msg(M_ERR, "setgroups('%s') failed", state->groupname); @@ -225,13 +235,13 @@ platform_user_group_set(const struct platform_state_user *user_state, * new_uid/new_gid defaults to -1, which will not make * libcap-ng change the UID/GID unless configured */ - if (group_state->groupname && group_state->gr) + if (group_state->groupname && group_state->gid >= 0) { - new_gid = group_state->gr->gr_gid; + new_gid = group_state->gid; } - if (user_state->username && user_state->pw) + if (user_state->username && user_state->uid >= 0) { - new_uid = user_state->pw->pw_uid; + new_uid = user_state->uid; } /* Prepare capabilities before dropping UID/GID */ diff --git a/src/openvpn/platform.h b/src/openvpn/platform.h index a35a571c..d8dad74b 100644 --- a/src/openvpn/platform.h +++ b/src/openvpn/platform.h @@ -63,7 +63,7 @@ struct context; struct platform_state_user { #if defined(HAVE_GETPWNAM) && defined(HAVE_SETUID) const char *username; - struct passwd *pw; + uid_t uid; #else int dummy; #endif @@ -74,7 +74,7 @@ struct platform_state_user { struct platform_state_group { #if defined(HAVE_GETGRNAM) && defined(HAVE_SETGID) const char *groupname; - struct group *gr; + gid_t gid; #else int dummy; #endif @@ -97,10 +97,7 @@ static inline int platform_state_user_uid(const struct platform_state_user *s) { #if defined(HAVE_GETPWNAM) && defined(HAVE_SETUID) - if (s->pw) - { - return s->pw->pw_uid; - } + return s->uid; #endif return -1; } @@ -109,10 +106,7 @@ static inline int platform_state_group_gid(const struct platform_state_group *s) { #if defined(HAVE_GETGRNAM) && defined(HAVE_SETGID) - if (s->gr) - { - return s->gr->gr_gid; - } + return s->gid; #endif return -1; } diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index a1414d23..87033277 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -2242,7 +2242,7 @@ tuncfg(const char *dev, const char *dev_type, const char *dev_node, { msg(M_ERR, "Cannot get user entry for %s", username); } - else if (ioctl(tt->fd, TUNSETOWNER, platform_state_user.pw->pw_uid) < 0) + else if (ioctl(tt->fd, TUNSETOWNER, platform_state_user.uid) < 0) { msg(M_ERR, "Cannot ioctl TUNSETOWNER(%s) %s", username, dev); } @@ -2255,7 +2255,7 @@ tuncfg(const char *dev, const char *dev_type, const char *dev_node, { msg(M_ERR, "Cannot get group entry for %s", groupname); } - else if (ioctl(tt->fd, TUNSETGROUP, platform_state_group.gr->gr_gid) < 0) + else if (ioctl(tt->fd, TUNSETGROUP, platform_state_group.gid) < 0) { msg(M_ERR, "Cannot ioctl TUNSETGROUP(%s) %s", groupname, dev); }