From patchwork Thu May 16 11:58:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Gert Doering X-Patchwork-Id: 3713 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:1443:b0:57d:b2cb:6cf with SMTP id q3csp1733907man; Thu, 16 May 2024 05:05:20 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCUk6HMPbCPqh6XkKWRFAzkxM8jz6ThPkXp2FmCxFgvE6usfV1Q1YNCpRlMGg+8ozcLpaHsGFdmwtrSnXApgqzYFRd0nLYA= X-Google-Smtp-Source: AGHT+IGjcdBwbMj3Va5uPtjrVaM6/+LLEhrSeQYOPq1GU3sfiIO9rYhlRingjNz8efVIFphinwEm X-Received: by 2002:a17:903:2286:b0:1eb:5c40:55d7 with SMTP id d9443c01a7336-1ef433d32a4mr216256925ad.0.1715861120105; Thu, 16 May 2024 05:05:20 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1715861120; cv=none; d=google.com; s=arc-20160816; b=fuTR+5oedJSPT+3pRUu5IO++J0CFLVa1Ds4w7QG9DY0yQjpl7xHmJBPI5d0WpXHe0z O4n9t2WhO6Ux8gAP1Yq4/5qflBqWuNcTn3SUA/mLL24Nb/vGLVFTIW4Y6RuSTk+dPhOI lFWPM+CS4Fv37YcIfywgJDdVIawCC01EsK65KfttzanRcYvFN4ltWF/yBn28aGyGNqGD rR9mhriSbhTPcrTG0MnURT2turfjuSEPelSaFt8FBwZz7fbdKqQM25EYOPZgGDImj3Mc OPGpAUcRlyJa9sqA97rDWuVY7Z735CKXFN6NrGCRzWPtS+lK2sWModWUDrY5ckbz0ny7 eYDQ== 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:references:in-reply-to:message-id:date:to:from :dkim-signature:dkim-signature; bh=N+hi1l93xJQ+wL8plZl/Z2weM6NQeXM3pOaTpNz/4Ag=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=pNsXLl8xCzYsUGtQ/BUPrrFX/Q+F3ct2mucTMYoFIwb161YWRMX1+nTOJXxFjy4qes QzmWSLGDEkdIDiTeMZEr8PA8NSneH0gX60vfRKeRKA7IAIO++qWcCKqkPVb3Ip9YTGBy jJzOcA4UpNkW8nvLIBM5NOsFXVeTs5tt7NwlmAfxpVHpD2OLNS37D1fNpps43vOvSXPw DRw8zYTLIzUU2OoDD0l1y6CayO0+1MESvKt/KJC6Kg3wB2d2pCwtBLn3UM/nrycTFLQ9 kQNlgf75p+tSMVCy+mCbGJQ1eJRwDbj9E/5JXwSo6hq2JeYxVzrXmIZBeYz0wpnJ+Hnl mwtQ==; 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=WN1GKMP0; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=InPeTDxr; 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=muc.de Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id d9443c01a7336-1ef0c26a44fsi163100545ad.599.2024.05.16.05.05.19 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 16 May 2024 05:05:20 -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=WN1GKMP0; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=InPeTDxr; 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=muc.de 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 1s7Zqo-00040j-Id; Thu, 16 May 2024 12:04:47 +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 1s7Zqm-00040a-Pz for openvpn-devel@lists.sourceforge.net; Thu, 16 May 2024 12:04:46 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:Content-Type:MIME-Version :References:In-Reply-To:Message-ID:Date:Subject:To:From:Sender:Reply-To:Cc: 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=/OQMJPlOAA98yiUXT1aZLpiQDYoYwSn2Er+V4TSTmds=; b=WN1GKMP0+6Foyqyu9Oo3lc3w1I /L/KplIhHzN4DCsw5NhA2hHWXU3pZvFZKP55IjazaY8Q6K3cBRjbrZpX4RsVRnadbf+0GQw0X2wjS oHvbORCDp3WOj0qkiT1nEFT9M1Nab/4eJBJTfOYsuzgk+5C/xAdem1Gza1XLA6WK59tI=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:Content-Type:MIME-Version:References: In-Reply-To:Message-ID:Date:Subject:To:From:Sender:Reply-To:Cc: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=/OQMJPlOAA98yiUXT1aZLpiQDYoYwSn2Er+V4TSTmds=; b=InPeTDxr0XezzjmWtR3Unjwa3H t+spX5hPlW0FSVpvw3QXDBkvAY7gO5guWZYgiKJFki3VtF9jLZvxNDjJiYc30KIkbhwLWb9HNfsFZ gag59CNRxk0ICM+q9vIX9Hz/o3lquPntortC4LyLiQuryiPaVIJ68sOc+T9FCF3niJpc=; Received: from dhcp-174.greenie.muc.de ([193.149.48.174] helo=blue.greenie.muc.de) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1s7Zqm-0001Nx-6E for openvpn-devel@lists.sourceforge.net; Thu, 16 May 2024 12:04:45 +0000 Received: from blue.greenie.muc.de (localhost [127.0.0.1]) by blue.greenie.muc.de (8.17.1.9/8.17.1.9) with ESMTP id 44GC4aex024339 for ; Thu, 16 May 2024 14:04:36 +0200 Received: (from gert@localhost) by blue.greenie.muc.de (8.17.1.9/8.17.1.9/Submit) id 44GC4aF8024338 for openvpn-devel@lists.sourceforge.net; Thu, 16 May 2024 14:04:36 +0200 From: Gert Doering To: openvpn-devel@lists.sourceforge.net Date: Thu, 16 May 2024 13:58:08 +0200 Message-ID: <20240516120434.23499-1-gert@greenie.muc.de> X-Mailer: git-send-email 2.43.2 In-Reply-To: References: MIME-Version: 1.0 X-Spam-Score: 0.0 (/) X-Spam-Report: =?unknown-8bit?q?Spam_detection_software=2C_running_on_the_sy?= =?unknown-8bit?q?stem_=22util-spamd-2=2Ev13=2Elw=2Esourceforge=2Ecom=22=2C?= =?unknown-8bit?q?_has_NOT_identified_this_incoming_email_as_spam=2E__The_ori?= =?unknown-8bit?q?ginal?= =?unknown-8bit?q?_message_has_been_attached_to_this_so_you_can_view_it_or_la?= =?unknown-8bit?q?bel?= =?unknown-8bit?q?_similar_future_email=2E__If_you_have_any_questions=2C_see?= =?unknown-8bit?q?_the_administrator_of_that_system_for_details=2E?= =?unknown-8bit?q?_?= =?unknown-8bit?q?_Content_preview=3A__From=3A_Reynir_Bj=C3=B6rnsson_=3Creyni?= =?unknown-8bit?q?r=40reynir=2Edk=3E_If_an_exit_has?= =?unknown-8bit?q?_already_been_scheduled_we_should_not_schedule_it_again=2E_?= =?unknown-8bit?q?Otherwise=2C_the_exit?= =?unknown-8bit?q?_signal_is_never_emitted_if_the_peer_reschedules_the_exit_b?= =?unknown-8bit?q?efore_the_timeout?= =?unknown-8bit?q?_occurs=2E_?= =?unknown-8bit?q?_?= =?unknown-8bit?q?_Content_analysis_details=3A___=280=2E0_points=2C_6=2E0_req?= =?unknown-8bit?q?uired=29?= =?unknown-8bit?q?_?= =?unknown-8bit?q?_pts_rule_name______________description?= =?unknown-8bit?q?_----_----------------------_------------------------------?= =?unknown-8bit?q?--------------------?= =?unknown-8bit?q?_0=2E0_RCVD=5FIN=5FDNSWL=5FBLOCKED__RBL=3A_ADMINISTRATOR_NO?= =?unknown-8bit?q?TICE=3A_The_query_to?= =?unknown-8bit?q?_DNSWL_was_blocked=2E__See?= =?unknown-8bit?q?_http=3A//wiki=2Eapache=2Eorg/spamassassin/DnsBlocklists=23?= =?unknown-8bit?q?dnsbl-block?= =?unknown-8bit?q?_for_more_information=2E?= =?unknown-8bit?q?_=5B193=2E149=2E48=2E174_listed_in_list=2Ednswl=2Eorg=5D?= =?unknown-8bit?q?_0=2E0_URIBL=5FBLOCKED__________ADMINISTRATOR_NOTICE=3A_The?= =?unknown-8bit?q?_query_to_URIBL_was?= =?unknown-8bit?q?_blocked=2E__See?= =?unknown-8bit?q?_http=3A//wiki=2Eapache=2Eorg/spamassassin/DnsBlocklists=23?= =?unknown-8bit?q?dnsbl-block?= =?unknown-8bit?q?_for_more_information=2E?= =?unknown-8bit?q?_=5BURIs=3A_openvpn=2Enet=5D?= =?unknown-8bit?q?_-0=2E0_SPF=5FPASS_______________SPF=3A_sender_matches_SPF_?= =?unknown-8bit?q?record?= =?unknown-8bit?q?_-0=2E0_SPF=5FHELO=5FPASS__________SPF=3A_HELO_matches_SPF_?= =?unknown-8bit?q?record?= X-Headers-End: 1s7Zqm-0001Nx-6E Subject: [Openvpn-devel] [PATCH v3] Only schedule_exit() once 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?1797322443577712396?= X-GMAIL-MSGID: =?utf-8?q?1799210789833121052?= From: Reynir Björnsson If an exit has already been scheduled we should not schedule it again. Otherwise, the exit signal is never emitted if the peer reschedules the exit before the timeout occurs. 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. This patch was assigned a CVE number after already reviewed and ACKed, because it was discovered that a misbehaving client can use the (now fixed) server behaviour to avoid being disconnected by means of a managment interface "client-kill" command - the security issue here is "client can circumvent security policy set by management interface". This only affects previously authenticated clients, and only management client-kill, so normal renegotion / AUTH_FAIL ("your session ends") is not affected. CVE: 2024-28882 Change-Id: I9457f005f4ba970502e6b667d9dc4299a588d661 Signed-off-by: Reynir Björnsson Acked-by: Arne Schwabe --- 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/+/555 This mail reflects revision 3 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 8d10f25..01165b2 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -514,17 +514,24 @@ } /* - * 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) { + const int n_seconds = c->options.scheduled_exit_interval; + /* don't reschedule if already scheduled. */ + if (event_timeout_defined(&c->c2.scheduled_exit)) + { + 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..17e7e80 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -204,7 +204,11 @@ * */ if (c->options.mode == MODE_SERVER) { - schedule_exit(c, c->options.scheduled_exit_interval, SIGTERM); + if (!schedule_exit(c)) + { + /* Return early when we don't need to notify management */ + return; + } } else { @@ -391,7 +395,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 +405,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 +494,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); }