From patchwork Thu Jun 10 05:30:11 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1858 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director9.mail.ord1d.rsapps.net ([172.28.255.1]) by backend30.mail.ord1d.rsapps.net with LMTP id IONxK1QwwmD4OwAAIUCqbw (envelope-from ) for ; Thu, 10 Jun 2021 11:31:32 -0400 Received: from proxy4.mail.ord1c.rsapps.net ([172.28.255.1]) by director9.mail.ord1d.rsapps.net with LMTP id SPR0K1QwwmBTJwAAalYnBA (envelope-from ) for ; Thu, 10 Jun 2021 11:31:32 -0400 Received: from smtp19.gate.ord1c ([172.28.255.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy4.mail.ord1c.rsapps.net with LMTPS id eCYYK1QwwmBocQAAjcXvpA (envelope-from ) for ; Thu, 10 Jun 2021 11:31:32 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp19.gate.ord1c.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=rfc2549.org X-Suspicious-Flag: YES X-Classification-ID: ee351244-ca00-11eb-8b7f-bc305bf036e4-1-1 Received: from [216.105.38.7] ([216.105.38.7:48100] helo=lists.sourceforge.net) by smtp19.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id A2/53-29981-35032C06; Thu, 10 Jun 2021 11:31:31 -0400 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.90_1) (envelope-from ) id 1lrMdZ-0003Rn-Sh; Thu, 10 Jun 2021 15:30:29 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lrMdY-0003RY-6O for openvpn-devel@lists.sourceforge.net; Thu, 10 Jun 2021 15:30:28 +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:To:From:Sender:Reply-To:Cc: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=tQaoxE4681cHuD5up4bcdcBQHcVMbinChc7RQ1xF1+U=; b=lSVnMNDLVjp+K2MIM6aa83WsAJ oWoU2Q7F7yMxyUG/gBdnvlRDZDYIeeBNsoZFBcbaPMVAsXI0nN3HtnuNTw2x37D2pSZ0quQg3uwYK UeyhVgz7hTtqQA2HRKEmiu6Uod9gU/p1VPszBZEG+Xgz74j2PhA7Ufs/TKk8DmtafFP8=; 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:To:From: Sender:Reply-To:Cc: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=tQaoxE4681cHuD5up4bcdcBQHcVMbinChc7RQ1xF1+U=; b=F 7+RtE5z04XEKWK/vWEuSxqgRpN5rKYZ8g2wUTrG5nCqcjy9/hNUTS73FUZ29pUt8wiVZwwwhi93Uj Qpykmql7aJjNg+pZcFay7xzszFHyBzY7H+JsqNb+KQ0AYuEaTZk1WJ9rMgTI7J80umFHImIMGzti8 hZXB2zQ/nfXU8Tzw=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.3) id 1lrMdR-0003Qu-NS for openvpn-devel@lists.sourceforge.net; Thu, 10 Jun 2021 15:30:30 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.94.2 (FreeBSD)) (envelope-from ) id 1lrMdH-000EPb-AU for openvpn-devel@lists.sourceforge.net; Thu, 10 Jun 2021 17:30:11 +0200 Received: (nullmailer pid 1730718 invoked by uid 10006); Thu, 10 Jun 2021 15:30:11 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Thu, 10 Jun 2021 17:30:11 +0200 Message-Id: <20210610153011.1730670-1-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 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: rfc2549.org] 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record X-Headers-End: 1lrMdR-0003Qu-NS Subject: [Openvpn-devel] [PATCH] Avoid resending reset reply more than once per client packet 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 For the second reply of a OpenVPN we have no completed the three way handshake yet and the client IP address is still untrusted. When retransmitting the reset packet multiple times when timing out for an ACK response to it, we send the packet multiple times to an untrusted IP which is nowadys considered bad in a protocol. This patch fixes the problem by keeping normal original retry logic intact but adds a flags to initial packets that they are are held back to be retrasmitted until we have another packet from the client. Signed-off-by: Arne Schwabe Acked-by: Antonio Quartulli --- src/openvpn/reliable.c | 52 +++++++++++++++++++++++++++++++----------- src/openvpn/reliable.h | 36 +++++++++++++++++++++++------ src/openvpn/ssl.c | 16 +++++++++++-- 3 files changed, 82 insertions(+), 22 deletions(-) diff --git a/src/openvpn/reliable.c b/src/openvpn/reliable.c index 15b90fbe4..da660023c 100644 --- a/src/openvpn/reliable.c +++ b/src/openvpn/reliable.c @@ -549,6 +549,13 @@ reliable_get_buf_sequenced(struct reliable *rel) return NULL; } +static inline bool +entry_ready_for_resend(const struct reliable_entry *e) +{ + return ((now >= e->next_try || e->n_acks >= N_ACK_RETRANSMIT) + && (!(e->flags & REL_FLAG_RESEND_INITIAL) || (e->flags & REL_FLAG_RESEND_ALLOW))); +} + /* return true if reliable_send would return a non-NULL result */ bool reliable_can_send(const struct reliable *rel) @@ -562,7 +569,7 @@ reliable_can_send(const struct reliable *rel) if (e->active) { ++n_active; - if (now >= e->next_try || e->n_acks >= N_ACK_RETRANSMIT) + if (entry_ready_for_resend(e)) { ++n_current; } @@ -583,7 +590,6 @@ reliable_send(struct reliable *rel, int *opcode) { int i; struct reliable_entry *best = NULL; - const time_t local_now = now; for (i = 0; i < rel->size; ++i) { @@ -592,8 +598,7 @@ reliable_send(struct reliable *rel, int *opcode) /* If N_ACK_RETRANSMIT later packets have received ACKs, we assume * that the packet was lost and resend it even if the timeout has * not expired yet. */ - if (e->active - && (e->n_acks >= N_ACK_RETRANSMIT || local_now >= e->next_try)) + if (e->active && entry_ready_for_resend(e)) { if (!best || reliable_pid_min(e->packet_id, best->packet_id)) { @@ -605,17 +610,18 @@ reliable_send(struct reliable *rel, int *opcode) { #ifdef EXPONENTIAL_BACKOFF /* exponential backoff */ - best->next_try = local_now + best->timeout; + best->next_try = now + best->timeout; best->timeout *= 2; #else /* constant timeout, no backoff */ - best->next_try = local_now + best->timeout; + best->next_try = now + best->timeout; #endif best->n_acks = 0; + best->flags &= ~REL_FLAG_RESEND_ALLOW; *opcode = best->opcode; dmsg(D_REL_DEBUG, "ACK reliable_send ID " packet_id_format " (size=%d to=%d)", (packet_id_print_type)best->packet_id, best->buf.len, - (int)(best->next_try - local_now)); + (int)(best->next_try - now)); return &best->buf; } return NULL; @@ -639,6 +645,22 @@ reliable_schedule_now(struct reliable *rel) } } + +void +reliable_allow_reschedule_initial(struct reliable *rel) +{ + dmsg(D_REL_DEBUG, "ACK %s", __func__); + for (int i = 0; i < rel->size; ++i) + { + struct reliable_entry *e = &rel->array[i]; + if (e->flags & REL_FLAG_RESEND_INITIAL) + { + e->flags |= REL_FLAG_RESEND_ALLOW; + } + } +} + + /* in how many seconds should we wake up to check for timeout */ /* if we return BIG_TIMEOUT, nothing to wait for */ interval_t @@ -647,21 +669,20 @@ reliable_send_timeout(const struct reliable *rel) struct gc_arena gc = gc_new(); interval_t ret = BIG_TIMEOUT; int i; - const time_t local_now = now; for (i = 0; i < rel->size; ++i) { const struct reliable_entry *e = &rel->array[i]; if (e->active) { - if (e->next_try <= local_now) + if (e->next_try <= now) { ret = 0; break; } else { - ret = min_int(ret, e->next_try - local_now); + ret = min_int(ret, e->next_try - now); } } } @@ -712,10 +733,10 @@ reliable_mark_active_incoming(struct reliable *rel, struct buffer *buf, */ void -reliable_mark_active_outgoing(struct reliable *rel, struct buffer *buf, int opcode) +reliable_mark_active_outgoing(struct reliable *rel, struct buffer *buf, + int opcode, bool initial_response) { - int i; - for (i = 0; i < rel->size; ++i) + for (int i = 0; i < rel->size; ++i) { struct reliable_entry *e = &rel->array[i]; if (buf == &e->buf) @@ -731,6 +752,11 @@ reliable_mark_active_outgoing(struct reliable *rel, struct buffer *buf, int opco e->next_try = 0; e->timeout = rel->initial_timeout; dmsg(D_REL_DEBUG, "ACK mark active outgoing ID " packet_id_format, (packet_id_print_type)e->packet_id); + + if (initial_response) + { + e->flags |= REL_FLAG_RESEND_INITIAL | REL_FLAG_RESEND_ALLOW; + } return; } } diff --git a/src/openvpn/reliable.h b/src/openvpn/reliable.h index 97e6dce7e..902483685 100644 --- a/src/openvpn/reliable.h +++ b/src/openvpn/reliable.h @@ -66,19 +66,29 @@ struct reliable_ack packet_id_type packet_id[RELIABLE_ACK_SIZE]; }; + +/** Use logic for initial packet to resend. I.e. only allow to resend the + * packet if also the \c REL_FLAG_RESEND_ALLOW is set */ +#define REL_FLAG_RESEND_INITIAL (1<<0) +/** The peer has sent a packet, this allows resending of our reply */ +#define REL_FLAG_RESEND_ALLOW (1<<1) + /** - * The structure in which the reliability layer stores a single incoming - * or outgoing packet. - */ +* The structure in which the reliability layer stores a single incoming +* or outgoing packet. +*/ struct reliable_entry { bool active; interval_t timeout; time_t next_try; packet_id_type packet_id; - size_t n_acks; /* Number of acks received for packets with higher PID. - * Used for fast retransmission when there were at least - * N_ACK_RETRANSMIT. */ + size_t n_acks; /**< Number of acks received for packets with higher PID. + * Used for fast retransmission when there were at least + * N_ACK_RETRANSMIT. */ + unsigned int flags; /**< flags to control resending to avoid + * sending more responses than client's initial + * packets to avoid amplification */ int opcode; struct buffer buf; }; @@ -378,8 +388,12 @@ struct buffer *reliable_get_buf_output_sequenced(struct reliable *rel); * reliable_get_buf_output_sequenced() into which the packet has been * copied. * @param opcode The packet's opcode. + * @param initial_response consider this packet an initial response to + * on a connection and apply non amplification + * retry rules */ -void reliable_mark_active_outgoing(struct reliable *rel, struct buffer *buf, int opcode); +void reliable_mark_active_outgoing(struct reliable *rel, struct buffer *buf, + int opcode, bool initial_response); /** @} name Functions for inserting outgoing packets */ @@ -462,6 +476,14 @@ interval_t reliable_send_timeout(const struct reliable *rel); */ void reliable_schedule_now(struct reliable *rel); +/** + * Marks all packets that are currently held back to avoid amplification + * as ready to be resend with the normal resend/timers. + * @param rel The reliable structure of which the entries should be + * modified. + */ +void reliable_allow_reschedule_initial(struct reliable *rel); + void reliable_debug_print(const struct reliable *rel, char *desc); /* set sending timeout (after this time we send again until ACK) */ diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 70b36779a..8a53817db 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -2735,8 +2735,15 @@ tls_process(struct tls_multi *multi, ks->must_negotiate = now + session->opt->handshake_window; ks->auth_deferred_expire = now + auth_deferred_expire_window(session->opt); + /* session id defined serves here as a proxy for this is a + * response to a reset from the other side or our initial + * packet */ + bool initial_reply = ks->initial_opcode != P_CONTROL_SOFT_RESET_V1 + && session_id_defined(&ks->session_id_remote); + /* null buffer */ - reliable_mark_active_outgoing(ks->send_reliable, buf, ks->initial_opcode); + reliable_mark_active_outgoing(ks->send_reliable, buf, + ks->initial_opcode, initial_reply); INCR_GENERATED; ks->state = S_PRE_START; @@ -2972,7 +2979,7 @@ tls_process(struct tls_multi *multi, } if (status == 1) { - reliable_mark_active_outgoing(ks->send_reliable, buf, P_CONTROL_V1); + reliable_mark_active_outgoing(ks->send_reliable, buf, P_CONTROL_V1, false); INCR_GENERATED; state_change = true; dmsg(D_TLS_DEBUG, "Outgoing Ciphertext -> Reliable"); @@ -3652,6 +3659,11 @@ tls_pre_decrypt(struct tls_multi *multi, /* Let our caller know we processed a control channel packet */ ret = true; + /* we we have a packet from the peer, allow another initial response + * to the packet, on the first packet the queue is empty so nothing + * happens */ + reliable_allow_reschedule_initial(ks->send_reliable); + /* * Set our remote address and remote session_id */