[WIP] Etcd tls #218

colhom · 2016-10-12T06:22:33Z

The first commit is from #217, disregard for this review.

Here is the overview for kube-etcd-controller TLS flow, which should set us up well for integrating with pre-existing external PKI.

Trust delegation model:

kube-etcd-controller CA (self-signed or externally signed via CSR on startup)
    |
    |
    |-------->  etcd-cluster-A CA  (tpr object)
    |                  --------> etcd-cluster-A-0000 cert (pod)
    |                  --------> etcd-cluster-A-0001 cert 
    |                  --------> etcd-cluster-A-0002 cert 
    |           
    | ---------> etcd-cluster-B CA
                      --------> etcd-cluster-B-0000 cert
                      --------> etcd-cluster-B-0001 cert
                      --------> etcd-cluster-B-0002 cert

Here are the steps in order:

kube-etcd-controller pod startup:
- generate controller CA private key
- generate controller CA certificate
  - generate self-signed cert (default for now, useful for development mode)
  - generate CSR, wait for external entity to sign it and return cert via Kubernetes API (production mode, allows integration with arbitrary external PKI systems)
etcd cluster creation (in controller pod):
- generate private key
- generate controller CA as a subordinate CA of cluster CA using parameter from cluster spec
etcd node pod startup (in the etcd pod, prior to etcd application starting):
- generate private key
- generate a CSR for CN=etcd-cluster-xxxx, submit for signing via annotation
etcd node enrollment (in controller pod)
- observe new CSR annotation on pod/etcd-cluster-xxxx
- sign CSR with the cluster CA for pod/etcd-cluster-xxxx
  - --> return certificate via annotation to pod/etcd-cluster-xxxx
etcd node startup (in etcd pod)
- observe new signed certificate in annotation on pod/etcd-cluster-xxxx
- bring up etcd application with private key, cert and cluster CA certificate

So far I have implemented up through step 2

\cc @hongchaodeng @brianredbeard

also tear down backup replicaset before etcd pods. fixes #216

only support self-signed mode for now, appropriate hook in place for remote signing complete with todo's. This will involve exchanging a CSR for a Certificate via the Kubernetes API, allowing the delegating CA's private key material to remain private.

colhom · 2016-10-12T06:28:10Z

fixes #12

hongchaodeng · 2016-10-12T16:30:27Z

pkg/util/tlsutil/file.go

