subject:"\[TLS\] Draft 18 review \: Message splitting and interleaving"

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

2016-11-23 Thread Olivier Levillain

Le 23/11/2016 00:24, Eric Rescorla a écrit :
> On Tue, Nov 22, 2016 at 2:18 PM, David Benjamin 
> wrote:
>
>> On Tue, Nov 22, 2016 at 2:05 PM Olivier Levillain <
>> olivier.levill...@ssi.gouv.fr> 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

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls

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

2016-11-22 Thread Eric Rescorla

On Tue, Nov 22, 2016 at 4:36 PM, Martin Thomson 
wrote:

> On 23 November 2016 at 10:24, Eric Rescorla  wrote:
> >>   [EncryptedExtensions Certifi]
> >>   [cateRequest Certificate Cer]
> >>   [tificateVerify Finished]
> >
> >
> > Yeah, that's how this works in NSS.
>
> To be clear, NSS buffers an entire flight of messages and then sends
> them.  It might fragment things between TCP segments as a result, but
> usually fits everything in a single record (with some exceptions,
> thanks to CertificateRequest being bloated, foor example).  (In DTLS,
> it's more complicated because we have MTU detection, but the same
> basic principle applies.)
>

Yes, this is what I meant to say. Basically, it tries to cram as much as it
can into
one record and the answer to "as much as it can" is "it depends"

-Ekr


Like others, I would find stricter rules around record splits very
> hard to enforce, and for not much gain.
>
___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls

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

2016-11-22 Thread Martin Thomson

On 23 November 2016 at 10:24, Eric Rescorla  wrote:
>>   [EncryptedExtensions Certifi]
>>   [cateRequest Certificate Cer]
>>   [tificateVerify Finished]
>
>
> Yeah, that's how this works in NSS.

To be clear, NSS buffers an entire flight of messages and then sends
them.  It might fragment things between TCP segments as a result, but
usually fits everything in a single record (with some exceptions,
thanks to CertificateRequest being bloated, foor example).  (In DTLS,
it's more complicated because we have MTU detection, but the same
basic principle applies.)

Like others, I would find stricter rules around record splits very
hard to enforce, and for not much gain.

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls

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

2016-11-22 Thread Eric Rescorla

On Tue, Nov 22, 2016 at 2:18 PM, David Benjamin 
wrote:

> On Tue, Nov 22, 2016 at 2:05 PM Olivier Levillain <
> olivier.levill...@ssi.gouv.fr> 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!
>>
>> It is next present in section 4.5.3 (P.64):
>>Handshake messages MUST NOT span key changes.  Because the
>>ServerHello, Finished, and KeyUpdate messages signal a key change,
>>upon receiving these messages a receiver MUST verify that the end of
>>these messages aligns with a record boundary; if not, then it MUST
>>terminate the connection with an "unexpected_message" alert.
>>
>> And then in section 5 (mostly P.64 and P.65) plus a repetition P.69.
>>
>>
>> It is worth noting that this specific point was (at least partially)
>> the origin of several security issues:
>>  - the alert attack (https://www.mitls.org/pages/attacks/Alert);
>>  - Heartbleed (where OpenSSL answered with a fragmented record whereas
>>it was not the spirit of the spec);
>>  - the OpenSSL Downgrade attack in 2014.
>>
>> Some of this is covered in the current specification, but it might be
>> worth being even stricter:
>>  - regarding splitting/merging, they should be forbidden by default;
>>  - the default would apply to alerts (currently, the spec states that
>>alerts MUST not be split, but they should not be merged either
>>for simplicity's sake and since it is useless);
>>
>
> +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.


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

-Ekr

I share your dislike of the way handshake messages and record boundaries
> interact (or, rather, don't), but I'm not sure how this rule helps.
>
>
>> These constraints can be expressed in 5.1 and cover most of the
>> problematic cases.  They are easy to check and to enforce.
>>
>>
>> However, the problem of the OpenSSL Downgrade flaw (where the first
>> record containing the beginning of the ClientHello was cut before the
>> ClientHello.version field) will still stand, but there is no simple
>> way to fix this, since version negotiation can now happen anywhere in
>> the ClientHello extensions, so it might be impossible to enforce that
>> the client versions are sent in the first record.
>>
>> I might volunteer some text if there is an interest of the WG.
>>
>>
>> Olivier Levillain
>>
>> ___
>> TLS mailing list
>> TLS@ietf.org
>> https://www.ietf.org/mailman/listinfo/tls
>>
>
> ___
> TLS mailing list
> TLS@ietf.org
>

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

2016-11-22 Thread David Benjamin

On Tue, Nov 22, 2016 at 2:05 PM Olivier Levillain <
olivier.levill...@ssi.gouv.fr> 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!
>
> It is next present in section 4.5.3 (P.64):
>Handshake messages MUST NOT span key changes.  Because the
>ServerHello, Finished, and KeyUpdate messages signal a key change,
>upon receiving these messages a receiver MUST verify that the end of
>these messages aligns with a record boundary; if not, then it MUST
>terminate the connection with an "unexpected_message" alert.
>
> And then in section 5 (mostly P.64 and P.65) plus a repetition P.69.
>
>
> It is worth noting that this specific point was (at least partially)
> the origin of several security issues:
>  - the alert attack (https://www.mitls.org/pages/attacks/Alert);
>  - Heartbleed (where OpenSSL answered with a fragmented record whereas
>it was not the spirit of the spec);
>  - the OpenSSL Downgrade attack in 2014.
>
> Some of this is covered in the current specification, but it might be
> worth being even stricter:
>  - regarding splitting/merging, they should be forbidden by default;
>  - the default would apply to alerts (currently, the spec states that
>alerts MUST not be split, but they should not be merged either
>for simplicity's sake and since it is useless);
>

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


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

I share your dislike of the way handshake messages and record boundaries
interact (or, rather, don't), but I'm not sure how this rule helps.


> These constraints can be expressed in 5.1 and cover most of the
> problematic cases.  They are easy to check and to enforce.
>
>
> However, the problem of the OpenSSL Downgrade flaw (where the first
> record containing the beginning of the ClientHello was cut before the
> ClientHello.version field) will still stand, but there is no simple
> way to fix this, since version negotiation can now happen anywhere in
> the ClientHello extensions, so it might be impossible to enforce that
> the client versions are sent in the first record.
>
> I might volunteer some text if there is an interest of the WG.
>
>
> Olivier Levillain
>
> ___
> TLS mailing list
> TLS@ietf.org
> https://www.ietf.org/mailman/listinfo/tls
>
___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls

[TLS] Draft 18 review : Message splitting and interleaving

2016-11-22 Thread Olivier Levillain

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!

It is next present in section 4.5.3 (P.64):
   Handshake messages MUST NOT span key changes.  Because the
   ServerHello, Finished, and KeyUpdate messages signal a key change,
   upon receiving these messages a receiver MUST verify that the end of
   these messages aligns with a record boundary; if not, then it MUST
   terminate the connection with an "unexpected_message" alert.

And then in section 5 (mostly P.64 and P.65) plus a repetition P.69.


It is worth noting that this specific point was (at least partially)
the origin of several security issues:
 - the alert attack (https://www.mitls.org/pages/attacks/Alert);
 - Heartbleed (where OpenSSL answered with a fragmented record whereas
   it was not the spirit of the spec);
 - the OpenSSL Downgrade attack in 2014.

Some of this is covered in the current specification, but it might be
worth being even stricter:
 - regarding splitting/merging, they should be forbidden by default;
 - the default would apply to alerts (currently, the spec states that
   alerts MUST not be split, but they should not be merged either
   for simplicity's sake and since it is useless);
 - 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.

These constraints can be expressed in 5.1 and cover most of the
problematic cases.  They are easy to check and to enforce.


However, the problem of the OpenSSL Downgrade flaw (where the first
record containing the beginning of the ClientHello was cut before the
ClientHello.version field) will still stand, but there is no simple
way to fix this, since version negotiation can now happen anywhere in
the ClientHello extensions, so it might be impossible to enforce that
the client versions are sent in the first record.

I might volunteer some text if there is an interest of the WG.


Olivier Levillain

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls

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

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

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

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

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

[TLS] Draft 18 review : Message splitting and interleaving

6 matches

Site Navigation

Mail list logo

Footer information