On Monday, 5 September 2016 11:02:57 CEST Eric Rescorla wrote: > PR: https://github.com/tlswg/tls13-spec/pull/625
Finally found some time to take a look at it. So in general I like the change to "abort the handshake with a <type> alert", the "send a fatal <type> alert and close the connection" was a bit clunky. I was actually planning to do something like this myself. Few other items carried over from my proposed PR[1]: --- I still think we should include generic advice for handling parser errors in the parser (Presentation Language/Vectors and Constructed Types) section; something like this: Peers that receive a encoding of a vector in a message with a length that does not match specification MUST abort the connection with a "decode_error" alert unless more specific section states otherwise. and this: Peers that receive messages that doesn't match the expected constructed type in expected message MUST abort the connection with a "decode_error" alert unless more specific section states otherwise. --- I think it's a bit confusing to have this sentence: Servers MUST select the lower of the highest supported server version and the version offered by the client in the ClientHello. In particular, servers MUST accept ClientHello messages with versions higher than those supported and negotiate the highest mutually supported version. in the Server Hello section. Everything before the very last "and" is more applicable to ClientHello. --- I think that this sentence in Handshake Protocol: Sending handshake messages in an unexpected order results in an "unexpected_message" fatal error. should use a MUST and be prescribed to the receiving side, for example: Peer that receives handshake messages in unexpected order MUST abort the handshake with an "unexpected_message" alert. --- Description what should client do when the ServerHello.cipher_suite is the one it did not advertise in ClientHello.cipher_suites is missing. So is description of how to handle ServerHello.version being higher than ClientHello.max_supported_version. At least, I don't see it in Server Hello section. --- missing_extension description in Alert Protocol probably should be extended from: Sent by endpoints that receive a hello message not containing an extension that is mandatory to send for the offered TLS version. to: Sent by endpoints that receive a hello message not containing an extension that is mandatory to send for the offered TLS version or the offered key exchange mechanisms. (thinking of handling situations where "key_share" or "pre_shared_key" is missing) --- Hello Extensions section doesn't specify how to handle messages with duplicate extensions (e.g. second "key_share" extension added in the second ClientHello message in connection) --- Signature Algorithms doesn't say how to handle RSA-SSA signatures with salt length that doesn't match hash size. --- Key Share doesn't say how to handle server_share that doesn't match client_shares. --- Finished section doesn't describe what to do if the contents don't match the expected value. Should it be illegal_parameter or bad_record_mac is more appropriate? --- Record Layer doesn't describe what to do if a record with zero-length payload and handshake or alert type is received. --- Record Layer includes legacy_record_version : (...) This field is deprecated and MUST be ignored for all purposes. Record Layer Protection does not. --- 1 - https://github.com/tlswg/tls13-spec/pull/621 -- Regards, Hubert Kario Senior Quality Engineer, QE BaseOS Security team Web: www.cz.redhat.com Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno, Czech Republic
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ TLS mailing list TLS@ietf.org https://www.ietf.org/mailman/listinfo/tls
