Re: [TLS] PR#625: Change alert requirements

Eric Rescorla <ekr@rtfm.com> Tue, 20 September 2016 01:13 UTC

Return-Path: <ekr@rtfm.com>
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 311DA127078 for <tls@ietfa.amsl.com>; Mon, 19 Sep 2016 18:13:07 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.599
X-Spam-Level:
X-Spam-Status: No, score=-2.599 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=rtfm-com.20150623.gappssmtp.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 VhKgipKgFvdE for <tls@ietfa.amsl.com>; Mon, 19 Sep 2016 18:13:05 -0700 (PDT)
Received: from mail-yw0-x230.google.com (mail-yw0-x230.google.com [IPv6:2607:f8b0:4002:c05::230]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id A35E012B162 for <tls@ietf.org>; Mon, 19 Sep 2016 18:13:03 -0700 (PDT)
Received: by mail-yw0-x230.google.com with SMTP id g192so1777207ywh.1 for <tls@ietf.org>; Mon, 19 Sep 2016 18:13:03 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rtfm-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=uqo36i0dgF1DyVnQK5NgbTubqPki3hG6PHPtvZZ8sDs=; b=d3g0EAys9nXypdi6vitORDqxrzCKT1zxpJnk0bLVgXj/xAYY6j17OxH7ZakrRO716D IMNU+NcdVbGwVhuuVjiSiHzqEanQmJuAkdeEVAH/CbAhmAQYQL+TEjbMYlrEICAWBad0 cVrhUcjzFIDJlLj0yaZjvnhcvTSi6iL+J/l4hL4062mBuU8CFzrzfDGA1iPUxHZcAmUq pBhlRdSVptNORWZqrIRlrK4XVk2tqncciAzCS3ASfymNO0KcTxV9PCSMP60BYcm/Zr6D V+CnN7DbVfqaNS3BsOtiztj/eLi2akZbvLP/7L/HxLYYCKHogvHqCM6cSoJZosumxEzZ OJ9A==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=uqo36i0dgF1DyVnQK5NgbTubqPki3hG6PHPtvZZ8sDs=; b=TFy91hBwsMatHYWTRBDCl4fdOR5Mb3j+Z4oFnd9+M5FqZ3j/S2vlKYEnU04+SJKF2b gKFOSc2+CKpxwnP9M9KivsMZ9N9Ctb3+cJ1FZmOQEiVyIN9u1NIleRYzGUUq+Ax4IMEM QPA3QvDHWSh1nFpuTL+OiEc5smvYtCiabcP20QLzNuNZHIY/VPUce+RRnxobmnI4wPbX lPkBnFN+2QqZzvZ03yvjcUhosElGd/d0hGJrRaKG5488zp/LDtbD4x5F1k2tEH+wLTNE hHAB7+ECuvQDMLKFztDruVv51jeSvCKoMX5Zj7IxoCFP/s5ZOtKCaRUaRKFfS0dY178p 6ldQ==
X-Gm-Message-State: AE9vXwO5h9vSrXdHFSwvKM547hDb2/hDultMnCAYRWnfIf8YDXj1cQ4tv6wuC7nZk9PNvhPWia0WGFFX9pPXnw==
X-Received: by 10.129.41.196 with SMTP id p187mr27630429ywp.149.1474333982908; Mon, 19 Sep 2016 18:13:02 -0700 (PDT)
MIME-Version: 1.0
Received: by 10.129.160.10 with HTTP; Mon, 19 Sep 2016 18:12:22 -0700 (PDT)
In-Reply-To: <47532130.8rB6yCJVvA@pintsize.usersys.redhat.com>
References: <CABcZeBMeLgqjvr2cjWL=AHTQJbS9siNBB6U2=0654yigbBGkYA@mail.gmail.com> <47532130.8rB6yCJVvA@pintsize.usersys.redhat.com>
From: Eric Rescorla <ekr@rtfm.com>
Date: Mon, 19 Sep 2016 18:12:22 -0700
Message-ID: <CABcZeBOsN+_gUUb=HoUsoPOTBgANedT5Y5O+pAGXn0qTYjq1jg@mail.gmail.com>
To: Hubert Kario <hkario@redhat.com>
Content-Type: multipart/alternative; boundary="001a1142154e555f07053ce6215a"
Archived-At: <https://mailarchive.ietf.org/arch/msg/tls/1Nq0T0sIzkTcDM-XkuwRc__lR4s>
Cc: "tls@ietf.org" <tls@ietf.org>
Subject: Re: [TLS] PR#625: Change alert requirements
X-BeenThere: tls@ietf.org
X-Mailman-Version: 2.1.17
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: Tue, 20 Sep 2016 01:13:07 -0000

Resolutions below.

On Thu, Sep 8, 2016 at 11:08 AM, Hubert Kario <hkario@redhat.com> wrote:

> On Monday, 5 September 2016 11:02:57 CEST Eric Rescorla wrote:
> > PR: https://github.com/tlswg/tls13-spec/pull/625
>
> Finally found some time to take a look at it.
>
> So in general I like the change to "abort the handshake with a <type>
> alert",
> the "send a fatal <type> alert and close the connection" was a bit clunky.
> I
> was actually planning to do something like this myself.
>
> Few other items carried over from my proposed PR[1]:
>
> ---
>
> I still think we should include generic advice for handling parser errors
> in
> the parser (Presentation Language/Vectors and Constructed Types) section;
> something like this:
>
>    Peers that receive a encoding of a vector in a message with a length
> that
>    does not match specification MUST abort the connection with a
>    "decode_error" alert unless more specific section states otherwise.
>
> and this:
>
>    Peers that receive messages that doesn't match the expected constructed
>    type in expected message MUST abort the connection with a "decode_error"
>    alert unless more specific section states otherwise.
>

I added some text in the syntax section.


> I think it's a bit confusing to have this sentence:
>
>    Servers MUST select the lower of the highest supported server version
> and
>    the version offered by the client in the ClientHello. In particular,
>    servers MUST accept ClientHello messages with versions higher than those
>    supported and negotiate the highest mutually supported version.
>
> in the Server Hello section. Everything before the very last "and" is more
> applicable to ClientHello.


I am going to leave this because I anticipate adopting the new version
syntax.


I think that this sentence in Handshake Protocol:

>
>    Sending handshake messages in an unexpected order results in an
>    "unexpected_message" fatal error.
>
> should use a MUST and be prescribed to the receiving side, for example:
>
>    Peer that receives handshake messages in unexpected order MUST abort the
>    handshake with an "unexpected_message" alert.
>

I changed this.



Description what should client do when the ServerHello.cipher_suite is the
> one
> it did not advertise in ClientHello.cipher_suites is missing.
>

Changed..


> So is description of how to handle ServerHello.version being higher than
> ClientHello.max_supported_version.
>
> At least, I don't see it in Server Hello section.
>

See above re: version


missing_extension description in Alert Protocol probably should be extended
> from:
>
>    Sent by endpoints that receive a hello message not containing an
> extension
>    that is mandatory to send for the offered TLS version.
>
> to:
>
>    Sent by endpoints that receive a hello message not containing an
> extension
>    that is mandatory to send for the offered TLS version or the offered key
>    exchange mechanisms.
>
> (thinking of handling situations where "key_share" or "pre_shared_key" is
> missing)
>

Updated a bit.


> ---
>
> Hello Extensions section doesn't specify how to handle messages with
> duplicate
> extensions (e.g. second "key_share" extension added in the second
> ClientHello
> message in connection)
>
> ---
>
> Signature Algorithms doesn't say how to handle RSA-SSA signatures with salt
> length that doesn't match hash size.
>
> ---
>
> Key Share doesn't say how to handle server_share that doesn't match
> client_shares.
>

I'm comfortable leaving these as-is.



> Finished section doesn't describe what to do if the contents don't match
> the
> expected value.
>
> Should it be illegal_parameter or bad_record_mac is more appropriate?
>

This is specified in the decrypt_error section.


> ---
>
> Record Layer doesn't describe what to do if a record with zero-length
> payload
> and handshake or alert type is received.


I'm comfortable leaving these as-is.


> ---
>
> Record Layer includes
>
>    legacy_record_version : (...) This field is deprecated and MUST be
> ignored
>    for all purposes.
>
> Record Layer Protection does not.
>

I don't follow.



>
> ---
>
> 1 - https://github.com/tlswg/tls13-spec/pull/621
> --
> Regards,
> Hubert Kario
> Senior Quality Engineer, QE BaseOS Security team
> Web: www.cz.redhat.com
> Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno, Czech Republic
>