Closed ekr closed 7 years ago
@ilaril I think I have dealt with most of these.
On Sat, Sep 03, 2016 at 12:49:39PM -0700, ekr wrote:
@ilaril I think I have dealt with most of these.
- You say "Later the document seems to imply that the limit for for content+type+ padding is in fact 16385 bytes." If you point out where I will fix it.
Probably just me misreading the 255 byte limit for expansion from protection and the 16384+1+255 formula for the 16384+256 byte limit.
Also, what's the protected limit if max_fragment_length is in effect? 256 bytes above the plaintext limit (so e.g. 768 bytes for 512 byte plaintext limit)?
- I don't think max_fragment_length should go in ServerHello because the client doesn't need it to interpret messages from the server.
Actually, probably OK to keep it until EE if included at all because the strictest limit one can apply is 512 octets, and the maximum size of ServerHello is (there is protection change afterwards, so one can't combine the mesage):
This is 56 bytes, plus key share data, leaving at least 456 bytes for the key share. Now, the only DHFs that exceed that are FFDH4k, FFDH6k and FFDH8k (and possible PQ exchanges).
And any device that is puny enough to want 512-byte fragments is very unlikely to support any of those.
- If you want to provide a PR about RSA in S 8.5 that would be useful.
The first length byte thing?
Actually, that's a special case of doing RSA PKCS#1 v1.5 (CA certs) verification by doing RSA op and then decoding the result, instead of the proper way, which is to do RSA op, encode the expected result and then compare.
-Ilari
Closing this as Ilari did a new review. @ilaril please let me know if there's anything that got missed.
Copying here so they don't get lost.
On Mon, Jul 11, 2016 at 12:08:00PM -0700, Eric Rescorla wrote:
Did a readthrough, here's a bunch of comments (didn't check the issues list):
One should note that it is not clear what sort of continuity properties should be provoded:
The last choice obviously limits lifetime of derived PSK to the parent certificate.
Also, extension matching is quite different between those three (and the last two are rather different from the current rules, tuned for 0-RTT only being used with PSK.
Well, I think the DH parameters are gone, but DH public keys are still there, and those AFAIK are padded with zeroes.
Should the "ServerHello" be "HelloRetryRequest"?
Also, adding cookie extension if that was present in HRR?
BTW, with the early_data context and cross-connection cookies gone, I have figured out how to delta-compress the first client_hello and its rejection against to-be-sent second client hello plus connection parameters server selected into 6 bytes(!) of space (not including return-reachability check or MAC).
Isn't this the true ciphersuite used on this connection, "resumption" or not? Otherwise you can get into all sorts of crazy situations that WILL be sources of implementation bugs.
The idea that it isn't true ciphersuite brings me bad flashbacks about TLS 1.2 ticket "maybe resume" craziness (except this would be even worse).
Hasn't early_data just moved to EncryptedExtensions?
Hello Retry Request
What is written into this field if server selects pure-PSK ciphersuite and then decides to reject the ClientHello? Or connections that use pure-PSK just plain can't be rejected for any reason (including IP address verification in DTLS?)
Currently assuming pure-PSK rejections are possible, cipher_suite is needed to tell if the next group field contains anything sane or not...
What about cookie? IIRC, it is allowed to be in HRR even without being in CH...
Again, rejections with cookie aren't spurious, and the cookie needs to be added.
Hello Extensions
Cookie in HRR might not follow that....
I would say that it is a bad idea for any extension to do anything special with "resumption" without very good reasons.
In fact, the only non-connection-control extension that is relevant for PSK I know is server_name.
Now, ALPN (connection-control by definition!) behaves specially with early_data for darn good reasons (the ALP is locked).
This is made worse by the fact that what consistutes "resumption" is not clearly defined (the ALPN rules are related to early_data, not choosing PSK #0!).
Also, for interoperability, any legal TLS 1.3 meaning of these algorithms must be extended to apply to TLS 1.2, even for TLS 1.2 ClientHello. Anything else would be pointless trap.
This impiles that RSA PSS and EdDSA work in TLS 1.2 and also how those work.
Also, even if meaning of the ECDSA codepoints changed, those would not break that rule, since new definition is strict subset of old.
Are those supposed to be filtered to be subset of ones client advertised or not? E.g. if client didn't indicate support for x448, can the server still send x448?
Doesn't seem to consistent with the other descriptions of this and pre_shared_key (those are just "MUST NOT select covered ciphersuite if missing"). Of course, in practice, it might be more this (at least that's how I would implement it)...
Bit crazy algorithm: If you haven't heard of this server before, pick smallest you support, if you have, pick the one it selected the last time.
The temporal ordering in messages is actually selecting different PSK and then rejecting 0-RTT.
And the addition also prevents correlating session with its parent, even in case of reuse (this was the reason for switching to addition from XOR).
Good luck with that...
Also, requirement that the server MUST proceed with the handshake and reject 0-RTT if that validation fails would be good here...
This does not talk about if end_of_early_data alert is encrypted or not (obviously encrypted one won't work if server rejected 0-RTT, unless server trial-decrypts).
Oh, still trial decryption... Got it.
I think there was some proposal to dump the Finished. Would simplify implementation. In TLS, there is at least one appdata record with its MAC to be deprotected. That's not true in DTLS.
However, even in DTLS, any wrong keys almost certainly cause handshake to immediately blow up as handshake keys won't match up...
I don't think variations in clocks are minimal...
I wonder what 95% timeskew interval per day is...
(Oh, and have fun with leap seconds!)
This seems inconsistent. In implementation, I would write explicit disjoint whitelists of extensions for both (and non-whitelisted one is a fatal error). Explicit whitelisting is safe even on client side, since the extensions are bounded by client-supported ones.
How are multiple OIDs represented? Multiple EKU OID/value pairs?
I think the certificate should be REQUIRED to match (as in, not be inconsistent with it) server_name, even if not ACK'd.
The exception on not having valid value can not work at all. With certificates, even if the EE cert is signed with unknown algorithm, there is a small chance things will work. In contrast, there is absolutely no chance of success if either endpoint tries ot use this fallback.
Does this apply to in-handshake or post-handshake auth? I thought in-handshake auth with PSK is not possible.
If it is intended to be possible, that's certainly going to be surprising to implementers, and there are probably going to be multiple implementations that outright choke if one tries to send CertificateRequest in (DHE-)PSK mode
Later the document seems to imply that the limit for for content+type+ padding is in fact 16385 bytes.
Maybe add requirement that record with sequence number of 2^64-1 MUST be either fatal alert or KeyUpdate.
I would expand that 2^24.5 to some rough number ("about 24 million"?).
And also "will wrap" -> "would wrap". Because such wrapping can't happen (even if endpoint delays KeyUpdate to RSN 2^64-1).
This might cause fun with future key exchange types...
I think this needs to be in ServerHello on server-side, given how low- level it is.
These darn things use Supplemental Data messages, would need to define where the data goes. And one can't stick it to extension for user_mapping and client_authz, because those are sent by the client, not the server.
Isn't that "Client/HelloRetryRequest"? I don't think EncryptedExtensions can have Cookie...
Should the two registeries shadowed by this closed?
Isn't the RFC already published, so the codepoints are stable?
This seems really obsolete. The timers have not been 18.2Hz for years, and applications running on operating systems damn better use OS services for random numbers, given that anything else is fraught with peril.
Better to just nuke the code entierely for all versions.
"Disabled" code has nasty tendency of coming back to life.
The extensions field can't be omitted in TLS 1.3. And I would consider TLS 1.2 client implementations that send such messages as quite pathological.
Also, ECDSA signing operations do have problems with timing attacks.
Also, the lengths need to be correct (not verifying this leads to an attack). Best just to treat first length bytes >0x82 or >0x83 as malformed. There is no way to send legimate message with first length byte >0x83 in TLS.
I think properties of the exporter should be covered as well:
(The TLS 1.2 exporter without EMS failed the middle requirement, creating security issues in applications that assumed the data was connection-unique.
-Ilari
Dave Garrett 10:52 PM (6 hours ago)
to tls, Ilari, me Just replying to a few points.
On Tuesday, July 12, 2016 12:16:24 am Ilari Liusvaara wrote:
The HelloRetryRequest message is not applicable to pure-PSK, which does not use the KeyShare extension at all. PSK has its own separate extension. ((EC)DHE-PSK uses both together)
The purpose of HelloRetryRequest is to allow for clients to not send full keys for every single algorithm they support, yet allow the server to still pick from that entire list if it needs to. PSK has no equivalent system; pre-shared keys are first-flight or bust. If an (EC)DHE-PSK suite is selected, and a valid PSK identity is selected, the server can use HelloRetryRequest to reject a group in favor of another supported by the client, but it can't reject the suite or identity in this manner. The response to no acceptable PSK identity is either to negotiate another suite or to abort with a fatal alert.
See draft 14 section 4.2.5: https://tools.ietf.org/html/draft-ietf-tls-tls13-14#section-4.2.5
xiaoyinl fixed the second one of those notes, but that was merged after the checkpoint for draft 14. I'll fix this one to just note for ECDHE PSK AES.
Emphatically agreed, however I worded it this way to be slightly more general. If I said that all that code should be universally gutted, I'd risk more people ignoring it due to the severe status quo bias that is very common. In lieu of modernizing fully and dropping it completely, having these old features disabled via the same compile-time option that enables building of TLS 1.3 would be acceptable, IMO (though, more trouble than it should be worth, and still not ideal at all). Bikeshedding better wordings in this section would not be unwelcome, if you think anything here can be made better.
Implementations should be expected to handle pathological cases gracefully, even if only to recognize and reject. ;)
Ilari Liusvaara 2:00 AM (3 hours ago)
to Dave, tls, me On Tue, Jul 12, 2016 at 01:52:57AM -0400, Dave Garrett wrote:
Yes, rejection because of group mismatch can't occur. However, I don't see any requirement not to reject pure-PSK for cookie check (might be feasible, if the computational and network load is determined "small enough").
IIRC, there was merged patch that changed the reference, but not the text. Just checked, seems to still be there in Editor's Copy.