On 04/11/2017 12:32 PM, Eric Rescorla wrote: > > 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. >
I guess I didn't consult back with what I had put in this mail when preparing my editorial-changes pull request :) > > > 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. > (For the gallery, there were some tweaks in this area in #936) > > > 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. > It looks like this changed a little via #936 as well; I'm fine with your followup change there. > > > 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? > That's probably fine; I expect there is some additional attack in there where an attacker could force 1.0 if 1.1 would otherwise have been used, but both of those are not in a great place right now, so we don't need to try too hard to help them out. > > > > 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 > Okay. > > > 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. > Yeah, I missed the bits I called out in #935 when I was doing my WGLC review, but the two are related and can be handled together. > > > Should Alert.level be Alert.legacy_level? > > i think we went over this and decided not to. > There was a pull request from not-me, yes. Though IIRC it only changed the name locally when describing alerts, and was rejected on the grounds that "we don't use the legacy_level version for this anywhere else in the spec", which is a little bit of circular reasoning. I'm okay with leaving it as-is. > > > 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. > Well, maybe. ContentType.invalid is the only one that I marked up in red pen on my paper copy, but I can't certify that I compared the entire list against the IANA registry. I did look at the extensions registry when preparing #936, though. > > > 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..." > > Yeah, I guess I snuck that fix into #936. So much for keeping things separate... > > 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. > It looks like that was just by removing the note. Is a channel binding spec for 1.3 still a needed work item, then? -Ben > > On Mon, Mar 27, 2017 at 11:23 PM, Kaduk, Ben <bka...@akamai.com > <mailto:bka...@akamai.com>> wrote: > > On 3/13/17, 12:30, "Sean Turner" <s...@sn3rd.com > <mailto: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 <mailto:TLS@ietf.org> > https://www.ietf.org/mailman/listinfo/tls > > <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_tls&d=DwMFaQ&c=96ZbZZcaMF4w0F4jpN6LZg&r=sssDLkeEEBWNIXmTsdpw8TZ3tAJx-Job4p1unc7rOhM&m=zDb_mbfXWxowypc9E4E6zZZ_lXTab2DcV9qm--twWoM&s=WD0bv4QMm5OHI8RqolDFScW-e1jMk-YNzkVmbC4cmEw&e=> > >
_______________________________________________ TLS mailing list TLS@ietf.org https://www.ietf.org/mailman/listinfo/tls
