From patchwork Wed Dec 26 17:10:01 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 208178 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 44CA12C00B9 for ; Thu, 27 Dec 2012 04:10:09 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753413Ab2LZRKG (ORCPT ); Wed, 26 Dec 2012 12:10:06 -0500 Received: from mail-pa0-f52.google.com ([209.85.220.52]:35268 "EHLO mail-pa0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753060Ab2LZRKF (ORCPT ); Wed, 26 Dec 2012 12:10:05 -0500 Received: by mail-pa0-f52.google.com with SMTP id fb1so4995609pad.25 for ; Wed, 26 Dec 2012 09:10:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:subject:from:to:cc:content-type:date:message-id :mime-version:x-mailer:content-transfer-encoding; bh=2oGf96lU3JwxV2eWZWOBm7QdxKBCU9tNAd15STn7crY=; b=fl95iiOHCBUAfGRIYffHZq72DGrK1zOVvE2cQDzOPLTebCJD7mJ6sc7ORsGDFYYPPv IdrGWYhu0Z/fUL0skv6il1WT0Xg6luzb7h2qojfcTpb6+e4SE7gVKM73bvfky/HThIQ5 eej36v7g70BvHjShDH4iQ8UJJgCnIXJ26CFsJbYJmEvbAt1vq00HeZvzd1ktSuYVKHP+ 3bicJEPmuUa0QKBGz7UxGFWXcws1eBKQpoyJYUYrd6D+tCACbg8CTnfv2nbbhWZsJWwm O0l1o7E20d8SuBzc/FiBkDgZz2j1oXERUT2yW2iyO8CcWStnYUzd0o+DFumHQ09HJff4 qa9Q== X-Received: by 10.68.189.163 with SMTP id gj3mr87562915pbc.110.1356541804022; Wed, 26 Dec 2012 09:10:04 -0800 (PST) Received: from [172.19.254.60] ([172.19.254.60]) by mx.google.com with ESMTPS id vs3sm16177765pbc.61.2012.12.26.09.10.01 (version=SSLv3 cipher=OTHER); Wed, 26 Dec 2012 09:10:02 -0800 (PST) Subject: [PATCH] tcp: should drop incoming frames without ACK flag set From: Eric Dumazet To: David Miller Cc: netdev , Zhiyun Qian , Nandita Dukkipati , Neal Cardwell , John Dykstra Date: Wed, 26 Dec 2012 09:10:01 -0800 Message-ID: <1356541801.20133.20615.camel@edumazet-glaptop> Mime-Version: 1.0 X-Mailer: Evolution 2.28.3 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Eric Dumazet In commit 96e0bf4b5193d (tcp: Discard segments that ack data not yet sent) John Dykstra enforced a check against ack sequences. In commit 354e4aa391ed5 (tcp: RFC 5961 5.2 Blind Data Injection Attack Mitigation) I added more safety tests. But we missed fact that these tests are not performed if ACK bit is not set. RFC 793 3.9 mandates TCP should drop a frame without ACK flag set. " fifth check the ACK field, if the ACK bit is off drop the segment and return" Not doing so permits an attacker to only guess an acceptable sequence number, evading stronger checks. Many thanks to Zhiyun Qian for bringing this issue to our attention. See : http://web.eecs.umich.edu/~zhiyunq/pub/ccs12_TCP_sequence_number_inference.pdf Reported-by: Zhiyun Qian Signed-off-by: Eric Dumazet Acked-by: Nandita Dukkipati Cc: Neal Cardwell Cc: John Dykstra --- Notes - I left a "if (true)" block that I'll remove in a cleanup patch in linux-3.9, to permit this patch being easily back-ported to stable branches. - A followup patch will be sent (in net-next) to have stronger checks before sending dupack net/ipv4/tcp_input.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index a136925..903d0ef 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -5540,6 +5540,9 @@ no_ack: } slow_path: + if (!th->ack) + goto discard; + if (len < (th->doff << 2) || tcp_checksum_complete_user(sk, skb)) goto csum_error; @@ -5551,7 +5554,7 @@ slow_path: return 0; step5: - if (th->ack && tcp_ack(sk, skb, FLAG_SLOWPATH) < 0) + if (tcp_ack(sk, skb, FLAG_SLOWPATH) < 0) goto discard; /* ts_recent update must be made after we are sure that the packet @@ -5984,11 +5987,15 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb, if (tcp_check_req(sk, skb, req, NULL, true) == NULL) goto discard; } + + if (!th->ack) + goto discard; + if (!tcp_validate_incoming(sk, skb, th, 0)) return 0; /* step 5: check the ACK field */ - if (th->ack) { + if (true) { int acceptable = tcp_ack(sk, skb, FLAG_SLOWPATH) > 0; switch (sk->sk_state) { @@ -6138,8 +6145,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb, } break; } - } else - goto discard; + } /* ts_recent update must be made after we are sure that the packet * is in window.