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

Wip client auth revision #316

Closed
wants to merge 3 commits into from

Conversation

@ekr
Copy link
Contributor

commented Oct 21, 2015

This is still a WIP but is posted so that people can review.

Client Server

Key ^ ClientHello
Exch v + KeyShare -------->

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 21, 2015

Contributor

The ^ and v without a line joining them are a little hard to follow. What about:

v---- Key exchange
 ClientHello
       + KeyShare              -------->
                                                       ServerHello
                                                      + KeyShare
v---- Server parameters
                                             {EncryptedExtensions}
                                          {ServerConfiguration*}
                                           {CertificateRequest*} 
v---- Authentication
                                                    {Certificate*}
                                            {CertificateVerify*}
                                <--------              {Finished}
     {Certificate*}
     {CertificateVerify*}
     {Finished}                -------->

Also, this doesn't allow the server to speak first, is that intentional?

This comment has been minimized.

Copy link
@ekr

ekr Oct 21, 2015

Author Contributor

Why do you think it doesn't allow that?

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 21, 2015

Contributor

Because it shows the server's application data after the server has received the Finished.

: a MAC over the entire handshake computed using the Static Secret
and providing key confirmation. [{{server-finished}}]
: a MAC over the entire handshake. This message provides key confirmation, binds the endpoint's identity
to the exchanged keys, and in some modes (see {{zero-rtt-exchange}})

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 21, 2015

Contributor

Do you mean to reference the 0RTT handshake, or do you mean to refer to PSK/resumption?

This comment has been minimized.

Copy link
@ekr

ekr Oct 21, 2015

Author Contributor

Both, actually.

to the 1-RTT handshake in the first flight. Specifically, the
client sends its Authentication messages after the ClientHello,
followed by any application data. The rest of the handshake
messages are the same as with Figure {{tls-full}}. This implies

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 21, 2015

Contributor

Figure {{figure-ref}} puts the word "Figure" into the final text twice.

messages are the same as with Figure {{tls-full}}. This implies
that the server can request client authentication even if the
client offers a certificate on its first flight. This isn't
necessarily useful but is consistent with the server being

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 21, 2015

Contributor

I think that you might want to avoid statements like "this isn't necessarily useful"

@@ -2887,6 +2791,8 @@ Structure of this message:
CertificateExtension certificate_extensions<0..2^16-1>;
} CertificateRequest;

[[TODO: We agreed to add a context string here.]]

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 21, 2015

Contributor

a context string, fresh entropy, or both?

---- ----------------- --------
0-RTT ClientHello + ServerConfiguration xSS
+ Server Certificate
+ CertificateRequest

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 21, 2015

Contributor

I think that this needs to make it clear that these extras are cached, otherwise ServerConfiguration here can be confused for ServerConfiguration in the 1-RTT case.

And no, I don't think that the note isn't quite enough.


Note 2: The Handshake Context for the last three rows does not include
any 0-RTT handshake messages, regardless of whether 0-RTT is used.

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 21, 2015

Contributor

An extra note required to point out that ClientHello ... HelloRetryRequest ... ClientHello is included in all cases.

This comment has been minimized.

Copy link
@ekr

ekr Oct 21, 2015

Author Contributor

Still an open issue.

opaque ASN1Cert<1..2^24-1>;

struct {
ASN1Cert certificate_list<0..2^24-1>;

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 21, 2015

Contributor

TODO: add correlator/context id

This comment has been minimized.

Copy link
@ekr

ekr Oct 21, 2015

Author Contributor

I am trying to decide whether it should go here or in CertificateVerify.

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 21, 2015

Contributor

Ack, I don't think that it matters, but CertificateVerify has a certain merit to it. Of course, we should merge all the messages and then it will matter much less.

indicated supported pairs, then it SHOULD continue the handshake by sending
the client a certificate chain of its choice that may include algorithms
that are not known to be supported by the client. This fallback chain MAY
use the deprecated SHA-1 hash algorithm.

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 21, 2015

Contributor

... if the client indicated that it supported SHA-1 in combination with the signature algorithm in "signature_algorithms".

This comment has been minimized.

Copy link
@ekr

ekr Oct 21, 2015

Author Contributor

Actually, I'm not sure that this is right, since we already agreed you can just send one that violates the rules.

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 21, 2015

Contributor

I think that this is important though. If we allow inclusion of sha-1 in signature_algorithms, and not permit violations of the rules, then we have a way to signal that a client doesn't want sha-1 any more. If you let servers just use sha-1 without signaling, then you are in a perpetual sha-1 limbo.

This comment has been minimized.

Copy link
@ekr

ekr Oct 21, 2015

Author Contributor

This is in the existing text, so why don't you send a PR. I suspect
@davegarrett would approve :)

