From 7901659fa169db8ac5ffd7c610a798c785a3556b Mon Sep 17 00:00:00 2001

From: Vojtech Polasek <vpolasek@redhat.com>

Date: Fri, 9 Jul 2021 14:41:03 +0200

Subject: [PATCH 01/12] ensure that higher policy levels can override variables

 of lower levels

---

 ssg/controls.py | 13 ++++++++++---

 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/ssg/controls.py b/ssg/controls.py

index 297d80e46c5..165cdf0511a 100644

--- a/ssg/controls.py

+++ b/ssg/controls.py

@@ -202,9 +202,16 @@ def get_all_controls_of_level(self, policy_id, level_id):

         all_policy_controls = self.get_all_controls(policy_id)

         eligible_controls = []

-        for c in all_policy_controls:

-            if len(level_ids.intersection(c.levels)) > 0:

-                eligible_controls.append(c)

+        defined_variables = []

+        # we will go level by level, from top to bottom

+        # this is done to enable overriding of variables by higher levels

+        for lv in level_ids:

+            for c in all_policy_controls:

+                if lv in c.levels:

+                    # if the control has a variable, check if it is not already defined

+                    if c.variables.keys().isdisjoint(defined_variables):

+                        eligible_controls.append(c)

+                        defined_variables += [*c.variables.keys()]

         return eligible_controls

     def get_all_controls(self, policy_id):

From 66e612a9668009cc553fcf1abbf2c9477155c0c2 Mon Sep 17 00:00:00 2001

From: Vojtech Polasek <vpolasek@redhat.com>

Date: Thu, 5 Aug 2021 14:02:25 +0200

Subject: [PATCH 02/12] use ordered sets emulated by ordereddict

because of compatibility with python2

---

 ssg/controls.py | 21 ++++++++++++++-------

 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/ssg/controls.py b/ssg/controls.py

index 165cdf0511a..611a647e125 100644

--- a/ssg/controls.py

+++ b/ssg/controls.py

@@ -2,6 +2,7 @@

 import logging

 import os

 from glob import glob

+from collections import OrderedDict

 import ssg.build_yaml

 import ssg.yaml

@@ -152,16 +153,18 @@ def get_level(self, level_id):

             raise ValueError(msg)

     def get_level_with_ancestors(self, level_id):

-        levels = set()

+        # use OrderedDict for Python2 compatibility instead of ordered set

+        levels = OrderedDict()

         level = self.get_level(level_id)

-        levels.add(level)

+        levels[level] = ""

         if level.inherits_from:

             for lv in level.inherits_from:

-                levels.update(self.get_level_with_ancestors(lv))

+                eligible_levels = [l for l in self.get_level_with_ancestors(lv).keys() if l not in levels.keys()]

+                for l in eligible_levels:

+                    levels[l] = ""

         return levels

-

 class ControlsManager():

     def __init__(self, controls_dir, env_yaml=None):

         self.controls_dir = os.path.abspath(controls_dir)

@@ -198,20 +201,24 @@ def _get_policy(self, policy_id):

     def get_all_controls_of_level(self, policy_id, level_id):

         policy = self._get_policy(policy_id)

         levels = policy.get_level_with_ancestors(level_id)

-        level_ids = set([lv.id for lv in levels])

+        # we use OrderedDict here with empty values instead of ordered set

+        # cause we want to be compatible with python 2

+        level_ids = OrderedDict()

+        for lv in levels.keys():

+            level_ids[lv.id] = ""

         all_policy_controls = self.get_all_controls(policy_id)

         eligible_controls = []

         defined_variables = []

         # we will go level by level, from top to bottom

         # this is done to enable overriding of variables by higher levels

-        for lv in level_ids:

+        for lv in level_ids.keys():

             for c in all_policy_controls:

                 if lv in c.levels:

                     # if the control has a variable, check if it is not already defined

                     if c.variables.keys().isdisjoint(defined_variables):

                         eligible_controls.append(c)

-                        defined_variables += [*c.variables.keys()]

