[TLS] New review through the TLS 1.3 Editor's Copy

[Ilari Liusvaara <[e-mail]>]

Did a new review of the document, given that stuff from the older
review got addressed.

Consulted the older review when making this, but not the issue lists:


> EncryptedExtensions.
> : responses to any extensions that are not required to
>   determine the cryptographic parameters. [{{encrypted-extensions}}]

Also, with Certificate extensions, the extensions here need to not
be specific to a certificate or certificate chain.

> Finally, the client and server exchange Authentication messages. TLS
> uses the same set of messages every time that authentication is needed.
> Specifically:
> 
> Certificate.
> : the certificate of the endpoint. This message is omitted if the
>   server is not authenticating with a certificate. Note that if raw
>   public keys {{RFC7250}} or the cached information extension
>   {{?RFC7924}} are in use, then this message will not
>   contain a certificate but rather some other value corresponding to
>   the server's long-term key.  [{{certificate}}]

This is also omitted for client if CertificateRequest was not sent,
right?

> CertificateVerify.
> : a signature over the entire handshake using the public key
>   in the Certificate message. This message is omitted if the
>   server is not authenticating via a certificate. [{{certificate-verify}}]

Same as above. And additionally, for client, doesn't sending an
empty certificate omit this?

> ## Zero-RTT Data
> 
> With the Zero-RTT mode, clients can send data on their first flight
> ("early data") whereby the client uses a PSK either obtained 
> out-of-band or as a ticket (i.e., one with the "early_data_info" 
> extension) from an earlier exchange to authenticate to the server. 
> 
> When clients use a PSK obtained out-of-band then the following 
> information MUST be provisioned to both parties:
> 
>   * The PSK identity
>   * The cipher suite for use with this PSK
>   * The key exchange and authentication modes this PSK is allowed to be used with
>   * The Application-Layer Protocol Negotiation (ALPN) label(s)
>   * The Server Name Indication (SNI), if any is to be used

The ALPN labels can't be plural. There must (nothing else will work) be
a single ALP (possibly being the implicit default ALP) for any 0-RTT-
capable PSK.

Also, AFAIK, the reason why ticket_age exists in PSK is to support
0-RTT anti-replay. Without 0-RTT data, there is no point in using
anything that doesn't involve server-side contribution anyway (and
thus would get superrior security properties).

> 2. There are no guarantees of non-replay between connections.
> Unless the server takes special measures outside those provided by TLS,
> the server has no guarantee that the same
> 0-RTT data was not transmitted on multiple 0-RTT connections
> (See {{replay-time}} for more details).
> This is especially relevant if the data is authenticated either
> with TLS client authentication or inside the application layer
> protocol. However, 0-RTT data cannot be duplicated within a connection (i.e., the server
> will not process the same data twice for the same connection) and
> an attacker will not be able to make 0-RTT data appear to be
> 1-RTT data (because it is protected with different keys.)

Of course, if the server TLS library provodes the difference between
0-RTT and 1-RTT data is a different matter...

> ## Decoding Errors
> 
> TLS defines two generic alerts (see {{alert-protocol}}) to use upon failure to parse
> a message. Peers which receive a message which cannot be parsed according to the syntax
> (e.g., have a length extending beyond the message boundary or contain an out-of-range
> length) MUST terminate the connection with a "decoding_error" alert. Peers which receive
> a message which is syntactically correct but semantically invalid (e.g., a DHE share of p - 1)
> MUST terminate the connection with an "illegal_parameter" alert.

What alert is used if some field is an non-extensible enumeration and
the value transmitted is outside the legal values?

E.g. An unknown max_fragment_length value.

I have used illegal_parameter for errors like this.

Also, isn't the alert name decode_error, not decoding_error?

> ###  Client Hello
>
> In the event that a client requests additional functionality using
> extensions, and this functionality is not supplied by the server, the
> client MAY abort the handshake. Note that TLS 1.3 ClientHello messages
> MUST always contain extensions, and a TLS 1.3 server MUST respond to
> any TLS 1.3 ClientHello without extensions or with data following
> the extensions block with a "decode_error"
> alert. TLS 1.3 servers may receive TLS 1.2 ClientHello messages
> without extensions. If negotiating TLS 1.2, a server MUST check that
> the message either contains no data after legacy_compression_methods
> or that it contains a valid extensions block with no data following.
> If not, then it MUST abort the handshake with a "decode_error" alert.

