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

[Olivier Levillain <[e-mail]>]

Le 22/11/2016 22:19, Eric Rescorla a écrit :
> On Tue, Nov 22, 2016 at 11:02 AM, Olivier Levillain <
> [e-mail]> wrote:
>> 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...)
>>
> 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 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.

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

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

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