Subject: [TLS] Draft 18 review : Extensions and message reuse

From: Olivier Levillain <[email-masked]>
Date: Tue, 22 Nov 2016 20:02:04 +0100

Hi list,

I am sorry for the very late answer concerning draft 18, but we
(ANSSI) have several remarks after proof-reading the current
specification.

We are sorry for the multiple long messages.

If the WG is interested by some of our concerns/proposals, we would be
glad to propose some PRs.

= Extensions and message reuse =

We were sorry to find that some TLS 1.2 extensions were reused with a
different meaning, or that some TLS 1.3 extensions were context
dependent, which does not allow for a clear separation between the
parsing step (which might lead to a decode_error alert) and the
message validation step (which might lead to an invalid_parameter
alert).  It is especially disappointing since the choice made for the
ciphersuites is a very clean one.

The first example of such a problem is the signature_scheme and
signature_algorithms enums.  In practice, the signature_algorithm must
contain information valid for TLS 1.2 and TLS 1.3, but the exact
meaning is not the same in both cases.  It would be cleaner to have a
new, separate extension called signature_scheme, defining the new
enum.  This way, a TLS 1.2/1.3 client implementations would send both
extensions.  We would spend 10 bytes on the wire, but it is not that
important for the first message which can be kept reasonably long
(and which is sometimes padded via the corresponding extension...)

Another example is the key_share extension: it is in fact made of
three different extensions, depending on the message where it is
included.  It would be simpler for a (almost-)stateless parser* to
either use three different extensions or to use the same content each
time.  In the second case, the extension would each time consist of a
list of (group + optional keyshare).  The advantages of such a unified
keyshare extension would be:
 - stateless parsing*
 - no reuse of supported_groups which can be merged with key_share
 - no more validation complexity since higher-level checks were
   already required (the groups sent by HRR did not came with a key
   share for example).
The major con one might see is that the server can not advertise its
supported groups in an encrypted manner anymore (since
supported_groups is not used anymore).  However, we do believe
simplifying the implementation is worth it.

* Of course the parser still has to use the extension id to know how
  to parse the value, but it would not need to have broader context
  (the message where the extension is included).  In other words, we
  would like select syntax in structs to only use local information.

I can try and propose a PR for each extension if there is an interest
in the WG.

Olivier Levillain

---

From: Eric Rescorla <[email-masked]>
Date: Tue, 22 Nov 2016 13:19:11 -0800

> (and which is sometimes padded via the corresponding extension...)

We certainly could do this, but I don't think it's the right answer, for
two reasons:

1. From a protocol perspective, some of the things you could say with
signature_algorithms weren't coherent. For instance: I support ECDSA_SHA512
but only with P256 (via supported_curves). So it's actually a retroactive
improvement to TLS 1.2 to use the code points this way. NSS, at least
interprets these the same way in TLS 1.2 and TLS 1.3. I believe BoringSSL
does as well.

2. From an implementation perspective, it's easier to just have a single
external facing surface, which should be the more sensible one, namely
signature schemes.

> I can try and propose a PR for each extension if there is an interest

It seems to me you are making two points here:

1. That it would be good to unify supported_groups and key_share
2. That we should use separate code points wherever extensions differ
between the messages they appear (this is far from the only one).

We've had some conversation about the first approach, which seems like kind
of a stylistic/taste issue whether to have the "I can do" and "here is" in
one
place or two, and on balance I think it's better to not change this at this
point
without a really compelling reason.

I can see the appeal of having one parser per extension ID, but it isn't
actually that much easier from an implementation perspective. You already
need a mapping table from extension ID to handler and that table
also needs to be able to detect common problems (e.g., extension reuse
or extensions appearing in the wrong place) and it's not hard at that
point to also have it point to message-specific parsers (this is how NSS
mostly works). ISTM that the alternative you propose would either result
in a proliferation of extensions (which confuses the matching rules)
or having extensions which are syntactically valid but not legal in
their context (e.g., >1 key share from the server). This pushes these
checks further away from the decoder, and makes things which would
otherwise be syntax errors (e.g., a "pre_shared_key" extension from
the server which contains keys) into semantic errors which require
explicit checks.

-Ekr

>
> Olivier Levillain
>
> _______________________________________________
> TLS mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tls
>

---

From: Olivier Levillain <[email-masked]>
Date: Wed, 23 Nov 2016 09:50:48 +0100

> external facing surface, which should be the more sensible one, namely

I understand your point, but there were different curves with the same
security parameter in TLS 1.2, and advertising some combinations in a
TLS 1.2 / 1.3 (supporting Brainpool curves but not the NIST equivalent)
implementation might be a pain. Considering the current deployment, we
certainly can live with that.

We also were wondering why not reuse the values for hash algorithms for
PSS: if we use 0x04XX for PSS-sha256 and 0x05XX for PSS-sha384, etc. it
would align with existing TLS 1.2 behaviour, and make it easier to
include PSS support in these implementations, as is recommended by the
specification.

> without a really compelling reason.

I have no compelling reason, but I don't remember why the WG chose to
stick with two extensions where there was a working design with only one.

> the server which contains keys) into semantic errors which require

I understand your point of view (and we know that key_share is not the
only extension with this property) and this can be expressed rather
easily in code.

(However, it is important to remember that there must still be checks to
implement at the semantic level when receiving this extension, as
explained in the spec:
 - HRR: check that the group was not proposed in the first place in
keyshare (but that it is supported)
 - Second CH: check that there is only one key share matching the
required group
 - SH: check that the key share matches one of the group sent in the CH)

olivier