+                        defined_variables += list(c.variables.keys())

         return eligible_controls

     def get_all_controls(self, policy_id):

From 95a23a31293a0a63361ddf1831866cd5ae1ab61e Mon Sep 17 00:00:00 2001

From: Vojtech Polasek <vpolasek@redhat.com>

Date: Thu, 5 Aug 2021 16:30:10 +0200

Subject: [PATCH 03/12] rework handling of variables when returning all

 controls of a level

currently only the top most level variables are kept in the controls

if there is a control with lower level which has the same variable defined, it is deep copied and the variable definition is removed only from the resulting control

the original control stays in tact

---

 ssg/controls.py | 27 +++++++++++++++++++++------

 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/ssg/controls.py b/ssg/controls.py

index 611a647e125..4ebb8bda3d7 100644

--- a/ssg/controls.py

+++ b/ssg/controls.py

@@ -1,8 +1,8 @@

 import collections

 import logging

 import os

+import copy

 from glob import glob

-from collections import OrderedDict

 import ssg.build_yaml

 import ssg.yaml

@@ -154,7 +154,7 @@ def get_level(self, level_id):

     def get_level_with_ancestors(self, level_id):

         # use OrderedDict for Python2 compatibility instead of ordered set

-        levels = OrderedDict()

+        levels = collections.OrderedDict()

         level = self.get_level(level_id)

         levels[level] = ""

         if level.inherits_from:

@@ -201,24 +201,39 @@ def _get_policy(self, policy_id):

     def get_all_controls_of_level(self, policy_id, level_id):

         policy = self._get_policy(policy_id)

         levels = policy.get_level_with_ancestors(level_id)

+        print ("getting levels of " + level_id)

+        print ([ l.id for l in levels.keys()])

         # we use OrderedDict here with empty values instead of ordered set

         # cause we want to be compatible with python 2

-        level_ids = OrderedDict()

+        level_ids = collections.OrderedDict()

         for lv in levels.keys():

             level_ids[lv.id] = ""

-

+        print (level_ids.keys())

         all_policy_controls = self.get_all_controls(policy_id)

         eligible_controls = []

         defined_variables = []

         # we will go level by level, from top to bottom

         # this is done to enable overriding of variables by higher levels

         for lv in level_ids.keys():

+            print ("going through level " +lv)

             for c in all_policy_controls:

+                print (c.levels)

                 if lv in c.levels:

                     # if the control has a variable, check if it is not already defined

-                    if c.variables.keys().isdisjoint(defined_variables):

+                    variables = list(c.variables.keys())

+                    if len(variables) == 0:

                         eligible_controls.append(c)

-                        defined_variables += list(c.variables.keys())

+                    for var in variables:

+                        if var in defined_variables:

+                            # if it is, create new instance of the control and remove the variable

+                            # we are going from the top level to the bottom

+                            # so we don't want to overwrite variables

+                            new_c = copy.deepcopy(c)

+                            del new_c.variables[var]

+                            eligible_controls.append(new_c)

+                        else:

+                            defined_variables.append(var)

+                            eligible_controls.append(c)

         return eligible_controls

     def get_all_controls(self, policy_id):

From a2dd7e9386c757a523b57646bdc5a9ffa99f68c5 Mon Sep 17 00:00:00 2001

From: Vojtech Polasek <vpolasek@redhat.com>

Date: Thu, 5 Aug 2021 16:31:25 +0200

Subject: [PATCH 04/12] add tests for defining of variables

---

 tests/unit/ssg-module/data/controls_dir/abcd-levels.yml | 6 ++++++

 tests/unit/ssg-module/test_controls.py                  | 5 +++++

 2 files changed, 11 insertions(+)

diff --git a/tests/unit/ssg-module/data/controls_dir/abcd-levels.yml b/tests/unit/ssg-module/data/controls_dir/abcd-levels.yml

index aded77c12a6..b98a7cd4e19 100644

--- a/tests/unit/ssg-module/data/controls_dir/abcd-levels.yml

