[TLS] Let's review: draft-ietf-tls-tls13-07 (abridged)

[Ilari Liusvaara <[e-mail]>]

Let's review: draft-ietf-tls-tls13-07

This is abridged version of mail I sent earlier, but was too large for
the list due to its sheer size.

(Note: I omitted some stuff I saw recently discussed (e.g. pruning
unused crypto algorithms) or I remember discussed. I didn't explicitly
check issue list when doing this).

Note: The issue list could use some cleanup. It has multiple duplicate
issues (especially about fixing THS) and also some issues that no longer
look applicable.


> Header

Isn't 4346 already obsoleted by 5246, which this document also obsoletes?

4366 seems to be jointly obsoleted by 5246 and 6066.

5246 and 5077 are not in numerical order, whereas the rest are.

> 1. (Introduction)

DSA should be replaced by ECDSA (DSA is pretty much obsolete)?

> 1.2. (Major Differences from TLS 1.2)

Is this meant to be changelog or list of changes? It in current form
looks more like a changelog.

> 4.9.1. (Digital Signing)

I think someone wanted randoms back here in order to support privilege
separation (which I think is important to support, I consider it much
more important than being "HSM friendly")?

Reading what current draft of 4492-bis says, the hash function used is
determined by signature_algorithms (or presumably the corresponding
mechanism in CertificateRequest for client certs).

Also, to my knowledge, there is no mechanism to indicate in ECDSA
certificate what hash algorithms are allowed.

> 4.9.2. (Authenticated Encryption with Additional Data (AEAD))

The example looks like it belongs to section 4.9.1, as it is about
signatures, not AEAD construct.

> 5. (The TLS Record Protocol)

Documenting the security properties of TLS would be useful...

The lack of record length hiding may be problematic in protocols that
have no place for cover traffic (e.g. can DNS requests contain padding,
DPRIVE WG is apparently planning on putting DNS into (D)TLS?).

> 5.2.1. (Fragmentation)

Zero-length fragments of application data are very much visible in
ciphertext (unless record padding is added), so those are not currently
useful as traffic analysis countermeasure.

> 5.2.2. (Record Payload Protection)

There looks to be latter limits that restrict ciphertext size to 2^14
+1024, which is smaller than 2^14+2048 here (but those limits might be
tightened further).

As for amount of expansion needed for length-hiding, I think that being
able to represent 16384-byte record with no padding would be enough
(since record sizes cap at 16384 bytes anyway).

> 6. (The TLS Handshaking Protocols)

Are encryption keys, finished value (tls-unique) and exporter secret
part of session or not?

> 6.1.1. (Closure Alerts)

The semantics of closure alerts seem incompatible with half-closes,
which some protocols actually use.
 
> 6.1.2. (Error Alerts)

Could use another example of warning alert, now that no_renegotiation
is not a warning anymore?
 
> 6.2.1. (Incorrect DHE Share)

EncryptedExtensions is marked optional in Figure 2, but not Figure 1?

The relationship between session hash and handshake restarts seems
like a hairy problem.

Also, I figured out a downgrade attack that works against careless
_server_ (not requiring client to do anything else than have weak
crypto enabled). Continuing hashes looks to block that attack.

It involves attacker sending ClientHello with arbitrary parameters
that triggers a retry (very easy to trigger a retry), eating the
reply, followed by sending client's original ClientHello. That
could trigger crypto downgrade in some badly made servers.

> 6.2.2. (Cached Server Configuration)

Issue #184 manifests here too. I think both accepting and provoding
configuration in the same handshake is sensible (key rollover), and
later the draft talks about exactly that case.
 
Also, maybe note that provoding the message does not alter the
configuration hash (even if there is no existing one) could be useful.

> 6.2.3. (Zero-RTT Exchange)

No EncryptedExtensions?

How would the server know when 0-RTT data ended, so it could send its
ServerHello?
 
Also, how is the 0-RTT data bound to session if accepted, or is there
a security analysis for leaving this binding out?

Can anyone expand on the note about impersonation with compromised
server key? I can't offhand figure out how attacker can calculate
server-side ES (without having also compromised (possibly former)
client or (current) server exchange key).

> 6.3. (Handshake Protocol)

Missing encrypted_extensions (it is a handshake message, right)?
 
> 6.3.1.1. (Client Hello)

The non-match case gives HelloRetryRequest, not ServerHello, right?

s/should/SHOULD/ in description of session_id?

I don't think client extensions are optional anymore in TLS 1.3 (being
required for successful handshake.

