From b205a61ec92f6e67bc1157ba2bd37ef9b7e46a5d Mon Sep 17 00:00:00 2001 From: Marco Kawajiri Date: Tue, 21 Jan 2025 07:59:55 -0800 Subject: [PATCH] Handle host identities Summary: Adding host client certificate support to `cached_acl_provider.py` to support host certificates other than the managed host This will allow client certificates with a `host:/` Subject Common Name to be referenced in `/etc/aclcache.json.gz` and `acl_config.py` (for example, to allow cert-authenticated requests from other hosts in the same rack as the wedge RSW). Test Plan: Unit tests (`./tests2/ptest-runner rest-api`), setup `acl_config.py` and `aclcache.json.gz` files to allow a test certificate with Subject CN `host:a_hostname.facebook.com` to do `POST /redfish/v1/Chassis/Rack/Actions/Chassis.Reset`: ``` echo ' RULES = {"/redfish/v1/Chassis/Rack/Actions/Chassis.Reset": {"POST": ["MANAGED_HOST_ANY", "POST:redfish_rack_actions_chassis_reset"]}} RULES_REGEXP = {} ' > /usr/local/fbpackages/rest-api/acl_config.py echo '{"host:a_hostname.facebook.com": ["POST:redfish_rack_actions_chassis_reset"]}' | gzip > /etc/aclcache.json.gz sv restart restapi ``` Run curl command, ensure it succeeds: ``` curl -f -v -k --cert client-cert.pem --key client-key.pem -X POST "https://$TARGET_BMC/redfish/v1/Chassis/Rack/Actions/Chassis.Reset" -d '{"ResetType": "ForceOff"}' ``` Reviewed By: alandau Differential Revision: D68328510 fbshipit-source-id: 7d1924cc1e3ec8938114eda0954ac816b302df75 --- .../acl_providers/cached_acl_provider.py | 16 ++++++++++++++ .../acl_providers/common_acl_provider_base.py | 5 ++--- .../files/test_cached_acl_provider.py | 21 ++++++++++++++++++- .../files/test_common_acl_provider_base.py | 21 +++++++++++-------- 4 files changed, 50 insertions(+), 13 deletions(-) diff --git a/common/recipes-rest/rest-api/files/acl_providers/cached_acl_provider.py b/common/recipes-rest/rest-api/files/acl_providers/cached_acl_provider.py index 8beee7fa939a..b6b8394f8d2b 100644 --- a/common/recipes-rest/rest-api/files/acl_providers/cached_acl_provider.py +++ b/common/recipes-rest/rest-api/files/acl_providers/cached_acl_provider.py @@ -65,6 +65,22 @@ def is_user_authorized(self, identity: Identity, permissions: t.List[str]) -> bo user_roles = self._get_permissions_for_user_identity(identity) return any(role in permissions for role in user_roles) + def is_host_authorized(self, identity: Identity, permissions: t.List[str]) -> bool: + """ + Returns True if any role matching the host identity are in the given + permissions list. + """ + if not identity.host: + return False + + # Check if it's a managed host + if super().is_host_authorized(identity, permissions): + return True + + # Check host roles in ACL + host_roles = self.aclrules.get("host:" + str(identity.host), []) + return any(role in permissions for role in host_roles) + def _load_aclrules(self) -> None: if not self.cachepath: server_logger.warning( diff --git a/common/recipes-rest/rest-api/files/acl_providers/common_acl_provider_base.py b/common/recipes-rest/rest-api/files/acl_providers/common_acl_provider_base.py index 5a67b135e8e0..03de7f122bfe 100644 --- a/common/recipes-rest/rest-api/files/acl_providers/common_acl_provider_base.py +++ b/common/recipes-rest/rest-api/files/acl_providers/common_acl_provider_base.py @@ -60,13 +60,12 @@ def is_user_authorized(self, identity: Identity, permissions: t.List[str]) -> bo """ pass - @classmethod - def is_host_authorized(cls, identity: Identity, permissions: t.List[str]) -> bool: + def is_host_authorized(self, identity: Identity, permissions: t.List[str]) -> bool: """ Returns True if any role matching the host identity are in the given permissions list. """ - host_roles = cls._get_roles_for_host(identity) + host_roles = self._get_roles_for_host(identity) return any(role in permissions for role in host_roles) ## Utils diff --git a/common/recipes-rest/rest-api/files/test_cached_acl_provider.py b/common/recipes-rest/rest-api/files/test_cached_acl_provider.py index c1db59373e1f..3f46ad9261a3 100644 --- a/common/recipes-rest/rest-api/files/test_cached_acl_provider.py +++ b/common/recipes-rest/rest-api/files/test_cached_acl_provider.py @@ -34,7 +34,10 @@ class TestCachedAclProvider(unittest.TestCase): def setUp(self): self.loop = asyncio.new_event_loop() with tempfile.NamedTemporaryFile("wb") as tmpfile: - rules = {"deathowl": ["test", "awesome"]} + rules = { + "deathowl": ["test", "awesome"], + "host:a_hostname.facebook.com": ["example_host_permission"], + } tmpfile.write(gzip.compress(bytes(json.dumps(rules), encoding="utf-8"))) tmpfile.seek(0) self.subject = cached_acl_provider.CachedAclProvider(cachepath=tmpfile.name) @@ -62,6 +65,22 @@ def testAclProviderDeniesPermissionIfIdentityIsInvalid(self): identity = common_auth.Identity(user="nosuchthing", host=None) self.assertFalse(self.subject.is_user_authorized(identity, ["derp"])) + def testAclProvderHostIdentityGrantsPermissionIfMatching(self): + identity = common_auth.Identity(user=None, host="a_hostname.facebook.com") + self.assertTrue( + self.subject.is_host_authorized(identity, ["example_host_permission"]) + ) + + def testAclProvderHostIdentityDeniesPermissionIfNotMatching(self): + identity = common_auth.Identity(user=None, host="a_hostname.facebook.com") + self.assertFalse(self.subject.is_host_authorized(identity, ["awesome"])) + + def testAclProviderHostIdentityDeniesPermissionIfIdentityIsInvalid(self): + identity = common_auth.Identity(user=None, host="another_hostname.facebook.com") + self.assertFalse( + self.subject.is_user_authorized(identity, ["example_host_permission"]) + ) + def test_signal_handler(self): with tempfile.NamedTemporaryFile("wb") as tmpfile: rules = {"deathowl": ["test", "awesome"]} diff --git a/common/recipes-rest/rest-api/files/test_common_acl_provider_base.py b/common/recipes-rest/rest-api/files/test_common_acl_provider_base.py index 39efe059d52f..78b201e17cb6 100644 --- a/common/recipes-rest/rest-api/files/test_common_acl_provider_base.py +++ b/common/recipes-rest/rest-api/files/test_common_acl_provider_base.py @@ -35,7 +35,6 @@ class MockAclProvider(AclProviderBase): always_authorize_user = False - always_authorize_host = False def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -45,9 +44,6 @@ def __init__(self, *args, **kwargs): def is_user_authorized(self, identity, permissions): return self.always_authorize_user - def is_host_authorized(self, identity, permissions): - return self.always_authorize_host - class TestCachedAclProvider(unittest.TestCase): def setUp(self): @@ -63,15 +59,18 @@ def setUp(self): self.addCleanup(p.stop) def test_is_host_authorized_no_identity(self): - res = AclProviderBase.is_host_authorized(common_auth.NO_IDENTITY, []) + acl_provider = MockAclProvider() + res = acl_provider.is_host_authorized(common_auth.NO_IDENTITY, []) self.assertEqual(res, False) def test_is_host_authorized_no_permissions(self): - res = AclProviderBase.is_host_authorized(EXAMPLE_MANAGED_HOST_IDENTITY, []) + acl_provider = MockAclProvider() + res = acl_provider.is_host_authorized(EXAMPLE_MANAGED_HOST_IDENTITY, []) self.assertEqual(res, False) def test_is_host_authorized(self): - res = AclProviderBase.is_host_authorized( + acl_provider = MockAclProvider() + res = acl_provider.is_host_authorized( EXAMPLE_MANAGED_HOST_IDENTITY, ["MANAGED_HOST_ANY"] ) self.assertEqual(res, True) @@ -125,9 +124,13 @@ def test_is_authorized_user(self): def test_is_authorized_host(self): acl_provider = MockAclProvider() - acl_provider.always_authorize_host = True - self.assertEqual(acl_provider.is_authorized(common_auth.NO_IDENTITY, []), True) + self.assertEqual( + acl_provider.is_authorized( + EXAMPLE_MANAGED_HOST_IDENTITY, ["MANAGED_HOST_ANY"] + ), + True, + ) def test_is_authorized_unauthorized(self): acl_provider = MockAclProvider()