From patchwork Thu Jan 9 14:42:44 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "plaisthos (Code Review)" X-Patchwork-Id: 4040 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:1539:b0:5e7:b9eb:58e8 with SMTP id a25csp1379398mar; Thu, 9 Jan 2025 06:43:10 -0800 (PST) X-Forwarded-Encrypted: i=2; AJvYcCWqtwwlO+N2Te+c+bxj7mLRfyZyHfsdVuV25fPn5A8FigOsHRfsP+y8HR8e1lF63udwWElQCm/MZ1M=@openvpn.net X-Google-Smtp-Source: AGHT+IEZuiJLrFEGV8+/wbIWXMvMhYDNTHZxT04GpUGR4hpMxKoFKpBzH1VtjCDPDVv+sm7DlAEN X-Received: by 2002:a05:6830:3509:b0:71d:6221:d4b7 with SMTP id 46e09a7af769-721e2ef58a3mr4342083a34.28.1736433790065; Thu, 09 Jan 2025 06:43:10 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1736433790; cv=none; d=google.com; s=arc-20240605; b=gNClDdy2k3Z7wPq+14oFQNnYVs5YAeee7F8TmjFwERf7/C3ksOu+42xJc4HSku5iT3 GpLYjpKscWoi3ZAJl+KIONhHtfLCuDD917xuV3ycih7c+EvtiB8iHaOjanOG6tGemCPk PlsgOUThNDfOG6LlujYrg/9aQ8OjDBhDE7gzWtKweicvKPfhR3p8H88/2J5pC4s4cuaR hhaDXu1YwQ9OFOaImEjoy8Mt3d4Nb+hfjJulYHGTLW0Td6z0c+YZPhZiMu3YlTlAgB/3 27WMoQ+omzdzYGwokvgbkIQAMiKgWuiguvJk1XEpgxUmIdzFVnfN4Z/n24a7REkHr2Ei O1vg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=errors-to:cc:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:user-agent :mime-version:message-id:references:auto-submitted:to:date:from :dkim-signature:dkim-signature:dkim-signature; bh=iihpinF9qwCoyr1XR5SwUkwsm5E6i1qYNtE+jxBXnFY=; fh=lm0MLPW7DntlrDqRECIiC9JlE1uPxhepE0URYHIf+eE=; b=AxAzff0jXgnCesDfcSPxbvu4DVoGUcJan8z1sf4KCcGZAMq9KMnE2TlOao+3DZHjoR QFX4fBmcrUEeWvnCR7ovmLJs/kO5RtZ7mi6gWOryqYUv/oHM+9T2QhfAfCm4dEfvrhAh lqx9AXC/qN3IaHJBkOZVy977ZCrUHLRYWwhLZB91v6jD47RixrHe5lryG6kr82kWTmuH LlJ9c6O1tKVKEZYQ4eOy10YjiMLzBzX+ZGY3lGcqIrxmlaHSk3YQkOgZdsRjsd8Zyy1q 9xr8HV/U4J1+2sgQ41tGFdjYwYpoo5d8+JwKJKYiziKs51+JuL6lpEyGCNed4uIdKe4T GC2Q==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=FfLIDwHV; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=UTMpZ2qF; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=KgW2iVRP; 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=NONE dis=NONE) header.from=openvpn.net; dara=fail header.i=@openvpn.net Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id 46e09a7af769-7231865df2esi945458a34.249.2025.01.09.06.43.09 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 09 Jan 2025 06:43:10 -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=FfLIDwHV; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=UTMpZ2qF; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=KgW2iVRP; 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=NONE dis=NONE) header.from=openvpn.net; dara=fail header.i=@openvpn.net Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1tVtkS-0005F9-7i; Thu, 09 Jan 2025 14:43:00 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1tVtkQ-0005F2-R4 for openvpn-devel@lists.sourceforge.net; Thu, 09 Jan 2025 14:42:58 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Type:Content-Transfer-Encoding:MIME-Version :Message-ID:Reply-To:References:Subject:List-Unsubscribe:List-Id:Cc:To:Date: From:Sender:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help: List-Subscribe:List-Post:List-Owner:List-Archive; bh=+bzxdf+eSv9I2Kd3W5Pynkf1PIHyY/1i8aa+Kv+E3E4=; b=FfLIDwHV6xy4TRMYRKcyluHefw dAbFTDSrEX6wwBwnwMrBCPhLQ0gdLto6OMPESSpOnpCDCHE+GVcyBofsEfCHvhx7qCf5o6VuNqHA+ M6a4/VQ73aLZXuvBGq/nNz3dv8QBGRW4K5L8+LYSlng5hDMifVhFj20mXIxFYiy8q3is=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Type:Content-Transfer-Encoding:MIME-Version:Message-ID:Reply-To: References:Subject:List-Unsubscribe:List-Id:Cc:To:Date:From:Sender:Content-ID :Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To: Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help:List-Subscribe:List-Post: List-Owner:List-Archive; bh=+bzxdf+eSv9I2Kd3W5Pynkf1PIHyY/1i8aa+Kv+E3E4=; b=U TMpZ2qFa2P6/gtQZvCdTIgSMeAS39PNSRhSLEyo8Ay+3qALlB8enZmq0wa8aeVu7AOBZ72F9eu0BP ukwFlpdH5ypAQ3aC569NKY1aAzTOuHjFHeLmQhhqG6P/h3DA5xZ6H7ePyagZgaTEKlRsHJMZeo+Bf z5xROulPTPmq8uGc=; Received: from mail-wr1-f49.google.com ([209.85.221.49]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1tVtkP-0000RX-GV for openvpn-devel@lists.sourceforge.net; Thu, 09 Jan 2025 14:42:58 +0000 Received: by mail-wr1-f49.google.com with SMTP id ffacd0b85a97d-38637614567so527511f8f.3 for ; Thu, 09 Jan 2025 06:42:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=openvpn.net; s=google; t=1736433766; x=1737038566; darn=lists.sourceforge.net; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from:from:to:cc :subject:date:message-id:reply-to; bh=+bzxdf+eSv9I2Kd3W5Pynkf1PIHyY/1i8aa+Kv+E3E4=; b=KgW2iVRPyfnW0e1H/vZC5DNxOVKP1mCH3Vn2rVS43QgkQKmk53TFHlK6Y2cHLsSXKu pQ1wIQMcsMlvt3ZEmv9+NoRwR4rx/SE6uikCdhrRA76LZBYlULbb5TITOLHCMoQ6xj0u VtDvOgZXLfHwmkYYfhbVVx/URlao/pjLii7aeEt+lNlElHGjsQlkx96m2rEjVgH4NxBp EaAMYfUsIaoEQhtD9/h5G55zZS6JSk65h+7clZVpNQZt+7onqlDymqFKDXYyfErh4xXi Fs26fv0YTW4QDzvr1yKMedju2/zFLd0Ub7KZKrog1bGH19hWzhGyV5buoGgiQ9xW1MeN iryw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736433766; x=1737038566; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=+bzxdf+eSv9I2Kd3W5Pynkf1PIHyY/1i8aa+Kv+E3E4=; b=iQufek0bZamh16uoMDk2M1+62tJirt2SgshNpU3zvzT5wRyZ8Ro+Jml9SoEUBqrzjP Jnh210Z5rCWO6uSjAkt+xJSR18Nu6kDRKVYeKEH1Ma3LFOkHQGaHSNwuwC/JcRdDF/uk GXAb3KpKQC+vrcUsywq2lz82urik6ON558yVaI4nXG9pIfB9INTNxsvhJutXVl2ybzIc HFTcp/EiDtJ4cMSaMjBzyxWzjeF1X7ApqTm6LN/Yj7VtMGX3CFyb9TYuyDHWFeXMQvBb qFOQVeWKG1vAkhCGwzhMQ5FZtbsXCRpXBGSsVN6pYpleiGxVUOvelA5FeWPJDCd3gwKg E8Xw== X-Gm-Message-State: AOJu0YxkTJYx5u4fRFBTr1aFsyberiUHLo/VwIz787oyPc9boKd3F3gh a3qDXL0Qtybpo9dQKif+jUa/U6sdU6R7SPdWjT14lQiH/sHqPUu6P5bKyMnW4irDAflEiFke4wz N X-Gm-Gg: ASbGncsyKklPbVFog/b6T7hE3XOOa4va4TZdJ17EZDIldD+BSS4gI82cGWA4598cKGt HcuapS2vhAniAq7CrsDqVADEYwgJalf8n+6YfhPGU2uCkTkbODPeSdhdk10uXp2Nc9Izr29CQF3 5Eis60/s9XPwMV5N03Hi47BiczPZ0+5DP3fBFMeABf4Aycu8b146k796xrKPzoEi8jitvO9UQZp CCO3l0TUVPFWpWhoMzOrsafw7xsRQwQlE6tqiQFF8I+MD515rsprlp++kR+yiDPxPXOcRbVeJZo 6LAkavQoKfSy9KYjA+vBbrCPsy1gs0B4nnqQTFv5x2Nns4ek X-Received: by 2002:a5d:6d01:0:b0:38a:86fe:52dc with SMTP id ffacd0b85a97d-38a873036a8mr5920941f8f.13.1736433765589; Thu, 09 Jan 2025 06:42:45 -0800 (PST) Received: from gerrit.openvpn.in (ec2-18-159-0-78.eu-central-1.compute.amazonaws.com. [18.159.0.78]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38a8e37d154sm2068190f8f.10.2025.01.09.06.42.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Jan 2025 06:42:44 -0800 (PST) From: "plaisthos (Code Review)" X-Google-Original-From: "plaisthos (Code Review)" X-Gerrit-PatchSet: 1 Date: Thu, 9 Jan 2025 14:42:44 +0000 To: flichtenheld Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange X-Gerrit-Change-Id: I81cbecd19018290d85c6c77fba7769f040d66233 X-Gerrit-Change-Number: 855 X-Gerrit-Project: openvpn X-Gerrit-ChangeURL: X-Gerrit-Commit: 0b843016e8963db78a88e1283ae37bb25a43fa91 References: Message-ID: <857ead877a63a88afb17be062d04275cc49d1a51-HTML@gerrit.openvpn.net> MIME-Version: 1.0 User-Agent: Gerrit/3.8.2 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: Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit Content analysis details: (-0.2 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [209.85.221.49 listed in list.dnswl.org] 0.0 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. [209.85.221.49 listed in sa-accredit.habeas.com] 0.0 RCVD_IN_VALIDITY_RPBL_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. [209.85.221.49 listed in bl.score.senderscore.com] -0.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.221.49 listed in wl.mailspike.net] 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.0 SPF_PASS SPF: sender matches SPF record 0.0 WEIRD_PORT URI: Uses non-standard port number for HTTP 0.0 HTML_MESSAGE BODY: HTML included in message -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 -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.0 T_KAM_HTML_FONT_INVALID Test for Invalidly Named or Formatted Colors in HTML X-Headers-End: 1tVtkP-0000RX-GV Subject: [Openvpn-devel] [M] Change in openvpn[master]: Improve error reporting from AF_UNIX tun/tap support 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: , Reply-To: arne-openvpn@rfc2549.org, openvpn-devel@lists.sourceforge.net, frank@lichtenheld.com Cc: openvpn-devel Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1820782798174259054?= X-GMAIL-MSGID: =?utf-8?q?1820782798174259054?= X-getmail-filter-classifier: gerrit message type newchange Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/855?usp=email to review the following change. Change subject: Improve error reporting from AF_UNIX tun/tap support ...................................................................... Improve error reporting from AF_UNIX tun/tap support When having a non-existent lwipovpn binary or similar problems, the error reporting would often only report read error that were harder to identify the real problem. Add the openvpn_waitpid_check method that checks for error conditions and reports a better message in cases of problems. Change-Id: I81cbecd19018290d85c6c77fba7769f040d66233 Signed-off-by: Arne Schwabe --- M src/openvpn/run_command.c M src/openvpn/run_command.h M src/openvpn/tun_afunix.c 3 files changed, 62 insertions(+), 8 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/55/855/1 diff --git a/src/openvpn/run_command.c b/src/openvpn/run_command.c index d757823..7f2cfb3 100644 --- a/src/openvpn/run_command.c +++ b/src/openvpn/run_command.c @@ -107,6 +107,35 @@ } bool +openvpn_waitpid_check(pid_t pid, const char *error_message, int msglevel) +{ + if (pid == 0) + { + return false; + } + int status; + pid_t pidret = waitpid(pid, &status, WNOHANG); + if (pidret != pid) + { + return true; + } + + if (WIFEXITED(status)) + { + msg(msglevel, "%sexternal program exited with error status: %d", + error_message, WEXITSTATUS(status)); + + } + else if (WIFSIGNALED(status)) + { + msg(msglevel, "%sexternal program received signal %d", + error_message, WTERMSIG(status)); + } + + return false; +} + +bool openvpn_execve_allowed(const unsigned int flags) { if (flags & S_SCRIPT) diff --git a/src/openvpn/run_command.h b/src/openvpn/run_command.h index c92edbc..366acaa 100644 --- a/src/openvpn/run_command.h +++ b/src/openvpn/run_command.h @@ -59,6 +59,19 @@ int openvpn_execve_check(const struct argv *a, const struct env_set *es, const unsigned int flags, const char *error_message); + +/** Checks if a running process is still running. This is mainly useful + * for processes started with \c S_NOWAITPID + * @param pid pid of the process to be checked + * @param error_message prefixed of the message that be printed + * @param msglevel msglevel of the messages to be printed + * @return true if the process is still running, false if + * an error condition occurred + */ +bool +openvpn_waitpid_check(pid_t pid, const char *error_message, + int msglevel); + /** * Will run a script and return the exit code of the script if between * 0 and 255, -1 otherwise diff --git a/src/openvpn/tun_afunix.c b/src/openvpn/tun_afunix.c index 6b6c159..c626993 100644 --- a/src/openvpn/tun_afunix.c +++ b/src/openvpn/tun_afunix.c @@ -47,9 +47,12 @@ #include #include + + static void tun_afunix_exec_child(const char *dev_node, struct tuntap *tt, struct env_set *env) { + const char *msgprefix = "ERROR: failure executing process for tun:"; struct argv argv = argv_new(); /* since we know that dev-node starts with unix: we can just skip that @@ -58,10 +61,12 @@ argv_printf(&argv, "%s", program); - argv_msg(M_INFO, &argv); tt->afunix.childprocess = openvpn_execve_check(&argv, env, S_NOWAITPID, - "ERROR: failure executing " - "process for tun"); + msgprefix); + if (!openvpn_waitpid_check(tt->afunix.childprocess, msgprefix, M_WARN)) + { + tt->afunix.childprocess = 0; + } argv_free(&argv); } @@ -138,20 +143,27 @@ ssize_t write_tun_afunix(struct tuntap *tt, uint8_t *buf, int len) { - int ret; - pid_t pidret = waitpid(tt->afunix.childprocess, &ret, WNOHANG); - if (pidret == tt->afunix.childprocess) + const char *msg = "ERROR: failure during write to AF_UNIX socket: "; + if (!openvpn_waitpid_check(tt->afunix.childprocess, msg, M_WARN)) { - msg(M_INFO, "Child process PID %d for afunix dead? Return code: %d", - tt->afunix.childprocess, ret); + tt->afunix.childprocess = 0; return -ENXIO; } + return write(tt->fd, buf, len); } ssize_t read_tun_afunix(struct tuntap *tt, uint8_t *buf, int len) { + const char *msg = "ERROR: failure during read from AF_UNIX socket: "; + if (!openvpn_waitpid_check(tt->afunix.childprocess, msg, M_WARN)) + { + tt->afunix.childprocess = 0; + } + /* do an actual read on the file descriptor even in the error case since + * we otherwise loop on this on this from select and spam the console + * with error messages */ return read(tt->fd, buf, len); } #else /* ifndef WIN32 */