Open cpoma opened 4 years ago
It should be our first start that the tagging between r1v4 and r2v2
Could we add a tag that is an array of the STIG versions to which the check applies? That will make it easier to cull over time.
Thanks. Danny and I will chat about this today and hopefully able to start moving forward on integrating these sets of changes and tagging stuff.
Also with some of the strange behavior in 4.18 we need to test previous releases and the current releases to make sure everything works on both sides. Perhaps in 4.19 though back off a few of the issues.
On Thu, Oct 31, 2019, 8:50 AM Kevin J. Smith notifications@github.com wrote:
@kevin-j-smith requested changes on this pull request.
In controls/V-81009.rb https://github.com/simp/inspec-profile-disa_stig-el7/pull/113#discussion_r341105039 :
+
- If the \"nodev\" option is not present on the line for \"/dev/shm\", this is a finding.
- Verify \"/dev/shm\" is mounted with the \"nodev\" option:
mount | grep \"/dev/shm\" | grep nodev
- If no results are returned, this is a finding.
- "
- desc "fix", "
- Configure the \"/etc/fstab\" to use the \"nodev\" option for all lines containing \"/dev/shm\".
- "
- describe mount('/dev/shm') do
- its('options') { should include 'nodev' }
- end
This describe statement only takes into consideration what is currently mounted. It is important to indicate that would be mounted if DR happens (node bounces) and auto-mount procedures take place. We normally weight everything that is configurable upon 2 scales. 1) configuration (what happens when the node is rebooted) and 2) cached (what parameters/settings are currently affecting the node). I think in this case you should include the configuration settings. Here is what that would look like.
describe etc_fstab.where { mount_point == '/dev/shm' } do it { should be_configured } its('mount_options.flatten') { should include 'nodev' } end
It is also important to note that /etc/mtab also indicates the status of a healthy system. It is wholly possible for mount to not indicate proper settings (unlikely, but possible). In that case you might want to take this file into consideration.
describe etc_fstab('/etc/mtab') { mount_point == '/dev/shm' } do it { should be_configured } its('mount_options.flatten') { should include 'nodev' } end
The last possible location to look for 'proper working order' is /proc/mounts. Quote from the mount manual:
When the proc filesystem is mounted (say at /proc), the files /etc/mtab and /proc/mounts have very similar contents. The former has somewhat more information, such as the mount options used, but is not necessarily up-to-date.
It is also possible that you want to check the cached value in /proc/mounts in newer and newer OS versions due to the adoption of systemd *.mounts files.
In controls/V-81011.rb https://github.com/simp/inspec-profile-disa_stig-el7/pull/113#discussion_r341105266 :
- describe mount('/dev/shm') do
- its('options') { should include 'nosuid' }
- end
Same as above, but using 'nosuid'
In controls/V-81013.rb https://github.com/simp/inspec-profile-disa_stig-el7/pull/113#discussion_r341105341 :
- describe mount('/dev/shm') do
- its('options') { should include 'noexec' }
- end
ditto
In controls/V-81015.rb https://github.com/simp/inspec-profile-disa_stig-el7/pull/113#discussion_r341107499 :
+
- Off-loading is a common process in information systems with limited audit storage capacity.
- Without the configuration of the \"au-remote\" plugin, the audisp-remote daemon will not off-load the logs from the system being audited.
- "
- desc "fix", "
- Edit the /etc/audisp/plugins.d/au-remote.conf file and change the value of \"active\" to \"yes\".
- The audit daemon must be restarted for changes to take effect:
service auditd restart
- "
- if file('/etc/audisp/plugins.d/au-remote.conf').exist?
- describe parse_config_file('/etc/audisp/plugins.d/au-remote.conf') do
- its('active') { should match %r{yes$} }
using regex here is a bit unneeded. You should trust the parse_config_file, or supply the regex (options + assignment_regex) to extract the exact piece you are looking for. In your case match %r{yes$} is unneeded. Use a more simpler approach, cmp 'yes'
In controls/V-81015.rb https://github.com/simp/inspec-profile-disa_stig-el7/pull/113#discussion_r341110005 :
- else
- describe "File '/etc/audisp/plugins.d/au-remote.conf' cannot be found. This test cannot be checked in a automated fashion and you must check it manually" do
- skip "File '/etc/audisp/plugins.d/au-remote.conf' cannot be found. This check must be performed manually"
- end
It would be interesting to see what additional behavior is acceptable, in this case. The STIG does not allow for additional behavior, so I am uncertain why you are coding manual review as optional. This appears to be a clear pass/fail check. I would remove the if/else control structure and just search for the file.
Now the question is, if what you are intending is valid. That is, are there secondary approved options. If this is the case, I would just say implement them here. Give an attribute in that control structure to require organizational documentation stating why this option is valid for their use case. In the end, I would say this is unneeded. What I would expect is exceptions, the likes of which you are allowing to be manually reviewed, to be overridden by organizational inheritance.
In controls/V-81017.rb https://github.com/simp/inspec-profile-disa_stig-el7/pull/113#discussion_r341113630 :
- describe parse_config_file('/etc/audisp/plugins.d/au-remote.conf') do
- its('direction') { should match %r{out$} }
- its('path') { should match %r{/sbin/audisp-remote$} }
- its('type') { should match %r{always$} }
- end
Same as above...
Plus, the STIG specifically calls out direction, path and type, as I see you have noticed and implemented. It also makes mention of type and format. As it wouldn't hurt to include these, for completeness I would implement them as well. This is only a suggestion and not a must. We normally ask that each control can stand alone. So, consider that 'V-81017' is not being used to audit the system.
In controls/V-81017.rb https://github.com/simp/inspec-profile-disa_stig-el7/pull/113#discussion_r341113841 :
- describe "File '/etc/audisp/plugins.d/au-remote.conf' cannot be found. This test cannot be checked in a automated fashion and you must check it manually" do
- skip "File '/etc/audisp/plugins.d/au-remote.conf' cannot be found. This check must be performed manually"
- end
Same as V-81015, this control structure is not needed.
In controls/V-81019.rb https://github.com/simp/inspec-profile-disa_stig-el7/pull/113#discussion_r341115843 :
+
- If the \"overflow_action\" option is not \"syslog\", \"single\", or \"halt\", or the line is commented out, this is a finding.
- "
- desc "fix", "
- Edit the /etc/audisp/audispd.conf file and add or update the \"overflow_action\" option:
- overflow_action = syslog
- The audit daemon must be restarted for changes to take effect:
service auditd restart
- "
- if file('/etc/audisp/audispd.conf').exist?
- describe parse_config_file('/etc/audisp/audispd.conf') do
- its('overflow_action') { should match %r{syslog$|single$|halt$} }
It would be more clear to use arrays here vs regex.
its('overflow_action') { should be_in %w(syslog single halt) }
In controls/V-81019.rb https://github.com/simp/inspec-profile-disa_stig-el7/pull/113#discussion_r341115953 :
- describe "File '/etc/audisp/audispd.conf' cannot be found. This test cannot be checked in a automated fashion and you must check it manually" do
- skip "File '/etc/audisp/audispd.conf' cannot be found. This check must be performed manually"
- end
Same as above...unneeded control structure.
In controls/V-81021.rb https://github.com/simp/inspec-profile-disa_stig-el7/pull/113#discussion_r341117371 :
- describe "File '/etc/audisp/audispd.conf' cannot be found. This test cannot be checked in a automated fashion and you must check it manually" do
- skip "File '/etc/audisp/audispd.conf' cannot be found. This check must be performed manually"
- end
Same as above....unneeded control structure.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/simp/inspec-profile-disa_stig-el7/pull/113?email_source=notifications&email_token=AALK42BDRYROPECZLR676TDQRLIC5A5CNFSM4JAAH7W2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCJ33SNA#pullrequestreview-309836084, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALK42EFK5RWHKX2CVRAV4DQRLIC5ANCNFSM4JAAH7WQ .
Yep we internally had to restrict to 4.16 or below.
I believe Danny and Rony found all the issues and have a patch in a PR for being able to support current master for inspec.
On Thu, Oct 31, 2019, 9:39 AM Kevin J. Smith notifications@github.com wrote:
Yep we internally had to restrict to 4.16 or below.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/simp/inspec-profile-disa_stig-el7/pull/113?email_source=notifications&email_token=AALK42HLXEYFPWSCQI3N4TDQRLN2HA5CNFSM4JAAH7W2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECXZYUA#issuecomment-548379728, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALK42CZCMQQT2HTHAFUJDTQRLN2HANCNFSM4JAAH7WQ .
Adding: V-81009, V-81011, V-81013, V-81015, V-81017, V-81019, V-81021