Enable secure communication with TLS#10
Conversation
JustinCappos
left a comment
There was a problem hiding this comment.
Do we also need add a ticket to back out these changes later? It seems that X.509 verification is an important part of what we would actually eventually want.
| ccmsg.RegisterClientCacheServer(grpcServer, &grpcClientCacheServer{cache: c}) | ||
| ccmsg.RegisterPublisherCacheServer(grpcServer, &grpcPublisherCacheServer{cache: c}) | ||
| // Enable to observe gRPC latency | ||
| // grpc_prometheus.EnableHandlingTimeHistogram() |
There was a problem hiding this comment.
Why are we leaving in commented code?
There was a problem hiding this comment.
It is only required for performance analysis, so benchmarking. Therefore, I put this comment until adding a configuration parameter for relevant daemons to enable/disable this feature. A person who is not an expert on Prometheus may spend so much time to find out this feature.
| grpcServer := common.NewDBGRPCServer(conf.Insecure, db) | ||
| ccmsg.RegisterNodeBootstrapdServer(grpcServer, &grpcBootstrapServer{bootstrap: b}) | ||
| // Enable to observe gRPC latency | ||
| // grpc_prometheus.EnableHandlingTimeHistogram() |
There was a problem hiding this comment.
Why are we leaving this in?
There was a problem hiding this comment.
To have fast access to that until having a configuration parameter for this in relevant daemons.
|
|
||
| lp.server = common.NewDBGRPCServer(db) | ||
| // LogPipe configuration for secure communication missing | ||
| lp.server = common.NewDBGRPCServer(true, db) |
There was a problem hiding this comment.
What are the ramifications of setting this up without security?
There was a problem hiding this comment.
The first thing that comes to mind is that eavesdropping can occur if the logpipe daemon is outside the network or even on a local network. Data tampering may also be possible if I'm not mistaken, so a malicious actor could manipulate the log data.
|
There is a configuration parameter as |
In this pull request, you find three main changes affecting the gRPC helpers, the sections using these helpers, and the Prometheus registering.
How does it work?
First, it requires to generate the CA’s certificate and the CA’s private key. Following that, the server’s certificate and the server’s private key for each daemon, except for caches and logpiped for now, by using the CA’s certificate and the CA’s private key. If the secure mode is enabled using the configuration, the server-side TLS implementation loads the server’s certificate and private key from the
/tlsdirectory.The client-side TLS implementation works in the same way. If CacheCash runs in the secure mode, it loads the CA certificate that signed the server’s certificate. In this way, a client checks if it communicates with the right server by verifying the server’s certificate.
gRPC helpers:
These are the functions that generate gRPC clients and servers for all the relevant daemons. Passing the TLS credentials into the options was missing before. I implement a part that allows passing a secure bool in these helpers. And the helpers load the relevant certificate and private key if the security mode is on. The responsible person for the deployment of CacheCash needs to generate the certificates and put them in the
/tlsdirectory.Note: The file names to store the certificate and private keys at, it should be configurable.
There are comments in the relevant parts to show how to enable handling time histogram for gRPC services.
I propose to use a merge pull request if this passes the review.