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

Send "certificate_authorities" extension #372

Closed
wants to merge 9 commits into from
Closed

Send "certificate_authorities" extension #372

wants to merge 9 commits into from

Conversation

ocheron
Copy link
Contributor

@ocheron ocheron commented Jul 7, 2019

Main point here is to add "certificate_authorities" extension currently missing in TLS13 certificate requests so that we keep same capabilities than with lower versions. Unfortunetly I did not see earlier that the extension was missing so I had to change to a bit more complex design to send post-handshake requests.

The PR also allows to send CertificateRequired alerts to the client and adds a few checks.

Adds requirement from RFC 8446 section 4.4.2.4:

   If the server supplies an empty Certificate message, the client
   MUST abort the handshake with a "decode_error" alert.

This can apply before TLS 1.3 too because the public key is needed for
key exchange, and it then avoids the EmptyChain validation failure
which currently exists.
It's necessary to correctly send the alert to the peer.
@kazu-yamamoto kazu-yamamoto self-requested a review July 8, 2019 01:06
@kazu-yamamoto
Copy link
Collaborator

This looks good to me. But I have one question. What about certificate_authorities in client hello? Do you have any plans?

@ocheron
Copy link
Contributor Author

ocheron commented Jul 8, 2019

I don't have plan for client hello, there is no existing API this would be useful to.

Copy link
Collaborator

@kazu-yamamoto kazu-yamamoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

kazu-yamamoto added a commit to kazu-yamamoto/hs-tls that referenced this pull request Jul 9, 2019
@kazu-yamamoto
Copy link
Collaborator

Merged.

@ocheron ocheron deleted the cert-auth branch July 9, 2019 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants