From f6e1ff0f842309e347595d0f52b7a49d9ae3700e Mon Sep 17 00:00:00 2001
From: Sean Mooney <work@seanmooney.info>
Date: Tue, 9 Jan 2024 15:21:27 +0000
Subject: [PATCH] enable use of service_user tokens role enforcement

This change also removes outdated todo comments
from the init container script.

As part of CVE-2023-2088 a hardening recommendation was made
to start enforce that the service token user must have the service
role. This change updates the ironic config to enable that enfrocement.

Closes: OSPRH-3058
Closes: OSPRH-191
---
 templates/common/bin/common.sh                |  5 ----
 templates/common/config/ironic.conf           |  4 +++
 tests/functional/base_test.go                 |  6 ++---
 tests/functional/ironicapi_controller_test.go | 25 +++++++++++++++++++
 4 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/templates/common/bin/common.sh b/templates/common/bin/common.sh
index ac856fe8..3dd039f9 100755
--- a/templates/common/bin/common.sh
+++ b/templates/common/bin/common.sh
@@ -95,9 +95,4 @@ function common_ironic_config {
     crudini --set ${SVC_CFG_MERGED} nova password $IRONICPASSWORD
     crudini --set ${SVC_CFG_MERGED} swift password $IRONICPASSWORD
     crudini --set ${SVC_CFG_MERGED} inspector password $IRONICPASSWORD
-    # TODO: nova password
-    #crudini --set ${SVC_CFG_MERGED} nova password $NOVAPASSWORD
-    # TODO: service token
-    #crudini --set ${SVC_CFG_MERGED} service_user password $IronicPassword
-
 }
diff --git a/templates/common/config/ironic.conf b/templates/common/config/ironic.conf
index c31a2ff4..d74b828c 100644
--- a/templates/common/config/ironic.conf
+++ b/templates/common/config/ironic.conf
@@ -45,6 +45,10 @@ username={{ .ServiceUser }}
 www_authenticate_uri={{ .KeystonePublicURL }}
 auth_url={{ .KeystoneInternalURL }}
 auth_type=password
+# This is part of hardening related to CVE-2023-2088
+# https://docs.openstack.org/nova/latest/configuration/config.html#keystone_authtoken.service_token_roles_required
+# when enabled the service token user must have the service role to be considered valid.
+service_token_roles_required = true
 
 [service_catalog]
 auth_type=password
diff --git a/tests/functional/base_test.go b/tests/functional/base_test.go
index 51911567..f01fc669 100644
--- a/tests/functional/base_test.go
+++ b/tests/functional/base_test.go
@@ -41,9 +41,9 @@ const (
 	ContainerImage         = "test://ironic"
 	PxeContainerImage      = "test://pxe-image"
 	IronicPythonAgentImage = "test://ipa-image"
-	IronicInputHash        = "n5b6h585hf7h555h5ffh66ch5bdh55h695h97h558h7fh5bchbch5cch5dfh68bh667hd5h68h689hbch5b9h584h565hdfh56fh57h64fh58fh77h7fq"
-	ConductorInputHash     = "n5fbh587h74h4h78h5d6h676h677h77hd9hd6h68fh8dh74h5c8hc9hf6h75h64h65dhf8h8fh646h596h8dhf5h74h54chbchb6h558h68q"
-	APIInputHash           = "n6fh657h688h85h547h5d4h7fh5dch9h8h648hf8h8h695hd6h599hch558h5b9h5f5hd6h667h88h584h689h66h59h677hb7h8bh5f8h6dq"
+	IronicInputHash        = "n665h595h685hch649h579h9dh55h678hbch5d6h57ch564hd7hbdh5ddhd9h5c6h644h658h677hf9h5bch64bh574h654h5ddh59bh54ch579h56chc9q"
+	ConductorInputHash     = "n58dh5d4h58fh586h566h697h679h5fh68fhddh5c6h594h5f7h7fh594h5f7h88h588hf9hc4h5b6h5dh65ch95h77h5ddh7dh8fh599h579h55dh5fbq"
+	APIInputHash           = "n64fh5cfh9chfdh667hf9h654hd8h56dh568h675h5cfh8dh57h66bhbch5dfhdh5b4h54ch96h94h574h9h54fh649hf4h56bh566h694h664hfdq"
 )
 
 type IronicNames struct {
diff --git a/tests/functional/ironicapi_controller_test.go b/tests/functional/ironicapi_controller_test.go
index efb50da0..c36db228 100644
--- a/tests/functional/ironicapi_controller_test.go
+++ b/tests/functional/ironicapi_controller_test.go
@@ -16,6 +16,8 @@ limitations under the License.
 package functional_test
 
 import (
+	"fmt"
+
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	ironicv1 "github.com/openstack-k8s-operators/ironic-operator/api/v1beta1"
@@ -130,6 +132,29 @@ var _ = Describe("IronicAPI controller", func() {
 				corev1.ConditionTrue,
 			)
 		})
+		It("generated configs successfully", func() {
+			th.ExpectCondition(
+				ironicNames.APIName,
+				ConditionGetterFunc(IronicAPIConditionGetter),
+				condition.InputReadyCondition,
+				corev1.ConditionTrue,
+			)
+			instance := GetIronicAPI(ironicNames.APIName)
+			apiConfigMapName := types.NamespacedName{
+				Namespace: instance.Namespace,
+				Name:      fmt.Sprintf("%s-config-data", instance.Name),
+			}
+			configDataMap := th.GetConfigMap(apiConfigMapName)
+			Expect(configDataMap).ShouldNot(BeNil())
+			Expect(configDataMap.Data).Should(HaveKey("ironic.conf"))
+			configData := string(configDataMap.Data["ironic.conf"])
+			// as part of additional hardening we now require service_token_roles_required
+			// to be set to true to ensure that the service token is not just a user token
+			// ironic does not currently rely on the service token for enforcement of elevated
+			// privileges but this is a good practice to follow and might be required in the
+			// future
+			Expect(configData).Should(ContainSubstring("service_token_roles_required = true"))
+		})
 		It("Sets NetworkAttachmentsReady", func() {
 			th.ExpectCondition(
 				ironicNames.APIName,