Note that it is now impossible to receive a TLS 1.3 ClientHello without
extensions (any such would be interpretted as TLS 1.2 ClientHello
without extensions).

> ###  Server Hello
> 
> version
> : This field contains the version of TLS negotiated for this session.  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.  For this version of the
>   specification, the version is { 3, 4 }.  (See {{backward-compatibility}} for
>   details about backward compatibility.)

Err, don't they have to select highest client indicated version that they
support (now that version negotiation is with extensions)?


> ##  Hello Extensions
> 
> An extension type MUST NOT appear in the ServerHello or HelloRetryRequest
> unless the same extension type appeared in the corresponding ClientHello.
> If a client receives an extension type in ServerHello or HelloRetryRequest
> that it did not request in the associated ClientHello, it MUST abort the
> handshake with an "unsupported_extension" fatal alert.

Add note that Cookie is an exception to this?

> When multiple extensions of different types are present in the ClientHello or
> ServerHello messages, the extensions MAY appear in any order. There MUST NOT be
> more than one extension of the same type.

pre_shared_key is an exception to this in ClientHello, right?

> In general, the specification of each extension type needs to describe the
> effect of the extension both during full handshake and session resumption. Most
> current TLS extensions are relevant only when a session is initiated: when an
> older session is resumed, the server does not process these extensions in
> ClientHello, and does not include them in ServerHello. However, some
> extensions may specify different behavior during session resumption.
> [[TODO: update this and the previous paragraph to cover PSK-based resumption.]]

Basically, treating any extension specially with regards to full
handshake versus "resumption" is very likely to be a Bad Idea and
there should be good justifications on doing so.

(Such difference would likely be a major source of implementation bugs.)

> ###  Signature Algorithms
> 
> The client uses the "signature_algorithms" extension to indicate to the server
> which signature algorithms may be used in digital signatures. Clients which
> desire the server to authenticate via a certificate MUST send this extension.
> If a server
> is authenticating via a certificate and the client has not sent a
> "signature_algorithms" extension then the server MUST
> abort the handshake with a "missing_extension" alert
> (see {{mti-extensions}}).
> 
> Servers which are authenticating via a certificate MUST indicate so
> by sending the client an empty "signature_algorithms" extension.

This looks to be a Bad Idea. It was to support that poorly tought-out
server certificate authentication in PSK mode, right?

Basically, offering any means (even if there is "MUST" to "prevent" the
use) of disabling server authentication in non-PSK mode is a footgun.

> ### Negotiated Groups
> 
> As of TLS 1.3, servers are permitted to send the "supported_groups"
> extension to the client. If the server has a group it prefers to the
> ones in the "key_share" extension but is still willing to accept the
> ClientHello, it SHOULD send "supported_groups" to update the client's
> view of its preferences; this extension SHOULD contain all groups
> the server supports, regardless of whether they are currently
> supported by the client. Clients MUST NOT act upon any information
> found in "supported_groups" prior to successful completion of the
> handshake, but MAY use the information learned from a successfully
> completed handshake to change what groups they offer to a server in
> subsequent connections.

This means altering the set of key shares they offer to the server,
right? Not the set of groups they indicate as supported?

> ### Key Share
> 
> Upon receipt of this extension in a HelloRetryRequest, the client MUST first
> verify that the selected_group field corresponds to a group which was provided
> in the "supported_groups" extension in the original ClientHello. It MUST then
> verify that the selected_group field does not correspond to a group which was
> provided in the "key_share" extension in the original ClientHello. If either of
> these checks fails, then the client MUST abort the handshake with an
> "illegal_parameter" alert.  Otherwise, when sending the new ClientHello, the
> client MUST append a new KeyShareEntry for the group indicated in the
> selected_group field to the groups in its original KeyShare. The remaining
> KeyShareEntry values MUST be preserved.

Err... Hasn't the KeyShare behaviour been changed from "append" to
"replace"?

> ### Pre-Shared Key Extension
> 
> obfuscated_ticket_age
> : For each ticket, the time since the client learned about the server
>   configuration that it is using, in milliseconds.  This value is
>   added modulo 2^32 to with the "ticket_age_add" value that was
>   included with the ticket, see {{NewSessionTicket}}.  This addition
>   prevents passive observers from correlating sessions unless tickets
>   are reused.  Note: because ticket lifetimes are restricted to a
>   week, 32 bits is enough to represent any plausible age, even in
>   milliseconds. External tickets SHOULD use an obfuscated_ticket_age of
>   0; servers MUST ignore this value for external tickets.

The comment above about purpose of ticket_age also applies here.

> ### Pre-Shared Key Exchange Modes
> 
> In order to use PSKs, clients MUST also send a "psk_key_exchange_modes"
> extension. The semantics of this extension are that the client only
> supports the use of PSKs with these modes, which restricts both the
> use of PSKs offered in this ClientHello and those which the server
> might supply via NewSessionTicket.

Why is this a separate extension and not part of pre_shared_key, given
that if the latter is present, this must be too?

Seems like more complexity for absolutely no reason. I would understand
if this extension was optional if PSK is present, but it is not.


> ### Early Data Indication
> 
> When PSK resumption is used, the client can send application data
> in its first flight of messages. If the client opts to do so, it MUST
> supply an "early_data" extension as well as the "pre_shared_key"
> extension.

Can't early data now be used with any type of PSK, not just ones
provisioned by NewSessionTicket?

> A server MUST validate that the ticket age for the selected PSK
> identity (computed by un-masking PskIdentity.obfuscated_ticket_age)
> is within a small tolerance of the time since the ticket was
> issued (see {{replay-time}}).  If it is not, the server SHOULD proceed
> with the handshake but reject 0-RTT, and SHOULD NOT take any other action
> that assumes that this ClientHello is fresh.

PSKs that are externally provisioned don't have any valid ticket age...

> In order to accept early data, the server server MUST have accepted a
> PSK cipher suite and selected the the first key offered in the
> client's "pre_shared_key" extension. In addition, it MUST verify that
> the following values are consistent with those negotiated in the
> connection during which the ticket was established.
> 
> - The TLS version number, AEAD algorithm, and the hash for HKDF.
> - The selected ALPN {{!RFC7443}} value, if any.

Actually, it must verify that the record protection matches, not
just the HKDF.

> ## Server Parameters
> 
> The same extension types MUST NOT appear in both the ServerHello and
> EncryptedExtensions. All server-sent extensions other than those explicitly
> listed in {{server-hello}} or designated in the IANA registry MUST only
> appear in EncryptedExtensions. Extensions which are designated to
> appear in ServerHello MUST NOT appear in EncryptedExtensions. Clients
> MUST check EncryptedExtensions for the presence of any forbidden
> extensions and if any are found MUST abort the handshake with an
> "illegal_parameter" alert.

Also there are extensions in Certificates as well...

> #### Receiving a Certificate Message
>
> Any endpoint receiving any certificate signed using any signature algorithm
> using an MD5 hash MUST abort the handshake with a "bad_certificate" alert.
> SHA-1 is deprecated and it is RECOMMENDED that
> any endpoint receiving any certificate signed using any signature algorithm
> using a SHA-1 hash abort the handshake with a "bad_certificate" alert.
> All endpoints are RECOMMENDED to transition to SHA-256 or better as soon
> as possible to maintain interoperability with implementations
> currently in the process of phasing out SHA-1 support.

These requirements are problematic, and I have absolutely no intention
of ever implementing this in my TLS libs. They treat signatures involving
MD5 or SHA1 the same as unknown signature algorithm (which is not a fatal
error).

To be clear, MD5 (and SHOULD for SHA-1) signatures are absolutely not
to be trusted. But those should be treated as unknown with no special
handling.

Also, if there are self-signed certs, the self-signature is just to be
ignored. There are still CA root certs out there with MD5 signatures.

> %%% Authentication Messages

> If sent by a server, the signature algorithm MUST be one offered in the
> client's "signature_algorithms" extension unless no valid certificate chain can be
> produced without unsupported algorithms (see {{signature-algorithms}}).

This is seemingly about server signatures. In that context, an
unknown algorithm has absolutely no chance of working.

> ### New Session Ticket Message {#NewSessionTicket}
> 
> Any ticket MUST only be resumed with a cipher suite that is identical
> to that negotiated connection where the ticket was established.

This kind of requirement is problematic when "external" PSKs have
requirement that just the hash matches. (Implementation bugs).



-Ilari