Also, TLS 1.2 servers that care about security (are there actually any?)
already require extensions for successful handshake.

> 6.3.1.2. (Server Hello)

Well, at least it wouldn't be backward compatiblity hazard to remove
session_id_len, since it comes after server version.

Note: Unlike client extensions, I think server extensions might be empty
in TLS 1.3 handshake (since extensions required for GDHE_CERT type key
exchanges are not replied to by server).

> 6.3.1.3. (Hello Retry Request)

server_version and cipher_suite in HRR seem bit dangerous to me. If
HashAlgorithm contained all PRF hashes (currently it does), one could
use that to designate PRF hash.
 
Merging DTLS cookies might be a good idea. However, then group mismatch
wouldn't be the only source of failure (which needs some text adjustment
about the selected_group check).

s/use for/add to/ in description of selected_group?

What happens if Ciphersuite and NamedGroup don't match between HRR
and SH/SKS? I expect there be clients that don't check, no matter if it
is MUST or not.

Heck, I recently heard of servers that only partially(!!!) check client
Finished (or omit the check entierely).

> 6.3.1.4. (Hello Extensions)

supported_groups is 10, right?

Also, for matter of protocol testing, could be useful to give some
free values to temporary squat on for extensions that aren't stable
yet.

The restriction on extensions appearing also holds for
EncryptedExtensions, not just SH or HRR.

It could also be useful to deprecate some extensions (client MAY send
for backwards compat, server MUST NOT select).

Some candidates:
- truncated_hmac: Block modes have been removed.
- encrypt_then_mac: Block modes have been removed.
- extended_master_secret: The THS bug is fixed anyway.
- sessionticket TLS: Session tickets are deprecated.
- renegotiation_info: The renego bug is fixed anyway.

The interaction of some extensions with PSK-based resumption isn't
that clear. This is further complicated by the fact that PSK can also
start a new session.

Reference to "error alerts". Should that be "fatal alerts"?

The part about being careful with extensions modifying handshake
presumably also goes for functions that are sometimes used for
authentication like TLS-EXPORTER or TLS-UNIQUE.

> 6.3.1.4.1. (Signature Algorithms)

Anonymous should be removed. It is apparently not used (there is reference
to another section saying that section uses it, but I can't find in that
section how it is actually used).

Description of signature refers to 6.3,2, which is about SKS, which does
not seem to have anything to do with signatures (leftover from TLS 1.2?)

Then in description of the extensions there is reference to 6.3.4 and
6.3.2. 6.3.4 looks relevant, 6.3.2. doesn't look so.

Also, making signature_algorithms mandatory would allow dropping the
backward compatiblity special cases from here.

There is text that talks about "offering prior versions". Does that
refer to insecure version downgrade (I don't suppose TLS 1.1 maximum
client would care about what TLS 1.3 spec says).

> 6.3.1.4.2. (Negotiated Groups)

One chould add Curve25519 and Curve448, and the corresponding
signature groups (signature groups when CFRG signatures become
stable).

> 6.3.1.5.1. (Diffie-Hellman Parameters)
> 6.3.1.5.2. (ECDHE Parameters)

Drop the wrappers? Then DHE and ECDHE could be unified (the unification
is currently blocked by different lengths of the length field).
 
The point description might make it more clear that encodings are per-
curve (altough most curves use X9.62).

Does the note about only single point format apply to signatures too
(if so, ec_point_formats could be deprecated).

Regarding CFRG curves, the current draft (for TLS 1.2) specifies use
of the native point format (32/56 octets) as if it was uncompressed.
 
> 6.3.1.5.4. (Pre-Shared Key Extension)

Client SHOULD offer at most two PSK identities (one for DH-PSK,
the other for pure-PSK)?

This would imply that one can't resume pure-PSK sessions, but that
does not look to be useful anyway.

Also, could be useful to state that the PSK identity hint from
previous versions has been deprecated.
 
> 6.3.1.5.5. (Early Data Indication)

s/MUST not/MUST NOT/ in description of early_data with client auth?

Also, I would say that 0-RTT data with 0-RTT auth is more dangerous than
0-RTT data with 1-RTT auth (for reasons mentioned earlier in the draft).

Eeh, the rule for detecting end of 0RTT data, isn't the next client
handshake message from the next flight at least in some cases (and
waiting would obviously deadlock)?

Again, how is binding of early handshake messages done (or security
analysis showing this unnecressary)?

Client receiving rejection knows that auth failed and data failed, it
might try to fix things up in its next flight.

As for what to sign for client auth, the ClientHello and Certificate
(at least)? One thing to note is that if verify contains randoms, then
server_random isn't available.