+++ b/tests/unit/ssg-module/data/controls_dir/abcd-levels.yml

@@ -19,10 +19,14 @@ controls:

   - id: S2

     levels:

     - low

+    rules:

+      - var_password_pam_minlen=1

   - id: S3

     levels:

     - medium

+    rules:

+      - var_password_pam_minlen=2

   - id: S4

     title: Configure authentication

@@ -36,3 +40,5 @@ controls:

         title: Enforce password quality standards

         levels:

         - high

+        rules:

+          - var_password_pam_minlen=3

diff --git a/tests/unit/ssg-module/test_controls.py b/tests/unit/ssg-module/test_controls.py

index ff9b04f26c9..06fcb0c375d 100644

--- a/tests/unit/ssg-module/test_controls.py

+++ b/tests/unit/ssg-module/test_controls.py

@@ -87,6 +87,11 @@ def test_controls_levels():

     assert len(low_controls) == 4

     assert len(medium_controls) == 5

+    # test overriding of variables in levels

+    assert c_2.variables["var_password_pam_minlen"] == "1"

+    assert c_3.variables["var_password_pam_minlen"] == "2"

+    assert c_4b.variables["var_password_pam_minlen"] == "3"

+

 def test_controls_load_product():

     ssg_root = \

From 82b90a9720dadab7d6060f0ccbcd902b1c097904 Mon Sep 17 00:00:00 2001

From: Vojtech Polasek <vpolasek@redhat.com>

Date: Fri, 6 Aug 2021 09:30:47 +0200

Subject: [PATCH 05/12] make overriding of variables optional

---

 ssg/controls.py | 38 +++++++++++++++++++-------------------

 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/ssg/controls.py b/ssg/controls.py

index 4ebb8bda3d7..90639fbe4c7 100644

--- a/ssg/controls.py

+++ b/ssg/controls.py

@@ -198,42 +198,42 @@ def _get_policy(self, policy_id):

             raise ValueError(msg)

         return policy

-    def get_all_controls_of_level(self, policy_id, level_id):

+    def get_all_controls_of_level(self, policy_id, level_id, override_vars=True):

+        # if override_vars is enabled, then variables from higher levels will

+        # override variables efined in controls of lower levels

         policy = self._get_policy(policy_id)

         levels = policy.get_level_with_ancestors(level_id)

-        print ("getting levels of " + level_id)

-        print ([ l.id for l in levels.keys()])

         # we use OrderedDict here with empty values instead of ordered set

         # cause we want to be compatible with python 2

         level_ids = collections.OrderedDict()

         for lv in levels.keys():

             level_ids[lv.id] = ""

-        print (level_ids.keys())

         all_policy_controls = self.get_all_controls(policy_id)

         eligible_controls = []

         defined_variables = []

         # we will go level by level, from top to bottom

         # this is done to enable overriding of variables by higher levels

         for lv in level_ids.keys():

-            print ("going through level " +lv)

             for c in all_policy_controls:

-                print (c.levels)

                 if lv in c.levels:

-                    # if the control has a variable, check if it is not already defined

-                    variables = list(c.variables.keys())

-                    if len(variables) == 0:

+                    if override_vars == False:

                         eligible_controls.append(c)

-                    for var in variables:

-                        if var in defined_variables:

-                            # if it is, create new instance of the control and remove the variable

-                            # we are going from the top level to the bottom

-                            # so we don't want to overwrite variables

-                            new_c = copy.deepcopy(c)

-                            del new_c.variables[var]

-                            eligible_controls.append(new_c)

-                        else:

-                            defined_variables.append(var)

+                    else:

+                        # if the control has a variable, check if it is not already defined

+                        variables = list(c.variables.keys())

+                        if len(variables) == 0:

                             eligible_controls.append(c)

+                        for var in variables:

+                            if var in defined_variables:

+                                # if it is, create new instance of the control and remove the variable

+                                # we are going from the top level to the bottom

+                                # so we don't want to overwrite variables

