Re: [quicwg/base-drafts] Fix for off-path migration attack (#2033)

erickinnear <notifications@github.com> Thu, 22 November 2018 19:32 UTC

Return-Path: <noreply@github.com>
X-Original-To: quic-issues@ietfa.amsl.com
Delivered-To: quic-issues@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id B228B130DC2 for <quic-issues@ietfa.amsl.com>; Thu, 22 Nov 2018 11:32:14 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -9.46
X-Spam-Level:
X-Spam-Status: No, score=-9.46 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_HIGH=-1.46, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=0.001, MAILING_LIST_MULTI=-1, RCVD_IN_DNSWL_HI=-5, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=github.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id v0WsVbi8xlfK for <quic-issues@ietfa.amsl.com>; Thu, 22 Nov 2018 11:32:12 -0800 (PST)
Received: from out-5.smtp.github.com (out-5.smtp.github.com [192.30.252.196]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id EA76B12875B for <quic-issues@ietf.org>; Thu, 22 Nov 2018 11:32:11 -0800 (PST)
Date: Thu, 22 Nov 2018 11:32:10 -0800
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=github.com; s=pf2014; t=1542915130; bh=lnX+xS4Mfu4mDq6sDlU/NEnMgP+zN2eC+DbmnAdb9w0=; h=Date:From:Reply-To:To:Cc:In-Reply-To:References:Subject:List-ID: List-Archive:List-Post:List-Unsubscribe:From; b=LSjdmSsxQJD7ZhG0INmmAI2L6N5hpLA+LUg4Vuw5aAfHmZQ5uYnu340hUEPIRThTZ 8hrI80rWVTT9JbRZSG9UL/vIeB5FPpBmf3rGcn2f4MTpnitpukHLl7DNjken8SQmdl 0cbD5dTkeIkNc/wiYFhBNNZ/fF9Ixc8w6thWsqSI=
From: erickinnear <notifications@github.com>
Reply-To: quicwg/base-drafts <reply+0166e4ab10ed19041594a2a5f52395bd74aff5ab8588035492cf00000001180ec63a92a169ce16d3ac5a@reply.github.com>
To: quicwg/base-drafts <base-drafts@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Message-ID: <quicwg/base-drafts/pull/2033/review/177753396@github.com>
In-Reply-To: <quicwg/base-drafts/pull/2033@github.com>
References: <quicwg/base-drafts/pull/2033@github.com>
Subject: Re: [quicwg/base-drafts] Fix for off-path migration attack (#2033)
Mime-Version: 1.0
Content-Type: multipart/alternative; boundary="--==_mimepart_5bf7043a71e21_480d3fa7ec2d45b43134f"; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: list
X-GitHub-Sender: erickinnear
X-GitHub-Recipient: quic-issues
X-GitHub-Reason: subscribed
X-Auto-Response-Suppress: All
X-GitHub-Recipient-Address: quic-issues@ietf.org
Archived-At: <https://mailarchive.ietf.org/arch/msg/quic-issues/znmCRVIIMYiCHZ4Rcfp9hLFhfYU>
X-BeenThere: quic-issues@ietf.org
X-Mailman-Version: 2.1.29
List-Id: Notification list for GitHub issues related to the QUIC WG <quic-issues.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/quic-issues>, <mailto:quic-issues-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/quic-issues/>
List-Post: <mailto:quic-issues@ietf.org>
List-Help: <mailto:quic-issues-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/quic-issues>, <mailto:quic-issues-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 22 Nov 2018 19:32:15 -0000

erickinnear approved this pull request.

There is a fair bit of text here, but overall seems like a good balance of addressing the concern without adding significant complexity. Thanks for putting this together and writing it up! 

> @@ -1753,10 +1753,12 @@ have failed, even if the data matches that sent in the PATH_CHALLENGE.
 Additionally, the PATH_RESPONSE frame MUST be received on the same local address
 from which the corresponding PATH_CHALLENGE was sent.  If a PATH_RESPONSE frame
 is received on a different local address than the one from which the
-PATH_CHALLENGE was sent, path validation is considered to have failed, even if
-the data matches that sent in the PATH_CHALLENGE.  Thus, the endpoint considers
-the path to be valid when a PATH_RESPONSE frame is received on the same path
-with the same payload as the PATH_CHALLENGE frame.
+PATH_CHALLENGE was sent, path validation is not considered to be successful,
+even if the data matches that sent in the PATH_CHALLENGE.  This doesn't result
+in path validation failure, as it might be a result of a forwarded packet (see
+{{off-path-forward}}) or misrouting.  Thus, the endpoint considers the path to
+be valid when a PATH_RESPONSE frame is received on the same path with the same

We now have three options for path validation: success, failure, and neither. Are there any other places that reference this where we need to update the wording around expected behaviors? 

> +
+In response to an apparent migration, endpoints MUST validate the previously
+active path using a PATH_CHALLENGE frame.  This induces the sending of new
+packets on that path.  If the path is no longer viable, the validation attempt
+will time out and fail; if the path is viable, but no longer desired, the
+validation will succeed, but only result in a probing packet being sent on the
+path.
+
+An endpoint that receives a PATH_CHALLENGE on an active path SHOULD send a
+non-probing packet in response.  If the non-probing packet arrives before any
+copy made by an attacker, this results in the connection being migrated back to
+the original path.  Any subsequent migration to another path resets this entire
+process.
+
+Abandoning this validation attempt before it either succeeds or times out
+increases exposure to the packet copying attack.

Is this a necessary (independent) paragraph? 

> @@ -1963,13 +2012,16 @@ multiple paths will still send ACK frames covering all received packets.
 
 While multiple paths might be used during connection migration, a single
 congestion control context and a single loss recovery context (as described in
-{{QUIC-RECOVERY}}) may be adequate.  A sender can make exceptions for probe
-packets so that their loss detection is independent and does not unduly cause
-the congestion controller to reduce its sending rate.  An endpoint might set a
-separate timer when a PATH_CHALLENGE is sent, which is cancelled when the
-corresponding PATH_RESPONSE is received.  If the timer fires before the
-PATH_RESPONSE is received, the endpoint might send a new PATH_CHALLENGE, and
-restart the timer for a longer period of time.
+{{QUIC-RECOVERY}}) may be adequate.  For instance, an endpoint might delay
+switching to a new congestion control context until it is confirmed that an old
+path is no longer needed (for the case in {{off-path-forward}}).

Nit: wording
```suggestion
path is no longer needed (such as the case in {{off-path-forward}}).
```
(or something of that sort)

> +path.
+
+An endpoint that receives a PATH_CHALLENGE on an active path SHOULD send a
+non-probing packet in response.  If the non-probing packet arrives before any
+copy made by an attacker, this results in the connection being migrated back to
+the original path.  Any subsequent migration to another path resets this entire
+process.
+
+Abandoning this validation attempt before it either succeeds or times out
+increases exposure to the packet copying attack.
+
+This defense is imperfect, but this is not considered a serious problem. If the
+path via the attack is reliably faster than the original path despite multiple
+attempts to use that original path, it is not possible to distinguish between
+attack and an improvement in routing.
+

Right, since there will still be duplicate packets from the client and you can't make the client actually change the remote address that it sends to. So even if the attack succeeds, if the attacker stops delivering packets (or even starts delivering them more slowly), I think you would automatically just go back to the original path (as long as the client is still sending). 

My understanding here is that this overall change is mostly necessary to prompt traffic from a client that might otherwise have quiesced. 

> -packets so that their loss detection is independent and does not unduly cause
-the congestion controller to reduce its sending rate.  An endpoint might set a
-separate timer when a PATH_CHALLENGE is sent, which is cancelled when the
-corresponding PATH_RESPONSE is received.  If the timer fires before the
-PATH_RESPONSE is received, the endpoint might send a new PATH_CHALLENGE, and
-restart the timer for a longer period of time.
+{{QUIC-RECOVERY}}) may be adequate.  For instance, an endpoint might delay
+switching to a new congestion control context until it is confirmed that an old
+path is no longer needed (for the case in {{off-path-forward}}).
+
+A sender can make exceptions for probe packets so that their loss detection is
+independent and does not unduly cause the congestion controller to reduce its
+sending rate.  An endpoint might set a separate timer when a PATH_CHALLENGE is
+sent, which is cancelled when the corresponding PATH_RESPONSE is received.  If
+the timer fires before the PATH_RESPONSE is received, the endpoint might send a
+new PATH_CHALLENGE, and restart the timer for a longer period of time.

I thought we'd already addressed this elsewhere, but I can't find it anymore, so this looks good. 
My memory was that we said to retransmit PATH_CHALLENGE based on a timer until you got a response or decide that validation failed, although now we have three states for validation result. If we have that elsewhere (and I just missed it) then we should reconcile with this.

> @@ -1963,13 +2012,16 @@ multiple paths will still send ACK frames covering all received packets.
 
 While multiple paths might be used during connection migration, a single
 congestion control context and a single loss recovery context (as described in
-{{QUIC-RECOVERY}}) may be adequate.  A sender can make exceptions for probe
-packets so that their loss detection is independent and does not unduly cause
-the congestion controller to reduce its sending rate.  An endpoint might set a
-separate timer when a PATH_CHALLENGE is sent, which is cancelled when the
-corresponding PATH_RESPONSE is received.  If the timer fires before the
-PATH_RESPONSE is received, the endpoint might send a new PATH_CHALLENGE, and
-restart the timer for a longer period of time.
+{{QUIC-RECOVERY}}) may be adequate.  For instance, an endpoint might delay
+switching to a new congestion control context until it is confirmed that an old
+path is no longer needed (for the case in {{off-path-forward}}).
+
+A sender can make exceptions for probe packets so that their loss detection is
+independent and does not unduly cause the congestion controller to reduce its
+sending rate.  An endpoint might set a separate timer when a PATH_CHALLENGE is

...which makes sense, given that you don't retransmit PATH_CHALLENGE or PATH_RESPONSE, maybe we should note that here or nearby as well?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2033#pullrequestreview-177753396