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

Hubert Kario <hkario@redhat.com> Thu, 08 September 2016 18:08 UTC

Return-Path: <hkario@redhat.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 1397C12B206 for <tls@ietfa.amsl.com>; Thu, 8 Sep 2016 11:08:11 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -8.41
X-Spam-Level:
X-Spam-Status: No, score=-8.41 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_HI=-5, RP_MATCHES_RCVD=-1.508, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
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 ooRmcIqXLvyA for <tls@ietfa.amsl.com>; Thu, 8 Sep 2016 11:08:09 -0700 (PDT)
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 34C1212B1C9 for <tls@ietf.org>; Thu, 8 Sep 2016 11:08:09 -0700 (PDT)
Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DA59C2FB; Thu, 8 Sep 2016 18:08:08 +0000 (UTC)
Received: from pintsize.usersys.redhat.com (dhcp-0-191.brq.redhat.com [10.34.0.191]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u88I87Zd026099 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 8 Sep 2016 14:08:08 -0400
From: Hubert Kario <hkario@redhat.com>
To: tls@ietf.org
Date: Thu, 08 Sep 2016 20:08:02 +0200
Message-ID: <47532130.8rB6yCJVvA@pintsize.usersys.redhat.com>
User-Agent: KMail/5.2.3 (Linux/4.7.2-201.fc24.x86_64; KDE/5.25.0; x86_64; ; )
In-Reply-To: <CABcZeBMeLgqjvr2cjWL=AHTQJbS9siNBB6U2=0654yigbBGkYA@mail.gmail.com>
References: <CABcZeBMeLgqjvr2cjWL=AHTQJbS9siNBB6U2=0654yigbBGkYA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="nextPart2250422.MEA5QUzcTZ"; micalg="pgp-sha512"; protocol="application/pgp-signature"
X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 08 Sep 2016 18:08:08 +0000 (UTC)
Archived-At: <https://mailarchive.ietf.org/arch/msg/tls/o-EuQMsoRdPABpO2A7EiMfThc_U>
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: Thu, 08 Sep 2016 18:08:11 -0000

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 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 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.

---

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

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.

---

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)

---

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.

---

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?

---

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

---

Record Layer includes

   legacy_record_version : (...) This field is deprecated and MUST be ignored
   for all purposes.

Record Layer Protection does not.

---

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