Hello! I read the current specification and I have some feedback on it. I hope this can be useful. Sorry for the length or if I point to obvious things. Here is a link as well for readability: https://gist.github.com/mimoo/b6c64a133cd85da53f2f744dcdb39839
1. *1-Byte Length of Vectors*: One edge-case that is implied and not discussed in the Presentation Language section <https://tlswg.github.io/tls13-spec/#presentation-language> is: "do I need to write the length of a vector<0...255>?". It seems logical that it will always be 1 byte and that it might be an implicit length. It would be nice to have this edge case mentioned or say something along the lines "there are no exceptions when encoding the length of variable-length vectors". 2. *Lack of repetition*: The intro diagram <https://tlswg.github.io/tls13-spec/#rfc.section.2> tells you what messages are encrypted with what key. But after that, no reminders are present. For example, look at the Finished message. How do you know that this has to be encrypted with the handshake key? It is not written in the Finished message <https://tlswg.github.io/tls13-spec/#rfc.section.4.4.3> section, it is not written in the authentication messages <https://tlswg.github.io/tls13-spec/#rfc.section.4.4> parent section and it is not written in the handshake protocol <https://tlswg.github.io/tls13-spec/#rfc.section.4> parent-parent section either. Only in the overview section. 3. *How to define application data in an application data message*: It is not clear how "real" application data should appear in an application data message. Encrypted handshakes and alerts are clearly defined, but it takes some time to understand that "real" application data is written -- without structure or metadata -- into the content field of a TLSInnerPlaintext message. The current definition is "content: The cleartext of TLSPlaintext.fragment". I think a better one would be "content: the byte encoding of a handshake or alert message, or the raw bytes of the application's data to send". 4. *Unencrypted Application Data*: Does it say somewhere in the specification that real application data should never appear in a TLSPlaintext message? At the moment the application_data content type is present in the declaration of a TLSPlaintext structure, see the record layer section <https://tlswg.github.io/tls13-spec/#record-layer> 5. *What alert to use*: Sometimes it is not mentioned what level of alert should be used. For example: "Because TLS 1.3 forbids renegotiation, if a server receives a ClientHello at any other time, it MUST terminate the connection". I'm guessing it should be a fatal alert, but this is open to interpretation. 6. *Why force the signature algorithm extension*: If the client does not send a signature algorithm extension, why would the server abort the handshake? It would make more sense to continue with the server's prefered certificate. Especially since currently, most webservers only possess one kind of certificate anyway. 7. *Resumption_secret*: The Key Schedule <https://tlswg.github.io/tls13-spec/#rfc.section.7.1> section mentions a resumption_secret but nowhere in the spec does this keyword re-appear. It should be mentioned somewhere that this is the secret to be used as PSK in a future resumption handshake. 8. *Auth_modes*: there is a single mention of "auth_modes" in the Pre-Shared Key Extension <https://tlswg.github.io/tls13-spec/#pre-shared-key-extension> section. No definition of this variable is to be found. 9. *Computing Finished keys*: The section on how to compute Finished keys <https://tlswg.github.io/tls13-spec/#rfc.section.4.4.3> should be in the cryptographic computation <https://tlswg.github.io/tls13-spec/#cryptographic-computations> section in my opinion. It should probably be added to the key schedule diagram as well. 10. *Computation of the Binders*: From the relevant section <https://tlswg.github.io/tls13-spec/#psk-binder>: "*Each entry in the binders list is computed as an HMAC over the portion of the ClientHello up to and including the PreSharedKeyExtension.identities field. That is, it includes all of the ClientHello but not the binder list itself. The length fields for the message (including the overall length, the length of the extensions block, and the length of the “pre_shared_key” extension) are all set as if the binder were present*". There are several things that are unclear to me and I think they should be mentioned here: the fake binder should be of size hash.size, it should also say that the HMAC is computed over the handshake message containing the client hello, not only the client hello structure. 11. *What binder key to use*: It is not clear which binder are to be used when doing a session resumption. The previous handshake one or the new handshake one? The confusion comes from the fact that the old handshake's resumption_secret/PSK is to be used in the new handshake, it could be thought that the old handshake's binder key should be the one used in the new handshake as well. I think it would help if this resumption_secret is called something like next_handshake.resumption_secret. Or make it clearer from which key schedule the binder key comes from. 12. *PSK and Cipher Suite*: I think the following fact should be emphasized in more places: the client needs to advertise the single cipher suite they previously used when doing session resumption. It could be added to the cipher_suite definition of the client hello section <https://tlswg.github.io/tls13-spec/#client-hello> as well as in the overview of session resumption <https://tlswg.github.io/tls13-spec/#resumption-and-psk>. 13. *Implicit Lengths in The Presentation Language*: nested structures + implicit lengths = trouble. It's easy to forget parsing/writing all the different "sizes" when they are not clearly stated in the spec. Why not just having a length field in front of these variable-length vectors? It would add a bit of overhead but would make things clearer in my opinion. 14. *No Help For The Implementer*: If you look at the Noise Protocol Framework <http://www.noiseprotocol.org/noise.html>, you can clearly see that the specifications were written with implementers in mind: several states, and objects are defined. For example, one way of implementing TLS is to have some sort of complex state that will remember what state you are in the handshake, what is the cryptographic context, etc... here the implementer has to decide everything for himself. 15. *Whitelist Of State Transitions*: There is nothing about whitelisting state transitions. In my opinion there should be a list of allowed state transitions that the implementer could use. I made one here, I suspect it is still missing some states <https://gist.github.com/mimoo/779dcf8c44d80a2a34a1a2f2ed620711>. 16. *How to parse extensions*: I think that IANA's consideration <https://tlswg.github.io/tls13-spec/#iana-considerations> section's table would be useful to have in the earlier Extension section <https://tlswg.github.io/tls13-spec/#extensions> (probably only the extensions included in the RFC should be listed). Another useful pragraph: how to parse these extensions, making sure that different messages come with their own whitelist of extensions. 17. *Language <-> Bytes Translation*: There are no real introduction/no clear section about this. I understand that everything is in the Presentation Language <https://tlswg.github.io/tls13-spec/#presentation-language> section but for someone who discovers the specification it takes some time to find out that this section is about both the presentation language and its byte encoding. A better introduction in this section as well as an emphasis on the byte representation in each of the subsections of this section would in my opinion benefit the whole document. 18. *Asymmetrical MUSTs*: some "MUST" keywords are not symmetrical. What is the point of them? For example, the spec says that as a client, you MUST use a unique 0x0303 value in the legacy_version field of the client hello <https://tlswg.github.io/tls13-spec/#client-hello>. But if you do not, there are no problems from the server side in the specification and the client hello will still be well received. (Is this maybe for forward compatibility?) And finally, here are some minor nitpicks: 1. *PSK extension*: *debatable*, but I would rather have the PSK section <https://tlswg.github.io/tls13-spec/#pre-shared-key-extension> at the very end of the Extensions section <https://tlswg.github.io/tls13-spec/#extensions> as it MUST be the final extension of a client hello. 2. *No Indication Of Encryption*: This is *debatable*, but I find it inelegant that there are no indication in a record that it is encrypted. The implementation is supposed to keep track of it. 3. *What PSK mode does a server support*: I think the server should indicate what mode (psk_dh_ke or psk_ke) it supports in a server_hello.psk_mode extension or in the new_session_ticket message. Like that the client would know if it has to send a key_share or not in the resumption. Right now, the only indication of a psk_dh_ke is that the server will send a key_share even when presented with a PSK. *this is debatable*. 4. *The Key Derivation*: The key derivation section is the least clear section in this specification. The diagrams kind of help, but are limited by the RFC format... I have nothing else to say here, the whole section is confusing to say the least. No solution offered :o)
_______________________________________________ TLS mailing list TLS@ietf.org https://www.ietf.org/mailman/listinfo/tls
