[PATCH 0/2] Two fixes for OVAL checks accounts_dangerous_path_for_root and package_red_hat_gpgkeys_installed

Maura Dailey maura at eclipse.ncsc.mil
Tue Apr 23 21:03:58 UTC 2013


I considered it dangerous to alter the PATH for a user account, which is 
why I initially chose to use grep. The regex was at fault in the 
existing check, and I proved that the regex was wrong using grep.

However, Jeff was correct to point out that testing with testcheck.py 
was necessary because OVAL is weird and the documentation for it is a 
little misleading. I discovered that check="none satisfy" is not a 
direct replacement for check="none exist", despite wording that seems to 
suggest the contrary. So while my regex was, in fact, correct for all my 
test cases, I was getting weird results of true and false when I finally 
figured out how to test everything with testcheck.py directly. Perhaps 
someone else can figure out what combination of check and 
check_existence will work with multiple states, but it was far easier to 
break the states out into individual tests.

I'll post the full new patch for accounts_dangerous_path_for_root.xml 
after this, but here's the blurb to apply to the last patch (scroll past 
it to see the actual results of testcheck.py testing):

diff --git a/RHEL6/input/checks/accounts_dangerous_path_for_root.xml 
b/RHEL6/input/checks/accounts_dangerous_path_for_root.xml
index aa38d43..d78f49a 100644
--- a/RHEL6/input/checks/accounts_dangerous_path_for_root.xml
+++ b/RHEL6/input/checks/accounts_dangerous_path_for_root.xml
@@ -9,31 +9,39 @@
      </metadata>
      <criteria comment="environment variable PATH contains dangerous 
path" operator="AND">
        <criterion comment="environment variable PATH starts with : or 
." test_ref="test_env_var_begins" />
-      <criterion comment="environment variable PATH contains : or . 
twice in a row" test_ref="test_env_var_contains" />
+      <criterion comment="environment variable PATH contains : twice in 
a row" test_ref="test_env_var_contains_doublecolon" />
+      <criterion comment="environment variable PATH contains . twice in 
a row" test_ref="test_env_var_contains_doubleperiod" />
        <criterion comment="environment variable PATH ends with : or ." 
test_ref="test_env_var_ends" />
        <criterion comment="environment variable PATH doesn't begin with 
a /" test_ref="test_env_var_begins_slash" />
+      <criterion comment="environment variable PATH doesn't contain 
relative paths" test_ref="test_env_var_contains_relative_path" />
      </criteria>
    </definition>
    <ind:environmentvariable_test check="none satisfy" 
comment="environment variable PATH starts with : or ." 
id="test_env_var_begins" version="1">
      <ind:object object_ref="object_env_var_path" />
      <ind:state state_ref="state_begins_colon_period" />
    </ind:environmentvariable_test>
-  <ind:environmentvariable_test check="none satisfy" 
comment="environment variable PATH contains . or : twice in a row" 
id="test_env_var_contains" version="1">
+  <ind:environmentvariable_test check="none satisfy" 
comment="environment variable PATH contains : twice in a row" 
id="test_env_var_contains_doublecolon" version="1">
      <ind:object object_ref="object_env_var_path" />
      <ind:state state_ref="state_contains_double_colon" />
+  </ind:environmentvariable_test>
+  <ind:environmentvariable_test check="none satisfy" 
comment="environment variable PATH contains . twice in a row" 
id="test_env_var_contains_doubleperiod" version="1">
+    <ind:object object_ref="object_env_var_path" />
      <ind:state state_ref="state_contains_double_period" />
    </ind:environmentvariable_test>
    <ind:environmentvariable_test check="none satisfy" 
comment="environment variable PATH ends with : or ." 
id="test_env_var_ends" version="1">
      <ind:object object_ref="object_env_var_path" />
      <ind:state state_ref="state_ends_colon_period" />
    </ind:environmentvariable_test>
