Re: [TLS] Working Group Last Call for draft-ietf-tls-tls13-18

["Kaduk, Ben" <[e-mail]>]

Unfortunately, Outlook seems to be incapable of properly quoting a message for inline replies, so I will have to top-post with the relevant bits.

I’ll try to put together some text on side-channel resistance along with my pull request for editorial changes.

With respect to psk_key_exchange_modes, PreSharedKey and KeyShare are enough for the existing modes, but my understanding was that the plan with splitting the modes out this way was to allow adding new modes in later documents, including PSK for server authentication.  Can we guarantee that for all future modes we might add, there will be some other extension that indicates what was picked?  (I guess we can probably arrange for that to happen with how we specify the new modes, so it’s not actually a big deal, but might or might not make things more awkward in the future.)

The problematic text about ciphertext length/expansion for tag and other things was in the description of the TLSCiphertext ‘length’ field, which says:

   length  The length (in bytes) of the following
      TLSCiphertext.fragment, which is the sum of the lengths of the
      content and the padding, plus one for the inner content type.  The
      length MUST NOT exceed 2^14 + 256.  An endpoint that receives a
      record that exceeds this length MUST terminate the connection with
      a "record_overflow" alert.

(There is no TLSCiphertext.fragment field.)  Assuming it’s talking about the length of the “opaque encrypted_record[length]” member (which seems obvious), I do not see how the length is the sum of the length of the (plaintext) content and padding and content-type octet, since the AEAD-encryption is supposed to add some amount of expansion.  Probably the answer is to just remove any discussion about what the length represents, since it is not really helping anything; the specification for the AEAD in use will specify the ciphertext length.

On 11/14/16, 10:44, "Eric Rescorla" <[e-mail]> wrote:

    
    
    On Mon, Nov 14, 2016 at 8:54 AM, Kaduk, Ben 
    <[e-mail]> wrote:
    
    I have reviewed this document and have a few minor comments.  I also have many editorial notes that can be addressed out of band.
    
    In the abstract and introduction, we have text about the properties that TLS is expected to provide, like confidentiality, authentication, etc.  Do we want to say anything about avoiding side channels that might leak information about the data being exchanged? 
     I know that we are not 100% confident at what exactly we currently achieve in this area, but since we have mechanisms that attempt to achieve it, maybe it is worth mentioning.  (In a similar vein, as davidben reminded me recently, an attacker “who has complete
     control of the network”, as is our stated adversary, can do things like trickle packets in one by one and try to see, e.g., where the end of early data is and query handshake state.  So some things may not be avoidable.)
    
    
    
    
    I'd be interested in seeing a PR in this area.
    
    
    
    
    
    In section 4.2.5.1 we talk about how for FFDH peers SHOULD validate each other’s public key Y by ensuring that 1 < Y < p-1, which is supposed to ensure that the peer is well-behaved and isn’t forcing the local system into a small subgroup.  Somehow I thought
     that additional checks were needed to avoid being forced into a small subgroup, but I guess that depends on what group it’s in, and I didn’t pull up the RFC 7919 reference when I was reading this document.
    
    
    
    
    These are the recommendations in 7919, I believe
    
    
    
    
    In section 4.2.7, we note that the server MUST NOT send the “psk_key_exchange_modes” extension, with the implication that the client must observer the types of handshake messages that are subsequently received in order to determine what the server selected. 
     I think that we had talked about this already some time ago, but just wanted to confirm that we are okay with increasing the size of the client state machine in this manner.
    
    
    
    
    It's not subsequent messages. You determine it from the PreSharedKey and KeyShare extensions.
    
    
    
    
    In section 4.5.1 we note that when client auth is not used, servers MAY compute the remainder of the client-sent messages for the transcript so as to issue a NewSessionTicket immediately after the server Finished.  Do we want to say anything about why a server
     might wish to do so?
    
    
    
    
    I think I would rather not.
    
    
    
    
    
    The coverage of record payload protection (around section 5.2) seems to not always make careful distinction between TLSPlaintext, TLSCiphertext, TLSInnerPlaintext, and the fields therein.  In some sense this is editorial, but there were a lot of places that
     I flagged, so I wanted to call it out to the broader audience and get more eyes on it.  I also didn’t find a description of where the length of the AEAD authentication tag gets included into a length field for the ciphertext.
    
    
    
    
    I'm not following this point. The way to think about this is that the ciphertext is a specific size. That may be encryption + auth tag or not (it's possible to design an AEAD algorithm that doesn't have a separate auth tag).
    
    
    -Ekr
    
    
    
    At the end of section 6.1 we have this little note that “it is assumed that closing a connection reliably delivers pending data before destroying the transport”, which, if I remember correctly previous discussion on this list, is not actually true for linux. 
     It is perhaps problematic to have an assumption that we know is not going to be held for a large proportion of implementations.
    
    -Ben
    
    _______________________________________________
    TLS mailing list
    [e-mail]
    https://www.ietf.org/mailman/listinfo/tls <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_tls&d=DgMFaQ&c=96ZbZZcaMF4w0F4jpN6LZg&r=sssDLkeEEBWNIXmTsdpw8TZ3tAJx-Job4p1unc7rOhM&m=4W-krMGHeyoUsqPM6uRCNxkjaJzO9tQcda4N2STFkzg&s=pxgTG28tIN2VVwYGzXG5fI2_1SiJEWfPkCBK7RpVmoM&e=>