+                                new_c = copy.deepcopy(c)

+                                del new_c.variables[var]

+                                eligible_controls.append(new_c)

+                            else:

+                                defined_variables.append(var)

+                                eligible_controls.append(c)

         return eligible_controls

     def get_all_controls(self, policy_id):

From 47df80d086e96deb4eab88d5f813bffb380006a8 Mon Sep 17 00:00:00 2001

From: Vojtech Polasek <vpolasek@redhat.com>

Date: Wed, 11 Aug 2021 12:38:42 +0200

Subject: [PATCH 06/12] fix a typo

---

 ssg/controls.py | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ssg/controls.py b/ssg/controls.py

index 90639fbe4c7..10a304bf8c2 100644

--- a/ssg/controls.py

+++ b/ssg/controls.py

@@ -200,7 +200,7 @@ def _get_policy(self, policy_id):

     def get_all_controls_of_level(self, policy_id, level_id, override_vars=True):

         # if override_vars is enabled, then variables from higher levels will

-        # override variables efined in controls of lower levels

+        # override variables defined in controls of lower levels

         policy = self._get_policy(policy_id)

         levels = policy.get_level_with_ancestors(level_id)

         # we use OrderedDict here with empty values instead of ordered set

From 8e59037ed07aad33a55e8297ee5bce0f51c0dee6 Mon Sep 17 00:00:00 2001

From: Vojtech Polasek <vpolasek@redhat.com>

Date: Wed, 11 Aug 2021 17:02:11 +0200

Subject: [PATCH 07/12] update tests to check that overriding of variables

 works

---

 .../ssg-module/data/controls_dir/abcd-levels.yml |  4 +---

 tests/unit/ssg-module/test_controls.py           | 16 ++++++++++++++--

 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/tests/unit/ssg-module/data/controls_dir/abcd-levels.yml b/tests/unit/ssg-module/data/controls_dir/abcd-levels.yml

index b98a7cd4e19..99efafd832e 100644

--- a/tests/unit/ssg-module/data/controls_dir/abcd-levels.yml

+++ b/tests/unit/ssg-module/data/controls_dir/abcd-levels.yml

@@ -25,8 +25,6 @@ controls:

   - id: S3

     levels:

     - medium

-    rules:

-      - var_password_pam_minlen=2

   - id: S4

     title: Configure authentication

@@ -41,4 +39,4 @@ controls:

         levels:

         - high

         rules:

-          - var_password_pam_minlen=3

+          - var_password_pam_minlen=2

diff --git a/tests/unit/ssg-module/test_controls.py b/tests/unit/ssg-module/test_controls.py

index 06fcb0c375d..124b344d141 100644

--- a/tests/unit/ssg-module/test_controls.py

+++ b/tests/unit/ssg-module/test_controls.py

@@ -89,8 +89,20 @@ def test_controls_levels():

     # test overriding of variables in levels

     assert c_2.variables["var_password_pam_minlen"] == "1"

-    assert c_3.variables["var_password_pam_minlen"] == "2"

-    assert c_4b.variables["var_password_pam_minlen"] == "3"

+    assert "var_password_pam_minlen" not in c_3.variables.keys()

+    assert c_4b.variables["var_password_pam_minlen"] == "2"

+

+    for c in low_controls:

+        if "var_password_pam_minlen" in c.variables.keys():

+            assert c.variables["var_password_pam_minlen"] == "1"

+

+    for c in medium_controls:

+        if "var_password_pam_minlen" in c.variables.keys():

+            assert c.variables["var_password_pam_minlen"] == "1"

+

+    for c in high_controls:

+        if "var_password_pam_minlen" in c.variables.keys():

+            assert c.variables["var_password_pam_minlen"] == "2"

 def test_controls_load_product():

From dae4fc52a627eac6595bb73e3ffb1a0c50e78fdd Mon Sep 17 00:00:00 2001

From: Vojtech Polasek <vpolasek@redhat.com>

Date: Wed, 11 Aug 2021 17:02:32 +0200

