Re: [quicwg/base-drafts] Specify behavior for post-handshake CRYPTO messages (#2524)

Martin Thomson <notifications@github.com> Sat, 16 March 2019 22:34 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 72BFC130DC2 for <quic-issues@ietfa.amsl.com>; Sat, 16 Mar 2019 15:34:12 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -8.001
X-Spam-Level:
X-Spam-Status: No, score=-8.001 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, 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 Vk7Tj8xaQpnv for <quic-issues@ietfa.amsl.com>; Sat, 16 Mar 2019 15:34:10 -0700 (PDT)
Received: from out-3.smtp.github.com (out-3.smtp.github.com [192.30.252.194]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id DA3B5129BBF for <quic-issues@ietf.org>; Sat, 16 Mar 2019 15:34:09 -0700 (PDT)
Date: Sat, 16 Mar 2019 15:34:08 -0700
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=github.com; s=pf2014; t=1552775648; bh=COqP6bSbr9w6AbgB80j6mSBrPQO0OZm9lczgIPS991w=; h=Date:From:Reply-To:To:Cc:In-Reply-To:References:Subject:List-ID: List-Archive:List-Post:List-Unsubscribe:From; b=XYFeJe9optZ+p3GBfCRH8ADfb7SkRUJutA9Lj3IE7s+gx4GRDOKU3oysZjdJ+x5pG 07M/NiBhjv7cADTjQTRMV6GlAXAPwQfPHEBW7WE/uaPfY/37HKpfcHbpuFseXx7YXB v02Woaww20nUgLq3VerwbEbd7KgI6PGBtJ9rcZvE=
From: Martin Thomson <notifications@github.com>
Reply-To: quicwg/base-drafts <reply+0166e4abfe960bf15d27edc2583ff1a5470f55f8cc3c6ca492cf0000000118a53be092a169ce192348df@reply.github.com>
To: quicwg/base-drafts <base-drafts@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Message-ID: <quicwg/base-drafts/pull/2524/review/215332111@github.com>
In-Reply-To: <quicwg/base-drafts/pull/2524@github.com>
References: <quicwg/base-drafts/pull/2524@github.com>
Subject: Re: [quicwg/base-drafts] Specify behavior for post-handshake CRYPTO messages (#2524)
Mime-Version: 1.0
Content-Type: multipart/alternative; boundary="--==_mimepart_5c8d79e0703f1_ebf3fc3a94d45c014266f1"; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: list
X-GitHub-Sender: martinthomson
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/vt_jFJnZ8npJt-CK1fA__vupIP4>
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: Sat, 16 Mar 2019 22:34:13 -0000

martinthomson requested changes on this pull request.

Thanks for writing this up Nick.

> @@ -1232,6 +1232,19 @@ handshake data start from zero in each packet number space.
 Endpoints MUST explicitly negotiate an application protocol.  This avoids
 situations where there is a disagreement about the protocol that is in use.
 
+Implementations need to maintain a buffer of CRYPTO data received out of order.
+Because there is no flow control of CRYPTO frames, and endpoint could
+potentially force its peer to buffer an unbounded amount of data.
+Implementations MUST support buffering at least 4096 bytes of data.
+
+Once the handshake completes, if an endpoint is unable to buffer all data in a
+CRYPTO frame, it MAY discard all subsequent CRYPTO frames, or it MAY close the
+connection with an INTERNAL_ERROR code. In the case where an endpoint chooses to
+discard all subsequent CRYPTO frames, the packets containing these CRYPTO frames
+are still ACKed. An endpoint in this condition MAY tear down state for its

```suggestion
are still ackknowledged. An endpoint in this condition MAY tear down state for its
```

> @@ -1232,6 +1232,19 @@ handshake data start from zero in each packet number space.
 Endpoints MUST explicitly negotiate an application protocol.  This avoids
 situations where there is a disagreement about the protocol that is in use.
 
+Implementations need to maintain a buffer of CRYPTO data received out of order.
+Because there is no flow control of CRYPTO frames, and endpoint could
+potentially force its peer to buffer an unbounded amount of data.
+Implementations MUST support buffering at least 4096 bytes of data.
+
+Once the handshake completes, if an endpoint is unable to buffer all data in a
+CRYPTO frame, it MAY discard all subsequent CRYPTO frames, or it MAY close the
+connection with an INTERNAL_ERROR code. In the case where an endpoint chooses to

```suggestion
connection with an INTERNAL_ERROR code. If an endpoint chooses to
```

> @@ -1232,6 +1232,19 @@ handshake data start from zero in each packet number space.
 Endpoints MUST explicitly negotiate an application protocol.  This avoids
 situations where there is a disagreement about the protocol that is in use.
 
+Implementations need to maintain a buffer of CRYPTO data received out of order.
+Because there is no flow control of CRYPTO frames, and endpoint could
+potentially force its peer to buffer an unbounded amount of data.
+Implementations MUST support buffering at least 4096 bytes of data.
+
+Once the handshake completes, if an endpoint is unable to buffer all data in a
+CRYPTO frame, it MAY discard all subsequent CRYPTO frames, or it MAY close the
+connection with an INTERNAL_ERROR code. In the case where an endpoint chooses to
+discard all subsequent CRYPTO frames, the packets containing these CRYPTO frames
+are still ACKed. An endpoint in this condition MAY tear down state for its
+cryptographic handshake, keeping only the material needed for the current
+encryption keys and any future key updates.

I would drop the last sentence here.  It's an optimization that is sufficiently obvious.

> @@ -1232,6 +1232,19 @@ handshake data start from zero in each packet number space.
 Endpoints MUST explicitly negotiate an application protocol.  This avoids
 situations where there is a disagreement about the protocol that is in use.
 
+Implementations need to maintain a buffer of CRYPTO data received out of order.

I think that this needs a new section heading.  Section 7.4 "After Handshake Completion" perhaps, after the transport parameters, could be what it is, but it might be better to say "Cryptographic Message Buffering" and discuss the handshake a little (more below).

> @@ -1232,6 +1232,19 @@ handshake data start from zero in each packet number space.
 Endpoints MUST explicitly negotiate an application protocol.  This avoids
 situations where there is a disagreement about the protocol that is in use.
 
+Implementations need to maintain a buffer of CRYPTO data received out of order.
+Because there is no flow control of CRYPTO frames, and endpoint could
+potentially force its peer to buffer an unbounded amount of data.
+Implementations MUST support buffering at least 4096 bytes of data.

Does this requirement apply during the handshake?  Perhaps document some recommendations.

Being unable to buffer CRYPTO frames during the handshake leads to connection failure. Endpoints MAY choose to allow more data to be buffered during the handshake and reduce buffering once the handshake completes.  A larger limit during the handshake could allow for larger keys or credentials to be exchanged.

-- 
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/2524#pullrequestreview-215332111