Fix validation for self-signed cert and addressed comments

Signed-off-by: Cheng Cheng <chengcheng@apple.com>
kubevirt · Sep 9, 2020 · 14b4c53 · 14b4c53
1 parent f88c194
commit 14b4c53
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 10 deletions.
diff --git a/pkg/certificates/bootstrap/cert-manager.go b/pkg/certificates/bootstrap/cert-manager.go
@@ -107,13 +107,13 @@ func (f *FileCertificateManager) Start() {
 	certDir := filepath.Dir(f.certBytesPath)
 	err = watcher.Add(certDir)
 	if err != nil {
-		log.DefaultLogger().Reason(err).Criticalf("Failed to establish a watch on %s", certDir)
+		log.DefaultLogger().Reason(err).Criticalf("Failed to establish a watch on %s", f.certBytesPath)
 	}
 	keyDir := filepath.Dir(f.keyBytesPath)
 	if keyDir != certDir {
 		err = watcher.Add(keyDir)
 		if err != nil {
-			log.DefaultLogger().Reason(err).Criticalf("Failed to establish a watch on %s", keyDir)
+			log.DefaultLogger().Reason(err).Criticalf("Failed to establish a watch on %s", f.keyBytesPath)
 		}
 	}
 

diff --git a/pkg/util/webhooks/tls.go b/pkg/util/webhooks/tls.go
@@ -121,12 +121,16 @@ func SetupTLSForVirtHandlerServer(caManager ClientCAManager, certManager certifi
 					if externallyManaged {
 						intermediatePool = x509.NewCertPool()
 						for _, rawIntermediate := range rawIntermediates {
-							if c, err := x509.ParseCertificate(rawIntermediate); err != nil {
+							if ic, err := x509.ParseCertificate(rawIntermediate); err != nil {
 								log.Log.Warningf("failed to parse peer intermediate certificate: %v", err)
 							} else {
-								intermediatePool.AddCert(c)
+								intermediatePool.AddCert(ic)
 							}
 						}
+					} else {
+						if c.Subject.CommonName != "kubevirt.io:system:client:virt-handler" {
+							return fmt.Errorf("common name is invalid, expected %s, but got %s", "kubevirt.io:system:client:virt-handler", c.Subject.CommonName)
+						}
 					}
 
 					_, err = c.Verify(x509.VerifyOptions{
@@ -176,8 +180,8 @@ func SetupTLSForVirtHandlerClients(caManager ClientCAManager, certManager certif
 				return fmt.Errorf("no client certificate provided.")
 			}
 
-			rawClient, rawIntermediates := rawCerts[0], rawCerts[1:]
-			c, err := x509.ParseCertificate(rawClient)
+			rawServer, rawIntermediates := rawCerts[0], rawCerts[1:]
+			c, err := x509.ParseCertificate(rawServer)
 			if err != nil {
 				return fmt.Errorf("failed to parse peer certificate: %v", err)
 			}
@@ -186,12 +190,16 @@ func SetupTLSForVirtHandlerClients(caManager ClientCAManager, certManager certif
 			if externallyManaged {
 				intermediatePool = x509.NewCertPool()
 				for _, rawIntermediate := range rawIntermediates {
-					if c, err := x509.ParseCertificate(rawIntermediate); err != nil {
+					if ic, err := x509.ParseCertificate(rawIntermediate); err != nil {
 						log.Log.Warningf("failed to parse peer intermediate certificate: %v", err)
 					} else {
-						intermediatePool.AddCert(c)
+						intermediatePool.AddCert(ic)
 					}
 				}
+			} else {
+				if c.Subject.CommonName != "kubevirt.io:system:node:virt-handler" {
+					return fmt.Errorf("common name is invalid, expected %s, but got %s", "kubevirt.io:system:node:virt-handler", c.Subject.CommonName)
+				}
 			}
 
 			_, err = c.Verify(x509.VerifyOptions{

diff --git a/pkg/util/webhooks/tls_test.go b/pkg/util/webhooks/tls_test.go
@@ -78,8 +78,8 @@ var _ = Describe("TLS", func() {
 	})
 
 	table.DescribeTable("on virt-handler should", func(serverSecret, clientSecret string, errStr string) {
-		serverTLSConfig := webhooks.SetupTLSForVirtHandlerServer(caManager, certmanagers[serverSecret], true)
-		clientTLSConfig := webhooks.SetupTLSForVirtHandlerClients(caManager, certmanagers[clientSecret], true)
+		serverTLSConfig := webhooks.SetupTLSForVirtHandlerServer(caManager, certmanagers[serverSecret], false)
+		clientTLSConfig := webhooks.SetupTLSForVirtHandlerClients(caManager, certmanagers[clientSecret], false)
 		srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			fmt.Fprintln(w, "hello")
 		}))