On Wed, Oct 21, 2015 at 10:44 AM, Martin Thomson notifications@github.com
wrote:

In draft-ietf-tls-tls13.md
#316 (comment):

  •                   the client, as described in [RFC4492].
    
    +~~~~
    +
    +- The "server_name" and "trusted_ca_keys" extensions {{RFC6066}} are used to
  • guide certificate selection. As servers MAY require the presence of the server_name
  • extension, clients SHOULD send this extension.

+All certificates provided by the server MUST be signed by a
+hash/signature algorithm pair that appears in the "signature_algorithms"
+extension provided by the client, if they are able to provide such
+a chain (see {{signature-algorithms}}).
+If the server cannot produce a certificate chain that is signed only via the
+indicated supported pairs, then it SHOULD continue the handshake by sending
+the client a certificate chain of its choice that may include algorithms
+that are not known to be supported by the client. This fallback chain MAY
+use the deprecated SHA-1 hash algorithm.

I think that this is important though. If we allow inclusion of sha-1 in
signature_algorithms, and not permit violations of the rules, then we have
a way to signal that a client doesn't want sha-1 any more. If you let
servers just use sha-1 without signaling, then you are in a perpetual sha-1
limbo.


Reply to this email directly or view it on GitHub
https://github.com/tlswg/tls13-spec/pull/316/files#r42656846.

{:br }
the string "server finished" [[OPEN ISSUE: Do we still need these?
labels? Should we expand them to indicate which phase we are
sending the messages in?]]

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 21, 2015

Contributor

If you do keep the labels, then something needs to change. Having both "server finished" and "server_finished" is going to be quite confusing.

with the exception of the Finished message, including the type and
length fields of the handshake messages. This is the concatenation
of all the exchanged Handshake structures in plaintext form (even if
they were encrypted on the wire).

This comment has been minimized.

Copy link
@martinthomson

martinthomson Oct 21, 2015

Contributor

Need to explicitly exclude post-handshake messages. NewSessionTicket, CertificateRequest, etc...

This comment has been minimized.

Copy link
@ekr

ekr Nov 14, 2015

Author Contributor

Also the 0-RTT messages, for the 1-RTT modes. This needs to be clearer.

@ekr ekr force-pushed the ekr:WIP_client_auth_revision branch from 6ffdc8f to 995e203 Nov 9, 2015

@ekr ekr added the Editor Ready label Nov 20, 2015

ekr added 3 commits Oct 17, 2015
Harmonize all of the authentication modes per offline
discussions.

Checkpoint

Checkpoint

Checkpoint

Restructure a bit

Checkpoint

Checkpoint

Post-handshake messages

Fix section levels

Update Finished

Checkpoint

Editorial

Fix build error

Trivial

Add open issue

Minor text

Remove Finished labels. Other nits

Fix verify_data description to re-add accidentally removed Hash())

@ekr ekr force-pushed the ekr:WIP_client_auth_revision branch from 2a0e3f6 to 4798eda Dec 1, 2015

@ekr

This comment has been minimized.

Copy link
Contributor Author

commented Dec 1, 2015

I've rebased this version and made some updates. I need to go over the diff again before I merge so there may be some issues, but I wanted to put it up so others could comment ahead of that. Expected merge tonight or tomorrow.

@ekr ekr closed this Dec 1, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.