Regarding finished, isn't Finished in second flight? Or is this another
client Finished?

Regarding cryptographic parameter set, I think one better prepare for
failure (there are multiple causes of server failing to compute keys or
decrypt data).

Also, how does known_configuration interact with PSK (other than
configurations seemingly not being usable with PSK)?

Also, the record protection used for early handshake messages should be
indicated.

I think there are basically two things to signal for 0RTT:
- What key material is used (either by PSK ID or configuration ID)
- What record protection (and KDF to derive keys) is used?

Tying the latter to client offered ciphersuites is probably not wanted.

> 6.3.2. (Server Key Share)

Add that connection will be rekeyed immediately after this message, and
that it MUST be the last message in record it is in?
 
> 6.3.3. (Encrypted Extensions)

Some of the figures have this as mandatory, some optional. Which is it?

Yeah, would be good to have explicit list of what goes to SH, and what
goes to EE.

Also, the list is affected by #184, as some extensions introduce extra
messages (esp. status_request{,_v2} and signed_certificate_timestamp).

> 6.3.4. (Server Certificate)
> 
>       The certificate MUST be appropriate for the negotiated cipher
>       suite's key exchange algorithm and any negotiated extensions.

If signature_algorithms is mandatory, this would then key to that for
signature algorithm, not ciphersuites (and even if it isn't, it would
then key to its fallback default).

Could change example of alternative certificate formats to RFC7250 (it
seems better designed than RFC5081).

Doesn't TLS 1.3 always use the certificate for signing, with indicated
algorithm (which obviously needs to be supported by peer)? Wouldn't it
make more sense to talk about signature algorithms instead of key
exchange algorithms. (Unless some key exchange that uses non-signing
certificates is added).
 
> 6.3.6. (Server Configuration)
 
Use TTL instead of of explicit time for expiration, as broken clocks
are abound (also, just 32-bit field that only goes up to 2106 or so)?

Also, considering the dangers of configuration, it might be appropriate
to have other permission flags (especially client and server auth) too?

Also, Could be useful for client to know what ciphers it can use for
0-RTT data?

> 6.3.7. (Server Certificate Verify)

Should PRF hash designation be added in order to avoid attacks from
weak PRF hashes?

I think note about checking signature algorithm against ciphersuite
should be removed.

Also, with regards to complications of DSA, just dump it? :-)

> 6.3.9. (Client Certificate)
> 
Doesn't CertificateRequest override algorithms anyway (and CR isn't an
extension)?

Again, could use RFC7250 instead of RFC5081 as an example?
 
> 6.3.10. (Client Certificate Verify)

Similar problems as in DSA for server auth. And missing identifier
for PRF too.

> 6.3.11. (New Session Ticket Message)

Eh, if server starts transmitting immediately without waiting for
client Finished (as is permitted for case where 1-RTT client auth
is not performed), it can't resume the session?

> 7.2.3. (Elliptic Curve Diffie-Hellman)

Probably needs different description for Curve25519 and Curve448.
Those are functions that take in octet strings and output octet strings
(32 or 56 bytes).

> 11. (IANA Considerations)

Yeah, looks to need a rewrite. Doesn't register the new stuff, and
furthermore has problems like:

- signature_algorithms is already defined by RFC5246.
- SignatureAlgorithm registry is already defined.
- HashAlgorithm registry is already defined.
 
> A.4. (The Cipher Suite)

Probably remove note about 001C and 001D, since lots of ciphersuites are
now reserved to avoid collisions with old ones.

> C.1. (Random Number Generation and Seeding)

Replace SHA-1 with SHA-256?

The example is completely obsolete. Any sort of modern PCs have much
faster timers (some reaching multi-GHz rates) and one really should use
Operating System for (seeding) random numbers.

> C.4. (Implementation Pitfalls)

- Do you handle required handshake messages being missing or handshake
  messages being in wrong order properly (i.e. terminate the connection)?

Also, ECDSA signing operations need timing protection.

s/DSA/ECDSA/ in seeding? And shouldn't k be determinstically generated?

> E.1.1. (Authentication and Key Exchange)

Would be useful to list what can be used to authenticate peers, in
practicular, are the following valid:

- Signing TLS-Unique value?
- Signing value from TLS-EXPORTER?

Both have been seen in the wild, so these better be secure ways. This
also goes for "anonymous" key exchange.
 
> E.1.2. (Version Rollback Attacks)

Is the note about PKCS #1 block type 2 about RSA key exchange, which is
deprecated?


-Ilari