*: support ssl/tls #868
*: support ssl/tls #868
Conversation
Connor1996
force-pushed
the
support-ssl/tls
branch
from
86d10c6
to
5388ea3
Compare
pd-client/client.go
Outdated
| } | ||
|
|
||
| // NewClient creates a PD client. | ||
| func NewClient(pdAddrs []string) (Client, error) { | ||
| func NewClient(pdAddrs []string, tlsCAPath, tlsCertPath, tlsKeyPath string) (Client, error) { |
There was a problem hiding this comment.
move these to a SecurityOption struct.
|
I prefer to follow https://coreos.com/etcd/docs/latest/op-guide/security.html and use the same name flag. Btw, I don't think we need to support auto TLS now. /cc @disksing |
conf/config.toml
Outdated
| # Path of file that contains X509 key in PEM format. | ||
| tls-key-path = "" | ||
| # enable client certificate auth, if true following two settings shouldn't be empty | ||
| client-cert-auth = false |
There was a problem hiding this comment.
As follower pd-servers redirect all requests to leader, so typically if if TLS is enabled on server side, client side must enable it too. Therefore I think this flag looks redundant to me.
There was a problem hiding this comment.
Maybe we should let the client redirect to the new leader to reduce the latency.
Connor1996
force-pushed
the
support-ssl/tls
branch
from
8447579
to
2f89728
Compare
pd-client/client.go
Outdated
| } | ||
|
|
||
| // Append the certificates from the CA | ||
| if ok := certPool.AppendCertsFromPEM(ca); !ok { |
There was a problem hiding this comment.
if !certPool.AppendCertsFromPEM(ca) {
pdctl/command/global.go
Outdated
| } | ||
|
|
||
| // Append the certificates from the CA | ||
| if ok := certPool.AppendCertsFromPEM(ca); !ok { |
There was a problem hiding this comment.
if !certPool.AppendCertsFromPEM(ca) {
|
LGTM. Have you tested it with tikv-server? |
|
@disksing Yes |
|
Rest LGTM |
server/config.go
Outdated
| @@ -85,6 +85,10 @@ type Config struct { | |||
| // ElectionInterval is the interval for etcd Raft election. | |||
| ElectionInterval typeutil.Duration `toml:"election-interval"` | |||
|
|
|||
| TLSCAPath string `toml:"tls-cacert-path" json:"tls-cacert-path"` | |||
There was a problem hiding this comment.
I think we can remove tls and use a security section like TiKV does
cmd/pd-ctl/main.go
Outdated
| url string | ||
| detach bool | ||
| version bool | ||
| CAPath string |
|
PTAL @siddontang @disksing |
|
Fix #865 |
As mentioned in #865
pd-ctl and pd-client can connect with pd in ssl/tls connection, and etcd-client with etcd-server too.
when tls-ca-path in config.toml is set, the connection among etcd peers will also use ssl/tls with auto generated cert and key file.
note: relative urls should use https schema, or it will still not use ssl/tls
when using ssl/tls, there is a bug of embed etcd that will cause panic when stopping pd process.
etcd-io/etcd#8916