Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove 0-RTT Finished and add a PSK binder instead #672

Merged
merged 6 commits into from Oct 16, 2016
Merged

Conversation

@ekr
Copy link
Contributor

ekr commented Oct 7, 2016

No description provided.

Copy link
Contributor

kaduk left a comment

Some mostly editorial stuff inline

which authentication modes it can be used with (PSK alone or PSK with
signatures). The server can then select those key establishment and
authentication parameters to be consistent both with the PSK and the
If the server selects a PSK, then it MUST only be used with

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 7, 2016

Contributor

Should probably be consistent about "PSK" vs "a PSK" (vs. "a PSK mode"?) here and L1364

If the server selects a PSK, then it MUST only be used with
the establishment modes indicated by "psk_key_exchange_modes" (PSK alone or with (EC)DHE).
The server can then select the key establishment and
parameters to be consistent both with the PSK and the

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 7, 2016

Contributor

Looks like the "authentication" got dropped from "authentication parameters".

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

Yes, that's correct. Because I removed psk_auth_modes, signature or not is decided entirely by whether PSK is in use.

Prior to accepting PSK key establishment, the server MUST validate the
corresponding binder value (see {{psk-binder}} below) If this
value is not present or does not validate the server MUST abort the
handshake. Servers SHOULD NOT attempt to validate multiple binders;

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 7, 2016

Contributor

Do we want to set an alert to use in this case?

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

I would be willing to, but I figured it was the usual ones

This comment has been minimized.

Copy link
@tomato42

tomato42 Oct 13, 2016

Contributor

+1

explicit aborts should result in sending Alert messages

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

I added the relevant text to decrypt_error.

corresponding binder value (see {{psk-binder}} below) If this
value is not present or does not validate the server MUST abort the
handshake. Servers SHOULD NOT attempt to validate multiple binders;
rather they SHOULD select a single PSK and validate solely its binder.

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 7, 2016

Contributor

This behavior (select, then verify) could lead to cases where the connection aborts when it could have succeeded, but I am okay with that, since it involves tampering or a misbehaving client.

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

Yes, that's intentional.

rather they SHOULD select a single PSK and validate solely its binder.

In order to accept PSK key establishment, the server sends a
"pre_shared_key" extension with the selected identity.

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 7, 2016

Contributor

"with" implies that it echos back the raw contents; maybe "indicating" or "indicating the index of"? (I hope we specify 0- vs. 1-indexing somewhere...)

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

Yes, indicating would be an improvement

Each entry in the binders list is computed as an HMAC over the portion
of the ClientHello up to and including the PreSharedKeyExtension.identities
field. That is, it includes all of the ClientHello but not the binders
list itself.

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 7, 2016

Contributor

And this answers my question about why it's a separate list of binders with matching index, as opposed to a list of structures with identity and binder together.

binder_key =
HKDF-Expand-Label(BaseKey, [label], "", Hash.length)

binder = HMAC(binding_key, Hash(ClientHello[truncated]))

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 7, 2016

Contributor

binder_key/binding_key don't match; pick one.

If the server responds with HelloRetryRequest, and the client then sends
ClientHello2, its binder will be computed over:

ClientHello1 + HelloRetryRequest + ClientHello[truncated]

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 7, 2016

Contributor

Missing '2'.

which established the PSK. The PSK used to encrypt the early data
MUST be the first PSK listed in the client's "pre_shared_key" extension.
which established the PSK. The PSK used to encrypt the early data
MUST be the first PSK listed in the client's "pre_shared_key"

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 7, 2016

Contributor

I think this change was spurious?

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

yeah, thanks

* "cookie" must always be supported if sent by the server.

If a required extension was not provided. endpoints MUST abort the
handshake with a "missing_extension" alert Any endpoint that receives

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 7, 2016

Contributor

Missing sentence break

hash algorithm. This restriction is automatically enforced
for PSKs established via NewSessionTicket ({{NewSessionTicket}})
but any externally-established PSKs MUST also follow this rule.
ClientHello1 + HelloRetryRequest + ClientHello[truncated]

This comment has been minimized.

Copy link
@nharper

nharper Oct 11, 2016

Contributor

This should be ClientHello2 instead of ClientHello (for the second one)?

of symmetric keys known to the client and the key exchange
modes which each PSK supports.
- A "pre_shared_key" ({{pre-shared-key-extension}}) extension which contains a symmetric
keys known to the client and a "psk_key_exchange_modes" ({{pre-shared-key-exchange-modes}}) extension which indicates

This comment has been minimized.

Copy link
@nharper

nharper Oct 11, 2016

Contributor

"which contains a symmetric keys": something sounds off in that wording to me. Is it "a list of symmetric keys" or "a symmetric key" or something else?

