Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update disable_users_coredumps.xml #358

Merged
merged 1 commit into from
Jan 8, 2015
Merged

Update disable_users_coredumps.xml #358

merged 1 commit into from
Jan 8, 2015

Conversation

fcaviggia
Copy link
Contributor

Hard or '-' works because '-' sets both hard and soft.

Hard or '-' works because '-' sets both hard and soft.
@@ -24,7 +24,7 @@
</ind:textfilecontent54_state>
<ind:textfilecontent54_object id="object_core_dumps_limitsconf" version="1">
<ind:filepath>/etc/security/limits.conf</ind:filepath>
<ind:pattern operation="pattern match">^[\s]*\*[\s]+hard[\s]+core[\s]+([\d]+)</ind:pattern>
<ind:pattern operation="pattern match">^[\s]*\*[\s]+(hard|-)[\s]+core[\s]+([\d]+)</ind:pattern>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the observation / catch behind proposed enhancement of this OVAL check is correct (the OVAL object comment mentions it will be checking for both (hard|-) values and also limits.conf manual page suggests when '-' character at particular field is used, both soft & hard resource limits will be enforced), the current implementation is wrong.

The problem is in the form the new regex is written - while the intention is to allow both hard and - values to be allowed in the second column of the core row of /etc/security/limits.conf, the regex instead of allowing both of these options defines new regex match group with id #1 (due to adding brackets):

  • the first captured group will be either hard or -,
  • the second capturing group will be the actual number setting (4-th column of the core row)

meaning the new implementation won't pass even when the system is configured properly. Data collected from the system with new implementation:

        <collected_objects>
          <object id="oval:ssg:obj:2230" version="1" flag="complete">
            <reference item_ref="1044271"/>
          </object>
        </collected_objects>
        <system_data>
          <ind-sys:textfilecontent_item id="1044271" status="exists">
            <ind-sys:filepath>/etc/security/limits.conf</ind-sys:filepath>
            <ind-sys:path>/etc/security</ind-sys:path>
            <ind-sys:filename>limits.conf</ind-sys:filename>
            <ind-sys:pattern>^[\s]*\*[\s]+(hard|-)[\s]+core[\s]+([\d]+)</ind-sys:pattern>
            <ind-sys:instance datatype="int">1</ind-sys:instance>
            <ind-sys:line>^[\s]*\*[\s]+(hard|-)[\s]+core[\s]+([\d]+)</ind-sys:line>
            <ind-sys:text>*     -   core        0</ind-sys:text>
            <ind-sys:subexpression>-</ind-sys:subexpression>
            <ind-sys:subexpression>0</ind-sys:subexpression>
          </ind-sys:textfilecontent_item>
        </system_data>

As can be seen in the new implementation there will be two subexpressions, one of them ('-') not matching the requirement ('0'), therefore the check will fail also on properly configured system.

To fix the problem the regex modification should be to use non-capturing group for the first case, IOW something like:

+ <ind:pattern operation="pattern match">^[\s]*\*[\s]+(?:hard|-)[\s]+core[\s]+([\d]+)</ind:pattern>

so the first regex match group remains the digit one, and only that one will be compared with expected state.

@iankko
Copy link

iankko commented Dec 18, 2014

NACK for now due to previous comment.

@shawndwells
Copy link
Member

bump to @fcaviggia

@shawndwells
Copy link
Member

bump to @fcaviggia, as it's likely he's just returning from PTO :)

@shawndwells shawndwells merged commit 658c212 into ComplianceAsCode:master Jan 8, 2015
@shawndwells
Copy link
Member

Updated @fcaviggia's patch with @iankko's comments and merged

@shawndwells shawndwells added this to the 0.1.20 milestone Jan 8, 2015
brett060102 added a commit to brett060102/content that referenced this pull request Apr 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants