From 4359323558403b2e9b02ae3d20aea96ce56f5639 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Mon, 27 Nov 2023 13:46:34 -0800 Subject: [PATCH] Disable legacy RBAC policy by default. Change the default RBAC policy in ironic such that the new RBAC policy is enforced by default and the legacy policy is not usable unless explicitly re-enabled. Depends-On: https://review.opendev.org/c/openstack/metalsmith/+/905012 Change-Id: Id559f1d8b9a76c8a570b598585c2d58c56d08837 --- devstack/lib/ironic | 49 +++++++++++----- devstack/upgrade/upgrade.sh | 2 +- ironic/common/policy.py | 4 +- ironic/tests/unit/api/test_acl_basic.yaml | 8 +-- ironic/tests/unit/common/test_policy.py | 4 +- ...-default-rbac-policy-f2f154043910f26a.yaml | 57 +++++++++++++++++++ zuul.d/ironic-jobs.yaml | 7 ++- 7 files changed, 109 insertions(+), 22 deletions(-) create mode 100644 releasenotes/notes/change-default-rbac-policy-f2f154043910f26a.yaml diff --git a/devstack/lib/ironic b/devstack/lib/ironic index 917246b908..788d2555db 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -184,10 +184,8 @@ if [[ "$hostdomain" =~ "rax" ]] || [[ "$hostdomain" =~ "iweb" ]]; then fi -# Oslo Policy, as of Wallaby defaults to not enforcing request scope -# against requestors. This is anticipated to change in Xena or after -# the Xena release of OpenStack. -IRONIC_ENFORCE_SCOPE=$(trueorfalse False IRONIC_ENFORCE_SCOPE) +# Oslo Policy enforcement for Scope and new policy enforcement. +IRONIC_ENFORCE_SCOPE=$(trueorfalse True IRONIC_ENFORCE_SCOPE) if [[ "$IRONIC_ENFORCE_SCOPE" == "True" ]]; then IRONIC_OS_CLOUD=devstack-system-admin @@ -202,6 +200,12 @@ fi # we need this variable for Xena and possibly Wallaby grenade. OS_CLOUD=${OS_CLOUD:-devstack-admin} +# If the node owner needs to be set on nodes, for example, jobs +# such as metalsmith have hardcoded OS_CLOUD variables which mean +# an ironic in a full system scope enforced mode can't find nodes +# for an allocation as suddenly the owner is applied. +IRONIC_SET_NODE_OWNER=${IRONIC_SET_NODE_OWNER:-} + # Versions and command line for API client IRONIC_DEFAULT_API_VERSION=${IRONIC_DEFAULT_API_VERSION:-} IRONIC_CMD="openstack --os-cloud $IRONIC_OS_CLOUD baremetal" @@ -1529,9 +1533,9 @@ function configure_ironic { if [[ "$IRONIC_JSON_RPC_AUTH_STRATEGY" == "" ]] || [[ "$IRONIC_JSON_RPC_AUTH_STRATEGY" == "keystone" ]]; then configure_client_for json_rpc fi - if [[ "$IRONIC_ENFORCE_SCOPE" == "True" ]]; then - iniset $IRONIC_CONF_FILE oslo_policy enforce_scope true - iniset $IRONIC_CONF_FILE oslo_policy enforce_new_defaults true + if [[ "$IRONIC_ENFORCE_SCOPE" == "False" ]]; then + iniset $IRONIC_CONF_FILE oslo_policy enforce_scope false + iniset $IRONIC_CONF_FILE oslo_policy enforce_new_defaults false fi # Set fast track options @@ -1878,11 +1882,11 @@ function create_ironic_cache_dir { # create_ironic_accounts() - Set up common required ironic accounts -# Project User Roles +# Project User Roles # ------------------------------------------------------------------ -# service ironic admin -# service nova baremetal_admin -# demo demo baremetal_observer +# service ironic admin +# service nova baremetal_admin # Legacy, Pre-SRBAC effort +# demo demo baremetal_observer # Legacy, Pre-SRBAC effort function create_ironic_accounts { if [[ "$ENABLED_SERVICES" =~ "ir-api" && "$ENABLED_SERVICES" =~ "key" ]]; then # Define service and endpoints in Keystone @@ -1897,6 +1901,7 @@ function create_ironic_accounts { create_service_user "ironic" "admin" # Create additional bare metal tenant and roles + # TODO(TheJulia): Remove when we remove the legacy RBAC policies. get_or_create_role baremetal_admin get_or_create_role baremetal_observer if is_service_enabled nova; then @@ -1993,6 +1998,14 @@ function start_ironic_api { function start_ironic_conductor { run_process ir-cond "$IRONIC_BIN_DIR/ironic-conductor --config-file=$IRONIC_CONF_FILE" + # Unset variables which we shouldn't have... Grenade resets these :\ + # in grenade/projects/60_nova/resources.sh as part of the resource + # creation tests. + unset OS_TENANT_NAME + unset OS_USERNAME + unset OS_PROJECT_NAME + unset OS_PASSWORD + # Wait up to 30 seconds for ironic-conductor to start and register itself local attempt local max_attempts=7 @@ -2302,10 +2315,12 @@ function wait_for_nova_resources { local resource_class=${IRONIC_DEFAULT_RESOURCE_CLASS^^} # TODO(dtantsur): switch to Placement OSC plugin, once it exists + # Normally, we would want to use IRONIC_OS_CLOUD, but tokens need to be + # scoped. In this case, devstack-admin is the appropriate os-cloud to use. local token - token=$(openstack --os-cloud $IRONIC_OS_CLOUD token issue -f value -c id) + token=$(openstack --os-cloud devstack-admin token issue -f value -c id) local endpoint - endpoint=$(openstack --os-cloud $IRONIC_OS_CLOUD endpoint list --service placement --interface public -f value -c URL) + endpoint=$(openstack --os-cloud devstack-admin endpoint list --service placement --interface public -f value -c URL) die_if_not_set $LINENO endpoint "Cannot find Placement API endpoint" local i @@ -2626,6 +2641,14 @@ function enroll_nodes { $IRONIC_CMD node add trait $node_id $IRONIC_DEFAULT_TRAITS fi + if [[ -n "$IRONIC_SET_NODE_OWNER" ]]; then + # If set, apply an owner project project ID to the node, + # to allow tools like metalsmith to be abel to view and + # allocate the node. + owner_project_id=$(openstack project show $IRONIC_SET_NODE_OWNER -f value -c id) + $IRONIC_CMD node set --owner $owner_project_id $node_id + fi + $IRONIC_CMD node manage $node_id --wait $IRONIC_MANAGE_TIMEOUT || \ die $LINENO "Node did not reach manageable state in $IRONIC_MANAGE_TIMEOUT seconds" diff --git a/devstack/upgrade/upgrade.sh b/devstack/upgrade/upgrade.sh index a3d51696ac..5a2554a8ca 100755 --- a/devstack/upgrade/upgrade.sh +++ b/devstack/upgrade/upgrade.sh @@ -144,7 +144,7 @@ ensure_services_started $ensure_started # internal tag, that was assigned to network will be the same. As result we need to update # tag on link between br-int and brbm to new value after restart. if [[ -z "${IRONIC_PROVISION_NETWORK_NAME}" ]]; then - net_id=$(openstack network show ironic_grenade -f value -c id) + net_id=$(openstack --os-cloud devstack-admin network show ironic_grenade -f value -c id) create_ovs_taps $net_id fi diff --git a/ironic/common/policy.py b/ironic/common/policy.py index 8343afef62..776c94538e 100644 --- a/ironic/common/policy.py +++ b/ironic/common/policy.py @@ -36,7 +36,9 @@ LOG = log.getLogger(__name__) # once oslo_policy change the default value to 'policy.yaml'. # https://github.com/openstack/oslo.policy/blob/a626ad12fe5a3abd49d70e3e5b95589d279ab578/oslo_policy/opts.py#L49 DEFAULT_POLICY_FILE = 'policy.yaml' -opts.set_defaults(cfg.CONF, DEFAULT_POLICY_FILE) +opts.set_defaults(cfg.CONF, DEFAULT_POLICY_FILE, + enforce_scope=True, + enforce_new_defaults=True) # Generic policy check string for system administrators. These are the people # who need the highest level of authorization to operate the deployment. diff --git a/ironic/tests/unit/api/test_acl_basic.yaml b/ironic/tests/unit/api/test_acl_basic.yaml index 1ec0ab07a7..10243a60a3 100644 --- a/ironic/tests/unit/api/test_acl_basic.yaml +++ b/ironic/tests/unit/api/test_acl_basic.yaml @@ -13,13 +13,13 @@ unauthenticated_user_cannot_get_node: path: &node_path '/v1/nodes/{node_uuid}' assert_status: 404 +# With new defaults, by default admin rights don't let you see +# everything without rights. Since this is default/basic behavior +# it doesn't make sense to mock it out. project_admin_can_get_node: path: *node_path headers: *project_admin_headers - assert_dict_contains: - uuid: '{node_uuid}' - driver: 'fake-hardware' - assert_status: 200 + assert_status: 404 project_member_cannot_get_node: path: *node_path diff --git a/ironic/tests/unit/common/test_policy.py b/ironic/tests/unit/common/test_policy.py index beb5b62fd8..533cb397bf 100644 --- a/ironic/tests/unit/common/test_policy.py +++ b/ironic/tests/unit/common/test_policy.py @@ -174,7 +174,7 @@ class PolicyInCodeTestCase(base.TestCase): }]), dict( rule='baremetal:node:get', - check=True, + check=False, targets=[], creds=[{ 'roles': ['baremetal_observer'], @@ -188,7 +188,7 @@ class PolicyInCodeTestCase(base.TestCase): creds=[{'roles': ['generic_user'], 'tenant': 'demo'}]), dict( rule='baremetal:node:create', - check=True, + check=False, targets=[], creds=[{ 'roles': ['baremetal_admin'], diff --git a/releasenotes/notes/change-default-rbac-policy-f2f154043910f26a.yaml b/releasenotes/notes/change-default-rbac-policy-f2f154043910f26a.yaml new file mode 100644 index 0000000000..22e1ee7f78 --- /dev/null +++ b/releasenotes/notes/change-default-rbac-policy-f2f154043910f26a.yaml @@ -0,0 +1,57 @@ +--- +upgrade: + - | + The Ironic service API Role Based Access Control policy has been updated + to disable the legacy RBAC policy by default. The effect of this is that + deprecated legacy roles of ``baremetal_admin`` and ``baremetal_observer`` + are no longer functional by default, and policy checks may prevent actions + such as viewing nodes when access rights do not exist by default. + + This change is a result of the new policy which was introduced as part of + `Secure Role Based Access Control`_ effort along with the + `Consistent and Secure RBAC`_ community goal and the underlying + ``[oslo_policy] enforce_scope`` and ``[oslo_policy] enforce_new_defaults`` + settings being changed to ``True``. + + The Ironic project believes most operators will observe no direct impact + from this change, unless they are specifically running legacy access + configurations utilizing the legacy roles for access. + + Operators which are suddenly unable to list or deploy nodes may have + a misconfiguration in credentials, or need to allow the user's project + the ability to view and act upon the node through the node ``owner`` or + ``lessee`` fields. By default, the `Ironic API policy`_ permits + authenticated requests with a ``system`` scoped token to access + all resources, and applies a finer grained access model across the API + for project scoped users. + + Ironic users who have not already changed their ``nova-compute`` service + settings for connecting to Ironic may also have issues scheduling + Bare Metal nodes. Use of a ``system`` scoped user is available, by + setting ``[ironic] system_scope`` to a value of ``all`` in your + nova-compute service configuration, which can be done independently + of other services, as long as the credentials supplied are also valid + with Keystone for system scoped authentication. + + Heat users which encounter any issues after this upgrade, should check + their user's roles. Heat's execution and model is entirely project scoped, + which means users will need to have access granted through the ``owner`` + or ``lessee`` field to work with a node. + + Operators wishing to revert to the old policy configuration may do so + by setting the following values in ``ironic.conf``.:: + + [oslo_policy] + enforce_new_defaults=False + enforce_scope=False + + Operators who revert the configuration are encourated to make the + necessary changes to their configuration, as the legacy RBAC policy + will be removed at some point in the future in alignment with + `2024.1-Release Timeline`_. Failure to do so will + may force operators to craft custom policy override configuration. + + .. _`Secure Role Based Access Control`: https://specs.openstack.org/openstack/ironic-specs/specs/17.0/secure-rbac.html + .. _`Ironic API Policy`: https://docs.openstack.org/ironic/latest/configuration/sample-policy.html + .. _`Consistent and Secure RBAC`: https://governance.openstack.org/tc/goals/selected/consistent-and-secure-rbac.html + .. _`2024.1-Release Timeline`: https://governance.openstack.org/tc/goals/selected/consistent-and-secure-rbac.html#id3 diff --git a/zuul.d/ironic-jobs.yaml b/zuul.d/ironic-jobs.yaml index dc8179c353..ab445d89bc 100644 --- a/zuul.d/ironic-jobs.yaml +++ b/zuul.d/ironic-jobs.yaml @@ -379,7 +379,9 @@ IRONIC_TEMPEST_WHOLE_DISK_IMAGE: True IRONIC_VM_EPHEMERAL_DISK: 0 IRONIC_AUTOMATED_CLEAN_ENABLED: False - IRONIC_ENFORCE_SCOPE: True + # NOTE(TheJulia): Explicitly set scope enforcement to False until we + # remove the legacy policies. + IRONIC_ENFORCE_SCOPE: False IRONIC_BOOT_MODE: bios Q_AGENT: ovn Q_ML2_TENANT_NETWORK_TYPE: vlan @@ -974,6 +976,9 @@ INSTANCE_WAIT: 120 MYSQL_GATHER_PERFORMANCE: False CIRROS_VERSION: 0.6.1 + # Required as different access rights are used by default + # and the classic devstack config which is defaulted doesn't work. + IRONIC_ENFORCE_SCOPE: True old: IRONIC_VM_LOG_DIR: '{{ devstack_bases.old }}/ironic-bm-logs' grenade_localrc: