MGMT-16721: Handle luks == nil and luks.Clevis == nil

In line 41 of internal/host/hostcommands/tang_connectivity_check_cmd.go Both `luks` and `luks.Clevis` are pointers which can be nil, there was no checking in place to ensure that we avoid a nil pointer dereference This PR fixes that by making appropriate checks to ensure that neither of these pointers are nil.
openshift · Feb 7, 2024 · 9a82bd4 · 9a82bd4
1 parent df15ca0
commit 9a82bd4
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 30 deletions.
diff --git a/internal/host/hostcommands/tang_connectivity_check_cmd.go b/internal/host/hostcommands/tang_connectivity_check_cmd.go
@@ -38,6 +38,14 @@ func (c *tangConnectivityCheckCmd) getTangServersFromHostIgnition(host *models.H
 		if err != nil {
 			return nil, err
 		}
+		if luks == nil {
+			return nil, nil
+		}
+		if luks.Clevis == nil {
+			// This cluster does not use tang, so we shouldn't perform a tang connectivity check
+			c.log.Warn("luks clevis configuration is missing or incomplete in the host ignition, disk encryption will be assumed to be disabled. (MGMT-16721)")
+			return nil, nil
+		}
 		if len(luks.Clevis.Tang) != 0 {
 			if tangServers, err = json.Marshal(luks.Clevis.Tang); err != nil {
 				return nil, err

diff --git a/internal/host/hostcommands/tang_connectivity_check_cmd_test.go b/internal/host/hostcommands/tang_connectivity_check_cmd_test.go
@@ -39,36 +39,88 @@ var _ = Describe("tangConnectivitycheckcmd", func() {
 
 	})
 
-	It("get_step: Tang imported cluster should use host ignition", func() {
+	const hostIgnition = `{
+		"ignition": {
+		  "config": {},
+		  "version": "3.2.0"
+		},
+		"storage": {
+			"luks": [
+				{
+				  "clevis": {
+					"tang": [
+					  {
+						"thumbprint": "nWW89qAs1hDPKiIcae-ey2cQmUk",
+						"url": "http://foo.bar"
+					  }
+					]
+				  },
+				  "device": "/dev/disk/by-partlabel/root",
+				  "name": "root",
+				  "options": [
+					"--cipher",
+					"aes-cbc-essiv:sha256"
+				  ],
+				  "wipeVolume": true
+				}
+			],
+		  "files": []
+		}
+	  }`
+
+	const hostIgnitionWithoutLuks = `{
+		"ignition": {
+		  "config": {},
+		  "version": "3.2.0"
+		},
+		"storage": {
+		  "files": []
+		}
+	  }`
+
+	const hostIgnitionWithoutClevis = `{
+		"ignition": {
+		  "config": {},
+		  "version": "3.2.0"
+		},
+		"storage": {
+			"luks": [
+			],
+		  "files": []
+		}
+	  }`
 
-		const hostIgnition = `{
-			"ignition": {
-			  "config": {},
-			  "version": "3.2.0"
-			},
-			"storage": {
-				"luks": [
-					{
-					  "clevis": {
-						"tang": [
-						  {
-							"thumbprint": "nWW89qAs1hDPKiIcae-ey2cQmUk",
-							"url": "http://foo.bar"
-						  }
-						]
-					  },
-					  "device": "/dev/disk/by-partlabel/root",
-					  "name": "root",
-					  "options": [
-						"--cipher",
-						"aes-cbc-essiv:sha256"
-					  ],
-					  "wipeVolume": true
-					}
-				],
-			  "files": []
-			}
-		  }`
+	It("Should not panic if Luks is undefined", func() {
+		imported := true
+		cluster.Imported = &imported
+		Expect(db.Create(&cluster).Error).ShouldNot(HaveOccurred())
+		apiVipConnectivity, err := json.Marshal(models.APIVipConnectivityResponse{
+			IsSuccess: true,
+			Ignition:  hostIgnitionWithoutLuks,
+		})
+		Expect(err).ToNot(HaveOccurred())
+		host.APIVipConnectivity = string(apiVipConnectivity)
+		Expect(db.Save(&host).Error).ShouldNot(HaveOccurred())
+		stepReply, stepErr = tangConnectivityCheckCmd.GetSteps(ctx, &host)
+		Expect(stepErr).ShouldNot(HaveOccurred())
+	})
+
+	It("Should not panic if Clevis is undefined", func() {
+		imported := true
+		cluster.Imported = &imported
+		Expect(db.Create(&cluster).Error).ShouldNot(HaveOccurred())
+		apiVipConnectivity, err := json.Marshal(models.APIVipConnectivityResponse{
+			IsSuccess: true,
+			Ignition:  hostIgnitionWithoutClevis,
+		})
+		Expect(err).ToNot(HaveOccurred())
+		host.APIVipConnectivity = string(apiVipConnectivity)
+		Expect(db.Save(&host).Error).ShouldNot(HaveOccurred())
+		stepReply, stepErr = tangConnectivityCheckCmd.GetSteps(ctx, &host)
+		Expect(stepErr).ShouldNot(HaveOccurred())
+	})
+
+	It("get_step: Tang imported cluster should use host ignition", func() {
 
 		By("Set cluster to imported and inject host ignition for test", func() {
 			imported := true

diff --git a/internal/host/validator.go b/internal/host/validator.go
@@ -420,7 +420,7 @@ func (v *validator) diskEncryptionRequirementsSatisfied(c *validationContext) (V
 			return ValidationPending, "Missing ignition information"
 		}
 		if luks == nil || luks.Clevis == nil {
-			// Disk encryption is disabled for workers on day1 cluster
+			// No tang servers to validate for the target cluster
 			return ValidationSuccessSuppressOutput, ""
 		}
 		c.cluster.DiskEncryption = &models.DiskEncryption{}