From 7212175803ea2fab60f5f49df699f9b3216be265 Mon Sep 17 00:00:00 2001 From: Stig Brautaset Date: Tue, 8 Mar 2016 17:07:09 +0000 Subject: [PATCH 1/6] Allow configuring mod_security's SecAuditLogParts The default configuration for this includes "I" which is not always always suitable, e.g. if you cannot tolerate POST parameters appearing in your modsec_audit.log You may want to omit `I` if mod_security is protecting a hypothetical web service that accepts credit card data in a POST request, for example. --- manifests/mod/security.pp | 2 ++ manifests/params.pp | 1 + spec/classes/mod/security_spec.rb | 8 ++++++++ templates/mod/security.conf.erb | 2 +- 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/manifests/mod/security.pp b/manifests/mod/security.pp index 10145d7f3b..7cd7114c4e 100644 --- a/manifests/mod/security.pp +++ b/manifests/mod/security.pp @@ -3,6 +3,7 @@ $activated_rules = $::apache::params::modsec_default_rules, $modsec_dir = $::apache::params::modsec_dir, $modsec_secruleengine = $::apache::params::modsec_secruleengine, + $audit_log_parts = $::apache::params::modsec_audit_log_parts, $secpcrematchlimit = $::apache::params::secpcrematchlimit, $secpcrematchlimitrecursion = $::apache::params::secpcrematchlimitrecursion, $allowed_methods = 'GET HEAD POST OPTIONS', @@ -35,6 +36,7 @@ # Template uses: # - $modsec_dir + # - $audit_log_parts # - secpcrematchlimit # - secpcrematchlimitrecursion file { 'security.conf': diff --git a/manifests/params.pp b/manifests/params.pp index 7b6e93cc01..6a40f4d9bf 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -159,6 +159,7 @@ $mellon_lock_file = '/run/mod_auth_mellon/lock' $mellon_cache_size = 100 $mellon_post_directory = undef + $modsec_audit_log_parts = 'ABIJDEFHZ' $modsec_crs_package = 'mod_security_crs' $modsec_crs_path = '/usr/lib/modsecurity.d' $modsec_dir = '/etc/httpd/modsecurity.d' diff --git a/spec/classes/mod/security_spec.rb b/spec/classes/mod/security_spec.rb index 2b6d16ecb7..1dd2e9571b 100644 --- a/spec/classes/mod/security_spec.rb +++ b/spec/classes/mod/security_spec.rb @@ -27,6 +27,7 @@ it { should contain_file('security.conf').with( :path => '/etc/httpd/conf.modules.d/security.conf' ) } + it { should contain_file('security.conf').with_content("^\s*SecAuditLogParts ABJDEFHZ$") } it { should contain_file('/etc/httpd/modsecurity.d').with( :ensure => 'directory', :path => '/etc/httpd/modsecurity.d', @@ -43,6 +44,13 @@ :path => '/etc/httpd/modsecurity.d/security_crs.conf' ) } it { should contain_apache__security__rule_link('base_rules/modsecurity_35_bad_robots.data') } + + describe 'with parameters' do + let :params do + { :modsec_audit_log_parts => "ABCDZ" + } + end + it { should contain_file('security.conf').with_content("^\s*SecAuditLogParts ABCDZ$") } end context "on Debian based systems" do diff --git a/templates/mod/security.conf.erb b/templates/mod/security.conf.erb index a71f5887d1..1ffd30bb9c 100644 --- a/templates/mod/security.conf.erb +++ b/templates/mod/security.conf.erb @@ -50,7 +50,7 @@ SecDebugLogLevel 0 SecAuditEngine RelevantOnly SecAuditLogRelevantStatus "^(?:5|4(?!04))" - SecAuditLogParts ABIJDEFHZ + SecAuditLogParts <%= @audit_log_parts %> SecAuditLogType Serial SecArgumentSeparator & SecCookieFormat 0 From d2699d18e5856ff8841373b3ebc9adc8c321564f Mon Sep 17 00:00:00 2001 From: Stig Brautaset Date: Tue, 8 Mar 2016 17:27:38 +0000 Subject: [PATCH 2/6] Add SecAuditLogParts tests for Debian-based systems --- spec/classes/mod/security_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/classes/mod/security_spec.rb b/spec/classes/mod/security_spec.rb index 1dd2e9571b..0590b21c1c 100644 --- a/spec/classes/mod/security_spec.rb +++ b/spec/classes/mod/security_spec.rb @@ -51,6 +51,7 @@ } end it { should contain_file('security.conf').with_content("^\s*SecAuditLogParts ABCDZ$") } + end end context "on Debian based systems" do @@ -79,6 +80,7 @@ it { should contain_file('security.conf').with( :path => '/etc/apache2/mods-available/security.conf' ) } + it { should contain_file('security.conf').with_content("^\s*SecAuditLogParts ABJDEFHZ$") } it { should contain_file('/etc/modsecurity').with( :ensure => 'directory', :path => '/etc/modsecurity', @@ -95,6 +97,14 @@ :path => '/etc/modsecurity/security_crs.conf' ) } it { should contain_apache__security__rule_link('base_rules/modsecurity_35_bad_robots.data') } + + describe 'with parameters' do + let :params do + { :modsec_audit_log_parts => "ACEZ" + } + end + it { should contain_file('security.conf').with_content("^\s*SecAuditLogParts ACEZ$") } + end end end From 78ee594d71921e86a80ccdfe84952b2cea0097f5 Mon Sep 17 00:00:00 2001 From: Stig Brautaset Date: Wed, 9 Mar 2016 10:36:19 +0000 Subject: [PATCH 3/6] Fix parameter name --- spec/classes/mod/security_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/classes/mod/security_spec.rb b/spec/classes/mod/security_spec.rb index 0590b21c1c..2730861f06 100644 --- a/spec/classes/mod/security_spec.rb +++ b/spec/classes/mod/security_spec.rb @@ -47,7 +47,7 @@ describe 'with parameters' do let :params do - { :modsec_audit_log_parts => "ABCDZ" + { :audit_log_parts => "ABCDZ" } end it { should contain_file('security.conf').with_content("^\s*SecAuditLogParts ABCDZ$") } @@ -100,7 +100,7 @@ describe 'with parameters' do let :params do - { :modsec_audit_log_parts => "ACEZ" + { :audit_log_parts => "ACEZ" } end it { should contain_file('security.conf').with_content("^\s*SecAuditLogParts ACEZ$") } From 388ab4b53de683039c120d138132eebf7c0fbee5 Mon Sep 17 00:00:00 2001 From: Stig Brautaset Date: Wed, 9 Mar 2016 15:04:22 +0000 Subject: [PATCH 4/6] Use regular expression rather than exact string match --- spec/classes/mod/security_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/classes/mod/security_spec.rb b/spec/classes/mod/security_spec.rb index 2730861f06..7a0ba8f299 100644 --- a/spec/classes/mod/security_spec.rb +++ b/spec/classes/mod/security_spec.rb @@ -27,7 +27,7 @@ it { should contain_file('security.conf').with( :path => '/etc/httpd/conf.modules.d/security.conf' ) } - it { should contain_file('security.conf').with_content("^\s*SecAuditLogParts ABJDEFHZ$") } + it { should contain_file('security.conf').with_content %r{^\s+SecAuditLogParts ABIJDEFHZ$} } it { should contain_file('/etc/httpd/modsecurity.d').with( :ensure => 'directory', :path => '/etc/httpd/modsecurity.d', @@ -50,7 +50,7 @@ { :audit_log_parts => "ABCDZ" } end - it { should contain_file('security.conf').with_content("^\s*SecAuditLogParts ABCDZ$") } + it { should contain_file('security.conf').with_content %r{^\s+SecAuditLogParts ABCDZ$} } end end @@ -80,7 +80,7 @@ it { should contain_file('security.conf').with( :path => '/etc/apache2/mods-available/security.conf' ) } - it { should contain_file('security.conf').with_content("^\s*SecAuditLogParts ABJDEFHZ$") } + it { should contain_file('security.conf').with_content %r{^\s+SecAuditLogParts ABIJDEFHZ$} } it { should contain_file('/etc/modsecurity').with( :ensure => 'directory', :path => '/etc/modsecurity', @@ -103,7 +103,7 @@ { :audit_log_parts => "ACEZ" } end - it { should contain_file('security.conf').with_content("^\s*SecAuditLogParts ACEZ$") } + it { should contain_file('security.conf').with_content %r{^\s+SecAuditLogParts ACEZ$} } end end From 3d5aa16b3fe927920599566ffda12da483d80124 Mon Sep 17 00:00:00 2001 From: Stig Brautaset Date: Wed, 9 Mar 2016 15:04:56 +0000 Subject: [PATCH 5/6] Move default value outside the redhat-specific section So it is valid for Debian-based systems also. --- manifests/params.pp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/manifests/params.pp b/manifests/params.pp index 6a40f4d9bf..19b3d2a460 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -47,6 +47,8 @@ $vhost_include_pattern = '*' + $modsec_audit_log_parts = 'ABIJDEFHZ' + if $::operatingsystem == 'Ubuntu' and $::lsbdistrelease == '10.04' { $verify_command = '/usr/sbin/apache2ctl -t' } else { @@ -159,7 +161,6 @@ $mellon_lock_file = '/run/mod_auth_mellon/lock' $mellon_cache_size = 100 $mellon_post_directory = undef - $modsec_audit_log_parts = 'ABIJDEFHZ' $modsec_crs_package = 'mod_security_crs' $modsec_crs_path = '/usr/lib/modsecurity.d' $modsec_dir = '/etc/httpd/modsecurity.d' From 9ac6064f58a304392184b10729993a5024641548 Mon Sep 17 00:00:00 2001 From: Stig Brautaset Date: Thu, 10 Mar 2016 14:33:12 +0000 Subject: [PATCH 6/6] Document mod_security's audit_log_parts parameter in README --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 5f1e77f89b..76c174cc56 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,8 @@ [Apache modules]: https://httpd.apache.org/docs/current/mod/ [array]: https://docs.puppetlabs.com/puppet/latest/reference/lang_data_array.html +[audit log]: https://github.com/SpiderLabs/ModSecurity/wiki/ModSecurity-2-Data-Formats#audit-log + [beaker-rspec]: https://github.com/puppetlabs/beaker-rspec [certificate revocation list]: https://httpd.apache.org/docs/current/mod/mod_ssl.html#sslcarevocationfile @@ -1706,6 +1708,7 @@ ${modsec\_dir}/activated\_rules. - `restricted_headers`: A list of restricted headers separated by slashes and spaces. Default: 'Proxy-Connection/ /Lock-Token/ /Content-Range/ /Translate/ /via/ /if/'. - `secpcrematchlimit`: Sets the number for the match limit in the PCRE library. Default: '1500' - `secpcrematchlimitrecursion`: Sets the number for the match limit recursion in the PCRE library. Default: '1500' +- `audit_log_parts`: Sets the sections to be put in the [audit log][]. Default: 'ABIJDEFHZ' ##### Class: `apache::mod::wsgi`