Improve certificate manager (#1133)

* improve certificate manager for yurthub

* fix hub root dir comment

cmd/yurt-tunnel-server/app/start.go

@@ -177,12 +177,12 @@ func Run(cfg *config.CompletedConfig, stopCh <-chan struct{}) error {
 	}, stopCh)
 
 	// 6. generate the TLS configuration based on the latest certificate
-	tlsCfg, err := certmanager.GenTLSConfigUseCertMgrAndCertPool(serverCertMgr, cfg.RootCert, "server")
+	tlsCfg, err := certmanager.GenTLSConfigUseCurrentCertAndCertPool(serverCertMgr.Current, cfg.RootCert, "server")
 	if err != nil {
 		return err
 	}
 
-	proxyClientTlsCfg, err := certmanager.GenTLSConfigUseCertMgrAndCertPool(tunnelProxyCertMgr, cfg.RootCert, "client")
+	proxyClientTlsCfg, err := certmanager.GenTLSConfigUseCurrentCertAndCertPool(tunnelProxyCertMgr.Current, cfg.RootCert, "client")
 	if err != nil {
 		return err
 	}

cmd/yurthub/app/config/config.go

@@ -65,9 +65,6 @@ type YurtHubConfiguration struct {
 	YurtHubProxyServerDummyAddr       string
 	YurtHubProxyServerSecureDummyAddr string
 	GCFrequency                       int
-	CertMgrMode                       string
-	KubeletRootCAFilePath             string
-	KubeletPairFilePath               string
 	NodeName                          string
 	HeartbeatFailedRetry              int
 	HeartbeatHealthyThreshold         int
@@ -92,6 +89,7 @@ type YurtHubConfiguration struct {
 	CertIPs                           []net.IP
 	CoordinatorServer                 *url.URL
 	MinRequestTimeout                 time.Duration
+	CaCertHashes                      []string
 }
 
 // Complete converts *options.YurtHubOptions to *YurtHubConfiguration
@@ -156,8 +154,6 @@ func Complete(options *options.YurtHubOptions) (*YurtHubConfiguration, error) {
 		YurtHubProxyServerDummyAddr:       proxyServerDummyAddr,
 		YurtHubProxyServerSecureDummyAddr: proxySecureServerDummyAddr,
 		GCFrequency:                       options.GCFrequency,
-		KubeletRootCAFilePath:             options.KubeletRootCAFilePath,
-		KubeletPairFilePath:               options.KubeletPairFilePath,
 		NodeName:                          options.NodeName,
 		HeartbeatFailedRetry:              options.HeartbeatFailedRetry,
 		HeartbeatHealthyThreshold:         options.HeartbeatHealthyThreshold,
@@ -180,6 +176,7 @@ func Complete(options *options.YurtHubOptions) (*YurtHubConfiguration, error) {
 		FilterManager:                     filterManager,
 		CertIPs:                           certIPs,
 		MinRequestTimeout:                 options.MinRequestTimeout,
+		CaCertHashes:                      options.CACertHashes,
 	}
 
 	return cfg, nil

cmd/yurthub/app/options/options.go

@@ -48,8 +48,6 @@ type YurtHubOptions struct {
 	YurtHubProxySecurePort    string
 	GCFrequency               int
 	YurtHubCertOrganizations  string
-	KubeletRootCAFilePath     string
-	KubeletPairFilePath       string
 	NodeName                  string
 	NodePoolName              string
 	LBMode                    string
@@ -74,6 +72,8 @@ type YurtHubOptions struct {
 	KubeletHealthGracePeriod  time.Duration
 	EnableNodePool            bool
 	MinRequestTimeout         time.Duration
+	CACertHashes              []string
+	UnsafeSkipCAVerification  bool
 }
 
 // NewYurtHubOptions creates a new YurtHubOptions with a default config.
@@ -85,8 +85,6 @@ func NewYurtHubOptions() *YurtHubOptions {
 		YurtHubPort:               util.YurtHubPort,
 		YurtHubProxySecurePort:    util.YurtHubProxySecurePort,
 		GCFrequency:               120,
-		KubeletRootCAFilePath:     util.DefaultKubeletRootCAFilePath,
-		KubeletPairFilePath:       util.DefaultKubeletPairFilePath,
 		LBMode:                    "rr",
 		HeartbeatFailedRetry:      3,
 		HeartbeatHealthyThreshold: 2,
@@ -106,6 +104,7 @@ func NewYurtHubOptions() *YurtHubOptions {
 		KubeletHealthGracePeriod:  time.Second * 40,
 		EnableNodePool:            true,
 		MinRequestTimeout:         time.Second * 1800,
+		UnsafeSkipCAVerification:  true,
 	}
 	return o
 }
@@ -132,6 +131,10 @@ func (options *YurtHubOptions) Validate() error {
 		return fmt.Errorf("dummy ip %s is not invalid, %w", options.HubAgentDummyIfIP, err)
 	}
 
+	if len(options.CACertHashes) == 0 && !options.UnsafeSkipCAVerification {
+		return fmt.Errorf("Set --discovery-token-unsafe-skip-ca-verification flag as true or pass CACertHashes to continue")
+	}
+
 	return nil
 }
 
@@ -144,8 +147,6 @@ func (o *YurtHubOptions) AddFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&o.YurtHubProxySecurePort, "proxy-secure-port", o.YurtHubProxySecurePort, "the port on which to proxy HTTPS requests to kube-apiserver")
 	fs.StringVar(&o.ServerAddr, "server-addr", o.ServerAddr, "the address of Kubernetes kube-apiserver,the format is: \"server1,server2,...\"")
 	fs.StringVar(&o.YurtHubCertOrganizations, "hub-cert-organizations", o.YurtHubCertOrganizations, "Organizations that will be added into hub's client certificate in hubself cert-mgr-mode, the format is: certOrg1,certOrg1,...")
-	fs.StringVar(&o.KubeletRootCAFilePath, "kubelet-ca-file", o.KubeletRootCAFilePath, "the ca file path used by kubelet.")
-	fs.StringVar(&o.KubeletPairFilePath, "kubelet-client-certificate", o.KubeletPairFilePath, "the path of kubelet client certificate file.")
 	fs.IntVar(&o.GCFrequency, "gc-frequency", o.GCFrequency, "the frequency to gc cache in storage(unit: minute).")
 	fs.StringVar(&o.NodeName, "node-name", o.NodeName, "the name of node that runs hub agent")
 	fs.StringVar(&o.LBMode, "lb-mode", o.LBMode, "the mode of load balancer to connect remote servers(rr, priority)")
@@ -171,6 +172,8 @@ func (o *YurtHubOptions) AddFlags(fs *pflag.FlagSet) {
 	fs.DurationVar(&o.KubeletHealthGracePeriod, "kubelet-health-grace-period", o.KubeletHealthGracePeriod, "the amount of time which we allow kubelet to be unresponsive before stop renew node lease")
 	fs.BoolVar(&o.EnableNodePool, "enable-node-pool", o.EnableNodePool, "enable list/watch nodepools resource or not for filters(only used for testing)")
 	fs.DurationVar(&o.MinRequestTimeout, "min-request-timeout", o.MinRequestTimeout, "An optional field indicating at least how long a proxy handler must keep a request open before timing it out. Currently only honored by the local watch request handler(use request parameter timeoutSeconds firstly), which picks a randomized value above this number as the connection timeout, to spread out load.")
+	fs.StringSliceVar(&o.CACertHashes, "discovery-token-ca-cert-hash", []string{}, "For token-based discovery, validate that the root CA public key matches this hash (format: \"<type>:<value>\").")
+	fs.BoolVar(&o.UnsafeSkipCAVerification, "discovery-token-unsafe-skip-ca-verification", o.UnsafeSkipCAVerification, "For token-based discovery, allow joining without --discovery-token-ca-cert-hash pinning.")
 }
 
 // verifyDummyIP verify the specified ip is valid or not and set the default ip if empty

cmd/yurthub/app/start.go

@@ -19,7 +19,6 @@ package app
 import (
 	"fmt"
 	"net/url"
-	"path/filepath"
 	"time"
 
 	"github.com/spf13/cobra"
@@ -33,7 +32,7 @@ import (
 	"github.com/openyurtio/openyurt/cmd/yurthub/app/options"
 	"github.com/openyurtio/openyurt/pkg/projectinfo"
 	"github.com/openyurtio/openyurt/pkg/yurthub/cachemanager"
-	"github.com/openyurtio/openyurt/pkg/yurthub/certificate/hubself"
+	"github.com/openyurtio/openyurt/pkg/yurthub/certificate/token"
 	"github.com/openyurtio/openyurt/pkg/yurthub/gc"
 	"github.com/openyurtio/openyurt/pkg/yurthub/healthchecker"
 	hubrest "github.com/openyurtio/openyurt/pkg/yurthub/kubernetes/rest"
@@ -87,24 +86,23 @@ func NewCmdStartYurtHub(stopCh <-chan struct{}) *cobra.Command {
 func Run(cfg *config.YurtHubConfiguration, stopCh <-chan struct{}) error {
 	trace := 1
 	klog.Infof("%d. register cert managers", trace)
-	certManager, err := hubself.NewYurtHubCertManager(cfg)
+	certManager, err := token.NewYurtHubCertManager(nil, cfg, stopCh)
 	if err != nil {
 		return fmt.Errorf("failed to create cert manager for yurthub, %v", err)
 	}
 	trace++
 
 	certManager.Start()
+	defer certManager.Stop()
 	err = wait.PollImmediate(5*time.Second, 4*time.Minute, func() (bool, error) {
-		curr := certManager.Current()
-		if curr != nil {
+		isReady := certManager.Ready()
+		if isReady {
 			return true, nil
 		}
-
-		klog.Infof("waiting for preparing client certificate")
 		return false, nil
 	})
 	if err != nil {
-		return fmt.Errorf("client certificate preparation failed, %v", err)
+		return fmt.Errorf("hub certificates preparation failed, %v", err)
 	}
 	trace++
 
@@ -137,16 +135,15 @@ func Run(cfg *config.YurtHubConfiguration, stopCh <-chan struct{}) error {
 	}
 	trace++
 
-	klog.Infof("%d. new restConfig manager for %s mode", trace, cfg.CertMgrMode)
-	restConfigMgr, err := hubrest.NewRestConfigManager(cfg, certManager, healthChecker)
+	klog.Infof("%d. new restConfig manager", trace)
+	restConfigMgr, err := hubrest.NewRestConfigManager(certManager, healthChecker)
 	if err != nil {
 		return fmt.Errorf("could not new restConfig manager, %w", err)
 	}
 	trace++
 
 	klog.Infof("%d. create tls config for secure servers ", trace)
-	cfg.TLSConfig, err = server.GenUseCertMgrAndTLSConfig(
-		restConfigMgr, certManager, filepath.Join(cfg.RootDir, "pki"), cfg.NodeName, cfg.CertIPs, stopCh)
+	cfg.TLSConfig, err = server.GenUseCertMgrAndTLSConfig(certManager)
 	if err != nil {
 		return fmt.Errorf("could not create tls config, %w", err)
 	}

pkg/controller/certificates/csrapprover.go

@@ -40,7 +40,7 @@ import (
 	"k8s.io/klog/v2"
 
 	"github.com/openyurtio/openyurt/pkg/projectinfo"
-	"github.com/openyurtio/openyurt/pkg/yurthub/certificate/hubself"
+	"github.com/openyurtio/openyurt/pkg/yurthub/certificate/token"
 	"github.com/openyurtio/openyurt/pkg/yurttunnel/constants"
 )
 
@@ -432,7 +432,7 @@ func isYurtHubNodeCert(csr *certificatesv1.CertificateSigningRequest, x509cr *x5
 		return false
 	} else {
 		for _, org := range x509cr.Subject.Organization {
-			if org != hubself.YurtHubCSROrg && org != user.NodesGroup && !strings.HasPrefix(org, yurtHubNodeCertOrgPrefix) {
+			if org != token.YurtHubCSROrg && org != user.NodesGroup && !strings.HasPrefix(org, yurtHubNodeCertOrgPrefix) {
 				return false
 			}
 		}

pkg/controller/certificates/csrapprover_test.go

@@ -31,7 +31,7 @@ import (
 	"k8s.io/client-go/util/cert"
 	"k8s.io/klog/v2"
 
-	"github.com/openyurtio/openyurt/pkg/yurthub/certificate/hubself"
+	"github.com/openyurtio/openyurt/pkg/yurthub/certificate/token"
 	"github.com/openyurtio/openyurt/pkg/yurttunnel/constants"
 )
 
@@ -91,7 +91,7 @@ func TestIsYurtCSR(t *testing.T) {
 						certificatesv1.UsageKeyEncipherment,
 						certificatesv1.UsageClientAuth,
 					},
-					Request: newCSRData("system:node:xxx", []string{hubself.YurtHubCSROrg, user.NodesGroup, "openyurt:tenant:xxx"}, []string{}, []net.IP{}),
+					Request: newCSRData("system:node:xxx", []string{token.YurtHubCSROrg, user.NodesGroup, "openyurt:tenant:xxx"}, []string{}, []net.IP{}),
 				},
 			},
 			exp: true,
@@ -106,7 +106,7 @@ func TestIsYurtCSR(t *testing.T) {
 						certificatesv1.UsageKeyEncipherment,
 						certificatesv1.UsageClientAuth,
 					},
-					Request: newCSRData("system:node:xxx", []string{hubself.YurtHubCSROrg, user.NodesGroup, "unknown org"}, []string{}, []net.IP{}),
+					Request: newCSRData("system:node:xxx", []string{token.YurtHubCSROrg, user.NodesGroup, "unknown org"}, []string{}, []net.IP{}),
 				},
 			},
 			exp: false,

pkg/node-servant/components/yurthub.go

@@ -30,10 +30,11 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/klog/v2"
 
+	"github.com/openyurtio/openyurt/pkg/projectinfo"
 	"github.com/openyurtio/openyurt/pkg/util/templates"
-	constants "github.com/openyurtio/openyurt/pkg/yurtadm/constants"
+	"github.com/openyurtio/openyurt/pkg/yurtadm/constants"
 	enutil "github.com/openyurtio/openyurt/pkg/yurtadm/util/edgenode"
-	"github.com/openyurtio/openyurt/pkg/yurthub/certificate/hubself"
+	"github.com/openyurtio/openyurt/pkg/yurthub/certificate/token"
 	"github.com/openyurtio/openyurt/pkg/yurthub/storage/disk"
 	"github.com/openyurtio/openyurt/pkg/yurthub/util"
 )
@@ -150,7 +151,7 @@ func getYurthubYaml(podManifestPath string) string {
 }
 
 func getYurthubConf() string {
-	return filepath.Join(hubself.HubRootDir, hubself.HubName)
+	return filepath.Join(token.DefaultRootDir, projectinfo.GetHubName())
 }
 
 func getYurthubCacheDir() string {

pkg/util/certmanager/factory/factory.go

@@ -28,6 +28,7 @@ import (
 	"k8s.io/client-go/util/certificate"
 	"k8s.io/klog/v2"
 
+	"github.com/openyurtio/openyurt/pkg/util"
 	"github.com/openyurtio/openyurt/pkg/util/certmanager/store"
 )
 
@@ -78,20 +79,33 @@ type CertManagerFactory interface {
 	New(*CertManagerConfig) (certificate.Manager, error)
 }
 
+type factory struct {
+	clientsetFn certificate.ClientsetFunc
+	fileStore   certificate.FileStore
+}
+
 func NewCertManagerFactory(clientSet kubernetes.Interface) CertManagerFactory {
 	return &factory{
-		clientset: clientSet,
+		clientsetFn: func(current *tls.Certificate) (kubernetes.Interface, error) {
+			return clientSet, nil
+		},
 	}
 }
 
-type factory struct {
-	clientset kubernetes.Interface
+func NewCertManagerFactoryWithFnAndStore(clientsetFn certificate.ClientsetFunc, store certificate.FileStore) CertManagerFactory {
+	return &factory{
+		clientsetFn: clientsetFn,
+		fileStore:   store,
+	}
 }
 
 func (f *factory) New(cfg *CertManagerConfig) (certificate.Manager, error) {
-	store, err := store.NewFileStoreWrapper(cfg.ComponentName, cfg.CertDir, cfg.CertDir, "", "")
-	if err != nil {
-		return nil, fmt.Errorf("failed to initialize the server certificate store: %w", err)
+	var err error
+	if util.IsNil(f.fileStore) {
+		f.fileStore, err = store.NewFileStoreWrapper(cfg.ComponentName, cfg.CertDir, cfg.CertDir, "", "")
+		if err != nil {
+			return nil, fmt.Errorf("failed to initialize the server certificate store: %w", err)
+		}
 	}
 
 	ips, dnsNames := cfg.IPs, cfg.DNSNames
@@ -139,12 +153,11 @@ func (f *factory) New(cfg *CertManagerConfig) (certificate.Manager, error) {
 	}
 
 	return certificate.NewManager(&certificate.Config{
-		ClientsetFn: func(current *tls.Certificate) (kubernetes.Interface, error) {
-			return f.clientset, nil
-		},
+		ClientsetFn:      f.clientsetFn,
 		SignerName:       cfg.SignerName,
 		GetTemplate:      getTemplate,
 		Usages:           usages,
-		CertificateStore: store,
+		CertificateStore: f.fileStore,
+		Logf:             klog.Infof,
 	})
 }