vitalif
/
etcd
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

etcdmain: grpc-proxy should only require CN-less certificates for --cert flags. 

We have following communication schema:
client --- 1 ---> grpc-proxy --- 2 --- > etcd-server

There are 2 sets of flags/certs in grpc proxy [ https://github.com/etcd-io/etcd/blob/master/etcdmain/grpc_proxy.go#L140 ]:
 A. (cert-file, key-file, trusted-ca-file, auto-tls) this are controlling [1] so client to proxy connection and in particular they are describing proxy public identity.
 B. (cert,key, cacert ) - these are controlling [2] so what's the identity that proxy uses to make connections to the etcd-server.

If 2 (B.) contains certificate with CN and etcd-server is running with --client-cert-auth=true, the CN can be used as identity of 'client' from service perspective. This is permission escalation, that we should forbid.

If 1 (A.) contains certificate with CN - it should be considered perfectly valid. The server can (should) have full identity.

So only --cert flag (and not --cert-file flag) should be validated for empty CN.
release-3.5
Piotr Tabor 2020-09-07 11:52:45 +02:00
parent 2c93127c7b
commit 2d0ce9de3d
2 changed files with 10 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
etcdmain/grpc_proxy.go
View File

@ -181,7 +181,11 @@ func startGRPCProxy(cmd *cobra.Command, args []string) {
}
grpclog.SetLoggerV2(gl)
tlsinfo := newTLS(grpcProxyListenCA, grpcProxyListenCert, grpcProxyListenKey)
// The proxy itself (ListenCert) can have not-empty CN.
// The empty CN is required for grpcProxyCert.
// Please see https://github.com/etcd-io/etcd/issues/11970#issuecomment-687875315 for more context.
tlsinfo := newTLS(grpcProxyListenCA, grpcProxyListenCert, grpcProxyListenKey, false)
if tlsinfo == nil && grpcProxyListenAutoTLS {
host := []string{"https://" + grpcProxyListenAddr}
dir := filepath.Join(grpcProxyDataDir, "fixtures", "proxy")
@ -320,7 +324,8 @@ func newClientCfg(lg *zap.Logger, eps []string) (*clientv3.Config, error) {
cfg.MaxCallRecvMsgSize = grpcMaxCallRecvMsgSize
}
tls := newTLS(grpcProxyCA, grpcProxyCert, grpcProxyKey)
lg.Info("grpcProxyCA for connections to etcd-server")
tls := newTLS(grpcProxyCA, grpcProxyCert, grpcProxyKey, true)
if tls == nil && grpcProxyInsecureSkipTLSVerify {
tls = &transport.TLSInfo{}
}
@ -339,11 +344,11 @@ func newClientCfg(lg *zap.Logger, eps []string) (*clientv3.Config, error) {
return &cfg, nil
}
func newTLS(ca, cert, key string) *transport.TLSInfo {
func newTLS(ca, cert, key string, requireEmptyCN bool) *transport.TLSInfo {
if ca == "" && cert == "" && key == "" {
return nil
}
return &transport.TLSInfo{TrustedCAFile: ca, CertFile: cert, KeyFile: key, EmptyCN: true}
return &transport.TLSInfo{TrustedCAFile: ca, CertFile: cert, KeyFile: key, EmptyCN: requireEmptyCN}
}
func mustListenCMux(lg *zap.Logger, tlsinfo *transport.TLSInfo) cmux.CMux {

2
pkg/transport/listener.go
View File

@ -437,7 +437,7 @@ func (info TLSInfo) ClientConfig() (*tls.Config, error) {
return tls.X509KeyPair(certPEMBlock, keyPEMBlock)
})
if hasNonEmptyCN {
return nil, fmt.Errorf("cert has non empty Common Name (%s)", cn)
return nil, fmt.Errorf("cert has non empty Common Name (%s): %s", cn, info.CertFile)
}
}