Re: [TLS] draft-ietf-tls-dtls13-31 comments

"Christopher Wood" <caw@heapingbits.net> Wed, 08 May 2019 15:48 UTC

Return-Path: <caw@heapingbits.net>
X-Original-To: tls@ietfa.amsl.com
Delivered-To: tls@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 67247120074 for <tls@ietfa.amsl.com>; Wed, 8 May 2019 08:48:32 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.701
X-Spam-Level:
X-Spam-Status: No, score=-2.701 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=heapingbits.net header.b=QWR789NM; dkim=pass (2048-bit key) header.d=messagingengine.com header.b=05M8Pgvd
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 lwQUJF4qIksK for <tls@ietfa.amsl.com>; Wed, 8 May 2019 08:48:28 -0700 (PDT)
Received: from wnew3-smtp.messagingengine.com (wnew3-smtp.messagingengine.com [64.147.123.17]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 1101C1200B7 for <TLS@ietf.org>; Wed, 8 May 2019 08:48:28 -0700 (PDT)
Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailnew.west.internal (Postfix) with ESMTP id 1475F489 for <TLS@ietf.org>; Wed, 8 May 2019 11:48:27 -0400 (EDT)
Received: from imap4 ([10.202.2.54]) by compute6.internal (MEProxy); Wed, 08 May 2019 11:48:27 -0400
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=heapingbits.net; h=mime-version:message-id:in-reply-to:references:date:from:to :subject:content-type; s=fm1; bh=PXPy4ANEskLW4h2apa6dznRgXfwobpu KpKagtvHl2uc=; b=QWR789NM7rby/e7lv9ua5BLYyUVjJ4WXRRO6DWrd2Svi4IK mhLFIwMyWHXtcCvR/kuUM1LNSnPiQJjVC7Be3gmgDErFiJLzAh7wN4l6zjOEJ/HE 40tpIBZpkZ66Kyug4Pr1C9s0dsDISdUhVvo26UBcQ2do+cVIMT7MCzLKhsYEjzIp p2xWE5Q8aNAHJ6rOU/xp/V3Z2pWRYp/4UYHDr0MqIUQwVHcrEBEiKIZ3/boDE6fs lYUpTawPGNtWFd47td7UX7j4aAnXcDU7C4zeCjfDMhKB9NCWURHVv8wA0gTH++1N lNNqeMniD/goMP94MNNcR5cBgN62CqB6vYdy2nw==
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; bh=PXPy4A NEskLW4h2apa6dznRgXfwobpuKpKagtvHl2uc=; b=05M8PgvdXhppBTyHidCYdc JFT4etNehkId0a7jbPt55CoM06Q4cgYl/eZ7A+0SuN30U5Ebm0N5OfJON86Wt+CU vJEYa57ajRSdhaioEQrOwj25GVH+FcAdK0Iqcq0F6QQiw8XUmbHcO1xgHrVkp+r+ UhHV4t3WKzCREZNL/2rlBRnKcbdb0+L8y3i+xIxIgYqpFVjyKD62Wo3IMWa5k2Yn XEw6me+2Qx1zvW4uB5U0hExnh/+DLiJiLX2hoOcUPLluMqFuc+KvZXw0oS7Uydq/ 2keeONfxL5rFWGgnmsxh6z5AT4m110PzHiKDB6K4wUYymVbtdpHi24Asy8CdXtGQ ==
X-ME-Sender: <xms:SfrSXDXj8Xd3WcDS4jWXWVEcbRzCm2mdOC0ZEvgjY_EKt9YUY6DK4g>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduuddrkeefgdelfecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecunecujfgurhepofgfggfkjghffffhvffutgesthdtre dtreertdenucfhrhhomhepfdevhhhrihhsthhophhhvghrucghohhougdfuceotggrfies hhgvrghpihhnghgsihhtshdrnhgvtheqnecuffhomhgrihhnpehgihhthhhusgdrtghomh enucfrrghrrghmpehmrghilhhfrhhomheptggrfieshhgvrghpihhnghgsihhtshdrnhgv thenucevlhhushhtvghrufhiiigvpedt
X-ME-Proxy: <xmx:SfrSXKBYvqJFjqeFW0KltSUl4Pd28458J8GJ8PlZD-RDIegsZQkZqA> <xmx:SfrSXCIH4d7oUV_sHeTzjHgbXYwWTKnQZZzFaK6jgrrvIIvoelKuqA> <xmx:SfrSXDJbB3l5hQkQxjQVr6JIw6oaSHYuvhXHASeDHgmN7bFNPa-qhA> <xmx:SvrSXLDzRxMZ7KJegGbbSf3UIpW4MbPk8SG0MwWhZuWHqAEnPyc9yZGPn-LYKgCO>
Received: by mailuser.nyi.internal (Postfix, from userid 501) id B3830260C3; Wed, 8 May 2019 11:48:25 -0400 (EDT)
X-Mailer: MessagingEngine.com Webmail Interface
User-Agent: Cyrus-JMAP/3.1.6-449-gfb3fc5a-fmstable-20190430v1
Mime-Version: 1.0
Message-Id: <a5b44d69-478c-4eb0-b4a7-dd437636af28@www.fastmail.com>
In-Reply-To: <CAF8qwaD+FYfSwhbOF9Y+3ExUJeAyB4J+ALKJ7OEh+HvM6LbLzg@mail.gmail.com>
References: <CAF8qwaD+FYfSwhbOF9Y+3ExUJeAyB4J+ALKJ7OEh+HvM6LbLzg@mail.gmail.com>
Date: Wed, 08 May 2019 11:48:20 -0400
From: "Christopher Wood" <caw@heapingbits.net>
To: "TLS@ietf.org" <TLS@ietf.org>
Content-Type: text/plain
Archived-At: <https://mailarchive.ietf.org/arch/msg/tls/87eUtqgJXH9lmTTh5x2s8UtZH_Q>
Subject: Re: [TLS] draft-ietf-tls-dtls13-31 comments
X-BeenThere: tls@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "This is the mailing list for the Transport Layer Security working group of the IETF." <tls.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/tls>, <mailto:tls-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/tls/>
List-Post: <mailto:tls@ietf.org>
List-Help: <mailto:tls-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/tls>, <mailto:tls-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 08 May 2019 15:48:32 -0000

Thanks for the review, David! Please see inline below for comments. 

On Wed, Apr 3, 2019, at 12:43 PM, David Benjamin wrote:
> Hi all,
> 
> I read through draft-ietf-tls-dtls13-31 and gathered a bunch of 
> comments. I uploaded https://github.com/tlswg/dtls13-spec/pull/85 for 
> the easy bits. The remainder I figured I should send to the list. 
> Hopefully this is a usable format. Apologies for the very late look 
> over it. I haven't had time to page it in before now.
> 
> In section 4, I assume that if not negotiated, CID cannot be present 
> and the receiver rejects any records with it? It might be worth 
> spelling that out explicitly.

That's my understanding as well.

> Section 4.2.1 reads odd to me. I assume the references to 1 and 2 are 
> just as an example and not a particular oddity from those epochs. Maybe 
> stick a "for example" or say "earlier and later epoch". It also says 
> that during the handshake, "implementations MUST accept packets from 
> the old epoch", which is ambiguous as there are several epochs in the 
> handshake. That sentence also runs counter to the SHOULD immediately at 
> the start of the paragraph, which is odd.

Agreed that some clarifying text would help hear. 

> Section 4..5.1 talks about timing channels for the sequence number. 
> Note even uncompressing the sequence number (finding the right upper 
> bytes) introduces a timing channel. Granted, it is a much much weaker 
> one than record deprotection, but invalid DTLS records being non-fatal 
> means signals can be amplified....

Perhaps the text could acknowledge this additional, albeit smaller timing channel?

> Section 5 says that implementations MUST ignore ChangeCipherSpec 
> messages. This contradicts section 4.1, which does not include 
> change_cipher_spec as one of the possible record rtypes for 
> DTLSPlaintext. It does fall through to section 4.5.2, but 4.5.2 allows 
> for implementations to reject these. (Is there actually a need to 
> ignore them in DTLS 1.3 if the middlebox hacks aren't applicable?)

Perhaps 4.1 could leave a comment saying change_cipher_spec is not permitted, along with a forward pointer to Section 5?

> I don't understand the last paragraph of section 5.1. What's it trying 
> to describe? If the client got its hands on an invalid cookie, there's 
> no recovering that handshake. The server won't accept the cookie, and 
> the client won't accept a second HRR.

Perhaps what's missing is precisely that clarifying statement. Namely, that clients which find themselves in such a situation cannot recover.

> Section 5.3 says that the supported_versions extension is used without 
> modification from TLS 1.3, but that still leaves its contents 
> ambiguous. Is DTLS 1.3 0304 or fefc? I assume 0304 since fefc isn't 
> mentioned. But then how are DTLS 1.0 and DTLS 1.2 encoded? (One could 
> imagine omitting them, but that doesn't match how TLS 1.3 did it.) I 
> assume they remain feff and fefd, so we don't need to answer whether 
> DTLS 1.0 is 0301 or 0302. Either way, this should probably be spelled 
> out.

My interpretation was 0304 as well. 

> This is a nitpick and exists in DTLS 1.2 too, but section 5.4 uses the 
> term "maximum handshake fragment size", but nowhere else in the 
> document is the term defined or referenced. This means the largest 
> handshake fragment such that the resulting DTLS record fits in a 
> packet. (Though even that's incomplete because you might have 
> previously allocated a smaller fragment and then have less room left 
> for the next fragment. It gets further messier when packing fragments 
> into records and having to worry about when you are and aren't forced 
> to add a record boundary for epoch change...)

+1 -- defining this term would be good.

> 
> Section 5.9 is weird. With warning alerts gone, all alerts are fatal. 
> Either you've torn down all local state and don't resend alerts (I 
> suspect most applications just do this, for better or worse), or maybe 
> you're hanging around for a while to resend them. If the latter, you 
> probably don't want to be trying to detect that the offending record 
> got resent, since that involves processing records out of a connection 
> in an error state. It seems better to blindly retransmit on every 
> packet until you get bored.
> 
> Relatedly, what does close_notify mean in DTLS? This draft and DTLS 1.2 
> don't seem to make any mention of it. Bidirectional shutdown doesn't 
> make much sense. Not sure if unidirectional is meaningful, given lack 
> of retransmits. Maybe a best effort thing to save the peer a timeout.

+1

> 
> Section 6.1 says:
> 
>  Implementations that receive a payload with an epoch value for which
>  no corresponding cipher state can be determined MUST generate a
>  "unexpected_message" alert.
> 
> This seems to be cover data from the next epoch, which contradicts 
> section 4.2.1. (If I receive, say, EncryptedExtensions before 
> ServerHello, I can't determine the cipher state for that epoch.) 
> Section 4.2.1 says you should either buffer or drop those records, not 
> reject them.

Good catch. This should probably be cleaned up to ensure out-of-order handshake packets are handled correctly and do not yield an alert. 

> 
> Section 7 defines ACKs with a uint64 record_number. I assume this is a 
> concatenated epoch and sequence number, but I don't see anywhere 
> defines this. Perhaps just write it as:
> 
>  struct {
>  uint16 epoch;
>  uint48 sequence_number;
>  } RecordNumber;
> 
>  struct {
>  RecordNumber record_numbers<0..2^16-1>;
>  } ACK;

Good clarification! 

> 
> Relatedly, what is the "record sequence number" input to the AEAD (RFC 
> 8446, section 5.3)? DTLS believes that the "sequence number" is 48 
> bits, but the rest of TLS 1.3 is written expecting a 64-bit sequence 
> number. Using the concatenation is simplest and also matches RFC 7905, 
> but should be mentioned explicitly. (RFC 6347 didn't have text to this 
> effect because it only had AES-GCM which, in (D)TLS 1.2, used explicit 
> IVs.)
> 
> (I will probably have other comments on the ACK bits, but I think I 
> need to digest it more carefully to try to understand what's going on. 
> It does seem a little overkill for what is ultimately a small handful 
> of packets...)

Sounds good. Thanks again for the review!

Ekr, Hannes, Nagendra: can you please have a look through the comments and try to address them?

Best,
Chris