-  <ind:environmentvariable_test check="none satisfy" 
comment="environment variable PATH begins with /" 
id="test_env_var_begins_slash" version="1">
+  <ind:environmentvariable_test check="none satisfy" 
comment="environment variable PATH starts with an absolute path /" 
id="test_env_var_begins_slash" version="1">
      <ind:object object_ref="object_env_var_path" />
      <ind:state state_ref="state_begins_slash" />
+  </ind:environmentvariable_test>
+  <ind:environmentvariable_test check="none satisfy" 
comment="environment variable PATH contains relative paths" 
id="test_env_var_contains_relative_path" version="1">
+    <ind:object object_ref="object_env_var_path" />
      <ind:state state_ref="state_contains_relative_path" />
    </ind:environmentvariable_test>


Here's the testing I performed with testcheck.py, altering the 
accounts_dangerous_path_for_root.xml file to read from PATHTEST instead 
of PATH:


The following tests should evaluate to true:

With PATHTEST initially set to the default root PATH: 
/usr/lib64/qt-3.3/bin:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/root/bin

Evaluating with OVAL tempfile : 
/tmp/accounts_dangerous_path_for_rootSpr4jL.xml
Definition oval:scap-security-guide.testing:def:134: true
Evaluation done.

To check for a valid use of : in the middle of a folder name, set 
PATHTEST to: 
/usr/lib64/qt\:-3.3/bin:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/root/bin

Evaluating with OVAL tempfile : 
/tmp/accounts_dangerous_path_for_root2ZMmwH.xml
Definition oval:scap-security-guide.testing:def:134: true
Evaluation done.


The following tests should evaluate to false:

To check for an invalid use of a double '.', set PATHTEST to: 
/usr/lib64/qt-3.3/bin:..:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/root/bin

Evaluating with OVAL tempfile : 
/tmp/accounts_dangerous_path_for_rootYJ_hVp.xml
Definition oval:scap-security-guide.testing:def:134: false
Evaluation done.

To check for an invalid use of a double ':', set PATHTEST to: 
/usr/lib64/qt-3.3/bin::/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/root/bin

Evaluating with OVAL tempfile : 
/tmp/accounts_dangerous_path_for_roothgOv2e.xml
Definition oval:scap-security-guide.testing:def:134: false
Evaluation done.

To check for relative paths, I'll set PATHTEST to: 
usr/lib64/qt-3.3/bin:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/root/bin

Evaluating with OVAL tempfile : 
/tmp/accounts_dangerous_path_for_rootjaK4O7.xml
Definition oval:scap-security-guide.testing:def:134: false
Evaluation done.

And to check for relative paths in the middle of the path, I'll set 
PATHTEST to: 
/usr/lib64/qt-3.3/bin:usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/root/bin

Evaluating with OVAL tempfile : 
/tmp/accounts_dangerous_path_for_rootjLpTgG.xml
Definition oval:scap-security-guide.testing:def:134: false
Evaluation done.

To check that the path doesn't end in ':', I'll set PATHTEST to: 
/usr/lib64/qt-3.3/bin:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/root/bin:

Evaluating with OVAL tempfile : 
/tmp/accounts_dangerous_path_for_root_9wdpm.xml
Definition oval:scap-security-guide.testing:def:134: false
Evaluation done.

To check that the path doesn't end in '.', I'll set PATHTEST to: 
/usr/lib64/qt-3.3/bin:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/root/bin.

Evaluating with OVAL tempfile : 
/tmp/accounts_dangerous_path_for_root4DcuhE.xml
Definition oval:scap-security-guide.testing:def:134: false
Evaluation done.

To check that the path doesn't begin in '.', I'll set PATHTEST to: 
./usr/lib64/qt-3.3/bin:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/root/bin

Evaluating with OVAL tempfile : 
/tmp/accounts_dangerous_path_for_rootFOOqRI.xml
Definition oval:scap-security-guide.testing:def:134: false
Evaluation done.

