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

[I trimmed a couple things that already had sub-threads spun off;
leaving in others even where I don't have a comment right now]

On 10/17/2016 01:10 PM, Ilari Liusvaara wrote:
>
>> 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?

That as my understanding.

>> 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?

For post-handshake, that is explicitly stated.  It is also implicitly
stated for in-handshake ("Clients MUST send this message
[CertificateVerify] whenever authenticating via a Certificate (i.e.,
when the Certificate message is non-empty)."

>> ## 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.

I pointed this out on Kyle's declined pull request and he fixed it in a
later one that has already been merged.

> 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).
>

I basically agree, and see only minimal reason why the ticket_age_add
needs to be mandatory.  Sure, it can give the server some insight into
rejecting stale tickets, but there are other ways to do so.

>> 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...

Yes, the library has to tell the application that 0-RTT data and 1-RTT
data are different, or bad things happen.  Watson had some text to that
effect in PR 694, which was not accepted (some text in a
counter-proposal by Martin Thomson was accepted).

>> ###  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)?
>

Basically.  It is not clear to me whether there is consensus to say
"highest version" or just "server picks".

>> ##  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?

I thought there was already an exception for cookie:

   [...]  As with
   ServerHello, a HelloRetryRequest MUST NOT contain any extensions that
   were not first offered by the client in its ClientHello, with the
   exception of optionally the "cookie" (see Section 4.2.2
<https://tools.ietf.org/html/draft-ietf-tls-tls13-16#section-4.2.2>) extension.

Though perhaps it is still worth duplicating that guidance so it's
easier to find.

>> 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?

Yes.

>> 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"?

I thought so, yes.

>> ### 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.

The justification for making it separate is that the
psk_key_exchange_modes also applies to what the server will send back in
NST, so in theory one could send psk_key_exchange_modes but not
pre_shared_key.  I'm still a little dubious, but accepted the justification.

>
>> ### 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?

Yes.

-Ben

>> 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
>
> _______________________________________________
> TLS mailing list
> TLS@ietf.org
> https://www.ietf.org/mailman/listinfo/tls