From patchwork Fri Oct 3 10:06:02 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Frank Lichtenheld X-Patchwork-Id: 4458 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:7505:b0:72f:f16c:e055 with SMTP id r5csp5074131mai; Fri, 3 Oct 2025 03:08:27 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCVmngEhhhY79buwyggFhT6ywYRDs9omjlM6WnGd6y4x/4Oa606pp/YQAkuXQF8Yhzdq3si8ysma9Pw=@openvpn.net X-Google-Smtp-Source: AGHT+IFQ6Zf6GbNKOOKhhB76o7ALT69P8EaPaIuDJEgBPaA1G2QvjN5TK/SCoB20S0lrCRVNG/l2 X-Received: by 2002:a05:6808:1815:b0:43f:7e97:397f with SMTP id 5614622812f47-43fc18385bbmr1161656b6e.41.1759486107817; Fri, 03 Oct 2025 03:08:27 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1759486107; cv=none; d=google.com; s=arc-20240605; b=QsPwvYiJS9uNha7NIsqUhUKS4tdUzAhfudStNgYtXfvU23mgdGWkR8zprl37zNybr6 La4Fz6bnXk6L2wYxG/9/KhHt0em0O/ZwJT7IZNcrh04jmVSUD6EXLRpYlilcnoG+vAzc upnEVl0gvJ1q3CWFbv7QqvL4IXnsfq9b2MaweXWukKxOUzbIo/4YSu8MlNQyq0cJK6Qe HguKwWcTGb7ncT0k34Ey18hXtlvlU9jKEkbpI1ERFHHUQY/KdB6fX1dc2lWdWdeY5Ftz 8yH1MFZgo01LRRUl24XKTA4ec4GXKsmr4aTZs5zQQMX7ytusrlrOXXwuY/L1K2GkeIK0 jvkQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=errors-to:content-transfer-encoding:cc:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :mime-version:references:in-reply-to:message-id:date:to:from :dkim-signature:dkim-signature:dkim-signature:dkim-signature; bh=g93yS8ZyMQRySs9DNM5xe3OSmfuOVlJKC0HQaCznYPE=; fh=247L+IztxEdcidbjmJCC8yDC2mCuyVhfy37dKhZO6zE=; b=gidfEndjCDrs4Mc4idZb+BjVCZf7zc28XFzB1e1jVef/55G4YxHRD72chixIOEnHZM sqX5CvUc5FiKxqgS3jVbu4rD/u+xUoW51vH3OJJVtRy22To5E827xAEAAOI+q0rkC4eO NYQJOjuwDiKDC3HhBrQ2qYjqmfMNje+gnalROCac2PUbCV8iHG2ih896D6wND2Vg1PTa kIgIKr26QW7mb4x1E88R0gmGtSAE6lGVoRwndPH8m0iNkVZsreo4w/Li8Rkkg7JPnoh/ rBUEn+z7vwQZNfg5DQldwY6QEtC5PGI7oI4Gta6jibqybA2JOxRXwPMfiyW3f3DxUxZV 0/lA==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@lists.sourceforge.net header.s=beta header.b=cjjqncOL; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=c9DGTTZn; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=E4JJZ4+e; dkim=neutral (body hash did not verify) header.i=@lichtenheld.com header.s=MBO0001 header.b=0+uf3Z8U; 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 Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id 5614622812f47-43fc19a04d1si410750b6e.392.2025.10.03.03.08.27 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 03 Oct 2025 03:08:27 -0700 (PDT) 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=pass header.i=@lists.sourceforge.net header.s=beta header.b=cjjqncOL; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=c9DGTTZn; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=E4JJZ4+e; dkim=neutral (body hash did not verify) header.i=@lichtenheld.com header.s=MBO0001 header.b=0+uf3Z8U; 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 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.sourceforge.net; s=beta; h=Content-Transfer-Encoding:Content-Type:Cc: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: Subject:MIME-Version:References:In-Reply-To:Message-Id:Date:To:From:Sender: Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender :Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=g93yS8ZyMQRySs9DNM5xe3OSmfuOVlJKC0HQaCznYPE=; b=cjjqncOLAKoIGSkgSKh00kBF87 vb4yUhFre4wb7W1vHvW79aihc5/okcEbi43adXdt/knudskBE3dJl/tW3AjB6msGmI+KNo+PbQsFn T8aB399aZ6ra+D3G6Fp/0U5PphrnplQsTqn5Xbf2m3g6RONTunSLeQ4GYOBaFEOr0v5s=; 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 1v4ci6-0008LL-5I; Fri, 03 Oct 2025 10:08:22 +0000 Received: from [172.30.29.66] (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 1v4ci2-0008L1-Mj for openvpn-devel@lists.sourceforge.net; Fri, 03 Oct 2025 10:08:19 +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:References: In-Reply-To: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:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=ElKl8nJDDW8zkJkQTEWqbLtrNPd8cVysCX+MTglCoxI=; b=c9DGTTZnPBBKh0/WkgH626QbFl 12dOM60ZcOwPzBW//5K4TYEyRHtFsRrfg/0WTvKL+QyOae7gjrOYUOUhDcT6napL7Oj/ArgQQgv8Q 8Kif9VyS4HxbDFxeMF39swyrOZbAeXEMvEkoToTZUTaG0AqJn4na9LzmfLlGUbB3AwJg=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: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:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=ElKl8nJDDW8zkJkQTEWqbLtrNPd8cVysCX+MTglCoxI=; b=E4JJZ4+elN3UR3erzEBwy3I50c K68BRIIl/cjZ5EE5fwz6BKriWa/pW5+ceIxKakd9H1hE1tWpkVbG8clFOYJIEHKhfXWd+avKRbPvP WL0DQQOWSBJhZPGzWT35u/Fnmmfa+uqfJdz5L1EBV0TsQpjTWaqyQG9g9rQ4pArL8ilM=; Received: from mout-p-202.mailbox.org ([80.241.56.172]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1v4ci1-0004UN-Hr for openvpn-devel@lists.sourceforge.net; Fri, 03 Oct 2025 10:08:18 +0000 Received: from smtp1.mailbox.org (smtp1.mailbox.org [IPv6:2001:67c:2050:b231:465::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-202.mailbox.org (Postfix) with ESMTPS id 4cdPVr0JKKz9tbX; Fri, 3 Oct 2025 12:08:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=lichtenheld.com; s=MBO0001; t=1759486084; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ElKl8nJDDW8zkJkQTEWqbLtrNPd8cVysCX+MTglCoxI=; b=0+uf3Z8UeUFDNURzhJ9FiUr/hbbJWkZrWNTPCB1mxrnp09U2zCsYN5ZG23oO1GtTFeEmX5 Eqci2xyX16Zsuue/36XrDos0hmh32x4Zh/ovFdS6sf0OYoneQ5Huthasv/kmzBELrfhf1b aDvzspiK8OLmgCPfOEs/wTNNwxcI3OBvUZsSPHxVkID4IofuvBZOB51q3rI526mKs+c+W0 VWNbvtU7rNd+Q4EtwcRqZbYp5mF/cUHptdWbr5kwIkM/HW9+gMD+RzZ9rsCMadgwaTZzRT Tiof6/uW/2PrP8o+WzlJzzkFtt471ZzNoEeOnevAOytB+LDQ8NLG8bLRoQO1Cw== Authentication-Results: outgoing_mbo_mout; dkim=none; spf=pass (outgoing_mbo_mout: domain of frank@lichtenheld.com designates 2001:67c:2050:b231:465::1 as permitted sender) smtp.mailfrom=frank@lichtenheld.com From: Frank Lichtenheld To: openvpn-devel@lists.sourceforge.net Date: Fri, 3 Oct 2025 12:06:02 +0200 Message-Id: <20251003100602.375062-1-frank@lichtenheld.com> In-Reply-To: References: MIME-Version: 1.0 X-Rspamd-Queue-Id: 4cdPVr0JKKz9tbX X-Spam-Score: -0.2 (/) X-Spam-Report: Spam detection software, running on the system "sfi-spamd-2.hosts.colo.sdot.me", 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: uid_t/gid_t are int on many platform but unsigned on at least Linux. So rewrite the code in a way that does not make any assumptions about the types. Mainly this means storing the information whether [...] Content analysis details: (-0.2 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -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 X-Headers-End: 1v4ci1-0004UN-Hr Subject: [Openvpn-devel] [PATCH v2] platform: Do not assume uid_t/gid_t are signed 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: , Cc: MaxF Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1844954905339692525?= X-GMAIL-MSGID: =?utf-8?q?1844954905339692525?= uid_t/gid_t are int on many platform but unsigned on at least Linux. So rewrite the code in a way that does not make any assumptions about the types. Mainly this means storing the information whether the value is valid in a separate bool and not in the value itself. Note that this changes the return behavior of platform_{user,group}_get but a review of the callers determined that this makes no actual difference. Change-Id: Ie6b4c41d13544d5ba71d441cc794c7abd12408f3 Signed-off-by: Frank Lichtenheld Acked-by: MaxF Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1206 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1206 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): MaxF diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c index 5a41a0f..1cb5c63 100644 --- a/src/openvpn/manage.c +++ b/src/openvpn/manage.c @@ -1847,25 +1847,26 @@ static bool man_verify_unix_peer_uid_gid(struct management *man, const socket_descriptor_t sd) { - if (socket_defined(sd) && (man->settings.client_uid != -1 || man->settings.client_gid != -1)) + if (socket_defined(sd) && (man->settings.user.user_valid || man->settings.group.group_valid)) { static const char err_prefix[] = "MANAGEMENT: unix domain socket client connection rejected --"; - int uid, gid; + uid_t uid; + gid_t gid; if (unix_socket_get_peer_uid_gid(man->connection.sd_cli, &uid, &gid)) { - if (man->settings.client_uid != -1 && man->settings.client_uid != uid) + if (man->settings.user.user_valid && man->settings.user.uid != uid) { msg(D_MANAGEMENT, "%s UID of socket peer (%d) doesn't match required value (%d) as given by --management-client-user", - err_prefix, uid, man->settings.client_uid); + err_prefix, uid, man->settings.user.uid); return false; } - if (man->settings.client_gid != -1 && man->settings.client_gid != gid) + if (man->settings.group.group_valid && man->settings.group.gid != gid) { msg(D_MANAGEMENT, "%s GID of socket peer (%d) doesn't match required value (%d) as given by --management-client-group", - err_prefix, gid, man->settings.client_gid); + err_prefix, gid, man->settings.group.gid); return false; } } @@ -2542,8 +2543,6 @@ CLEAR(*ms); ms->flags = flags; - ms->client_uid = -1; - ms->client_gid = -1; /* * Get username/password @@ -2553,27 +2552,21 @@ get_user_pass(&ms->up, pass_file, "Management", GET_USER_PASS_PASSWORD_ONLY); } +#if UNIX_SOCK_SUPPORT /* * lookup client UID/GID if specified */ if (client_user) { - struct platform_state_user s; - platform_user_get(client_user, &s); - ms->client_uid = platform_state_user_uid(&s); - msg(D_MANAGEMENT, "MANAGEMENT: client_uid=%d", ms->client_uid); - ASSERT(ms->client_uid >= 0); + ASSERT(platform_user_get(client_user, &ms->user)); + msg(D_MANAGEMENT, "MANAGEMENT: client_uid=%d", ms->user.uid); } if (client_group) { - struct platform_state_group s; - platform_group_get(client_group, &s); - ms->client_gid = platform_state_group_gid(&s); - msg(D_MANAGEMENT, "MANAGEMENT: client_gid=%d", ms->client_gid); - ASSERT(ms->client_gid >= 0); + ASSERT(platform_group_get(client_group, &ms->group)); + msg(D_MANAGEMENT, "MANAGEMENT: client_gid=%d", ms->group.gid); } -#if UNIX_SOCK_SUPPORT if (ms->flags & MF_UNIX_SOCK) { sockaddr_unix_init(&ms->local_unix, addr); diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h index bff96d3..a31eb06 100644 --- a/src/openvpn/manage.h +++ b/src/openvpn/manage.h @@ -242,14 +242,14 @@ struct addrinfo *local; #if UNIX_SOCK_SUPPORT struct sockaddr_un local_unix; + struct platform_state_user user; + struct platform_state_group group; #endif bool management_over_tunnel; struct user_pass up; int log_history_cache; int echo_buffer_size; int state_buffer_size; - int client_uid; - int client_gid; /* flags for handling the management interface "signal" command */ #define MANSIG_IGNORE_USR1_HUP (1u << 0) diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c index 880d14e..a1ffdaf 100644 --- a/src/openvpn/platform.c +++ b/src/openvpn/platform.c @@ -39,7 +39,7 @@ #include "platform.h" -#if _WIN32 +#ifdef _WIN32 #include #endif @@ -79,12 +79,10 @@ bool platform_user_get(const char *username, struct platform_state_user *state) { - bool ret = false; CLEAR(*state); if (username) { #if defined(HAVE_GETPWNAM) && defined(HAVE_SETUID) - state->uid = -1; const struct passwd *pw = getpwnam(username); if (!pw) { @@ -93,23 +91,23 @@ else { state->uid = pw->pw_uid; + state->user_valid = true; } state->username = username; - ret = true; #else /* if defined(HAVE_GETPWNAM) && defined(HAVE_SETUID) */ msg(M_FATAL, "cannot get UID for user %s -- platform lacks getpwname() or setuid() system calls", username); #endif } - return ret; + return state->user_valid; } static void platform_user_set(const struct platform_state_user *state) { #if defined(HAVE_GETPWNAM) && defined(HAVE_SETUID) - if (state->username && state->uid >= 0) + if (state->username && state->user_valid) { if (setuid(state->uid)) { @@ -125,12 +123,10 @@ bool platform_group_get(const char *groupname, struct platform_state_group *state) { - bool ret = false; CLEAR(*state); if (groupname) { #if defined(HAVE_GETGRNAM) && defined(HAVE_SETGID) - state->gid = -1; const struct group *gr = getgrnam(groupname); if (!gr) { @@ -139,23 +135,23 @@ else { state->gid = gr->gr_gid; + state->group_valid = true; } state->groupname = groupname; - ret = true; #else /* if defined(HAVE_GETGRNAM) && defined(HAVE_SETGID) */ msg(M_FATAL, "cannot get GID for group %s -- platform lacks getgrnam() or setgid() system calls", groupname); #endif } - return ret; + return state->group_valid; } static void platform_group_set(const struct platform_state_group *state) { #if defined(HAVE_GETGRNAM) && defined(HAVE_SETGID) - if (state->groupname && state->gid >= 0) + if (state->groupname && state->group_valid) { if (setgid(state->gid)) { @@ -237,13 +233,13 @@ * 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->gid >= 0) + if (group_state->groupname && group_state->group_valid) { - new_gid = group_state->gid; + new_gid = (int)group_state->gid; } - if (user_state->username && user_state->uid >= 0) + if (user_state->username && user_state->user_valid) { - new_uid = user_state->uid; + new_uid = (int)user_state->uid; } /* Prepare capabilities before dropping UID/GID */ diff --git a/src/openvpn/platform.h b/src/openvpn/platform.h index f1a2b01..0cb25f5 100644 --- a/src/openvpn/platform.h +++ b/src/openvpn/platform.h @@ -64,9 +64,8 @@ #if defined(HAVE_GETPWNAM) && defined(HAVE_SETUID) const char *username; uid_t uid; -#else - int dummy; #endif + bool user_valid; }; /* Get/Set GID of process */ @@ -76,9 +75,8 @@ #if defined(HAVE_GETGRNAM) && defined(HAVE_SETGID) const char *groupname; gid_t gid; -#else - int dummy; #endif + bool group_valid; }; bool platform_user_get(const char *username, struct platform_state_user *state); @@ -89,28 +87,6 @@ const struct platform_state_group *group_state, struct context *c); -/* - * Extract UID or GID - */ - -static inline int -platform_state_user_uid(const struct platform_state_user *s) -{ -#if defined(HAVE_GETPWNAM) && defined(HAVE_SETUID) - return s->uid; -#endif - return -1; -} - -static inline int -platform_state_group_gid(const struct platform_state_group *s) -{ -#if defined(HAVE_GETGRNAM) && defined(HAVE_SETGID) - return s->gid; -#endif - return -1; -} - void platform_chroot(const char *path); void platform_nice(int niceval); diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index 5fcf820..f71d891 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -3082,7 +3082,7 @@ } bool -unix_socket_get_peer_uid_gid(const socket_descriptor_t sd, int *uid, int *gid) +unix_socket_get_peer_uid_gid(const socket_descriptor_t sd, uid_t *uid, gid_t *gid) { #ifdef HAVE_GETPEEREID uid_t u; diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h index e45981f..2c79a11 100644 --- a/src/openvpn/socket.h +++ b/src/openvpn/socket.h @@ -371,7 +371,7 @@ void socket_delete_unix(const struct sockaddr_un *local); -bool unix_socket_get_peer_uid_gid(const socket_descriptor_t sd, int *uid, int *gid); +bool unix_socket_get_peer_uid_gid(const socket_descriptor_t sd, uid_t *uid, gid_t *gid); #endif /* if UNIX_SOCK_SUPPORT */