To check that the path doesn't begin in ':', I'll set PATHTEST to: 
:/usr/lib64/qt-3.3/bin:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/root/bin

Evaluating with OVAL tempfile : 
/tmp/accounts_dangerous_path_for_root5udYeZ.xml
Definition oval:scap-security-guide.testing:def:134: false
Evaluation done.

In each case, the file 
/tmp/accounts_dangerous_path_for_root??????.xml-results file is created, 
showing the results of each criteria and criterion check.

- Maura

On 04/23/2013 02:25 PM, Jeffrey Blank wrote:
> I mean, with regard to a positive and negative result.
>
> That is, did you alter the root path and achieve the expected results
> from the OVAL checks, for both negative and positive test cases, using
> testcheck.py?
>
>
> And indeed it is an interesting question, as to the value proposition
> for building and maintaining a test harness...
>
>
>
> On 04/23/2013 01:51 PM, Maura Dailey wrote:
>> I unit tested this to the best of my ability using grep's regular
>> expressions.
>>
>> - Maura Dailey
>>
>> On 04/23/2013 01:44 PM, Jeffrey Blank wrote:
>>> Did you unit test this?
>>>
>>>
>>>
>>> On 04/23/2013 01:35 PM, Maura Dailey wrote:
>>>> Forgive the cruddy formatting. I don't generate patches with git
>>>> often enough to get all the options to look right on the first try.
>>>>
>>>> For accounts_dangerous_path_for_root.xml:
>>>>    - First, I noticed that this check returned a result of true if any
>>>> of the conditions that we didn't want to see were satisfied. I
>>>> assumed that a false result was desired, so I made sure all tests had
>>>> a check value of "none satisfy" and that the criteria operator was
>>>> AND, not OR. A single failure will now give a result of false.
>>>>
>>>>    - Second, I noticed that the test_env_var_contains check would also
>>>> check for the values ".:" or ":.". If this is actually desired
>>>> behavior, I'll reverse this one, but for now, I split it into two
>>>> checks, one for "::" and one for "\.\."
>>>>
>>>>    - Third, no check was done to see if the path began with a / or if
>>>> elements in the path began with /. My checks ensure that PATH only
>>>> begins with / and that each instance of : is followed by a slash.
>>>> This is the riskiest change I made, since : is a valid character in
>>>> folder names. I tried to exclude : characters that begin with \.
>>>>
>>>> For package_red_hat_gpgkeys_installed.xml:
>>>>    - There were two problems in this particular check. The first was
>>>> that the check for gpg-pubkey-2fa658e0-45700c69 had its release and
>>>> version numbers reversed, so that it would always fail. The second
>>>> was that both rpminfo_state blocks shared the same id.
>>>>
>>>> Maura Dailey (2):
>>>>     Maura: Existing check returns false instead of true when the PATH
>>>>       passes all checks, and it doesn't check that path elements begin
>>>>       with /
>>>>     Maura: Duplicate state ids fixed, fixed reversed version and release
>>>>       info for the auxillary key
>>>>
>>>>    .../checks/accounts_dangerous_path_for_root.xml    |   30
>>>> ++++++++++++++----
>>>>    .../checks/package_red_hat_gpgkeys_installed.xml   |   32
>>>> +++++++-------------
>>>>    2 files changed, 34 insertions(+), 28 deletions(-)
>>>>
>>>> _______________________________________________
>>>> scap-security-guide mailing list
>>>> scap-security-guide at lists.fedorahosted.org
>>>> https://lists.fedorahosted.org/mailman/listinfo/scap-security-guide
>>>>
>>> _______________________________________________
>>> scap-security-guide mailing list
>>> scap-security-guide at lists.fedorahosted.org
>>> https://lists.fedorahosted.org/mailman/listinfo/scap-security-guide
>>



More information about the scap-security-guide mailing list