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

Support for dynamic SSL on the client side #15735

Closed
crazed opened this issue Jun 13, 2018 · 10 comments · Fixed by #19346
Closed

Support for dynamic SSL on the client side #15735

crazed opened this issue Jun 13, 2018 · 10 comments · Fixed by #19346
Labels
area/security disposition/help wanted Maintainers do not have enough resources to allocate to this at the moment. Help is appreciated! kind/enhancement lang/Python priority/P2

Comments

@crazed
Copy link

crazed commented Jun 13, 2018

What version of gRPC and what language are you using?

1.12.0 on Python

What operating system (Linux, Windows, …) and version?

Linux

What runtime / compiler are you using (e.g. python version or version of gcc)

Python 2.7.14

What did you do?

Try to use dynamic client side TLS certificates, in this case we are utilizing Hashicorp's Vault PKI backend to roll client certificates every 2 hours.

What did you expect to see?

Behaviour similar to Go's tls.Config#GetClientCertificate

What did you see instead?

No support exists, though things seem to work until the variable you used for the client certificate is updated at the same time as the channel is being used. At that point this results in a deadline exceeded on the client side.

Anything else we should know about your project / environment?

I don't think so, let me know if you need further information. I'm pretty sure this requires a change similar to #12644 and #12689.

@mehrdada
Copy link
Member

We are definitely happy to review PRs that implement this, as we have on the server side. I don't foresee we'll have resources in immediate term to tackle this problem ourselves though.

@euroelessar
Copy link
Contributor

@jiangtaoli2016 Hi, any updates? It looks like SPIFFE set of patches on master still does not support client-side reloading.

@euroelessar
Copy link
Contributor

@yihuazhang Hi, can you chime in, please?

@yihuazhang
Copy link
Contributor

@euroelessar Yes, in both SSL and SPIFFE implementation, we do not support client-side credential reload now. The client when creating a channel needs to provision a valid certificate which will be used throughout the life time of the connection.

Do you know if gRPC Java and Go support client-side reloading? I am not sure if we want to change the existing behavior but if do, I would like to have gRPC Java and Go to agree on the design. Maybe creating a gRFC will be a good start.

@euroelessar
Copy link
Contributor

@yihuazhang Feel free to correct me, but grpc/proposal#98 does not specify that ability to reload key material is a property of the server, therefore this proposal covers both client and server.

gRPC-Go currently supports it by providing generic TransportCredentials API (https://godoc.org/google.golang.org/grpc/credentials#TransportCredentials).
gRPC-Java currently supports it by allowing to customize io.netty.handler.ssl.SslContextBuilder.
gRPC-Core is the only implementation which does not provide any means to dynamically reload certificates on client side and therefore forces us to maintain our own fork (based on #16567) with added functionality.

@sanjaypujare
Copy link
Contributor

@euroelessar I am trying to understand the use-case for "ability to reload key material" on the client side. As I see:

  • a TLS connection is already established with a valid & unexpired cert but the cert expires afterwards and the connection is still active. Do you want to initiate TLS renegotiation to use the new key/certs? Most systems check cert validity only at the beginning during full handshake.

  • all you want is the ability to dynamically update keys/certs in memory so that the next TLS connection attempt from the client side will pick up the latest certs. I thought that capability exists, no?

@euroelessar
Copy link
Contributor

@sanjaypujare

  • TLS renegotiation (point 1) is explicitly non-goal due to security concerns, extra complexity, etc.
  • We only need an ability to reload a key material for newly established connections (point 2). This functionality is currently missing in either of grpc-core TLS implementations.

@yihuazhang
Copy link
Contributor

@euroelessar, Yes, you are right. grpc/proposal#98 does not restrict credential reload to be a server-only property.

I still have a hard time justify your comment - "need an ability to reload a key material for newly established connections". In your PR (#16567), it seems to refresh the client side's SSL credential in ssl_channel_add_handshakers(), which I believe is called only once for the lifetime of the connection. That part of code is similar to what we have at the server side. For the server side, it is needed in a sense that add_handshakers() will be called each time a new client initiates the connection, and we need to make sure the server has a valid credential for each client connection.

More explanation will be appreciated.

@euroelessar
Copy link
Contributor

euroelessar commented Jun 8, 2019

@yihuazhang Sure, let me provide a bit more details about our setup:

  1. TLS certificates are short-living (lifetime is few days)
  2. This TLS certificates are rotated every few hours
  3. Client channels are long-living (lifetime can be months)
  4. Client channels create new connections occasionally (e.g. on resolver updates, backend restarts, etc)
  5. By the moment of establishing new TCP connection (≈weeks after the process start) the original cert (with lifetime of ≈days) is likely already expired and can not be used, therefore client needs to use the latest available TLS certificate.

Please note that individual gRPC channel maps to multiple underlying TCP connections, and this connections are destroyed/created over time. While it's true that add_handshakers is called once per lifetime of the connection, it's called unbounded amount of times per lifetime of the channel (due to one channel to many connections mapping).

Please look at ReloadClientCert test in our fork of grpc-v1.21 which covers this use case (single grpc channel with periodically restarted server, which ensures that client can or can-not re-connect based on the certificate combination).

@jiangtaoli2016
Copy link

jiangtaoli2016 commented Jun 8, 2019

@euroelessar It is a valid use case to support credential reloading on the client side.
@yihuazhang I think we plan to do on both client and server side. I did not know you did not implement it on the client side.

@lock lock bot locked as resolved and limited conversation to collaborators Nov 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/security disposition/help wanted Maintainers do not have enough resources to allocate to this at the moment. Help is appreciated! kind/enhancement lang/Python priority/P2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants