sysctl_kernel_randomize_va_space: Check sysctl.d/*.conf in addition to sysctl.conf

[jglemza]

Currently, sysctl_kernel_randomize_va_space only checks the sysctl.conf file. This modification allows sysctl_kernel_randomize_va_space be configured in sysctl.d/*.conf or sysctl.conf.

Please let me know if this is not the proper place to make this edit.

[openscap-jenkins]

Can one of the admins verify this patch?

[mpreisler]

[ok to test]

[iankko]

Ditto. Will look at this one too.

[jglemza]

Sorry for the commit after the fact. I'm new to creating pull requests and did not realize it would attach to this pull request.

[iankko]

@jglemza

Hello Josh,

   thank you for the contribution.

Sorry for the commit after the fact. I'm new to creating pull requests and did not realize it would attach to this pull request.

Regarding the pull request:

It's actually expected the commit to be attached to the pull request. But rather than pushing the intended feature changes directly in the the master branch of your SSG repository fork, it's better to keep the master branch unchanged and only synchronized with the upstream SSG's master branch, and for each new feature / enhancement create own feature branch in your fork. Refer to e.g. page #36 of https://jlieskov.files.wordpress.com/2013/11/scap_security_guide_questions_answers_contributor_workshop_volume_1_november_2015.pdf document for the expected form of SSG developer workflow. Also GitHub help pages might provide further advice:

• https://help.github.com/articles/fork-a-repo/ (fork a repo && keep it synced),
• https://help.github.com/articles/creating-and-deleting-branches-within-your-repository/ (creating a feature branch),
• https://help.github.com/articles/creating-new-files (create or) https://help.github.com/articles/editing-files-in-your-repository/ (edit the necessary files)
• https://help.github.com/articles/creating-a-pull-request/ (create a pull request)

Then someone from SSG upstream would review your pull request, and if it got approved (it got ACK within a comment), they would merge it. If there would be some issues reported, they should be corrected by the contributor via subsequent commits and pushes into the very same feature branch of your fork.

Regarding the code changes themselves:

Your proposal is trying to change by hand OVAL check that has been created by the OVAL template (or supposed to be created by OVAL template). This can be observed by presence of <!-- THIS FILE IS GENERATED by create_sysctl_checks.py. DO NOT EDIT. --> string in it.

Rather than try to edit such a file manually by hand, it's better to modify the corresponding CSV file, and copy the already created OVAL file into the intended location (shared/oval/sysctl_kernel_randomize_va_space.xml in your case). This approach is more suitable because in the moment someone would edit the CSV file, your changes done in this PR would be lost. Also the corresponding OVAL template is more universal than just adding /etc/sysctl.d/*.conf location. Kindly have a look at it's implementation at:

• https://github.com/OpenSCAP/scap-security-guide/blob/master/RHEL/7/input/oval/templates/template_sysctl_static and
• https://github.com/OpenSCAP/scap-security-guide/blob/master/RHEL/7/input/oval/templates/template_sysctl_static_var

As can be seen these templates already count with four different locations (/etc/sysctl.conf, /etc/sysctl.d/*.conf, /run/sysctl.d/*.conf, and /usr/lib/sysctl.d/*.conf where sysctl configuration can be placed for Red Hat Enterprise Linux 7 system). Therefore rather than adding just support for /etc/sysct.d/*.conf, we might want the resulting OVAL to support also the remaining two possible locations.

So what's expected to be done:

• edit the sysctl corresponding CSV file:
  https://github.com/OpenSCAP/scap-security-guide/blob/master/RHEL/7/input/oval/templates/sysctl_values.csv

  adding kernel.randomize_va_space as the key, and 2 as the expected value

• from the scap-security-guide/RHEL/7/input/oval/templates call the make sysctls target

• this will create the following three files (except the others) for your check in the output subdirectory:

  templates]$ make clean
  rm -f output/*.xml
  rm -f output/*.sh
  rm -rf output/
  $ make sysctls
  ../../../../../shared/oval/templates/create_sysctl_checks.py sysctl_values.csv
  ../../../../../shared/oval/templates/create_sysctl_ipv6_checks.py sysctl_ipv6_values.csv
  templates]$ find . -name sysctl_*_randomize_va_space.xml
  ./output/sysctl_runtime_kernel_randomize_va_space.xml
  ./output/sysctl_static_kernel_randomize_va_space.xml
  ./output/sysctl_kernel_randomize_va_space.xml
  

  • Finally copy all three these files into the shared/oval location (replacing the current form of shared/oval/sysctl_kernel_randomize_va_space.xml OVAL check).

Side note: The _static_ version of the OVAL is checking static / persistent system configuration, the _runtime_ version of the OVAL is checking the runtime system configuration, finally the sysctl_kernel_randomize_va_space.xml is checking both of them (therefore three files are necessary).

Here's an example how the pull request (list of changed files) looked like in the case fs.suid_dumpable sysctl kernel parameter was subject of OVAL check addons:
    [==>] 6199203

Once all of the above is done:

• close this pull request (since it can't be merged in the current form),
• create a new branch for your changes, create the aforementioned changes [==>] but modified for the case of kernel.randomize_va_space case, push the changes into the feature branch of your fork,
• and create a new pull request for the review.

Hope the above being helpful (speak out if some parts of the above got left unclear yet).

Regards, Jan.

[jglemza]

Thanks for the guidance. I'll be in touch if I have any additional questions before I submit a new pull request.