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

proposal: crypto/tls: provide a way to access local certificate used to set up a connection #24673

Open
lyuxuan opened this issue Apr 3, 2018 · 5 comments

Comments

@lyuxuan
Copy link

commented Apr 3, 2018

While TLS provides ConnectionState() to share some connection security info, like remote certificate (PeerCertificates), it lacks the info about local certificate used to set up a connection. And it's not always possible to predict local certificate used as the selection depends on the remote end requirements.

Exposing the local certificate would be very helpful for debugging connection issues, for example, user may find certificate getter returns suboptimal certificate, which may be expiring soon or having a long verification chain. Moreover, it will also enable collecting certificate usage statistics, which could be valuable for service owners.

@FiloSottile Thanks!

@FiloSottile FiloSottile changed the title crypto/tls: provide a way to access local certificate used to set up a connection proposal: crypto/tls: provide a way to access local certificate used to set up a connection Apr 4, 2018

@gopherbot gopherbot added this to the Proposal milestone Apr 4, 2018

@gopherbot gopherbot added the Proposal label Apr 4, 2018

@FiloSottile FiloSottile added Proposal-Crypto and removed Proposal labels Apr 4, 2018

@gopherbot gopherbot added the Proposal label Apr 4, 2018

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Apr 4, 2018

I'll have to check if it would mess with 1.3, and think how to bundle chains, but I can see us adding ConnectionState.LocalCertificates.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 2018

Discussed with @agl and @FiloSottile about potentially adding

LocalCertificate *Certificate // (a *tls.Certificate)

but trying to understand the motivation a bit more. Compelling use cases would help with justification.

@lyuxuan

This comment has been minimized.

Copy link
Author

commented Apr 24, 2018

We have a specific use case in grpc channelz feature. It presents to users the current state of a connection, including the info about what's the local/remote certificate has been used. Users may use it to figure out what's the identity it presents to the peer, how long local certificate will last before expired, etc. Moreover, channel trace could record local certificates tried and failed before, which may be helpful for diagnosing problematic setup.

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Apr 24, 2018

Thanks for elaborating.

Is a ConnectionState field just more convenient, or does saving the result of GetCertificate /GetClientCertificate not work at all?

@agl I just saw this in the docs, what am I missing about TLS 1.3 that can lead to multiple client certs?

// GetClientCertificate may be called multiple times for the same
// connection if renegotiation occurs or if TLS 1.3 is in use.

@lyuxuan

This comment has been minimized.

Copy link
Author

commented Apr 27, 2018

Sorry about the delay in response.

GetCertificate and GetClientCertificate seems to be only part of the logic to select the local certificate, by reading the implementation of getCertificate function. If the returned result of GetCertificate and GetClientCertificate is nil, nil, then it will resort to NameToCertificate to find a certificate.

And yes, collocating local certificate with remote certificate in theConnectionState struct is convenient for usage.

Thank you.

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