+)
+
+// Parse from file if exists, else create new and write to file
+func InitializePrivateKeyFile(keyPath string) (*rsa.PrivateKey, error) {


Why is it called "InitializePrivateKeyFile"? Is it gonna initialize private key or initialize a file?

hongchaodeng · 2016-10-12T16:32:20Z

pkg/util/tlsutil/rsa.go

+	return rsa.GenerateKey(rand.Reader, RSAKeySize)
+}
+
+func writePrivateKeyToFile(keyPath string, key *rsa.PrivateKey) error {


These two funcs aren't used in this file. Could you move them to where they are used?

hongchaodeng · 2016-10-12T16:35:44Z

@colhom
The description of this PR is very good.
Could you write them as a design doc under design/ and submit in another PR?
For feature, we usually have design go in first and then implementation.

hongchaodeng · 2016-10-12T16:41:46Z

cmd/controller/main.go

@@ -16,17 +17,26 @@ var (
 func init() {
 	flag.StringVar(&cfg.PVProvisioner, "pv-provisioner", "kubernetes.io/gce-pd", "persistent volume provisioner type")
 	flag.BoolVar(&analyticsEnabled, "analytics", true, "Send analytical event (Cluster Created/Deleted etc.) to Google Analytics")
+	flag.StringVar(&cfg.EtcdTLSConfig.Dir, "etcd-tls-dir", "/etc/kube-etcd-controller/tls/", "directory to store etcd tls files. in most cases, this path will be mounted in a secret.")


Can you move these under "masterhost" flag?
It'd better group all TLS flags together, and more specifically all etcd TLS together

hongchaodeng · 2016-10-12T16:43:11Z

cmd/controller/main.go

 	flag.Parse()

 	cfg.Namespace = os.Getenv("MY_POD_NAMESPACE")
 	if len(cfg.Namespace) == 0 {
 		cfg.Namespace = "default"
 	}
+	if err := os.MkdirAll(cfg.EtcdTLSConfig.Dir, 0600); err != nil {


Why we create the dir?
Why not let user create the dir because they pass in.

hongchaodeng · 2016-10-12T16:44:03Z

pkg/cluster/cluster.go

-	idCounter int
-	eventCh   chan *clusterEvent
-	stopCh    chan struct{}
+	kclient         *unversioned.Client


don't group them together.
And group ca stuff together.

hongchaodeng · 2016-10-12T16:47:46Z

pkg/controller/controller.go

 	}
 }

+func (c *Controller) initializeClusterTLS(clusterName string, clusterSpec *spec.ClusterSpec) (*rsa.PrivateKey, *x509.Certificate, error) {


Could you put TLS methods into another file "controller/tls.go"?

hongchaodeng · 2016-10-12T16:48:22Z

pkg/spec/cluster.go

@@ -40,6 +42,16 @@ type ClusterSpec struct {
 	// If there is no seed member, a completely new cluster will be created.
 	// There is no seed member by default.
 	Seed *SeedPolicy `json:"seed,omitempty"`
+	// Common name for the cluster CA
+	// Defaults to the cluster name
+	CACommonName string `json:"caCommonName,omitempty"`


Could you abstract another struct "TLSPolicy"?

colhom · 2016-10-12T19:51:22Z

@hongchaodeng I'm going to pause on this implementation and produce a design document before continuing (as you suggest). A few of your questions on this PR will hopefully be answered by the design doc.

hongchaodeng · 2016-10-12T20:08:26Z

@colhom Thanks!

dhawal55 · 2017-01-11T20:17:28Z

Any update on this?

hongchaodeng · 2017-01-11T20:23:08Z

@dhawal55

We have decided to go with #409

philips · 2017-01-19T14:23:57Z

Closing this PR for now. We can revisit once #409 is in place and build on top of #409.

colhom added 3 commits October 11, 2016 22:07

tear down pvc after cluster pods/services

4fcb51b

also tear down backup replicaset before etcd pods. fixes #216

[etcd tls] introduce tlsutil package

5b8d105

hongchaodeng reviewed Oct 12, 2016

View reviewed changes

colhom mentioned this pull request Oct 13, 2016

[etcd tls] design doc #224

Merged

philips mentioned this pull request Dec 23, 2016

tls: static configuration #504

Closed

philips closed this Jan 19, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Etcd tls #218

[WIP] Etcd tls #218

colhom commented Oct 12, 2016

colhom commented Oct 12, 2016

hongchaodeng Oct 12, 2016

hongchaodeng Oct 12, 2016

hongchaodeng commented Oct 12, 2016

hongchaodeng Oct 12, 2016

hongchaodeng Oct 12, 2016

hongchaodeng Oct 12, 2016

hongchaodeng Oct 12, 2016

hongchaodeng Oct 12, 2016

colhom commented Oct 12, 2016

hongchaodeng commented Oct 12, 2016

dhawal55 commented Jan 11, 2017

hongchaodeng commented Jan 11, 2017

philips commented Jan 19, 2017

[WIP] Etcd tls #218

[WIP] Etcd tls #218

Conversation

colhom commented Oct 12, 2016

colhom commented Oct 12, 2016

hongchaodeng Oct 12, 2016

Choose a reason for hiding this comment

hongchaodeng Oct 12, 2016

Choose a reason for hiding this comment

hongchaodeng commented Oct 12, 2016

hongchaodeng Oct 12, 2016

Choose a reason for hiding this comment

hongchaodeng Oct 12, 2016

Choose a reason for hiding this comment

hongchaodeng Oct 12, 2016

Choose a reason for hiding this comment

hongchaodeng Oct 12, 2016

Choose a reason for hiding this comment

hongchaodeng Oct 12, 2016

Choose a reason for hiding this comment

colhom commented Oct 12, 2016

hongchaodeng commented Oct 12, 2016

dhawal55 commented Jan 11, 2017

hongchaodeng commented Jan 11, 2017

philips commented Jan 19, 2017