Skip to content

Commit 9121cc8

Browse files
Сарайкин Антон ЮрьевичСарайкин Антон Юрьевич
Сарайкин Антон Юрьевич
authored and
Сарайкин Антон Юрьевич
committed
Fixed log actions in rules without disruptive actions
1 parent 79a2a3c commit 9121cc8

5 files changed

+281
-6
lines changed

‎src/ngx_http_modsecurity_common.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ typedef struct {
5555
ngx_chain_t* temp_chain;
5656
ngx_chain_t* current_chain;
5757
unsigned response_body_filtered:1;
58+
unsigned logged:1;
5859
} ngx_http_modsecurity_ctx_t;
5960

6061

‎src/ngx_http_modsecurity_log.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ ngx_http_modsecurity_log_handler(ngx_http_request_t *r)
6767
return NGX_ERROR;
6868
}
6969

70+
if (ctx->logged) {
71+
dd("already logged earlier");
72+
return NGX_OK;
73+
}
74+
7075
dd("calling msc_process_logging for %p", ctx);
7176
old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
7277
msc_process_logging(ctx->modsec_transaction);

‎src/ngx_http_modsecurity_module.c

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,16 @@ ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_re
137137
intervention.url = NULL;
138138
intervention.log = NULL;
139139
intervention.disruptive = 0;
140+
ngx_http_modsecurity_ctx_t *ctx = NULL;
140141

141142
dd("processing intervention");
142143