Subject: [PATCH 08/12] make overriding of variables hardcoded when requesting

 controls of a certain level

---

 ssg/controls.py | 34 +++++++++++++++-------------------

 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/ssg/controls.py b/ssg/controls.py

index 10a304bf8c2..7923f0cb379 100644

--- a/ssg/controls.py

+++ b/ssg/controls.py

@@ -198,9 +198,7 @@ def _get_policy(self, policy_id):

             raise ValueError(msg)

         return policy

-    def get_all_controls_of_level(self, policy_id, level_id, override_vars=True):

-        # if override_vars is enabled, then variables from higher levels will

-        # override variables defined in controls of lower levels

+    def get_all_controls_of_level(self, policy_id, level_id):

         policy = self._get_policy(policy_id)

         levels = policy.get_level_with_ancestors(level_id)

         # we use OrderedDict here with empty values instead of ordered set

@@ -216,24 +214,22 @@ def get_all_controls_of_level(self, policy_id, level_id, override_vars=True):

         for lv in level_ids.keys():

             for c in all_policy_controls:

                 if lv in c.levels:

-                    if override_vars == False:

+                    # if the control has a variable, check if it is not already defined

+                    variables = list(c.variables.keys())

+                    if len(variables) == 0:

                         eligible_controls.append(c)

-                    else:

-                        # if the control has a variable, check if it is not already defined

-                        variables = list(c.variables.keys())

-                        if len(variables) == 0:

+                        continue

+                    for var in variables:

+                        if var in defined_variables:

+                            # if it is, create new instance of the control and remove the variable

+                            # we are going from the top level to the bottom

+                            # so we don't want to overwrite variables

+                            new_c = copy.deepcopy(c)

+                            del new_c.variables[var]

+                            eligible_controls.append(new_c)

+                        else:

+                            defined_variables.append(var)

                             eligible_controls.append(c)

-                        for var in variables:

-                            if var in defined_variables:

-                                # if it is, create new instance of the control and remove the variable

-                                # we are going from the top level to the bottom

-                                # so we don't want to overwrite variables

-                                new_c = copy.deepcopy(c)

-                                del new_c.variables[var]

-                                eligible_controls.append(new_c)

-                            else:

-                                defined_variables.append(var)

-                                eligible_controls.append(c)

         return eligible_controls

     def get_all_controls(self, policy_id):

From c051e11c70b7e23ce3d4a8e0670da4fae72833c6 Mon Sep 17 00:00:00 2001

From: Vojtech Polasek <vpolasek@redhat.com>

Date: Thu, 12 Aug 2021 15:30:39 +0200

Subject: [PATCH 09/12] get rid of one ordereddict

---

 ssg/controls.py | 9 ++-------

 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/ssg/controls.py b/ssg/controls.py

index 7923f0cb379..891b13c891c 100644

--- a/ssg/controls.py

+++ b/ssg/controls.py

@@ -201,19 +201,14 @@ def _get_policy(self, policy_id):

     def get_all_controls_of_level(self, policy_id, level_id):

         policy = self._get_policy(policy_id)

         levels = policy.get_level_with_ancestors(level_id)

-        # we use OrderedDict here with empty values instead of ordered set

-        # cause we want to be compatible with python 2

-        level_ids = collections.OrderedDict()

-        for lv in levels.keys():

-            level_ids[lv.id] = ""

         all_policy_controls = self.get_all_controls(policy_id)

         eligible_controls = []

         defined_variables = []

         # we will go level by level, from top to bottom

         # this is done to enable overriding of variables by higher levels

-        for lv in level_ids.keys():

+        for lv in levels.keys():

             for c in all_policy_controls:

-                if lv in c.levels:

+                if lv.id in c.levels:

                     # if the control has a variable, check if it is not already defined

                     variables = list(c.variables.keys())

                     if len(variables) == 0:

From 4dd5cb1326932cf020785a8c2472998eb2e7775e Mon Sep 17 00:00:00 2001

From: Vojtech Polasek <vpolasek@redhat.com>