Copy link
Contributor

martinthomson left a comment

A number of niggles here, but otherwise LGTM.

corresponding binder value (see {{psk-binder}} below) If this
value is not present or does not validate the server MUST abort the
handshake. Servers SHOULD NOT attempt to validate multiple binders;
rather they SHOULD select a single PSK and validate solely its binder.

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 12, 2016

Contributor

...and only validate the binder that corresponds to that PSK.

Each entry in the binders list is computed as an HMAC over the portion
of the ClientHello up to and including the PreSharedKeyExtension.identities
field. That is, it includes all of the ClientHello but not the binder
list itself.

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 12, 2016

Contributor

Probably worth pointing out that the length fields for ClientHello.extensions and the pre_shared_key extension are set as though the binder were present.

+--------> Derive-Secret(.,
| "external psk binder key" |
| "resumption psk binder key",
| "")

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 12, 2016

Contributor

Check the alignment on these changes, I think that it's a character off on several of these lines.

@@ -3799,6 +3832,14 @@ a string of Hash.length zeroes is used. Note that this does not mean skipping
rounds, so if PSK is not in use Early Secret will still be
HKDF-Extract(0, 0).

Note: There are multiple potential Early Secret values depending on
which PSK the server ultimately selects.

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 12, 2016

Contributor

You need to be clearer here about what this implies. With 0-RTT, only one of the the possible values of Early Secret is relevant. And I believe that is the only relevant factor, since early exporters aren't useful without 0-RTT.

The main implication is that if a client attempts to use 0-RTT but the server either picks a different PSK or rejects PSK, then the Early Secret has to be re-calculated.

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

This would actually be true in any case.

Note: For the computation of the binder_secret, the label is "external
psk binder key" for external PSKs and "resumption psk binder key" for
resumption PSKs. The different labels prevents the substitution of one
type of PSK for the other.

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 12, 2016

Contributor

Nit, neither of these paragraphs need to be a note.

establishment.
* "pre_shared_key" is REQUIRED for PSK key establishment.
* "psk_key_exchange_modes" is required when PSKs are offered.
* "cookie" must always be supported if sent by the server.

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 12, 2016

Contributor

MUST ?

* "supported_groups" and "key_share" are REQUIRED for DHE or ECDHE key
establishment.
* "pre_shared_key" is REQUIRED for PSK key establishment.
* "psk_key_exchange_modes" is required when PSKs are offered.

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 12, 2016

Contributor

I'm not 100% clear on whether the server is required to send this. It seems not. But you missed it from the IANA considerations table, so I can't verify.

* "psk_key_exchange_modes" is required when PSKs are offered.
* "cookie" must always be supported if sent by the server.

If a required extension was not provided. endpoints MUST abort the

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 12, 2016

Contributor

s/./,

used with collision resistant hash functions and producing output of
at least 256 bits. Any future replacement of these functions MUST
also provide collision resistance.
Note: the binder does not cover the binder values from other

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 12, 2016

Contributor

Nit: capitalize the T.

@@ -4550,6 +4595,20 @@ resumption master secret computed by connection N and needed to form
connection N+1 is separate from the traffic keys used by connection N,
thus providing forward secrecy between the connections.

The PSK binder value extension forms a binding between a PSK

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 12, 2016

Contributor

It's not an extension now.

@ekr ekr force-pushed the ekr:psk_binders branch from 5b46c59 to 21951c5 Oct 12, 2016
@ekr

This comment has been minimized.

Copy link
Contributor Author

ekr commented Oct 12, 2016

Note: I rebased for easier processing.

Copy link
Contributor

kaduk left a comment

editorial (modulo hash output length question)

of symmetric keys known to the client and the key exchange
modes which each PSK supports.
- A "pre_shared_key" ({{pre-shared-key-extension}}) extension which
contains a list of symmetric keys known to the client and a

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 14, 2016

Contributor

I think it's better to keep "identities" here.

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

Done

