From patchwork Mon Apr 22 19:08:25 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "plaisthos (Code Review)" X-Patchwork-Id: 3688 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:6bca:b0:576:af48:25c3 with SMTP id c10csp170425max; Mon, 22 Apr 2024 12:09:04 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCUBx8ilhMX7nbkCS1NlvgH3Fo1HQx51FLYaSyGqOPrhyBDagp8cPYa2TLKmDdovAHxoNatUMo1iCaWxB/kqtc8+xUbg4Vw= X-Google-Smtp-Source: AGHT+IEl7vkYSAMwsChUgdvquhGjqLEyoh4Uaaukddyd8E4CR2Xg7TxsRoyx9rjj9YMsfBv+NT3X X-Received: by 2002:a17:90b:151:b0:2ac:4da9:8f76 with SMTP id em17-20020a17090b015100b002ac4da98f76mr10662121pjb.3.1713812943825; Mon, 22 Apr 2024 12:09:03 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1713812943; cv=none; d=google.com; s=arc-20160816; b=WmXafM2GdJ85/5bPEuBlR+e5DttX6f+4bpLctZMVfa75AqLd7Yh1/4YiObvnbOlO/n vvFYWuiHazeI+wZhVQwAQMs3XS+r9OpU6ZOa7fI6++xquyEZ32yh0W2RcVgmNx8CFqje aSyXQtU4beSHwkpkHYjlgGMtJvXzG5U5sPYrogHwnDoCNMqCe1Fl8A0H2UJWGyjlMlDC 7Yh2FegwzTMRQUtNX2n7Vh+rRKGMDeO8v+WZPuchTEYJwJ9DlLWoRdfd1fi/9CRlF1qQ Oj6fUur2Y+XkuUjBeTybXFIcjVVhp6zvFzKd2xgzF60YDzkDTE30G9quekjlhzzcZDw3 h0mQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; 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=T75a7JbCpTsBHU0XRWZIAiBFrYETNtMR4XgjQSuocSQ=; fh=U7wEyxtwz2o5+UdevFSA47vNeG9knhWH0KV//QhD5a0=; b=NzA5zzBIb8LWH1jRGK5OVZZwKUMIF3cKb+rpXaqPfmOvslDNEnryTRAwEoPagbRNdF Dr2mzmoq3rq2CtL38zuPl6WDZZfLN/vbJy//W93SqFF1Zfbp5ToSlBkC31ySijODUSIA c4hd257NyhR/iJfm7sIt1MjFvX+VVipqbd8ml9Dl+gJ2m2wV6+T84ttI8iESlr5aWfhk 6/piR/AmGwBPyKdbyKviugBwG7Vko9li7yovxzxMV301Jrtfae4LLwGY6ZvI+CD4QmKQ 1xvM5l8viYZWQ/aZFV1jb4MwzkKif/04HzW3n4og5kcXkwosjI3GTNzMs9Hcz7fWev0u qq8g==; 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=UkRlZTMa; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=iCmFvbzX; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=JOkH6U15; 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 Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id ls15-20020a17090b350f00b002a571d72e62si9998650pjb.70.2024.04.22.12.09.03 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 22 Apr 2024 12:09:03 -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=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=UkRlZTMa; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=iCmFvbzX; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=JOkH6U15; 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 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 1ryz1s-0004Lv-OT; Mon, 22 Apr 2024 19:08:40 +0000 Received: from [172.30.20.202] (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 1ryz1r-0004Lp-BJ for openvpn-devel@lists.sourceforge.net; Mon, 22 Apr 2024 19:08:39 +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=keJ1MpLlf4HGuU5rMKz0j0UQkaQ7e3ItBqzbvl+aScY=; b=UkRlZTMaWhVogcBGytBbDdXx5E Qz65y2NXpLpB5AbWVKBTnIqCYei3YyCnRHTZw7McTGe8nSs/TuvcKblgFVhLFbZhdwVQp34VlqQSE DpSqF8iXrfHVEv/cZiQq68y99LPqUtajXjrFvR0HqFBsRIvKDGvAZtOTdScgFe9784G0=; 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=keJ1MpLlf4HGuU5rMKz0j0UQkaQ7e3ItBqzbvl+aScY=; b=i CmFvbzX7zuv74PJ5wibkNbi/LJnwRFvIIjfmrLPs8IA9PfziKyOcU+b4C3hpoiHjektgpeTz2jUJB XG+Bg/ndlBurw5+6OJCXWEYpsYE3CRMiTt6LLjvhaoU9wyU0zHKZWXor6RROLRXcWoJ1VnL4U2EL0 RPNDTNUIkdP9EOnE=; Received: from mail-wr1-f42.google.com ([209.85.221.42]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1ryz1p-0002ZP-Ii for openvpn-devel@lists.sourceforge.net; Mon, 22 Apr 2024 19:08:39 +0000 Received: by mail-wr1-f42.google.com with SMTP id ffacd0b85a97d-343c7fae6e4so4260884f8f.1 for ; Mon, 22 Apr 2024 12:08:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=openvpn.net; s=google; t=1713812906; x=1714417706; 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=keJ1MpLlf4HGuU5rMKz0j0UQkaQ7e3ItBqzbvl+aScY=; b=JOkH6U15LZOXDXBfGShadnWpmzQ9WUK2MRu5zpa+xetkiqab//89v/bPDSMCY63ugU C5aKQXslCD0Fpk2ltH75KXqGhXyozC9MLnNnh+DoiCWNzhy5eq4RM+FuggeX907oRJ6v giEURMURYBTjrRdI4aQIRh+kaln05lO5h6DK3l/NoZGlc0KQLyXXuv2DKa7fR5YHM62p 6N6GQQW7WVVk9Knoldi/LHtV6DL55fYnzauB0g/CQ9FBWIdEBi5eNTwbk8pujGaP0SDP 2nQOUIUw02U4p37EOKfRvWeW8uOUcMCH1b1lRR38mMl8cwsZ3aTtzmbbv1ZaN6GglRBX 66xA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713812906; x=1714417706; 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=keJ1MpLlf4HGuU5rMKz0j0UQkaQ7e3ItBqzbvl+aScY=; b=e6sSBHQAZLhiEErNPklwqYOaRERY6EMqTlhNALFa/XS17bZNu2V3OwsGkoAeMm6rQP pIsf6LvTjKjiOzITPX3Ui/WZeqItukWxVCDLJO4zSoRinVw6DI1w5bEaieDmMdAK2LPv wvc7qiOlkcqF6kPdQWjSb+ZDBUIUy8hDbrJJ+FwH6IctcdbG2JpUw4GofopDD7Lm6jt9 cG1RAI0Wkw5OFfSboD+qGC2HRHR8c5Z51oyjt2A88ihA8fIJF0GLc0zXINzcO8RDanT6 GHf9wAA5X0JK+nmT7UrtedNpWI7TQf9QTB70rr6je8ZoaEliRHmLrvpdbaWfn4v6GF5F QMrg== X-Gm-Message-State: AOJu0YxepTVCVnlX2jlSTNOd9sC9qM6P8ZtytJcMPy+itgCYFi3gC4mF mtwop6XzbDL4MEk+sP/z1j3ew8Sq4KmKrjQP9VqmIvOacGbSAyr5UKa49RIP7OKxjhN1nq5/Wuf H X-Received: by 2002:a5d:5090:0:b0:349:8fa4:1839 with SMTP id a16-20020a5d5090000000b003498fa41839mr6881244wrt.1.1713812906224; Mon, 22 Apr 2024 12:08:26 -0700 (PDT) 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 l18-20020a5d5612000000b0034a62e51429sm9612742wrv.112.2024.04.22.12.08.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 22 Apr 2024 12:08:25 -0700 (PDT) From: "reynir (Code Review)" X-Google-Original-From: "reynir (Code Review)" X-Gerrit-PatchSet: 1 Date: Mon, 22 Apr 2024 19:08:25 +0000 To: plaisthos , flichtenheld Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange X-Gerrit-Change-Id: I1cf589ecad82eecf167fe5cf28cda0fe4d42d42f X-Gerrit-Change-Number: 557 X-Gerrit-Project: openvpn X-Gerrit-ChangeURL: X-Gerrit-Commit: 09990e301902f92d2a18e3256f31f64f51f99987 References: Message-ID: <06e6119813c956c51c500ea004756c7fe47ebda4-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-2.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, plaisthos. Hello plaisthos, 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_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to DNSWL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [209.85.221.42 listed in list.dnswl.org] 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: openvpn.net] -0.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.221.42 listed in wl.mailspike.net] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an 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_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.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain 0.0 T_KAM_HTML_FONT_INVALID Test for Invalidly Named or Formatted Colors in HTML X-Headers-End: 1ryz1p-0002ZP-Ii Subject: [Openvpn-devel] [S] Change in openvpn[master]: Address schedule_exit() review comments 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: reynir@reynir.dk, 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?1797063120935335485?= X-GMAIL-MSGID: =?utf-8?q?1797063120935335485?= X-getmail-filter-classifier: gerrit message type newchange Attention is currently required from: flichtenheld, plaisthos. Hello plaisthos, flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/557?usp=email to review the following change. Change subject: Address schedule_exit() review comments ...................................................................... Address schedule_exit() review comments schedule_exit() now only takes the context as argument. The signal is hard coded to SIGTERM, and the interval is read directly from the context options. Furthermore, schedule_exit() now returns a bool signifying whether an exit was scheduled; false if exit is already scheduled. The call sites are updated accordingly. A notable difference is that management is only notified *once* when an exit is scheduled - we no longer notify management on redundant exit. Change-Id: I1cf589ecad82eecf167fe5cf28cda0fe4d42d42f --- M src/openvpn/forward.c M src/openvpn/forward.h M src/openvpn/push.c 3 files changed, 14 insertions(+), 14 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/57/557/1 diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 70f8e9d..937fae4 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -514,22 +514,23 @@ } /* - * Schedule a signal n_seconds from now. + * Schedule a SIGTERM signal c->options.scheduled_exit_interval seconds from now. */ -void -schedule_exit(struct context *c, const int n_seconds, const int signal) +bool +schedule_exit(struct context *c) { - /* don't reschedule if already scheduled; we drop the new signal, but it - * only ever seems to be SIGTERM anyway. */ + const int n_seconds = c->options.scheduled_exit_interval; + /* don't reschedule if already scheduled. */ if (event_timeout_defined(&c->c2.scheduled_exit)) { - return; + return false; } tls_set_single_session(c->c2.tls_multi); update_time(); reset_coarse_timers(c); event_timeout_init(&c->c2.scheduled_exit, n_seconds, now); - c->c2.scheduled_exit_signal = signal; + c->c2.scheduled_exit_signal = SIGTERM; msg(D_SCHED_EXIT, "Delayed exit in %d seconds", n_seconds); + return true; } /* diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h index 6fb5a18..422c591 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -303,7 +303,7 @@ void process_ip_header(struct context *c, unsigned int flags, struct buffer *buf); -void schedule_exit(struct context *c, const int n_seconds, const int signal); +bool schedule_exit(struct context *c); static inline struct link_socket_info * get_link_socket_info(struct context *c) diff --git a/src/openvpn/push.c b/src/openvpn/push.c index 1b406b9..db1fd2e 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -191,6 +191,7 @@ receive_exit_message(struct context *c) { dmsg(D_STREAM_ERRORS, "CC-EEN exit message received by peer"); + bool notify_management = true; /* With control channel exit notification, we want to give the session * enough time to handle retransmits and acknowledgment, so that eventual * retries from the client to resend the exit or ACKs will not trigger @@ -204,14 +205,14 @@ * */ if (c->options.mode == MODE_SERVER) { - schedule_exit(c, c->options.scheduled_exit_interval, SIGTERM); + notify_management = schedule_exit(c); } else { register_signal(c->sig, SIGUSR1, "remote-exit"); } #ifdef ENABLE_MANAGEMENT - if (management) + if (management && notify_management) { management_notify(management, "info", "remote-exit", "EXIT"); } @@ -391,7 +392,7 @@ void send_auth_failed(struct context *c, const char *client_reason) { - if (event_timeout_defined(&c->c2.scheduled_exit)) + if (!schedule_exit(c)) { msg(D_TLS_DEBUG, "exit already scheduled for context"); return; @@ -401,8 +402,6 @@ static const char auth_failed[] = "AUTH_FAILED"; size_t len; - schedule_exit(c, c->options.scheduled_exit_interval, SIGTERM); - len = (client_reason ? strlen(client_reason)+1 : 0) + sizeof(auth_failed); if (len > PUSH_BUNDLE_SIZE) { @@ -492,7 +491,7 @@ void send_restart(struct context *c, const char *kill_msg) { - schedule_exit(c, c->options.scheduled_exit_interval, SIGTERM); + schedule_exit(c); send_control_channel_string(c, kill_msg ? kill_msg : "RESTART", D_PUSH); }