Date: Thu, 12 Aug 2021 16:44:57 +0200

Subject: [PATCH 10/12] fix overriding of variables

when there were multiple variables overridden, it caused problems by creating multiple copies of controls

---

 ssg/controls.py | 16 +++++++++-------

 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/ssg/controls.py b/ssg/controls.py

index 891b13c891c..8b69676313c 100644

--- a/ssg/controls.py

+++ b/ssg/controls.py

@@ -214,17 +214,19 @@ def get_all_controls_of_level(self, policy_id, level_id):

                     if len(variables) == 0:

                         eligible_controls.append(c)

                         continue

+                    variables_to_remove = [] # contains list of variables which are already defined and should be removed from the control

                     for var in variables:

                         if var in defined_variables:

-                            # if it is, create new instance of the control and remove the variable

-                            # we are going from the top level to the bottom

-                            # so we don't want to overwrite variables

-                            new_c = copy.deepcopy(c)

-                            del new_c.variables[var]

-                            eligible_controls.append(new_c)

+                            variables_to_remove.append(var)

                         else:

                             defined_variables.append(var)

-                            eligible_controls.append(c)

+                    if len(variables_to_remove) == 0:

+                        eligible_controls.append(c)

+                    else:

+                        new_c = copy.deepcopy(c)

+                        for var in variables_to_remove:

+                            del new_c.variables[var]

+                        eligible_controls.append(new_c)

         return eligible_controls

     def get_all_controls(self, policy_id):

From fbebba524cab090bc4c2f92b75257a7cc881ef5e Mon Sep 17 00:00:00 2001

From: Vojtech Polasek <vpolasek@redhat.com>

Date: Thu, 12 Aug 2021 16:45:38 +0200

Subject: [PATCH 11/12] extended tests to test for multiple overridden

 variables

---

 .../data/controls_dir/abcd-levels.yml         |  2 ++

 tests/unit/ssg-module/test_controls.py        | 19 +++++++++++++++++++

 2 files changed, 21 insertions(+)

diff --git a/tests/unit/ssg-module/data/controls_dir/abcd-levels.yml b/tests/unit/ssg-module/data/controls_dir/abcd-levels.yml

index 99efafd832e..2e60ec43532 100644

--- a/tests/unit/ssg-module/data/controls_dir/abcd-levels.yml

+++ b/tests/unit/ssg-module/data/controls_dir/abcd-levels.yml

@@ -21,6 +21,7 @@ controls:

     - low

     rules:

       - var_password_pam_minlen=1

+      - var_some_variable=1

   - id: S3

     levels:

@@ -40,3 +41,4 @@ controls:

         - high

         rules:

           - var_password_pam_minlen=2

+          - var_some_variable=3

diff --git a/tests/unit/ssg-module/test_controls.py b/tests/unit/ssg-module/test_controls.py

index 124b344d141..1465661b04a 100644

--- a/tests/unit/ssg-module/test_controls.py

+++ b/tests/unit/ssg-module/test_controls.py

@@ -104,6 +104,25 @@ def test_controls_levels():

         if "var_password_pam_minlen" in c.variables.keys():

             assert c.variables["var_password_pam_minlen"] == "2"

+    # now test if controls of lower level has the variable definition correctly removed

+    # because it is overriden by higher level controls

+    s2_high = [c for c in high_controls if c.id == "S2"]

+    assert len(s2_high) == 1

+    assert "var_some_variable" not in s2_high[0].variables.keys()

+    assert "var_password_pam_minlen" not in s2_high[0].variables.keys()

+    s4b_high = [c for c in high_controls if c.id == "S4.b"]

+    assert len(s4b_high) == 1

+    assert s4b_high[0].variables["var_some_variable"] == "3"

+    assert s4b_high[0].variables["var_password_pam_minlen"] == "2"

+

+    # check that in low level the variable is correctly placed there in S2

+    s2_low = [c for c in low_controls if c.id == "S2"]

+    assert len(s2_low) == 1

