Re: [TLS] Alexey Melnikov's Discuss on draft-ietf-tls-tls13-26: (with DISCUSS and COMMENT)

[Alexey Melnikov <[e-mail]>]

Hi Ekr,

On Wed, Mar 7, 2018, at 2:16 PM, Eric Rescorla wrote:
> 
> 
> On Wed, Mar 7, 2018 at 5:29 AM, Alexey Melnikov
> <[e-mail]> wrote:>> Alexey Melnikov has entered the following ballot position for
>>  draft-ietf-tls-tls13-26: Discuss
>> 
>>  When responding, please keep the subject line intact and reply
>>  to all>>  email addresses included in the To and CC lines. (Feel free to
>>  cut this>>  introductory paragraph, however.)
>> 
>> 
>>  Please refer to
>>  https://www.ietf.org/iesg/statement/discuss-criteria.html>>  for more information about IESG DISCUSS and COMMENT positions.
>> 
>> 
>>  The document, along with other ballot positions, can be found here:>> https://datatracker.ietf.org/doc/draft-ietf-tls-tls13/
>> 
>> 
>> 
>>  ----------------------------------------------------------------
>>  ------>>  DISCUSS:
>>  ----------------------------------------------------------------
>>  ------>> 
>>  Thank you for a well written document. I will be switching to Yes
>>  once the>>  following is addressed/discussed:
>> 
>>  Relationship to TLS 1.2 needs to be clarified. The document is
>>  adding>>  requirements on TLS 1.2 clients. Implementors of TLS 1.2 are not
>>  going to (or>>  very unlikely to) read this document. This looks fishy to me. Two
>>  examples on>>  page 37:
>> 
>>    TLS 1.2 clients SHOULD also check that the last eight bytes  are
>>    not equal to>>    the second value if the ServerHello indicates TLS  1.1 or below
>> 
>>  and
>> 
>>   A legacy TLS client performing renegotiation with TLS 1.2 or prior
>>   and which>>   receives a TLS 1.3 ServerHello during renegotiation MUST  abort the
>>   handshake>>   with a "protocol_version" alert.  Note that  renegotiation is not
>>   possible>>   when TLS 1.3 has been negotiated.
>> 
>>  There are similar statements on page 45:
>> 
>>    TLS 1.2 implementations SHOULD also process this extension.
>> 
>>  and on page 48:
>> 
>>    However, the old semantics did not constrain the signing
>>    curve.  If TLS 1.2 is negotiated, implementations MUST be prepared>>    to accept a signature that uses any curve that they advertised in>>    the "supported_groups" extension.
>> 
>>  I think you need to clarify whether these normative requirements
>>  apply to pure>>  TLS 1.2 clients or TLS clients that implement both 1.2 and 1.3 and
>>  choose to>>  use 1.2 for some reason. Or maybe you need to say in the
>>  Abstract/Introduction>>  that although this document obsoletes TLS 1.2 it also specifies new>>  requirements on TLS 1.2 implementations. (So it is sort of both
>>  "Obsoletes" and>>  "Updates" TLS 1.2 RFC.)
> 
> 
> The intent is that these affect old TLS 1.2 implementations as well. S
> 1.4 tries> to be clear about this, but maybe it fails.
> 
> I suggest we:
> 
> (1) Add the following sentence to the abstract:
> "This document also specifies new requirements for TLS 1.2
> implementations.> 
> (2) Rewrite the first sentence of S 1.4 to say:
>    This document defines several changes that optionally affect
>    implementations of TLS 1.2, including those which do not also
>    support TLS 1.3
> 
> (3) Strike the following graf:
> 
>    An implementation of TLS 1.3 that also supports TLS 1.2 might
>    need to>    include changes to support these changes even when TLS 1.3 is
>    not in>    use.  See the referenced sections for more details.
I think this will address my concern.

>   
>> ---------------------------------------------------------------
>> ------->>  COMMENT:
>>  ----------------------------------------------------------------
>>  ------>> 
>>  1) On page 47:
>>   The length of the salt MUST be equal to the length of the digest
>>   algorithm>> 
>>  Length of algorithm?
> 
> Right. Output of.
> 
>  
>> 
>> 2) DER need a Normative Reference on first use. There are some
>>    references on>>  2nd/3rd use.
> 
> Thanks.
>  
>> 
>> 3) On page 57:
>> 
>>     The
>>     server then ignores early data by attempting to decrypt received>>     records in the handshake traffic keys until it is able to receive>>     the client's second flight and complete an ordinary 1-RTT
>>     handshake, skipping records that fail to decrypt, up to the
>>     configured max_early_data_size.
>> 
>>  I read this several times and still don't understand what this is
>>  saying. It is>>  saying "ignores ... until it is able to receive". I think you either
>>  don't mean>>  "ignore" (as in discard the rest) or I misunderstood. I clarifying
>>  example or a>>  reference to another section (e.g. with the diagram) would be very
>>  helpful here.> 
> We do mean discard. The idea here is that you try to decrypt
> those records> using the handshake keys and if that fails you ignore them.

Ok. So I suggest use "discard" and possibly delete or reword the "untill
it is able" part. Maybe split into 2 sentences?
> 
>> 
>> 4) On page 82:
>> 
>>     When record protection has not yet been engaged, TLSPlaintext
>>     structures>>     are written directly onto the wire.  Once record  protection has
>>     started,>>     TLSPlaintext records are protected and sent   as described in the
>>     following>>     section
>> 
>>  Just to double check: are you saying that before the handshake TLS
>>  application>>  layer effectively results in plain text messages (with some extra
>>  octets to>>  signal record type)?
> 
> No, you can't write application data prior to this point, as
> stated in S 2.> 
>    Application data MUST NOT be sent prior to sending the Finished
>    message and until the record layer starts using encryption keys.
>    Note that while the server may send application data prior to
>    receiving the client's Authentication messages, any data sent
>    at that>    point is, of course, being sent to an unauthenticated peer.
> 
> It's non-application data (handshake, alerts, acks) which is sent in
> this fashion> prior to the handshake.

Ok. Maybe say that in the document.
> 
>> 5) I am pretty sure that [RFC5116] is a Normative reference. It is
>>    required to>>  be understood to implemented TLS 1.3. Also, you have additional
>>  requirements on>>  AEADs, which again implies understanding of what they are:
>> 
>>  On page 84:
>> 
>>     An AEAD algorithm used in TLS 1.3 MUST NOT produce an expansion
>>     greater>>     than 255 octets
>> 
>>  and
>> 
>>     An AEAD algorithm where N_MAX is less than 8  bytes MUST NOT be
>>     used with TLS> 
> I concur. Thanks.
> 
> 
> 
>> 6) The diagram in section 7.1 was a bit cryptic. Maybe explain that
>>    when you>>  use '0' you mean as many bytes of 0s as needed for various
>>  functions.> 
> This is in the first paragraph after the diagram. Would you prefer it
> elsewhere?> 
>    If a given secret is not available, then the 0-value
>    consisting of a>    string of Hash.length bytes set to zeros is used.  Note that this

Maybe it was lack of sleep, but I didn't understand that you were
talking about the same thing. This also seems to apply more generically
then on the diagram. I suggest clarify:
  In the diagram, the value '0' denotes a string of Hash.length bytes
  set to zeros.
Or something like that.