Skip to content

Commit

Permalink
MGMT-16721: Handle luks == nil and luks.Clevis == nil
Browse files Browse the repository at this point in the history
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.

There are some unanswered questions around why luks.Clevis would be nil and whether or not we should take other actions, this will be investigated in https://issues.redhat.com/browse/MGMT-16721
  • Loading branch information
paul-maidment committed Feb 7, 2024
1 parent df15ca0 commit 6b6be5e
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 30 deletions.
8 changes: 8 additions & 0 deletions internal/host/hostcommands/tang_connectivity_check_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
110 changes: 81 additions & 29 deletions internal/host/hostcommands/tang_connectivity_check_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion internal/host/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down

0 comments on commit 6b6be5e

Please sign in to comment.