+    assert s2_low[0].variables["var_some_variable"] == "1"

+    assert s2_low[0].variables["var_password_pam_minlen"] == "1"

+

+

+

 def test_controls_load_product():

     ssg_root = \

From 369de6b8374084d9d607979b712285912dbb65aa Mon Sep 17 00:00:00 2001

From: Matej Tyc <matyc@redhat.com>

Date: Mon, 16 Aug 2021 10:39:22 +0200

Subject: [PATCH 12/12] Style improvements

- Renamed get_level_with_ancestors to get_level_with_ancestors_sequence,

  and made it return a list - a dictionary result is quite confusing.

- Removed some optimization in the variable deletion loops.

- Extracted functionality to a _get_control_without_variables static

  method.

- Defined variable removal steps using set operations.

---

 ssg/controls.py | 54 +++++++++++++++++++++++++------------------------

 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/ssg/controls.py b/ssg/controls.py

index 8b69676313c..ca3187d5b16 100644

--- a/ssg/controls.py

+++ b/ssg/controls.py

@@ -152,17 +152,17 @@ def get_level(self, level_id):

             )

             raise ValueError(msg)

-    def get_level_with_ancestors(self, level_id):

+    def get_level_with_ancestors_sequence(self, level_id):

         # use OrderedDict for Python2 compatibility instead of ordered set

         levels = collections.OrderedDict()

         level = self.get_level(level_id)

         levels[level] = ""

         if level.inherits_from:

             for lv in level.inherits_from:

-                eligible_levels = [l for l in self.get_level_with_ancestors(lv).keys() if l not in levels.keys()]

+                eligible_levels = [l for l in self.get_level_with_ancestors_sequence(lv) if l not in levels.keys()]

                 for l in eligible_levels:

                     levels[l] = ""

-        return levels

+        return list(levels.keys())

 class ControlsManager():

@@ -200,35 +200,37 @@ def _get_policy(self, policy_id):

     def get_all_controls_of_level(self, policy_id, level_id):

         policy = self._get_policy(policy_id)

-        levels = policy.get_level_with_ancestors(level_id)

+        levels = policy.get_level_with_ancestors_sequence(level_id)

         all_policy_controls = self.get_all_controls(policy_id)

         eligible_controls = []

-        defined_variables = []

+        already_defined_variables = set()

         # we will go level by level, from top to bottom

         # this is done to enable overriding of variables by higher levels

-        for lv in levels.keys():

-            for c in all_policy_controls:

-                if lv.id in c.levels:

-                    # if the control has a variable, check if it is not already defined

-                    variables = list(c.variables.keys())

-                    if len(variables) == 0:

-                        eligible_controls.append(c)

-                        continue

-                    variables_to_remove = [] # contains list of variables which are already defined and should be removed from the control

-                    for var in variables:

-                        if var in defined_variables:

-                            variables_to_remove.append(var)

-                        else:

-                            defined_variables.append(var)

-                    if len(variables_to_remove) == 0:

-                        eligible_controls.append(c)

-                    else:

-                        new_c = copy.deepcopy(c)

-                        for var in variables_to_remove:

-                            del new_c.variables[var]

-                        eligible_controls.append(new_c)

+        for lv in levels:

+            for control in all_policy_controls:

+                if lv.id not in control.levels:

+                    continue

+

+                variables = set(control.variables.keys())

+

+                variables_to_remove = variables.intersection(already_defined_variables)

+                already_defined_variables.update(variables)

+

+                new_c = self._get_control_without_variables(variables_to_remove, control)

+                eligible_controls.append(new_c)

+

         return eligible_controls

+    @staticmethod

+    def _get_control_without_variables(variables_to_remove, control):

+        if not variables_to_remove:

+            return control

+

+        new_c = copy.deepcopy(control)

+        for var in variables_to_remove:

+            del new_c.variables[var]

+        return new_c

+

     def get_all_controls(self, policy_id):

         policy = self._get_policy(policy_id)

         return policy.controls_by_id.values()