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. = Minor remarks = - P.14 (section 2): "Everything after this phase is encrypted" => "is protected" (since we also have integrity protection). - P.14 (section 2) and P.20 (section 2.3): the psk_key_exchange_modes extension is called "pre_shared_key_modes". - P.21 (section 2.3): Add a reference to the relevant section for "early exporter secret"? - P.41 and 42 (4.2.5): The fact that clients can send an empty client_shares vector is repeated 30 lines apart. Is it really needed? - P.43 (section 4.2.5.2): during the description of ECDHE parameters with secp*, the paragraph Although X9.62 supports multiple point formats, any given curve MUST specify only a single point format. All curves currently specified in this document MUST only be used with the uncompressed point format (the format for all ECDH functions is considered uncompressed). seems to imply that all ECDHE points are uncompressed, which is not the case. I would replace "All curves currently specified in this document" with "secp256r1, secp384r1 and secp521r1 curves". Similarly, I would remove the parenthesis. - P.44 (4.2.6): Unless I am mistaken, the identity field in the extension is the "ticket" (when using the older terminology). However, the term ticket is used in different places afterwards, without a proper definition. It might make sense to add a word in the identities definition. - P.44 (4.2.6): Similarly, it would be clearer to say "externally established PSKs" (as in P.45) instead of "external tickets". - P.44 (4.2.6) Similarly, instead of saying that a binder must be given "for each PSK offered", it might be clearer to say "for each PSK identity offered". (Sorry for th nitpick, but I believe it is important to be crystal clear in such a document). - P.46 (4.2.7): "Servers MUST NOT select a key exchange mode that is not listed by the client". As I understand it, the server advertises its "selection" by adding the keyshare extension or not. It might be useful to add this precision if I am not mistaken. - P.48 (4.2.8): "the server MUST have accepted a PSK ciphersuite" => since PSK ciphersuites do not exist anymore, should not it be "have accepted to use a PSK key exchange mode" or "have answered with a psk extension"? - P.53 (4.4): The Certificate message definition contains the following text: "Note that certificate-based client authentication is not available in the 0-RTT case." It might make more sense to move this text elsewhere in the section, and to explain it further: does it mean the 1-RTT following the early data must not contain a client authentication? - P.53 (4.4): What are "the pure 1-RTT cases"? - P.54 (4.4.1): Since the specification does not force the certificates to be ordered in the Certificate message anymore, what is the point of adding a SHOULD about the order? On the other hand, the text might include the requirement that all intermediate AC are to be provided in the Certificate message. - P. 55 (4.4.1.1): Since it is now possible to add per-certificate extension, I believe status_request_v2 does not make any sense now. Should not it be clearer to deprecate it with TLS 1.3 and to force multiple OCSP stapling to be made exclusively with multiple status_request extensions (one for each cert)? - P.56 (4.4.1.2): The text "As servers MAY require the presence of the "server_name" extension, clients SHOULD send this extension, when applicable." should be moved elsewhere (e.g. when discussing ClientHello). - It is not clear whether a client can authenticate with a certificate when the server has chosen PSK authentication. P.49 (4.3.2) states that "A server which is authenticating with a certificate can optionally request a certificate from the client." whereas 4.4.2 (P.59) states that "When used with non-certificate-based handshakes (e.g., PSK), the client's signature [in CertificateVerify] does not cover the server's certificate directly." How should this be interpreted? - P.60 (4.4), it is said that "The key used to compute the finished message is computed from the Base key defined in Section 4.4 using HKDF", but we are *in* section 4.4, so there must be a mistake? - P.61 (4.5.1), it is said that "the resumption master secret depends on the client's second flight" but this is only true when the client authenticates itself with a certificate. This point is related to a point sent by Ben Kaduk (https://www.ietf.org/mail-archive/web/tls/current/msg21829.html): "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?" (https://www.ietf.org/mail-archive/web/tls/current/msg21829.html). to which Ekr answered with "I think I would rather not." - P.63 (4.5.3): The definition of the "request_update" field is unclear. It is said to "indicate that the recipient of [...] should respond". Should not it say "Indicates *whether* the recipient" - P.65 (5.1): "Endpoints supporting other versions" => "earlier"? - P.66 (5.2): the content type if the TLS InnerPlaintext must be non null for the padding to work. This is correctly stated in the end of the specification where 0 is invalid_RESERVED, but it might be useful to add a statement in the type field definition: "Valid content types all have non-null values." - P.67 (5.2): The exact same text is repeated twice in the page: "An endpoint that receives a record that exceeds this length MUST terminate the connection with a "record_overflow" alert." - P.81 (8.1): RSA signature schemes are specified to be MTI for certificates and optionally for CertificateVerify. The precision should also be added for ecdsa_secp256r1-sha256. - P.83 and 85 (8.2): "Client" is supposed to mean that "the server shall not send them". Yet, the cookie extension is marked as Clear, and can be sent in the HRR message. - P.84 (8.2): Since it is said afterwards that truncated_hmac is a dangerous extension, it might be logical to mark it as *not* recommended? - P.84 and 85: I suppose that the order of the extensions in this table is given by the extension id. Yet it might be more practical to sort the table by extension name? - P.105 (B.4): "Do you verify that the RSA padding doesn't have additional data after the hash value? [FI06]" => the attack was about PKCS#1 v1.5, not PSS. Does it still apply, or should the text be amended? - P.105 (B.4): I believe the part about ECDSA "k" parameter states opposing statements, since deterministic ECDSA leads to a non-random k: Do you use a strong and, most importantly, properly seeded random number generator (see Appendix B.2) when generating Diffie-Hellman private values, the ECDSA "k" parameter, and other security- critical values? It is RECOMMENDED that implementations implement "deterministic ECDSA" as specified in [RFC6979]. Would not it be more logical to say: "Alternatively, it is RECOMMENDED..." - P.113 (D.2): There are missing references concerning the analysis of the TLS record layer = Typos = - P.28 (section 4.1.1): a quote is missing after '"psk_key_exchange_modes' - P.46 (4.2.7): "A clients MUST provide" => "client" - P.54 (4.4.1): "If an extension applies the the entire chain" => "to the" - P.58 (4.4.2): "A single 0 byte which servers as the separator" => "serves" - P.63 (4.5.3): "If the request_udate" => request_update - P.86 (8.2): "and the TLS SignatureAlgorithm Registry to list values 4-233 as "Reserved"" => It should be 223 (as 224-255 are reserved for private use) Olivier Levillain _______________________________________________ TLS mailing list TLS@ietf.org https://www.ietf.org/mailman/listinfo/tls