ansible-lockdown/RHEL6-STIG

Tasks that combine the remediation of multiple rules should be refactored

juliedavila opened this issue · 4 comments

Currently there are tasks that remediate multiple rules at the same time. This should not be the case.

Example:

- name: "V-38497 High  The system must not have accounts configured with blank or null passwords\n
        \tV-38658 Medium  The system must prohibit the reuse of passwords within twenty-four iterations\n
        \tV-38574 Medium  The system must use a FIPS 140-2 approved cryptographic hashing algorithm for generating account password hashes (system-auth)\n
        \tV-38482 Low     The system must require passwords to contain at least one numeric character\n
        \tV-38571 Low     The system must require passwords to contain at least one lowercase alphabetic character\n
        \tV-38572 Low     The system must require at least four characters be changed between the old and new passwords during a password change"
  lineinfile: >
    dest={{ item.dest }}
    regexp={{ item.rx }}
    line="{{ item.ln }}"
    backrefs=yes
    follow=yes owner=root group=root mode=0644
  with_items:
    - { rx: "'^(password\\s+requisite\\s+pam_cracklib.so\\s)(.*)$'", ln: "\\1{{rhel6stig_pam_cracklib_params}}", dest: '/etc/pam.d/system-auth' }
    - { rx: "'^(password\\s+requisite\\s+pam_cracklib.so\\s)(.*)$'", ln: "\\1{{rhel6stig_pam_cracklib_params}}", dest: '/etc/pam.d/password-auth' }
    - { rx: "'^(password\\s+sufficient\\s+pam_unix.so\\s)(.*)$'", ln: "\\1{{rhel6stig_pam_unix_params}}", dest: '/etc/pam.d/system-auth' }
    - { rx: "'^(password\\s+sufficient\\s+pam_unix.so\\s)(.*)$'", ln: "\\1{{rhel6stig_pam_unix_params}}", dest: '/etc/pam.d/password-auth' }
    - { rx: "'^(auth\\s+sufficient\\s+pam_unix.so)(.*)$'", ln: "auth        sufficient    {{rhel6stig_pam_auth_sufficient}}", dest: '/etc/pam.d/system-auth' }
    - { rx: "'^(auth\\s+sufficient\\s+pam_unix.so)(.*)$'", ln: "auth        sufficient    {{rhel6stig_pam_auth_sufficient}}", dest: '/etc/pam.d/password-auth' }
  tags: [ 'cat1' , 'cat2' , 'V-38497' , 'V-38658' , 'V-38574' , 'V-38482' , 'V-38571' , 'V-38572' , 'passwords' , 'accounts' ]

This makes it impossible to remediate any of 'V-38497' , 'V-38658' , 'V-38574' , 'V-38482' , 'V-38571' , 'V-38572' on an individual level, hitting one would hit them all. Also, combining varying severity levels is less than ideal as it limits how specific a user can harden a system.

This was something I wrestled with: granularity vs. efficiency. It appears granularity is more important.

One of the unpleasant ramifications of this is that, while a template that contains settings remediating CAT I and CAT II findings is very elegant, the template file must be replaced with several lineinfile tasks in order to allow granularity, introducing complexity (Granted, your specific example shows lots of lineinfile actions, so this is a pretty straightforward fix of making lots of individual tasks instead of a single task with a with_items. I'm thinking more about the audit.rules config file, which I believe contains settings that remediate findings across multiple criticality levels.).

I wonder if a better way to solve this would be multiple template files: one for the CAT I stuff, another for the CAT II stuff... Anyway, my general rule is three lineinfile tasks on the same file means I move to a template. I'm open to better ways to handling the need for granularity.

Actually, that's a great point and idea, I do like the idea of a template per severity. The change on the task/main.yml would mean it would have to hit the severities in reverse order (or place the logic elsewhere).

Another idea (or addition) would be to place the logic in the template itself. Something along the lines of
{% if rhel6stig_cat2 == true %}, etc.

I think I like the idea of using Jinja logic the best. It means we'll have the same template task in multiple playbooks when only one task would suffice, but that's a degree of redundancy I can live with. Much better than lots of lineinfile tasks.

This issue was addressed in the major refactoring of tasks. We may do more work in the future to move some things into templates since we've decided to have a bool for each finding ID, but I think the largest part of this issues has been addressed.