signatures). The server can then select those key establishment and
authentication parameters to be consistent both with the PSK and the
If the server selects a PSK, then it MUST only be used with the
establishment modes indicated by "psk_key_exchange_modes" (PSK alone

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 14, 2016

Contributor

The server does not send a psk_key_exchange_modes extension in its ServerHello/EncryptedExtensions, so the client has to figure out which mode is in use based on the messages the server sends. That's probably workable for now, but might cramp future extensibility.
However, in the CH-only world, this text should probably be cleaned up a bit, like "only be used with one of the establishment modes indicated by the client's psk_key_exchange_modes extension"

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

That's intentional, so we don't have duplicate and possible-conflicting indicators.

establishment modes indicated by "psk_key_exchange_modes" (PSK alone
or with (EC)DHE).
The server can then select the key establishment and
parameters to be consistent both with the PSK and the

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 14, 2016

Contributor

grammar needs help from 1411 to 1412

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

Cleaned this up a bit


The server indicates its selected parameters in the ServerHello as
follows:

- If PSK is being used then the server will send a
"pre_shared_key" extension indicating the selected key.
"pre_shared_key" extension.

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 14, 2016

Contributor

I'm not sure why it's helpful to lose this text.

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

me neither. reverted.

PskIdentity identities<6..2^16-1>;

PskIdentity identities<6..2^16-1>;
PskBinderEntry binders<0..2^16-1>;

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 14, 2016

Contributor

Is the zero length ever attainable given that the identities array must be nonempty and a binder is needed for each PSK offered?

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

nope. In fact I made it 33 so that we force binders to be >= 32 bytes long

"pre_shared_key" extension indicating the selected identity.

Clients MUST verify that the server's selected_identity is within the
range supplied by the client, that the same cipher suite was selected

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 14, 2016

Contributor

Same cipher suite as what?


Clients MUST verify that the server's selected_identity is within the
range supplied by the client, that the same cipher suite was selected
and that the, "key_share", and

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 14, 2016

Contributor

first comma is not needed.

field. That is, it includes all of the ClientHello but not the binder
list itself. The length fields for the message (including the overall
length, the length of the extensions block, and the length of the "pre_shared_key"
extensio) are all set as if the binder were present.

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 14, 2016

Contributor

"extension", and there's two spaces after the paren.

and the MAC used to compute the binder be collision
resistant. These are properties of HKDF and HMAC respectively when
used with collision resistant hash functions and producing output of
at least 256 bits. Any future replacement of these functions MUST

This comment has been minimized.

Copy link
@kaduk

kaduk Oct 14, 2016

Contributor

Wasn't there a comment about this being the hash output length and not necessarily 256 bits? I forget how that got resolved.

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 16, 2016

Contributor

This is how it was resolved.

If the server selects a PSK, then it MUST only be used with
the establishment modes indicated by "psk_key_exchange_modes" (PSK alone or with (EC)DHE).
The server can then select the key establishment and
parameters to be consistent both with the PSK and the

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

Yes, that's correct. Because I removed psk_auth_modes, signature or not is decided entirely by whether PSK is in use.

Prior to accepting PSK key establishment, the server MUST validate the
corresponding binder value (see {{psk-binder}} below) If this
value is not present or does not validate the server MUST abort the
handshake. Servers SHOULD NOT attempt to validate multiple binders;

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

I would be willing to, but I figured it was the usual ones

corresponding binder value (see {{psk-binder}} below) If this
value is not present or does not validate the server MUST abort the
handshake. Servers SHOULD NOT attempt to validate multiple binders;
rather they SHOULD select a single PSK and validate solely its binder.

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

Yes, that's intentional.

rather they SHOULD select a single PSK and validate solely its binder.

In order to accept PSK key establishment, the server sends a
"pre_shared_key" extension with the selected identity.

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

Yes, indicating would be an improvement

ke_modes and auth_modes values. If these values are not consistent,
the client MUST abort the handshake with an "illegal_parameter" alert.

This extension MUST be the last extension in the ClientHello (this

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

It's appreciated.

signatures). The server can then select those key establishment and
authentication parameters to be consistent both with the PSK and the
If the server selects a PSK, then it MUST only be used with the
establishment modes indicated by "psk_key_exchange_modes" (PSK alone

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

That's intentional, so we don't have duplicate and possible-conflicting indicators.

establishment modes indicated by "psk_key_exchange_modes" (PSK alone
or with (EC)DHE).
The server can then select the key establishment and
parameters to be consistent both with the PSK and the

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

Cleaned this up a bit


The server indicates its selected parameters in the ServerHello as
follows:

- If PSK is being used then the server will send a
"pre_shared_key" extension indicating the selected key.
"pre_shared_key" extension.

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

me neither. reverted.

PskIdentity identities<6..2^16-1>;

PskIdentity identities<6..2^16-1>;
PskBinderEntry binders<0..2^16-1>;

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

nope. In fact I made it 33 so that we force binders to be >= 32 bytes long

Prior to accepting PSK key establishment, the server MUST validate the
corresponding binder value (see {{psk-binder}} below) If this
value is not present or does not validate the server MUST abort the
handshake. Servers SHOULD NOT attempt to validate multiple binders;

This comment has been minimized.

Copy link
@ekr

ekr Oct 16, 2016

Author Contributor

I added the relevant text to decrypt_error.

@ekr ekr merged commit 21951c5 into tlswg:master Oct 16, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.