> It was already mentioned that the “major differences from TLS 1.2” > section should not be a changelog, but I agree with that.
Yes, this is on my plate. > Should Figure 4 (“message flow for a zero round trip handshake”) > include a “+ early_data” for the server’s flight? (The legend for > Figure 4 also lacks the explanation for the ‘+’ symbol.) I see you fixed this. > The language on page 30 is perhaps unclear: > > Because TLS 1.3 forbids renegotiation, if a server receives a > ClientHello at any other time, it MUST terminate the connection. > > Is that any TLS server, or just one that has negotiated and is using > TLS 1.3? The latter. Adjusted. > In the description of legacy_compression_methods on page 31, we make > restrictions on “every TLS 1.3 ClientHello”, but do not say how such > things are identified. (Hmm, maybe we also do so elsewhere in the > document, too, now that I search for where) we explicitly define what > a client “considered to be attempting to negotiate using this > specification (i.e., a TLS 1.3 ClientHEello) on page 87, as > supported_versions including 1.3. Which, is maybe not the most > future-proof thing. I think I feel OK about this. > The description of version negotiation (to populate > ServerHello.version) on page 32 seems to leave undefined what the > server should do when receiving a ClientHello that does not contain a > supported_versions extension. (Also, I don’t think > “ClientHello.supported_versions extension” is a well-defined syntax.) I think this clear in the section on Supported Versions. > When covering the server_random version downgrade sentinels, we do not > mention what is to be done when downgrading to TLS 1.0, which I > thought was still a permitted version by this spec. Interesting point. I'm trying to remember why we did things this way. I am tempted to just say "1.1 or 1.0". Thoughts? > It’s a little odd that we list in enum ExtensionType on page 35 a > strict subset of the extensions enumerated in the table on the > following pages. This got fixed in PR#936. > Do we want to add some commentary about the extant SHA1 collisions > when we say that {rsa_pkcs1,dsa,ecdsa}_sha1 are only SHOULD NOT? Nah. > I’ll note that we define 256 private use ECDHE group code points but > only four such FFDHE group code points. Probably fine, but a bit > surprising. Too late now!! > Should we forbid duplicate entries in > PreSharedKeyExtension.identities? I don't think that's necessary. > Conversely, we might want to explicitly say that duplicate > OIDFilter.certificate_extension_oid fields should be expected in > OIDFilterExtensions, to enable the case where multiple values must be > present. Or is that supposed to work by concatenating(?) the multiple > values’ DER encodings in the certificate_extension_values field? Yeah, I read this text as saying that those all go in the same DER, not that there can be multiple copies > I’ll call out for Russ’s attention at the end of Section 4.4.3 where > we say that “implementations MUST NOT combine external PSKs with > certificate-based authentication.” Is there any reason not to qualify > that as some sort of “don’t’ do it until it’s defined”? I'm actually going to move and modify this section per your issue #935. > Should Alert.level be Alert.legacy_level? i think we went over this and decided not to. > Appendix B has a claim that “values listed as _RESERVED were used in > previous versions of TLS and are listed here for completeness”, though > that is not exactly true, e.g., for ContentType.invalid_RESERVED(0) I see you fixed this as well. > Section C.3 notes that “Certificates should always be verified to > ensure proper signing by a trusted Certificate Authority”, which does > not use RFC 2119 language, but might be seen as in conflict with > opportunistic encryption in some circumstances. I don’t object to > this text, but it seems worth mentioning. I think the "Absent a specific..." > Page 113 still has the “[[NOTE: TLS 1.3 needs a new channel binding > definition that has not yet been defined.]]”, which should not make it > into the final spec! Fixed. On Mon, Mar 27, 2017 at 11:23 PM, Kaduk, Ben <bka...@akamai.com> wrote: > On 3/13/17, 12:30, "Sean Turner" <s...@sn3rd.com> wrote: > > This is a working group last call announcement for > draft-ietf-tls-tls13-19, to run through March 27. Please send your reviews > to the list as soon as possible so we can prepare for any discussion of > open issues at IETF 98 in Chicago. > > As the price of running the WGLC right during the meeting lead-up, my > review comes in at the last minute. > > Generally, it is in good shape. I think I still owe some text about what > we aim for and expect to achieve with respect to side channel resistance, > though at this point it may be too late to get that text in :( > > The following is basically a laundry list of the minor issues; I will send > editorial notes under separate cover, probably as a pull request. > > It was already mentioned that the “major differences from TLS 1.2” section > should not be a changelog, but I agree with that. > > Should Figure 4 (“message flow for a zero round trip handshake”) include a > “+ early_data” for the server’s flight? (The legend for Figure 4 also > lacks the explanation for the ‘+’ symbol.) > > The language on page 30 is perhaps unclear: > > Because TLS 1.3 forbids renegotiation, if a server receives a > ClientHello at any other time, it MUST terminate the connection. > > Is that any TLS server, or just one that has negotiated and is using TLS > 1.3? > > In the description of legacy_compression_methods on page 31, we make > restrictions on “every TLS 1.3 ClientHello”, but do not say how such things > are identified. (Hmm, maybe we also do so elsewhere in the document, too, > now that I search for where) we explicitly define what a client > “considered to be attempting to negotiate using this specification (i.e., a > TLS 1.3 ClientHEello) on page 87, as supported_versions including 1.3. > Which, is maybe not the most future-proof thing. > > The description of version negotiation (to populate ServerHello.version) > on page 32 seems to leave undefined what the server should do when > receiving a ClientHello that does not contain a supported_versions > extension. (Also, I don’t think “ClientHello.supported_versions > extension” is a well-defined syntax.) > > When covering the server_random version downgrade sentinels, we do not > mention what is to be done when downgrading to TLS 1.0, which I thought was > still a permitted version by this spec. > > It’s a little odd that we list in enum ExtensionType on page 35 a strict > subset of the extensions enumerated in the table on the following pages. > > Do we want to add some commentary about the extant SHA1 collisions when we > say that {rsa_pkcs1,dsa,ecdsa}_sha1 are only SHOULD NOT? > > I’ll note that we define 256 private use ECDHE group code points but only > four such FFDHE group code points. Probably fine, but a bit surprising. > > Should we forbid duplicate entries in PreSharedKeyExtension.identities? > > Conversely, we might want to explicitly say that duplicate > OIDFilter.certificate_extension_oid fields should be expected in > OIDFilterExtensions, to enable the case where multiple values must be > present. Or is that supposed to work by concatenating(?) the multiple > values’ DER encodings in the certificate_extension_values field? > > I’ll call out for Russ’s attention at the end of Section 4.4.3 where we > say that “implementations MUST NOT combine external PSKs with > certificate-based authentication.” Is there any reason not to qualify that > as some sort of “don’t’ do it until it’s defined”? > > Should Alert.level be Alert.legacy_level? > > The editors copy has already removed the reference to RFC 4507, which is > obsoleted by RFC 5077 (and was not cited anywhere, anyway). > > Appendix B has a claim that “values listed as _RESERVED were used in > previous versions of TLS and are listed here for completeness”, though that > is not exactly true, e.g., for ContentType.invalid_RESERVED(0) > > Section C.3 notes that “Certificates should always be verified to ensure > proper signing by a trusted Certificate Authority”, which does not use RFC > 2119 language, but might be seen as in conflict with opportunistic > encryption in some circumstances. I don’t object to this text, but it > seems worth mentioning. > > Page 113 still has the “[[NOTE: TLS 1.3 needs a new channel binding > definition that has not yet been defined.]]”, which should not make it into > the final spec! > > -Ben > > _______________________________________________ > TLS mailing list > TLS@ietf.org > https://www.ietf.org/mailman/listinfo/tls >
_______________________________________________ TLS mailing list TLS@ietf.org https://www.ietf.org/mailman/listinfo/tls