Re: [TLS] Draft 18 review : Message splitting and interleaving

[Olivier Levillain <[e-mail]>]

Le 23/11/2016 00:24, Eric Rescorla a écrit :
> On Tue, Nov 22, 2016 at 2:18 PM, David Benjamin <[e-mail]>
> wrote:
>
>> On Tue, Nov 22, 2016 at 2:05 PM Olivier Levillain <
>> [e-mail]> wrote:
>>
>>> 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.
>>>
>>>
>>> = Message splitting and interleaving =
>>>
>>> How to split and interleave subprotocols in TLS has not been clearly
>>> specified in the past, and it would be useful to be crystal clear on
>>> this point.
>>>
>>> In the specification, the subject is first presented in 4.5 (P.61):
>>>    Handshake messages sent after the handshake MUST NOT be interleaved
>>>    with other record types.  That is, if a message is split over two or
>>>    more handshake records, there MUST NOT be any other records between
>>>    them.
>>> But I am wondering why this text only concern "messages after the
>>> handshake".  It should always be the case!

I don't know what is your take on this first point, but I think I will
propose something in the PR.

I will try to add/move some text so the information is all present in
section 5.

>> +1. We (BoringSSL) and I believe NSS already forbid this for alerts at all
>> versions.
>>
>> This rule is actually implied by TLS 1.3 already, because we've gotten rid
>> of warning alerts. All alerts terminate the connection, except for
>> end_of_early_data, and end_of_early_data immediately signals a key change.
>> So it is not legal to send two alerts in the same epoch, much less in the
>> same record. (Being explicit about this is good, of course.)
>>
> This seems fine. I would take a PR for this.

Will do.


>  - incidentally, the default behaviour would apply to Heartbeat, as
>>>    was the intent of the specification;
>>>  - ApplicationData should be considered as a stream with possibly
>>>    0-length records
>>>  - Handshake messages should either come as a sequence of multiple
>>>    *entire* messages, or as a fraction of *one* message.  That is,
>>>    the number of HS messages inside one record should either be a
>>>    round number or strictly less than 1.
>>>
>> What simplifications were you expecting out of this? It seems to me this
>> would be a nuisance to both enforce as a receiver and honor as a sender.
>>
>> Our implementation doesn't try to pack handshake messages into records,
>> but I believe NSS does. NSS folks should confirm, but I expect such
>> implementations just buffer the messages up and flush when the buffer
>> exceeds the records size. That means all kinds of splits are plausible:
>>
>>   [EncryptedExtensions Certifi]
>>   [cateRequest Certificate Cer]
>>   [tificateVerify Finished]
>>
> Yeah, that's how this works in NSS.
>
> I'm not seeing a real benefit in prohibiting this behavior.

The expected benefit was that it was a way to enforce the rule from
section 4.6 (P.64), stating that a Handshake message should not span key
changes. That is why the receiver already had to do some work, and my
proposal was to restrict it even more. Yet I see your point that from
the sender's point of view, this is more complex.

After re-reading the 4.6 part, I find it sufficient. Sorry for the noise
about this.


olivier