144+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
145+
if (ctx == NULL)
146+
{
147+
return NGX_HTTP_INTERNAL_SERVER_ERROR;
148+
}
149+
143150
if (msc_intervention(transaction, &intervention) == 0) {
144151
dd("nothing to do");
145152
return 0;
@@ -195,6 +202,8 @@ ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_re
195202
if (intervention.status != 200)
196203
{
197204
ngx_http_modsecurity_log_handler(r);
205+
ctx->logged = 1;
206+
198207
if (r->header_sent)
199208
{
200209
dd("Headers are already sent. Cannot perform the redirection at this point.");
@@ -421,6 +430,7 @@ ngx_http_modsecurity_init(ngx_conf_t *cf)
421430
{
422431
ngx_http_handler_pt *h_rewrite;
423432
ngx_http_handler_pt *h_preaccess;
433+
ngx_http_handler_pt *h_log;
424434
ngx_http_core_main_conf_t *cmcf;
425435
int rc = 0;
426436

@@ -462,6 +472,22 @@ ngx_http_modsecurity_init(ngx_conf_t *cf)
462472
}
463473
*h_preaccess = ngx_http_modsecurity_pre_access_handler;
464474

475+
/**
476+
* Process the log phase.
477+
*
478+
* TODO: check if the log phase happens like it happens on Apache.
479+
* check if last phase will not hold the request.
480+
*
481+
*/
482+
h_log = ngx_array_push(&cmcf->phases[NGX_HTTP_LOG_PHASE].handlers);
483+
if (h_log == NULL)
484+
{
485+
dd("Not able to create a new NGX_HTTP_LOG_PHASE handle");
486+
return NGX_ERROR;
487+
}
488+
*h_log = ngx_http_modsecurity_log_handler;
489+
490+
465491
rc = ngx_http_modsecurity_header_filter_init();
466492
if (rc != NGX_OK) {
467493
return rc;

‎tests/modsecurity-config-auditlog.t

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ http {
6565
SecRule ARGS "@streq root" "id:21,phase:1,auditlog,status:302,redirect:http://www.modsecurity.org"
6666
SecDebugLog %%TESTDIR%%/auditlog-debug-root.txt
6767
SecDebugLogLevel 9
68-
SecAuditEngine On
68+
SecAuditEngine RelevantOnly
6969
SecAuditLogParts AB
7070
SecAuditLog %%TESTDIR%%/auditlog-root.txt
7171
SecAuditLogType Serial
@@ -76,9 +76,10 @@ http {
7676
location /subfolder1/subfolder2 {
7777
modsecurity_rules '
7878
SecRule ARGS "@streq subfolder2" "id:41,phase:1,status:302,auditlog,redirect:http://www.modsecurity.org"
79+
SecRule ARGS "@streq subfolder1" "id:42,phase:1,status:302,auditlog,redirect:http://www.modsecurity.org"
7980
SecDebugLog %%TESTDIR%%/auditlog-debug-subfolder2.txt
8081
SecDebugLogLevel 9
81-
SecAuditEngine On
82+
SecAuditEngine RelevantOnly
8283
SecAuditLogParts AB
8384
SecResponseBodyAccess On
8485
SecAuditLog %%TESTDIR%%/auditlog-subfolder2.txt
@@ -93,7 +94,7 @@ http {
9394
SecDebugLog %%TESTDIR%%/auditlog-debug-subfolder1.txt
9495
SecDebugLogLevel 9
9596
SecAuditLogParts AB
96-
SecAuditEngine On
97+
SecAuditEngine RelevantOnly
9798
SecAuditLog %%TESTDIR%%/auditlog-subfolder1.txt
9899
SecAuditLogType Serial
99100
SecAuditLogStorageDir %%TESTDIR%%/
@@ -104,10 +105,11 @@ http {
104105
modsecurity_rules '
105106
SecResponseBodyAccess On
106107
SecRule ARGS "@streq subfolder4" "id:61,phase:1,status:302,auditlog,redirect:http://www.modsecurity.org"
107-
SecRule ARGS "@streq subfolder4withE" "id:2,phase:1,ctl:auditLogParts=+E,auditlog"
108+
SecRule ARGS "@streq subfolder3" "id:62,phase:1,status:302,auditlog,redirect:http://www.modsecurity.org"
109+
SecRule ARGS "@streq subfolder4withE" "id:63,phase:1,ctl:auditLogParts=+E,auditlog"
108110
SecDebugLog %%TESTDIR%%/auditlog-debug-subfolder4.txt
109111
SecDebugLogLevel 9
110-
SecAuditEngine On
112+
SecAuditEngine RelevantOnly
111113
SecAuditLogParts AB
112114
SecAuditLog %%TESTDIR%%/auditlog-subfolder4.txt
113115
SecAuditLogType Serial
@@ -121,7 +123,7 @@ http {
121123
SecDebugLog %%TESTDIR%%/auditlog-debug-subfolder3.txt
122124
SecDebugLogLevel 9
123125
SecAuditLogParts AB
124-
SecAuditEngine On
126+
SecAuditEngine RelevantOnly
125127
SecAuditLog %%TESTDIR%%/auditlog-subfolder3.txt
126128
SecAuditLogType Serial
127129
SecAuditLogStorageDir %%TESTDIR%%/

‎tests/modsecurity-config-custom-error-page.t

Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
1+
#!/usr/bin/perl
2+
3+
#
4+
# ModSecurity, http://www.modsecurity.org/
5+
# Copyright (c) 2015 Trustwave Holdings, Inc. (http://www.trustwave.com/)
6+
#
7+
# You may not use this file except in compliance with
8+
# the License. You may obtain a copy of the License at
9+
#
10+
# http://www.apache.org/licenses/LICENSE-2.0
11+
#
12+
# If any of the files related to licensing are missing or if you have any
13+
# other questions related to licensing please contact Trustwave Holdings, Inc.
14+
# directly using the email address [email protected].
15+
#
16+
17+
18+
# Tests for ModSecurity module.
19+
20+
###############################################################################
21+
22+
use warnings;
23+
use strict;
24+
25+
use Test::More;
26+
27+
BEGIN { use FindBin; chdir($FindBin::Bin); }
28+
29+
use lib 'lib';
30+
use Test::Nginx;
31+
32+
###############################################################################
33+
34+
select STDERR; $| = 1;
35+
select STDOUT; $| = 1;
36+
37+
my $t = Test::Nginx->new()->has(qw/http/);
38+
39+
$t->write_file_expand('nginx.conf', <<'EOF');
40+
41+
%%TEST_GLOBALS%%
42+
43+
daemon off;
44+
45+
events {
46+
}
47+
48+
http {
49+
%%TEST_GLOBALS_HTTP%%
50+
51+
server {
52+
listen 127.0.0.1:8080;
53+
server_name localhost;
54+
55+
error_page 403 /403.html;
56+
57+
location /403.html {
58+
root %%TESTDIR%%/http;
59+
internal;
60+
}
61+
62+
location / {
63+
modsecurity on;
64+
modsecurity_rules '
65+
SecRuleEngine On
66+
SecRule ARGS "@streq root" "id:10,phase:1,auditlog,status:403,deny"
67+
SecDebugLog %%TESTDIR%%/auditlog-debug-local.txt
68+
SecDebugLogLevel 9
69+
SecAuditEngine RelevantOnly
70+
SecAuditLogParts AB
71+
SecAuditLog %%TESTDIR%%/auditlog-local.txt
72+
SecAuditLogType Serial
73+
SecAuditLogStorageDir %%TESTDIR%%/
74+
';
75+
}
76+
}
77+
78+
server {
79+
listen 127.0.0.1:8081;
80+
server_name localhost;
81+
82+
modsecurity on;
83+
modsecurity_rules '
84+
SecRuleEngine On
85+
SecRule ARGS "@streq root" "id:10,phase:1,auditlog,status:403,deny"
86+
SecDebugLog %%TESTDIR%%/auditlog-debug-global.txt
87+
SecDebugLogLevel 9
88+
SecAuditEngine RelevantOnly
89+
SecAuditLogParts AB
90+
SecAuditLog %%TESTDIR%%/auditlog-global.txt
91+
SecAuditLogType Serial
92+
SecAuditLogStorageDir %%TESTDIR%%/
93+
';
94+
95+
error_page 403 /403.html;
96+
97+
location /403.html {
98+
modsecurity off;
99+
root %%TESTDIR%%/http;
100+
internal;
101+
}
102+
103+
location / {
104+
}
105+
}
106+
}
107+
EOF
108+
109+
my $index_txt = "This is the index page.";
110+
my $custom_txt = "This is a custom error page.";
111+
112+
$t->write_file("/index.html", $index_txt);
113+
mkdir($t->testdir() . '/http');
114+
$t->write_file("/http/403.html", $custom_txt);
115+
116+
$t->run();
117+
$t->plan(8);
118+
119+
###############################################################################
120+
121+
my $d = $t->testdir();
122+
123+
my $t1;
124+
my $t2;
125+
my $t3;
126+
my $t4;
127+
128+
# Performing requests to a server with ModSecurity enabled at location context
129+
$t1 = http_get('/index.html?what=root');
130+
$t2 = http_get('/index.html?what=other');
131+
132+
# Performing requests to a server with ModSecurity enabled at server context
133+
$t3 = http_get2('/index.html?what=root');
134+
$t4 = http_get2('/index.html?what=other');
135+
136+
my $local = do {
137+
local $/ = undef;
138+
open my $fh, "<", "$d/auditlog-local.txt"
139+
or die "could not open: $!";
140+
<$fh>;
141+
};
142+
143+
my $global = do {
144+
local $/ = undef;
145+
open my $fh, "<", "$d/auditlog-global.txt"
146+
or die "could not open: $!";
147+
<$fh>;
148+
};
149+
150+
like($t1, qr/$custom_txt/, 'ModSecurity at location / root');
151+
like($t2, qr/$index_txt/, 'ModSecurity at location / other');
152+
like($local, qr/what=root/, 'ModSecurity at location / root present in auditlog');
153+
unlike($local, qr/what=other/, 'ModSecurity at location / other not present in auditlog');
154+
155+
like($t3, qr/$custom_txt/, 'ModSecurity at server / root');
156+
like($t4, qr/$index_txt/, 'ModSecurity at server / other');
157+
like($global, qr/what=root/, 'ModSecurity at server / root present in auditlog');
158+
unlike($global, qr/what=other/, 'ModSecurity at server / other not present in auditlog');
159+
160+
###############################################################################
161+
162+
sub http_get2($;%) {
163+
my ($url, %extra) = @_;
164+
return http2(<<EOF, %extra);
165+
GET $url HTTP/1.0
166+
Host: localhost
167+
168+
EOF
169+
}
170+
171+
sub http2($;%) {
172+
my ($request, %extra) = @_;
173+
174+
my $s = http_start2($request, %extra);
175+
176+
return $s if $extra{start} or !defined $s;
177+
return http_end2($s);
178+
}
179+
180+
sub http_start2($;%) {
181+
my ($request, %extra) = @_;
182+
my $s;
183+
184+
eval {
185+
local $SIG{ALRM} = sub { die "timeout\n" };
186+
local $SIG{PIPE} = sub { die "sigpipe\n" };
187+
alarm(8);
188+
189+
$s = $extra{socket} || IO::Socket::INET->new(
190+
Proto => 'tcp',
191+
PeerAddr => '127.0.0.1:' . port(8081)
192+
)
193+
or die "Can't connect to nginx: $!\n";
194+
195+
log_out($request);
196+
$s->print($request);
197+
198+
select undef, undef, undef, $extra{sleep} if $extra{sleep};
199+
return '' if $extra{aborted};
200+
201+
if ($extra{body}) {
202+
log_out($extra{body});
203+
$s->print($extra{body});
204+
}
205+
206+
alarm(0);
207+
};
208+
alarm(0);
209+
if ($@) {
210+
log_in("died: $@");
211+
return undef;
212+
}
213+
214+
return $s;
215+
}
216+
217+
sub http_end2($;%) {
218+
my ($s) = @_;
219+
my $reply;
220+
221+
eval {
222+
local $SIG{ALRM} = sub { die "timeout\n" };
223+
local $SIG{PIPE} = sub { die "sigpipe\n" };
224+
alarm(8);
225+
226+
local $/;
227+
$reply = $s->getline();
228+
229+
alarm(0);
230+
};
231+
alarm(0);
232+
if ($@) {
233+
log_in("died: $@");
234+
return undef;
235+
}
236+
237+
log_in($reply);
238+
return $reply;
239+
}
240+
241+
###############################################################################

0 commit comments

Comments
 (0)