From 0e23aa632cf17093a6d572ee3c7c82b8ac685313 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 10 Dec 2011 14:37:41 -0800 Subject: [PATCH 01/16] clean it up yo Change-Id: I67cf63e52870d36c29e3787fb06fb7abeb74fd3a --- etc/nova/policy.json | 14 +++++++------- nova/common/policy.py | 10 +++++++--- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/etc/nova/policy.json b/etc/nova/policy.json index 635ac956b24..4c56cde0d22 100644 --- a/etc/nova/policy.json +++ b/etc/nova/policy.json @@ -1,8 +1,8 @@ { - 'compute:get_volume': ('role:compute_admin', ('tenant_id:%(tenant_id)s', 'role:compute_sysadmin')), - 'compute:get_instance': ('role:compute_admin', ('tenant_id:%(tenant_id)s', 'role:compute_sysadmin')), - 'example:get_google': (('http:http://www.google.com',), ('role:compute_sysadmin',)), - 'example:my_file': ('role:compute_admin', ('tenant_id:%(tenant_id)s',)) - 'example:allowed' : (), - 'example:denied' : ('false:false',), -} \ No newline at end of file + "compute:get_volume": [["role:compute_admin"], ["tenant_id:%(tenant_id)s", "role:compute_sysadmin"]], + "compute:get_instance": [["role:compute_admin"], ["tenant_id:%(tenant_id)s", "role:compute_sysadmin"]], + "example:get_google": [["http:http://www.google.com"], ["role:compute_sysadmin"]], + "example:my_file": [["role:compute_admin"], ["tenant_id:%(tenant_id)s"]], + "example:allowed": [], + "example:denied": ["false:false"] +} diff --git a/nova/common/policy.py b/nova/common/policy.py index 6e591e6bab5..8b31a527bb0 100644 --- a/nova/common/policy.py +++ b/nova/common/policy.py @@ -65,8 +65,10 @@ def add_rule(self, key, match): def check(self, match_list, target_dict, cred_dict): for and_list in match_list: matched = False + if isinstance(and_list, basestring): + and_list = (and_list,) for match in and_list: - match_kind, match = match.split(':', 2) + match_kind, match = match.split(':', 1) if hasattr(self, '_check_%s' % match_kind): f = getattr(self, '_check_%s' % match_kind) rv = f(match, target_dict, cred_dict) @@ -74,7 +76,7 @@ def check(self, match_list, target_dict, cred_dict): matched = False break else: - rv = self._check(match, target_dict, cred_dict) + rv = self._check_generic(match, target_dict, cred_dict) if not rv: matched = False break @@ -103,7 +105,7 @@ def _check_generic(self, match, target_dict, cred_dict): # TODO(termie): do dict inspection via dot syntax match = match % target_dict - key, value = match.split(':', 2) + key, value = match.split(':', 1) if key in cred_dict: return value == cred_dict[key] return False @@ -129,4 +131,6 @@ def _check_http(self, match, target_dict, cred_dict): def load_json(path): rules_dict = json.load(open(path)) + import nose + nose.tools.set_trace() b = Brain(rules=rules_dict) From 0c49c3bab0a96a009213cbc6d1d79c875daa6319 Mon Sep 17 00:00:00 2001 From: Joshua McKenty Date: Sat, 10 Dec 2011 14:46:46 -0800 Subject: [PATCH 02/16] Fixed tests Change-Id: If0d485ec9ac2d2ea07858e881869ca34844797ab --- etc/nova/policy.json | 12 ++++++------ nova/common/policy.py | 15 +++++++++------ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/etc/nova/policy.json b/etc/nova/policy.json index 635ac956b24..9b7ff095c8b 100644 --- a/etc/nova/policy.json +++ b/etc/nova/policy.json @@ -1,8 +1,8 @@ { - 'compute:get_volume': ('role:compute_admin', ('tenant_id:%(tenant_id)s', 'role:compute_sysadmin')), - 'compute:get_instance': ('role:compute_admin', ('tenant_id:%(tenant_id)s', 'role:compute_sysadmin')), - 'example:get_google': (('http:http://www.google.com',), ('role:compute_sysadmin',)), - 'example:my_file': ('role:compute_admin', ('tenant_id:%(tenant_id)s',)) - 'example:allowed' : (), - 'example:denied' : ('false:false',), + "compute:get_volume": [["role:compute_admin"], ["tenant_id:%(tenant_id)s", "role:compute_sysadmin"]], + "compute:get_instance": [["role:compute_admin"], ["tenant_id:%(tenant_id)s", "role:compute_sysadmin"]], + "example:get_google": [["http:http://www.google.com"], ["role:compute_sysadmin"]], + "example:my_file": [["role:compute_admin"], ["tenant_id:%(tenant_id)s"]], + "example:allowed" : [], + "example:denied" : [["false:false"]] } \ No newline at end of file diff --git a/nova/common/policy.py b/nova/common/policy.py index 6e591e6bab5..592a724063c 100644 --- a/nova/common/policy.py +++ b/nova/common/policy.py @@ -22,7 +22,7 @@ import urllib2 -def NotAllowed(Exception): +class NotAllowed(Exception): pass @@ -65,8 +65,10 @@ def add_rule(self, key, match): def check(self, match_list, target_dict, cred_dict): for and_list in match_list: matched = False - for match in and_list: - match_kind, match = match.split(':', 2) + if type(and_list) == str: + and_list = (and_list,) + for match_value in and_list: + match_kind, match = match_value.split(':', 1) if hasattr(self, '_check_%s' % match_kind): f = getattr(self, '_check_%s' % match_kind) rv = f(match, target_dict, cred_dict) @@ -74,7 +76,7 @@ def check(self, match_list, target_dict, cred_dict): matched = False break else: - rv = self._check(match, target_dict, cred_dict) + rv = self._check_generic(match_value, target_dict, cred_dict) if not rv: matched = False break @@ -88,7 +90,7 @@ def check(self, match_list, target_dict, cred_dict): return False def _check_rule(self, match, target_dict, cred_dict): - new_match_list = self.rules.get(match[5:]) + new_match_list = self.rules.get(match) return self.check(new_match_list, target_dict, cred_dict) def _check_generic(self, match, target_dict, cred_dict): @@ -103,7 +105,8 @@ def _check_generic(self, match, target_dict, cred_dict): # TODO(termie): do dict inspection via dot syntax match = match % target_dict - key, value = match.split(':', 2) + print match + key, value = match.split(':', 1) if key in cred_dict: return value == cred_dict[key] return False From 4ee0a336e6a03ec35a92fd7160e57c844a3d96c4 Mon Sep 17 00:00:00 2001 From: Joshua McKenty Date: Sat, 10 Dec 2011 14:47:45 -0800 Subject: [PATCH 03/16] Fixed tests Change-Id: If44e296622a1ba99e8221aff222f0e35be1d5de0 --- nova/common/policy.py | 6 +++--- nova/policy.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/nova/common/policy.py b/nova/common/policy.py index 592a724063c..f0d614bac42 100644 --- a/nova/common/policy.py +++ b/nova/common/policy.py @@ -46,7 +46,7 @@ def enforce(match_list, target_dict, credentials_dict): performing the action. """ - b = Brain() + b = HttpBrain() if not b.check(match_list, target_dict, credentials_dict): raise NotAllowed() @@ -112,7 +112,7 @@ def _check_generic(self, match, target_dict, cred_dict): return False -class HttpBrain(object): +class HttpBrain(Brain): """A brain that can check external urls a Posts json blobs for target and credentials. @@ -132,4 +132,4 @@ def _check_http(self, match, target_dict, cred_dict): def load_json(path): rules_dict = json.load(open(path)) - b = Brain(rules=rules_dict) + b = HttpBrain(rules=rules_dict) diff --git a/nova/policy.py b/nova/policy.py index 57d6e3a25dc..7acde077a60 100644 --- a/nova/policy.py +++ b/nova/policy.py @@ -44,7 +44,7 @@ def enforce(context, action, target): :raises: `nova.exception.PolicyNotAllowed` if verification fails. """ - if not policy.Brain.rules: + if not policy.HttpBrain.rules: #TODO(vish): check mtime and reload path = utils.find_config(FLAGS.policy_file) policy.load_json(path) From de3e63e03398140102b48f28eea56d33f91e5508 Mon Sep 17 00:00:00 2001 From: Joshua McKenty Date: Sat, 10 Dec 2011 14:51:06 -0800 Subject: [PATCH 04/16] Post to pastebin instead of google.com Change-Id: I205e1437dbd7d8773b240ee6f2682d5bad4c51d8 --- etc/nova/policy.json | 2 +- nova/common/policy.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/etc/nova/policy.json b/etc/nova/policy.json index 9b7ff095c8b..a5d5153ca4b 100644 --- a/etc/nova/policy.json +++ b/etc/nova/policy.json @@ -1,7 +1,7 @@ { "compute:get_volume": [["role:compute_admin"], ["tenant_id:%(tenant_id)s", "role:compute_sysadmin"]], "compute:get_instance": [["role:compute_admin"], ["tenant_id:%(tenant_id)s", "role:compute_sysadmin"]], - "example:get_google": [["http:http://www.google.com"], ["role:compute_sysadmin"]], + "example:get_google": [["http:http://pastebin.com/"], ["role:compute_sysadmin"]], "example:my_file": [["role:compute_admin"], ["tenant_id:%(tenant_id)s"]], "example:allowed" : [], "example:denied" : [["false:false"]] diff --git a/nova/common/policy.py b/nova/common/policy.py index f0d614bac42..4e1a8e4699f 100644 --- a/nova/common/policy.py +++ b/nova/common/policy.py @@ -63,6 +63,9 @@ def add_rule(self, key, match): self.rules[key] = match def check(self, match_list, target_dict, cred_dict): + if not match_list: + return True + for and_list in match_list: matched = False if type(and_list) == str: From 12543faa91a3c27b0c212dbda8e1839134660022 Mon Sep 17 00:00:00 2001 From: Joshua McKenty Date: Sat, 10 Dec 2011 14:55:02 -0800 Subject: [PATCH 05/16] Fail out on missing actions Change-Id: Ib08d904a51b513fcd665c80e39cd5f5dc94a08ab --- nova/common/policy.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nova/common/policy.py b/nova/common/policy.py index 4e1a8e4699f..5668d8d18c2 100644 --- a/nova/common/policy.py +++ b/nova/common/policy.py @@ -94,6 +94,8 @@ def check(self, match_list, target_dict, cred_dict): def _check_rule(self, match, target_dict, cred_dict): new_match_list = self.rules.get(match) + if new_match_list is None: + return False return self.check(new_match_list, target_dict, cred_dict) def _check_generic(self, match, target_dict, cred_dict): From 19ad59dee963c65dfa6d66acaf12c2aadc706139 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 10 Dec 2011 14:56:30 -0800 Subject: [PATCH 06/16] fix tests Change-Id: Ic53e81e87535546e0e4e5f45d2c0dce84e05f136 --- nova/common/policy.py | 18 ++++++++++-------- nova/tests/test_policy.py | 18 ++++++++++++------ 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/nova/common/policy.py b/nova/common/policy.py index 8b31a527bb0..ef131e6bc17 100644 --- a/nova/common/policy.py +++ b/nova/common/policy.py @@ -22,7 +22,7 @@ import urllib2 -def NotAllowed(Exception): +class NotAllowed(Exception): pass @@ -46,7 +46,7 @@ def enforce(match_list, target_dict, credentials_dict): performing the action. """ - b = Brain() + b = HttpBrain() if not b.check(match_list, target_dict, credentials_dict): raise NotAllowed() @@ -63,15 +63,17 @@ def add_rule(self, key, match): self.rules[key] = match def check(self, match_list, target_dict, cred_dict): + if not match_list: + return True for and_list in match_list: matched = False if isinstance(and_list, basestring): and_list = (and_list,) for match in and_list: - match_kind, match = match.split(':', 1) + match_kind, match_value = match.split(':', 1) if hasattr(self, '_check_%s' % match_kind): f = getattr(self, '_check_%s' % match_kind) - rv = f(match, target_dict, cred_dict) + rv = f(match_value, target_dict, cred_dict) if not rv: matched = False break @@ -90,7 +92,9 @@ def check(self, match_list, target_dict, cred_dict): return False def _check_rule(self, match, target_dict, cred_dict): - new_match_list = self.rules.get(match[5:]) + new_match_list = self.rules.get(match) + if new_match_list is None: + return False return self.check(new_match_list, target_dict, cred_dict) def _check_generic(self, match, target_dict, cred_dict): @@ -111,7 +115,7 @@ def _check_generic(self, match, target_dict, cred_dict): return False -class HttpBrain(object): +class HttpBrain(Brain): """A brain that can check external urls a Posts json blobs for target and credentials. @@ -131,6 +135,4 @@ def _check_http(self, match, target_dict, cred_dict): def load_json(path): rules_dict = json.load(open(path)) - import nose - nose.tools.set_trace() b = Brain(rules=rules_dict) diff --git a/nova/tests/test_policy.py b/nova/tests/test_policy.py index 35b53bba933..b019866f348 100644 --- a/nova/tests/test_policy.py +++ b/nova/tests/test_policy.py @@ -23,27 +23,33 @@ class PolicyCheckTestCase(test.TestCase): - + + def test_enforce_nonexistent_action_throws(self): + context = {} + action = "example:noexist" + target = {} + self.assertRaises(exception.PolicyNotAllowed, policy.enforce, context, action, target) + def test_enforce_bad_action_throws(self): context = {} action = "example:denied" target = {} - self.assertRaises(exception.PolicyNotAllowed, policy.enforce, context, action, target) - + self.assertRaises(exception.PolicyNotAllowed, policy.enforce, context, action, target) + def test_enforce_good_action(self): context = {} action = "example:allowed" target = {} result = policy.enforce(context, action, target) self.assertEqual(result, None) - + def test_enforce_http_check(self): action = "example:get_google" context = {} target = {} result = policy.enforce(context, action, target) self.assertEqual(result, None) - + def test_templatized_enforcement(self): context = {'tenant_id' : 'bob'} target_mine = {'tenant_id' : 'bob'} @@ -51,4 +57,4 @@ def test_templatized_enforcement(self): action = "example:my_file" result = policy.enforce(context, action, target_mine) self.assertEqual(result, None) - self.assertRaises(exception.PolicyNotAllowed, policy.enforce, context, action, target_not_mine) \ No newline at end of file + self.assertRaises(exception.PolicyNotAllowed, policy.enforce, context, action, target_not_mine) From eeea292c29c75ee8072a183577c23c78f52410b9 Mon Sep 17 00:00:00 2001 From: Joshua McKenty Date: Sat, 10 Dec 2011 15:10:51 -0800 Subject: [PATCH 07/16] Early exit tests for both OR and AND Change-Id: I7ff06cdb5d4875b4d6c8a239470bd205c6e7ef23 --- etc/nova/policy.json | 5 ++++- nova/tests/test_policy.py | 33 ++++++++++++++++++--------------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/etc/nova/policy.json b/etc/nova/policy.json index a5d5153ca4b..795e74ef28e 100644 --- a/etc/nova/policy.json +++ b/etc/nova/policy.json @@ -3,6 +3,9 @@ "compute:get_instance": [["role:compute_admin"], ["tenant_id:%(tenant_id)s", "role:compute_sysadmin"]], "example:get_google": [["http:http://pastebin.com/"], ["role:compute_sysadmin"]], "example:my_file": [["role:compute_admin"], ["tenant_id:%(tenant_id)s"]], + "true" : [], "example:allowed" : [], - "example:denied" : [["false:false"]] + "example:denied" : [["false:false"]], + "example:early_and_fail" : [["false:false", "rule:true"]], + "example:early_or_success" : [["rule:true"], ["false:false"]] } \ No newline at end of file diff --git a/nova/tests/test_policy.py b/nova/tests/test_policy.py index 35b53bba933..a90a2e8d8f6 100644 --- a/nova/tests/test_policy.py +++ b/nova/tests/test_policy.py @@ -23,32 +23,35 @@ class PolicyCheckTestCase(test.TestCase): + def setUp(self): + super(PolicyCheckTestCase, self).setUp() + self.context = {'tenant_id' : 'bob'} + self.target = {} def test_enforce_bad_action_throws(self): - context = {} action = "example:denied" - target = {} - self.assertRaises(exception.PolicyNotAllowed, policy.enforce, context, action, target) + self.assertRaises(exception.PolicyNotAllowed, policy.enforce, self.context, action, self.target) def test_enforce_good_action(self): - context = {} action = "example:allowed" - target = {} - result = policy.enforce(context, action, target) - self.assertEqual(result, None) + policy.enforce(self.context, action, self.target) def test_enforce_http_check(self): action = "example:get_google" - context = {} - target = {} - result = policy.enforce(context, action, target) - self.assertEqual(result, None) + policy.enforce(self.context, action, self.target) def test_templatized_enforcement(self): - context = {'tenant_id' : 'bob'} target_mine = {'tenant_id' : 'bob'} target_not_mine = {'tenant_id' : 'fred'} action = "example:my_file" - result = policy.enforce(context, action, target_mine) - self.assertEqual(result, None) - self.assertRaises(exception.PolicyNotAllowed, policy.enforce, context, action, target_not_mine) \ No newline at end of file + policy.enforce(self.context, action, target_mine) + self.assertRaises(exception.PolicyNotAllowed, policy.enforce, self.context, action, target_not_mine) + + def test_early_AND_enforcement(self): + action = "example:early_and_fail" + self.assertRaises(exception.PolicyNotAllowed, policy.enforce, self.context, action, self.target) + + def test_early_OR_enforcement(self): + action = "example:early_or_success" + policy.enforce(self.context, action, self.target) + \ No newline at end of file From 2026d5187837d6c3ff3908b969ddf7b10f8346d2 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 10 Dec 2011 15:14:23 -0800 Subject: [PATCH 08/16] Stubout urlib and fix the remote call Change-Id: Id67db96b66cc18c2d77b5163c44caa7aec6e2e07 --- etc/nova/policy.json | 2 +- nova/common/policy.py | 7 +++++-- nova/tests/test_policy.py | 21 +++++++++++++++++++-- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/etc/nova/policy.json b/etc/nova/policy.json index 4c56cde0d22..23cc940912b 100644 --- a/etc/nova/policy.json +++ b/etc/nova/policy.json @@ -1,7 +1,7 @@ { "compute:get_volume": [["role:compute_admin"], ["tenant_id:%(tenant_id)s", "role:compute_sysadmin"]], "compute:get_instance": [["role:compute_admin"], ["tenant_id:%(tenant_id)s", "role:compute_sysadmin"]], - "example:get_google": [["http:http://www.google.com"], ["role:compute_sysadmin"]], + "example:get_http": [["http:http://www.example.com"]], "example:my_file": [["role:compute_admin"], ["tenant_id:%(tenant_id)s"]], "example:allowed": [], "example:denied": ["false:false"] diff --git a/nova/common/policy.py b/nova/common/policy.py index 91a4e3569dd..42eac79031a 100644 --- a/nova/common/policy.py +++ b/nova/common/policy.py @@ -125,10 +125,13 @@ class HttpBrain(Brain): def _check_http(self, match, target_dict, cred_dict): url = match % target_dict data = {'target': json.dumps(target_dict), - 'credentials': json.dumps(cred_dict)} + 'credentials': json.dumps(cred_dict)} post_data = urllib.urlencode(data) f = urllib2.urlopen(url, post_data) - if f.read(): + # NOTE(vish): This is to show how we could do remote requests, + # but some fancier method for response codes should + # probably be defined + if f.read() == "True": return True return False diff --git a/nova/tests/test_policy.py b/nova/tests/test_policy.py index b019866f348..a3dfec0257d 100644 --- a/nova/tests/test_policy.py +++ b/nova/tests/test_policy.py @@ -17,6 +17,9 @@ """Test of Policy Engine For Nova""" +import urllib2 +import StringIO + from nova import test from nova import policy from nova import exception @@ -43,13 +46,27 @@ def test_enforce_good_action(self): result = policy.enforce(context, action, target) self.assertEqual(result, None) - def test_enforce_http_check(self): - action = "example:get_google" + def test_enforce_http_true(self): + + def fakeurlopen(url, post_data): + return StringIO.StringIO("True") + self.stubs.Set(urllib2, 'urlopen', fakeurlopen) + action = "example:get_http" context = {} target = {} result = policy.enforce(context, action, target) self.assertEqual(result, None) + def test_enforce_http_false(self): + + def fakeurlopen(url, post_data): + return StringIO.StringIO("False") + self.stubs.Set(urllib2, 'urlopen', fakeurlopen) + action = "example:get_http" + context = {} + target = {} + self.assertRaises(exception.PolicyNotAllowed, policy.enforce, context, action, target) + def test_templatized_enforcement(self): context = {'tenant_id' : 'bob'} target_mine = {'tenant_id' : 'bob'} From f9a128086bc3a1e6da638f6b57b7fa65ab8caacb Mon Sep 17 00:00:00 2001 From: Joshua McKenty Date: Sat, 10 Dec 2011 15:19:15 -0800 Subject: [PATCH 09/16] Getting rid of old test. Change-Id: I400e72cfd6502c41fe9ef0395d2eed73ef3a3214 --- nova/tests/test_policy.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/nova/tests/test_policy.py b/nova/tests/test_policy.py index 3e3bd4ac5c3..63e7d3a6ce2 100644 --- a/nova/tests/test_policy.py +++ b/nova/tests/test_policy.py @@ -42,10 +42,6 @@ def test_enforce_bad_action_throws(self): def test_enforce_good_action(self): action = "example:allowed" policy.enforce(self.context, action, self.target) - - def test_enforce_http_check(self): - action = "example:get_http" - policy.enforce(self.context, action, self.target) def test_enforce_http_true(self): From 26b6f3c5e939ed7675a9c9a2dc19cd2f880114d3 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 10 Dec 2011 15:53:55 -0800 Subject: [PATCH 10/16] modify rules to reload on mtime and use inline rules for test Change-Id: I90f2e646182149eebaed708ab76d7d72b461a73e --- etc/nova/policy.json | 9 +----- nova/policy.py | 23 ++++++++++---- nova/tests/test_policy.py | 63 ++++++++++++++++++++++++++++----------- 3 files changed, 65 insertions(+), 30 deletions(-) diff --git a/etc/nova/policy.json b/etc/nova/policy.json index 241725b2227..fe337d85be5 100644 --- a/etc/nova/policy.json +++ b/etc/nova/policy.json @@ -1,11 +1,4 @@ { "compute:get_volume": [["role:compute_admin"], ["tenant_id:%(tenant_id)s", "role:compute_sysadmin"]], - "compute:get_instance": [["role:compute_admin"], ["tenant_id:%(tenant_id)s", "role:compute_sysadmin"]], - "example:get_http": [["http:http://www.example.com"]], - "example:my_file": [["role:compute_admin"], ["tenant_id:%(tenant_id)s"]], - "true" : [], - "example:allowed" : [], - "example:denied" : [["false:false"]], - "example:early_and_fail" : [["false:false", "rule:true"]], - "example:early_or_success" : [["rule:true"], ["false:false"]] + "compute:get_instance": [["role:compute_admin"], ["tenant_id:%(tenant_id)s", "role:compute_sysadmin"]] } diff --git a/nova/policy.py b/nova/policy.py index 7acde077a60..fc8380c95c0 100644 --- a/nova/policy.py +++ b/nova/policy.py @@ -17,6 +17,7 @@ """Policy Engine For Nova""" +import os from nova import exception from nova import flags @@ -27,6 +28,18 @@ flags.DEFINE_string('policy_file', 'policy.json', _('JSON file representing policy')) +_POLICY_PATH = None +_POLICY_MTIME = None + +def _load_if_modified(path): + global _POLICY_MTIME + mtime = os.path.getmtime(path) + if mtime != _POLICY_MTIME: + policy.load_json(path) + _POLICY_MTIME = mtime + + + def enforce(context, action, target): """Verifies that the action is valid on the target in this context. @@ -44,14 +57,14 @@ def enforce(context, action, target): :raises: `nova.exception.PolicyNotAllowed` if verification fails. """ - if not policy.HttpBrain.rules: - #TODO(vish): check mtime and reload - path = utils.find_config(FLAGS.policy_file) - policy.load_json(path) + global _POLICY_PATH + if not _POLICY_PATH: + _POLICY_PATH = utils.find_config(FLAGS.policy_file) + _load_if_modified(_POLICY_PATH) match_list = ('rule:%s' % action,) target_dict = target - credentials_dict = context + credentials_dict = context.to_dict() try: policy.enforce(match_list, target_dict, credentials_dict) except policy.NotAllowed: diff --git a/nova/tests/test_policy.py b/nova/tests/test_policy.py index 63e7d3a6ce2..8b5e0b36a9f 100644 --- a/nova/tests/test_policy.py +++ b/nova/tests/test_policy.py @@ -20,24 +20,52 @@ import urllib2 import StringIO -from nova import test -from nova import policy +from nova import context from nova import exception +from nova import flags +from nova import policy +from nova import test +from nova import utils +from nova.common import policy as common_policy + +FLAGS = flags.FLAGS -class PolicyCheckTestCase(test.TestCase): +class PolicyFileTestCase(test.TestCase): def setUp(self): - super(PolicyCheckTestCase, self).setUp() - self.context = {'tenant_id' : 'bob'} + self.flags(policy_file='nova/tests/policy.json') + + + +class PolicyTestCase(test.TestCase): + def setUp(self): + super(PolicyTestCase, self).setUp() + # NOTE(vish): preload rules to circumvent reloading from file + policy._load_if_modified(utils.find_config(FLAGS.policy_file)) + common_policy.Brain.rules = None + rules = { + "true" : [], + "example:allowed" : [], + "example:denied" : [["false:false"]], + "example:get_http": [["http:http://www.example.com"]], + "example:my_file": [["role:compute_admin"], + ["project_id:%(project_id)s"]], + "example:early_and_fail" : [["false:false", "rule:true"]], + "example:early_or_success" : [["rule:true"], ["false:false"]] + } + common_policy.HttpBrain(rules) + self.context = context.RequestContext('fake', 'fake') self.target = {} def test_enforce_nonexistent_action_throws(self): action = "example:noexist" - self.assertRaises(exception.PolicyNotAllowed, policy.enforce, self.context, action, self.target) + self.assertRaises(exception.PolicyNotAllowed, policy.enforce, + self.context, action, self.target) def test_enforce_bad_action_throws(self): action = "example:denied" - self.assertRaises(exception.PolicyNotAllowed, policy.enforce, self.context, action, self.target) + self.assertRaises(exception.PolicyNotAllowed, policy.enforce, + self.context, action, self.target) def test_enforce_good_action(self): action = "example:allowed" @@ -49,9 +77,8 @@ def fakeurlopen(url, post_data): return StringIO.StringIO("True") self.stubs.Set(urllib2, 'urlopen', fakeurlopen) action = "example:get_http" - context = {} target = {} - result = policy.enforce(context, action, target) + result = policy.enforce(self.context, action, target) self.assertEqual(result, None) def test_enforce_http_false(self): @@ -60,21 +87,23 @@ def fakeurlopen(url, post_data): return StringIO.StringIO("False") self.stubs.Set(urllib2, 'urlopen', fakeurlopen) action = "example:get_http" - context = {} target = {} - self.assertRaises(exception.PolicyNotAllowed, policy.enforce, context, action, target) + self.assertRaises(exception.PolicyNotAllowed, policy.enforce, + self.context, action, target) def test_templatized_enforcement(self): - target_mine = {'tenant_id' : 'bob'} - target_not_mine = {'tenant_id' : 'fred'} + target_mine = {'project_id' : 'fake'} + target_not_mine = {'project_id' : 'another'} action = "example:my_file" policy.enforce(self.context, action, target_mine) - self.assertRaises(exception.PolicyNotAllowed, policy.enforce, self.context, action, target_not_mine) - + self.assertRaises(exception.PolicyNotAllowed, policy.enforce, + self.context, action, target_not_mine) + def test_early_AND_enforcement(self): action = "example:early_and_fail" - self.assertRaises(exception.PolicyNotAllowed, policy.enforce, self.context, action, self.target) - + self.assertRaises(exception.PolicyNotAllowed, policy.enforce, + self.context, action, self.target) + def test_early_OR_enforcement(self): action = "example:early_or_success" policy.enforce(self.context, action, self.target) From d3eb99164b6e96f37ba84083eb5325e6492840b0 Mon Sep 17 00:00:00 2001 From: Joshua McKenty Date: Sat, 10 Dec 2011 15:54:56 -0800 Subject: [PATCH 11/16] Make role-checking rules in policy.py unique - is_admin is equivalent to having all other roles. Change-Id: If16a7a6aba228e0141562b805528a83d816fa725 --- etc/nova/policy.json | 11 +++-------- etc/nova/roles.yaml | 7 +++++++ nova/common/policy.py | 5 +++++ 3 files changed, 15 insertions(+), 8 deletions(-) create mode 100644 etc/nova/roles.yaml diff --git a/etc/nova/policy.json b/etc/nova/policy.json index 241725b2227..3396a23cb9a 100644 --- a/etc/nova/policy.json +++ b/etc/nova/policy.json @@ -1,11 +1,6 @@ { - "compute:get_volume": [["role:compute_admin"], ["tenant_id:%(tenant_id)s", "role:compute_sysadmin"]], - "compute:get_instance": [["role:compute_admin"], ["tenant_id:%(tenant_id)s", "role:compute_sysadmin"]], - "example:get_http": [["http:http://www.example.com"]], - "example:my_file": [["role:compute_admin"], ["tenant_id:%(tenant_id)s"]], "true" : [], - "example:allowed" : [], - "example:denied" : [["false:false"]], - "example:early_and_fail" : [["false:false", "rule:true"]], - "example:early_or_success" : [["rule:true"], ["false:false"]] + "compute:get_instance": [["role:compute_admin"], ["project_id:%(project_id)s", "role:sysadmin"]], + "volume:attach_volume": [["role:compute_admin"], ["project_id:%(project_id)s", "role:sysadmin"]], + "volume:create_volume": [["role:compute_admin"], ["project_id:%(project_id)s", "role:sysadmin"]] } diff --git a/etc/nova/roles.yaml b/etc/nova/roles.yaml new file mode 100644 index 00000000000..fd06672fc40 --- /dev/null +++ b/etc/nova/roles.yaml @@ -0,0 +1,7 @@ +roles: +- 'netadmin' +- 'sysadmin' +- 'admin' +- 'member' +- 'keystoneadmin' +- 'keystoneserviceadmin' \ No newline at end of file diff --git a/nova/common/policy.py b/nova/common/policy.py index 42eac79031a..20974668cbf 100644 --- a/nova/common/policy.py +++ b/nova/common/policy.py @@ -97,6 +97,11 @@ def _check_rule(self, match, target_dict, cred_dict): return False return self.check(new_match_list, target_dict, cred_dict) + def _check_role(self, match, target_dict, cred_dict): + if cred_dict['is_admin']: + return True + return match in cred_dict['roles'] + def _check_generic(self, match, target_dict, cred_dict): """Check an individual match. From 522d6a82746c12c7bf4b189fe0fa73c2382bb666 Mon Sep 17 00:00:00 2001 From: Joshua McKenty Date: Sat, 10 Dec 2011 16:01:17 -0800 Subject: [PATCH 12/16] Admin user has all roles. Change-Id: I383a9fd316a058fb147627e108419bb6c313c0e4 --- nova/tests/test_policy.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/nova/tests/test_policy.py b/nova/tests/test_policy.py index 8b5e0b36a9f..f5496f5f8a2 100644 --- a/nova/tests/test_policy.py +++ b/nova/tests/test_policy.py @@ -51,12 +51,20 @@ def setUp(self): "example:my_file": [["role:compute_admin"], ["project_id:%(project_id)s"]], "example:early_and_fail" : [["false:false", "rule:true"]], - "example:early_or_success" : [["rule:true"], ["false:false"]] + "example:early_or_success" : [["rule:true"], ["false:false"]], + "example:sysadmin_allowed" : [["role:sysadmin"]], } common_policy.HttpBrain(rules) self.context = context.RequestContext('fake', 'fake') + self.admin_context = context.RequestContext('admin', 'fake', is_admin=True) self.target = {} + def test_admin_has_all_roles(self): + action = "example:sysadmin_allowed" + self.assertRaises(exception.PolicyNotAllowed, policy.enforce, + self.context, action, self.target) + policy.enforce(self.admin_context, action, self.target) + def test_enforce_nonexistent_action_throws(self): action = "example:noexist" self.assertRaises(exception.PolicyNotAllowed, policy.enforce, From 0a5ff4c9c31edb212228584b1ee7069912a35d1a Mon Sep 17 00:00:00 2001 From: Joshua McKenty Date: Sat, 10 Dec 2011 16:16:34 -0800 Subject: [PATCH 13/16] Rolled out the is_admin use, using admin role instead. Change-Id: Ie47799b0eef7077df0d1fc7df8f19928b56d16bb --- etc/nova/policy.json | 6 +++--- nova/common/policy.py | 2 -- nova/tests/test_policy.py | 6 +++--- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/etc/nova/policy.json b/etc/nova/policy.json index d3603b2addc..532099df4a1 100644 --- a/etc/nova/policy.json +++ b/etc/nova/policy.json @@ -1,6 +1,6 @@ { "true" : [], - "compute:get_instance": [["role:admin"], ["project_id:%(project_id)s", "role:sysadmin"]], - "volume:attach_volume": [["role:admin"], ["project_id:%(project_id)s", "role:sysadmin"]], - "volume:create_volume": [["role:admin"], ["project_id:%(project_id)s", "role:sysadmin"]] + "compute:get_instance": [["role:admin"], ["project_id:%(project_id)s"]], + "volume:attach_volume": [["role:admin"], ["project_id:%(project_id)s"]], + "volume:create_volume": [["role:admin"], ["project_id:%(project_id)s"]] } diff --git a/nova/common/policy.py b/nova/common/policy.py index 20974668cbf..9d90bedb4cc 100644 --- a/nova/common/policy.py +++ b/nova/common/policy.py @@ -98,8 +98,6 @@ def _check_rule(self, match, target_dict, cred_dict): return self.check(new_match_list, target_dict, cred_dict) def _check_role(self, match, target_dict, cred_dict): - if cred_dict['is_admin']: - return True return match in cred_dict['roles'] def _check_generic(self, match, target_dict, cred_dict): diff --git a/nova/tests/test_policy.py b/nova/tests/test_policy.py index f5496f5f8a2..78ef6a18d9e 100644 --- a/nova/tests/test_policy.py +++ b/nova/tests/test_policy.py @@ -52,11 +52,11 @@ def setUp(self): ["project_id:%(project_id)s"]], "example:early_and_fail" : [["false:false", "rule:true"]], "example:early_or_success" : [["rule:true"], ["false:false"]], - "example:sysadmin_allowed" : [["role:sysadmin"]], + "example:sysadmin_allowed" : [["role:admin"], ["role:sysadmin"]], } common_policy.HttpBrain(rules) - self.context = context.RequestContext('fake', 'fake') - self.admin_context = context.RequestContext('admin', 'fake', is_admin=True) + self.context = context.RequestContext('fake', 'fake', roles=['member']) + self.admin_context = context.RequestContext('admin', 'fake', roles=['admin'], is_admin=True) self.target = {} def test_admin_has_all_roles(self): From 26c01f44f8b2f584d60b814c7d24b8fc7b137378 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 10 Dec 2011 16:24:22 -0800 Subject: [PATCH 14/16] add tests for mtime Change-Id: Icac07bb3beef6d5beb6c41f4f12baff12aeb63ed --- nova/tests/test_policy.py | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/nova/tests/test_policy.py b/nova/tests/test_policy.py index 8b5e0b36a9f..21620e64325 100644 --- a/nova/tests/test_policy.py +++ b/nova/tests/test_policy.py @@ -17,8 +17,10 @@ """Test of Policy Engine For Nova""" -import urllib2 +import os import StringIO +import tempfile +import urllib2 from nova import context from nova import exception @@ -33,8 +35,28 @@ class PolicyFileTestCase(test.TestCase): def setUp(self): - self.flags(policy_file='nova/tests/policy.json') + super(PolicyFileTestCase, self).setUp() + _, self.tmpfilename = tempfile.mkstemp() + self.flags(policy_file=self.tmpfilename) + self.context = context.RequestContext('fake', 'fake') + self.target = {} + def tearDown(self): + common_policy.Brain.rules = None + policy._POLICY_PATH = None + super(PolicyFileTestCase, self).tearDown() + + def test_modified_policy_reloads(self): + action = "example:test" + with open(self.tmpfilename, "w") as policyfile: + policyfile.write("""{"example:test": []}""") + policy.enforce(self.context, action, self.target) + with open(self.tmpfilename, "w") as policyfile: + policyfile.write("""{"example:test": ["false:false"]}""") + # NOTE(vish): reset stored mtime + policy._POLICY_MTIME = None + self.assertRaises(exception.PolicyNotAllowed, policy.enforce, + self.context, action, self.target) class PolicyTestCase(test.TestCase): @@ -57,6 +79,11 @@ def setUp(self): self.context = context.RequestContext('fake', 'fake') self.target = {} + def tearDown(self): + common_policy.Brain.rules = None + policy._POLICY_PATH = None + super(PolicyTestCase, self).tearDown() + def test_enforce_nonexistent_action_throws(self): action = "example:noexist" self.assertRaises(exception.PolicyNotAllowed, policy.enforce, From 5103356750e6095e8af23bd938064a034e8759cf Mon Sep 17 00:00:00 2001 From: Joshua McKenty Date: Sat, 10 Dec 2011 16:43:58 -0800 Subject: [PATCH 15/16] Added policy.enforce calls to instance creation, with simple default rules. Change-Id: Ib1ea3a6c54c5a36bd6b5a31467ea2b93010d36d1 --- etc/nova/policy.json | 11 ++++++++--- nova/compute/api.py | 15 +++++++++++++++ nova/tests/test_compute.py | 4 ++-- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/etc/nova/policy.json b/etc/nova/policy.json index 532099df4a1..4743d342b26 100644 --- a/etc/nova/policy.json +++ b/etc/nova/policy.json @@ -1,6 +1,11 @@ { "true" : [], - "compute:get_instance": [["role:admin"], ["project_id:%(project_id)s"]], - "volume:attach_volume": [["role:admin"], ["project_id:%(project_id)s"]], - "volume:create_volume": [["role:admin"], ["project_id:%(project_id)s"]] + "compute:create_instance" : [["role:admin"], ["project_id:%(project_id)s"]], + "compute:attach_network" : [["role:admin"], ["project_id:%(project_id)s"]], + "compute:attach_volume" : [["role:admin"], ["project_id:%(project_id)s"]], + "compute:list_instances": [["role:admin"], ["project_id:%(project_id)s"]], + "compute:get_instance": [["role:admin"], ["project_id:%(project_id)s"]], + "network:attach_network" : [["role:admin"], ["project_id:%(project_id)s"]], + "volume:create_volume": [["role:admin"], ["project_id:%(project_id)s"]], + "volume:attach_volume": [["role:admin"], ["project_id:%(project_id)s"]] } diff --git a/nova/compute/api.py b/nova/compute/api.py index f65b5cead38..a55efc3ab4b 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -29,6 +29,7 @@ import nova.image from nova import log as logging from nova import network +from nova import policy from nova import quota from nova import rpc from nova import utils @@ -542,6 +543,20 @@ def create(self, context, instance_type, could be 'None' or a list of instance dicts depending on if we waited for information from the scheduler or not. """ + target = {'project_id' : context.project_id, + 'user_id' : context.user_id, + 'availability_zone' : availability_zone} + policy.enforce(context, "compute:create_instance", target) + + if requested_networks: + target['requested_networks'] = requested_networks + policy.enforce(context, "compute:attach_network", target) + policy.enforce(context, "network:attach_network", target) + + if block_device_mapping: + target['block_device'] = block_device_mapping + policy.enforce(context, "compute:attach_volume", target) + policy.enforce(context, "volume:attach_volume", target) # We can create the DB entry for the instance here if we're # only going to create 1 instance and we're in a single diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py index e4c60e0682f..905c00b7c34 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -108,7 +108,7 @@ def setUp(self): self.compute = utils.import_object(FLAGS.compute_manager) self.user_id = 'fake' self.project_id = 'fake' - self.context = context.RequestContext(self.user_id, self.project_id) + self.context = context.RequestContext(self.user_id, self.project_id, roles=['member']) test_notifier.NOTIFICATIONS = [] def fake_show(meh, context, id): @@ -674,7 +674,7 @@ def test_lock(self): instance_uuid = instance['uuid'] self.compute.run_instance(self.context, instance_uuid) - non_admin_context = context.RequestContext(None, None, is_admin=False) + non_admin_context = context.RequestContext(None, None, roles=['member'], is_admin=False) # decorator should return False (fail) with locked nonadmin context self.compute.lock_instance(self.context, instance_uuid) From a493d74a6c90b8b1be0b551f8008946af7be619c Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 10 Dec 2011 18:30:06 -0800 Subject: [PATCH 16/16] add reset command to policy Change-Id: I5635d015467d45f08bb74b7638fa1ba616607375 --- nova/policy.py | 8 +++++++- nova/tests/test_policy.py | 11 ++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/nova/policy.py b/nova/policy.py index fc8380c95c0..84333286e8a 100644 --- a/nova/policy.py +++ b/nova/policy.py @@ -39,6 +39,13 @@ def _load_if_modified(path): _POLICY_MTIME = mtime +def reset(): + global _POLICY_PATH + global _POLICY_MTIME + _POLICY_PATH = None + _POLICY_MTIME = None + policy.Brain.rules = {} + def enforce(context, action, target): """Verifies that the action is valid on the target in this context. @@ -58,7 +65,6 @@ def enforce(context, action, target): """ global _POLICY_PATH - if not _POLICY_PATH: _POLICY_PATH = utils.find_config(FLAGS.policy_file) _load_if_modified(_POLICY_PATH) diff --git a/nova/tests/test_policy.py b/nova/tests/test_policy.py index 04a9438cd45..9558d75b89e 100644 --- a/nova/tests/test_policy.py +++ b/nova/tests/test_policy.py @@ -17,7 +17,6 @@ """Test of Policy Engine For Nova""" -import os import StringIO import tempfile import urllib2 @@ -36,17 +35,15 @@ class PolicyFileTestCase(test.TestCase): def setUp(self): super(PolicyFileTestCase, self).setUp() - common_policy.Brain.rules = None - policy._POLICY_PATH = None + policy.reset() _, self.tmpfilename = tempfile.mkstemp() self.flags(policy_file=self.tmpfilename) self.context = context.RequestContext('fake', 'fake') self.target = {} def tearDown(self): - common_policy.Brain.rules = None - policy._POLICY_PATH = None super(PolicyFileTestCase, self).tearDown() + policy.reset() def test_modified_policy_reloads(self): action = "example:test" @@ -64,6 +61,7 @@ def test_modified_policy_reloads(self): class PolicyTestCase(test.TestCase): def setUp(self): super(PolicyTestCase, self).setUp() + policy.reset() # NOTE(vish): preload rules to circumvent reloading from file policy._load_if_modified(utils.find_config(FLAGS.policy_file)) common_policy.Brain.rules = None @@ -85,8 +83,7 @@ def setUp(self): self.target = {} def tearDown(self): - common_policy.Brain.rules = None - policy._POLICY_PATH = None + policy.reset() super(PolicyTestCase, self).tearDown() def test_enforce_nonexistent_action_throws(self):