On Tue, Nov 22, 2016 at 2:18 PM, David Benjamin <david...@chromium.org> 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 > https://www.ietf.org/mailman/listinfo/tls > >
_______________________________________________ TLS mailing list TLS@ietf.org https://www.ietf.org/mailman/listinfo/tls
