From c027fc0e6247fd3cc93eb7702d3703b141071632 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Fri, 20 Mar 2020 13:35:13 -0700 Subject: [PATCH 1/3] Make certificates per-profile and consistent until IP or names change --- pkg/minikube/bootstrapper/certs.go | 219 ++++++++++++------- pkg/minikube/bootstrapper/certs_test.go | 17 +- pkg/minikube/bootstrapper/kubeadm/kubeadm.go | 3 +- pkg/minikube/localpath/localpath.go | 5 + pkg/minikube/node/config.go | 5 +- pkg/provision/buildroot.go | 2 +- 6 files changed, 147 insertions(+), 104 deletions(-) diff --git a/pkg/minikube/bootstrapper/certs.go b/pkg/minikube/bootstrapper/certs.go index 652392cb6522..675ba79ed2ae 100644 --- a/pkg/minikube/bootstrapper/certs.go +++ b/pkg/minikube/bootstrapper/certs.go @@ -17,6 +17,7 @@ limitations under the License. package bootstrapper import ( + "crypto/sha1" "encoding/pem" "fmt" "io/ioutil" @@ -25,9 +26,11 @@ import ( "os/exec" "path" "path/filepath" + "sort" "strings" "github.com/golang/glog" + "github.com/otiai10/copy" "github.com/pkg/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/clientcmd/api" @@ -40,63 +43,50 @@ import ( "k8s.io/minikube/pkg/minikube/localpath" "k8s.io/minikube/pkg/minikube/vmpath" "k8s.io/minikube/pkg/util" - "k8s.io/minikube/pkg/util/lock" - - "github.com/juju/mutex" -) - -var ( - certs = []string{ - "ca.crt", "ca.key", "apiserver.crt", "apiserver.key", "proxy-client-ca.crt", - "proxy-client-ca.key", "proxy-client.crt", "proxy-client.key", - } ) // SetupCerts gets the generated credentials required to talk to the APIServer. -func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig, n config.Node) error { - - localPath := localpath.MiniPath() +func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig, n config.Node) ([]assets.CopyableFile, error) { + localPath := localpath.Profile(k8s.ClusterName) glog.Infof("Setting up %s for IP: %s\n", localPath, n.IP) - // WARNING: This function was not designed for multiple profiles, so it is VERY racey: - // - // It updates a shared certificate file and uploads it to the apiserver before launch. - // - // If another process updates the shared certificate, it's invalid. - // TODO: Instead of racey manipulation of a shared certificate, use per-profile certs - spec := lock.PathMutexSpec(filepath.Join(localPath, "certs")) - glog.Infof("acquiring lock: %+v", spec) - releaser, err := mutex.Acquire(spec) + ccs, err := generateSharedCACerts() if err != nil { - return errors.Wrapf(err, "unable to acquire lock for %+v", spec) + return nil, errors.Wrap(err, "shared CA certs") } - defer releaser.Release() - if err := generateCerts(k8s, n); err != nil { - return errors.Wrap(err, "Error generating certs") + xfer, err := generateProfileCerts(k8s, n, ccs) + if err != nil { + return nil, errors.Wrap(err, "profile certs") } + + xfer = append(xfer, ccs.caCert) + xfer = append(xfer, ccs.caKey) + xfer = append(xfer, ccs.proxyCert) + xfer = append(xfer, ccs.proxyKey) + copyableFiles := []assets.CopyableFile{} - for _, cert := range certs { - p := filepath.Join(localPath, cert) + for _, p := range xfer { + cert := filepath.Base(p) perms := "0644" if strings.HasSuffix(cert, ".key") { perms = "0600" } certFile, err := assets.NewFileAsset(p, vmpath.GuestKubernetesCertsDir, cert, perms) if err != nil { - return err + return nil, errors.Wrapf(err, "key asset %s", cert) } copyableFiles = append(copyableFiles, certFile) } caCerts, err := collectCACerts() if err != nil { - return err + return nil, err } for src, dst := range caCerts { certFile, err := assets.NewFileAsset(src, path.Dir(dst), path.Base(dst), "0644") if err != nil { - return err + return nil, errors.Wrapf(err, "ca asset %s", src) } copyableFiles = append(copyableFiles, certFile) @@ -114,11 +104,11 @@ func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig, n config.Node) kubeCfg := api.NewConfig() err = kubeconfig.PopulateFromSettings(kcs, kubeCfg) if err != nil { - return errors.Wrap(err, "populating kubeconfig") + return nil, errors.Wrap(err, "populating kubeconfig") } data, err := runtime.Encode(latest.Codec, kubeCfg) if err != nil { - return errors.Wrap(err, "encoding kubeconfig") + return nil, errors.Wrap(err, "encoding kubeconfig") } kubeCfgFile := assets.NewMemoryAsset(data, vmpath.GuestPersistentDir, "kubeconfig", "0644") @@ -126,28 +116,32 @@ func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig, n config.Node) for _, f := range copyableFiles { if err := cmd.Copy(f); err != nil { - return errors.Wrapf(err, "Copy %s", f.GetAssetName()) + return nil, errors.Wrapf(err, "Copy %s", f.GetAssetName()) } } if err := installCertSymlinks(cmd, caCerts); err != nil { - return errors.Wrapf(err, "certificate symlinks") + return nil, errors.Wrapf(err, "certificate symlinks") } - return nil + return copyableFiles, nil } -func generateCerts(k8s config.KubernetesConfig, n config.Node) error { - serviceIP, err := util.GetServiceClusterIP(k8s.ServiceCIDR) - if err != nil { - return errors.Wrap(err, "getting service cluster ip") - } - - localPath := localpath.MiniPath() - caCertPath := filepath.Join(localPath, "ca.crt") - caKeyPath := filepath.Join(localPath, "ca.key") +type CACerts struct { + caCert string + caKey string + proxyCert string + proxyKey string +} - proxyClientCACertPath := filepath.Join(localPath, "proxy-client-ca.crt") - proxyClientCAKeyPath := filepath.Join(localPath, "proxy-client-ca.key") +// generateSharedCACerts generates CA certs shared among profiles, but only if missing +func generateSharedCACerts() (CACerts, error) { + globalPath := localpath.MiniPath() + cc := CACerts{ + caCert: filepath.Join(globalPath, "ca.crt"), + caKey: filepath.Join(globalPath, "ca.key"), + proxyCert: filepath.Join(globalPath, "proxy-client-ca.crt"), + proxyKey: filepath.Join(globalPath, "proxy-client-ca.key"), + } caCertSpecs := []struct { certPath string @@ -155,17 +149,41 @@ func generateCerts(k8s config.KubernetesConfig, n config.Node) error { subject string }{ { // client / apiserver CA - certPath: caCertPath, - keyPath: caKeyPath, + certPath: cc.caCert, + keyPath: cc.caKey, subject: "minikubeCA", }, { // proxy-client CA - certPath: proxyClientCACertPath, - keyPath: proxyClientCAKeyPath, + certPath: cc.proxyCert, + keyPath: cc.proxyKey, subject: "proxyClientCA", }, } + for _, ca := range caCertSpecs { + if canRead(ca.certPath) && canRead(ca.keyPath) { + glog.Infof("skipping %s CA generation: %s", ca.subject, ca.keyPath) + continue + } + + glog.Infof("generating %s CA: %s", ca.subject, ca.keyPath) + if err := util.GenerateCACert(ca.certPath, ca.keyPath, ca.subject); err != nil { + return cc, errors.Wrap(err, "generate ca cert") + } + } + + return cc, nil +} + +// generateProfileCerts generates profile certs for a profile +func generateProfileCerts(k8s config.KubernetesConfig, n config.Node, ccs CACerts) ([]string, error) { + profilePath := localpath.Profile(k8s.ClusterName) + + serviceIP, err := util.GetServiceClusterIP(k8s.ServiceCIDR) + if err != nil { + return nil, errors.Wrap(err, "getting service cluster ip") + } + apiServerIPs := append( k8s.APIServerIPs, []net.IP{net.ParseIP(n.IP), serviceIP, net.ParseIP(oci.DefaultBindIPV4), net.ParseIP("10.0.0.1")}...) @@ -174,9 +192,19 @@ func generateCerts(k8s config.KubernetesConfig, n config.Node) error { apiServerNames, util.GetAlternateDNS(k8s.DNSDomain)...) - signedCertSpecs := []struct { - certPath string - keyPath string + // Generate a hash input for certs that depend on ip/name combinations + hi := []string{} + hi = append(hi, apiServerAlternateNames...) + for _, ip := range apiServerIPs { + hi = append(hi, ip.String()) + } + sort.Strings(hi) + + specs := []struct { + certPath string + keyPath string + hash string + subject string ips []net.IP alternateNames []string @@ -184,56 +212,77 @@ func generateCerts(k8s config.KubernetesConfig, n config.Node) error { caKeyPath string }{ { // Client cert - certPath: filepath.Join(localPath, "client.crt"), - keyPath: filepath.Join(localPath, "client.key"), + certPath: filepath.Join(profilePath, "client.crt"), + keyPath: filepath.Join(profilePath, "client.key"), subject: "minikube-user", ips: []net.IP{}, alternateNames: []string{}, - caCertPath: caCertPath, - caKeyPath: caKeyPath, + caCertPath: ccs.caCert, + caKeyPath: ccs.caKey, }, { // apiserver serving cert - certPath: filepath.Join(localPath, "apiserver.crt"), - keyPath: filepath.Join(localPath, "apiserver.key"), + hash: fmt.Sprintf("%x", sha1.Sum([]byte(strings.Join(hi, "/"))))[0:8], + certPath: filepath.Join(profilePath, "apiserver.crt"), + keyPath: filepath.Join(profilePath, "apiserver.key"), subject: "minikube", ips: apiServerIPs, alternateNames: apiServerAlternateNames, - caCertPath: caCertPath, - caKeyPath: caKeyPath, + caCertPath: ccs.caCert, + caKeyPath: ccs.caKey, }, { // aggregator proxy-client cert - certPath: filepath.Join(localPath, "proxy-client.crt"), - keyPath: filepath.Join(localPath, "proxy-client.key"), + certPath: filepath.Join(profilePath, "proxy-client.crt"), + keyPath: filepath.Join(profilePath, "proxy-client.key"), subject: "aggregator", ips: []net.IP{}, alternateNames: []string{}, - caCertPath: proxyClientCACertPath, - caKeyPath: proxyClientCAKeyPath, + caCertPath: ccs.proxyCert, + caKeyPath: ccs.proxyKey, }, } - for _, caCertSpec := range caCertSpecs { - if !(canReadFile(caCertSpec.certPath) && - canReadFile(caCertSpec.keyPath)) { - if err := util.GenerateCACert( - caCertSpec.certPath, caCertSpec.keyPath, caCertSpec.subject, - ); err != nil { - return errors.Wrap(err, "Error generating CA certificate") - } + xfer := []string{} + for _, spec := range specs { + if spec.subject != "minikube-user" { + xfer = append(xfer, spec.certPath) + xfer = append(xfer, spec.keyPath) + } + + cp := spec.certPath + kp := spec.keyPath + if spec.hash != "" { + cp = cp + "." + spec.hash + kp = kp + "." + spec.hash + } + + if canRead(cp) && canRead(kp) { + glog.Infof("skipping %s signed cert generation: %s", spec.subject, kp) + continue } - } - for _, signedCertSpec := range signedCertSpecs { - if err := util.GenerateSignedCert( - signedCertSpec.certPath, signedCertSpec.keyPath, signedCertSpec.subject, - signedCertSpec.ips, signedCertSpec.alternateNames, - signedCertSpec.caCertPath, signedCertSpec.caKeyPath, - ); err != nil { - return errors.Wrap(err, "Error generating signed apiserver serving cert") + glog.Infof("generating %s signed cert: %s", spec.subject, kp) + err := util.GenerateSignedCert( + cp, kp, spec.subject, + spec.ips, spec.alternateNames, + spec.caCertPath, spec.caKeyPath, + ) + if err != nil { + return xfer, errors.Wrapf(err, "generate signed cert for %q", spec.subject) + } + + if spec.hash != "" { + glog.Infof("copying %s -> %s", cp, spec.certPath) + if err := copy.Copy(cp, spec.certPath); err != nil { + return xfer, errors.Wrap(err, "copy cert") + } + glog.Infof("copying %s -> %s", kp, spec.keyPath) + if err := copy.Copy(kp, spec.keyPath); err != nil { + return xfer, errors.Wrap(err, "copy key") + } } } - return nil + return xfer, nil } // isValidPEMCertificate checks whether the input file is a valid PEM certificate (with at least one CERTIFICATE block) @@ -355,9 +404,9 @@ func installCertSymlinks(cr command.Runner, caCerts map[string]string) error { return nil } -// canReadFile returns true if the file represented +// canRead returns true if the file represented // by path exists and is readable, otherwise false. -func canReadFile(path string) bool { +func canRead(path string) bool { f, err := os.Open(path) if err != nil { return false diff --git a/pkg/minikube/bootstrapper/certs_test.go b/pkg/minikube/bootstrapper/certs_test.go index d92e17466035..fd96b6a8385a 100644 --- a/pkg/minikube/bootstrapper/certs_test.go +++ b/pkg/minikube/bootstrapper/certs_test.go @@ -24,7 +24,6 @@ import ( "k8s.io/minikube/pkg/minikube/command" "k8s.io/minikube/pkg/minikube/config" "k8s.io/minikube/pkg/minikube/constants" - "k8s.io/minikube/pkg/minikube/localpath" "k8s.io/minikube/pkg/minikube/tests" "k8s.io/minikube/pkg/util" ) @@ -58,20 +57,8 @@ func TestSetupCerts(t *testing.T) { f := command.NewFakeCommandRunner() f.SetCommandToOutput(expected) - var filesToBeTransferred []string - for _, cert := range certs { - filesToBeTransferred = append(filesToBeTransferred, filepath.Join(localpath.MiniPath(), cert)) - } - filesToBeTransferred = append(filesToBeTransferred, filepath.Join(localpath.MiniPath(), "ca.crt")) - filesToBeTransferred = append(filesToBeTransferred, filepath.Join(localpath.MiniPath(), "certs", "mycert.pem")) - - if err := SetupCerts(f, k8s, config.Node{}); err != nil { + _, err := SetupCerts(f, k8s, config.Node{}) + if err != nil { t.Fatalf("Error starting cluster: %v", err) } - for _, cert := range filesToBeTransferred { - _, err := f.GetFileToContents(cert) - if err != nil { - t.Errorf("Cert not generated: %s", cert) - } - } } diff --git a/pkg/minikube/bootstrapper/kubeadm/kubeadm.go b/pkg/minikube/bootstrapper/kubeadm/kubeadm.go index 39618cf8a2b6..df5dc17a0efd 100644 --- a/pkg/minikube/bootstrapper/kubeadm/kubeadm.go +++ b/pkg/minikube/bootstrapper/kubeadm/kubeadm.go @@ -397,7 +397,8 @@ func (k *Bootstrapper) DeleteCluster(k8s config.KubernetesConfig) error { // SetupCerts sets up certificates within the cluster. func (k *Bootstrapper) SetupCerts(k8s config.KubernetesConfig, n config.Node) error { - return bootstrapper.SetupCerts(k.c, k8s, n) + _, err := bootstrapper.SetupCerts(k.c, k8s, n) + return err } // UpdateCluster updates the cluster diff --git a/pkg/minikube/localpath/localpath.go b/pkg/minikube/localpath/localpath.go index d9faef5d9a74..3dd6d4158ce1 100644 --- a/pkg/minikube/localpath/localpath.go +++ b/pkg/minikube/localpath/localpath.go @@ -54,6 +54,11 @@ func MakeMiniPath(fileName ...string) string { return filepath.Join(args...) } +// Profile returns the path to a profile +func Profile(name string) string { + return filepath.Join(MiniPath(), "profiles", name) +} + // MachinePath returns the Minikube machine path of a machine func MachinePath(machine string, miniHome ...string) string { miniPath := MiniPath() diff --git a/pkg/minikube/node/config.go b/pkg/minikube/node/config.go index b29867f1b6ae..1ee307dcc67b 100644 --- a/pkg/minikube/node/config.go +++ b/pkg/minikube/node/config.go @@ -122,6 +122,7 @@ func setupKubeAdm(mAPI libmachine.API, cfg config.ClusterConfig, node config.Nod if err := bs.UpdateCluster(cfg); err != nil { exit.WithError("Failed to update cluster", err) } + if err := bs.SetupCerts(cfg.KubernetesConfig, node); err != nil { exit.WithError("Failed to setup certs", err) } @@ -137,8 +138,8 @@ func setupKubeconfig(h *host.Host, cc *config.ClusterConfig, n *config.Node, clu kcs := &kubeconfig.Settings{ ClusterName: clusterName, ClusterServerAddress: addr, - ClientCertificate: localpath.MakeMiniPath("client.crt"), - ClientKey: localpath.MakeMiniPath("client.key"), + ClientCertificate: filepath.Join(localpath.Profile(cc.Name), "client.crt"), + ClientKey: filepath.Join(localpath.Profile(cc.Name), "client.key"), CertificateAuthority: localpath.MakeMiniPath("ca.crt"), KeepContext: viper.GetBool(keepContext), EmbedCerts: viper.GetBool(embedCerts), diff --git a/pkg/provision/buildroot.go b/pkg/provision/buildroot.go index 193478f21530..15d9d39d7447 100644 --- a/pkg/provision/buildroot.go +++ b/pkg/provision/buildroot.go @@ -160,7 +160,7 @@ WantedBy=multi-user.target return nil, err } - if err := p.Service("docker", serviceaction.Restart); err != nil { + if err := p.Service("docker", serviceaction.Start); err != nil { return nil, err } return dockerCfg, nil From 0711bd07bded295c09435baf3bc32b4fef1d2879 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Fri, 20 Mar 2020 13:38:42 -0700 Subject: [PATCH 2/3] Revert test change --- pkg/provision/buildroot.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/provision/buildroot.go b/pkg/provision/buildroot.go index 15d9d39d7447..193478f21530 100644 --- a/pkg/provision/buildroot.go +++ b/pkg/provision/buildroot.go @@ -160,7 +160,7 @@ WantedBy=multi-user.target return nil, err } - if err := p.Service("docker", serviceaction.Start); err != nil { + if err := p.Service("docker", serviceaction.Restart); err != nil { return nil, err } return dockerCfg, nil From 66e7acd39a7c36fe295cb1d6ffcca4f5aab18bd8 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Sat, 21 Mar 2020 08:15:05 -0700 Subject: [PATCH 3/3] Fix merge regression, add localpath functions for certs --- pkg/minikube/bootstrapper/certs.go | 6 +++--- pkg/minikube/localpath/localpath.go | 15 +++++++++++++++ pkg/minikube/node/start.go | 6 +++--- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/pkg/minikube/bootstrapper/certs.go b/pkg/minikube/bootstrapper/certs.go index e752b732a7da..de938082ccd6 100644 --- a/pkg/minikube/bootstrapper/certs.go +++ b/pkg/minikube/bootstrapper/certs.go @@ -139,7 +139,7 @@ type CACerts struct { func generateSharedCACerts() (CACerts, error) { globalPath := localpath.MiniPath() cc := CACerts{ - caCert: filepath.Join(globalPath, "ca.crt"), + caCert: localpath.CACert(), caKey: filepath.Join(globalPath, "ca.key"), proxyCert: filepath.Join(globalPath, "proxy-client-ca.crt"), proxyKey: filepath.Join(globalPath, "proxy-client-ca.key"), @@ -214,8 +214,8 @@ func generateProfileCerts(k8s config.KubernetesConfig, n config.Node, ccs CACert caKeyPath string }{ { // Client cert - certPath: filepath.Join(profilePath, "client.crt"), - keyPath: filepath.Join(profilePath, "client.key"), + certPath: localpath.ClientCert(k8s.ClusterName), + keyPath: localpath.ClientKey(k8s.ClusterName), subject: "minikube-user", ips: []net.IP{}, alternateNames: []string{}, diff --git a/pkg/minikube/localpath/localpath.go b/pkg/minikube/localpath/localpath.go index 3dd6d4158ce1..6bc9ef1239c3 100644 --- a/pkg/minikube/localpath/localpath.go +++ b/pkg/minikube/localpath/localpath.go @@ -59,6 +59,21 @@ func Profile(name string) string { return filepath.Join(MiniPath(), "profiles", name) } +// ClientCert returns client certificate path, used by kubeconfig +func ClientCert(name string) string { + return filepath.Join(Profile(name), "client.crt") +} + +// ClientKey returns client certificate path, used by kubeconfig +func ClientKey(name string) string { + return filepath.Join(Profile(name), "client.key") +} + +// CACert returns the minikube CA certificate shared between profiles +func CACert() string { + return filepath.Join(MiniPath(), "ca.crt") +} + // MachinePath returns the Minikube machine path of a machine func MachinePath(machine string, miniHome ...string) string { miniPath := MiniPath() diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index b46469837eda..d69bad75f754 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -250,9 +250,9 @@ func setupKubeconfig(h *host.Host, cc *config.ClusterConfig, n *config.Node, clu kcs := &kubeconfig.Settings{ ClusterName: clusterName, ClusterServerAddress: addr, - ClientCertificate: localpath.MakeMiniPath("client.crt"), - ClientKey: localpath.MakeMiniPath("client.key"), - CertificateAuthority: localpath.MakeMiniPath("ca.crt"), + ClientCertificate: localpath.ClientCert(cc.Name), + ClientKey: localpath.ClientKey(cc.Name), + CertificateAuthority: localpath.CACert(), KeepContext: viper.GetBool(keepContext), EmbedCerts: viper.GetBool(embedCerts), }