Skip to content

Commit b5398ab

Browse files
author
Felipe Zimmerle
committed
Forces downloads using https-only for resources or rules
This commit makes ModSecurity to refuse to download or install rules (SecRemoteRules) from sites that are not running HTTPS with a valid and trusted certificate.
1 parent 59fc243 commit b5398ab

File tree

5 files changed

+38
-10
lines changed

5 files changed

+38
-10
lines changed

apache2/apache2_config.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2266,11 +2266,9 @@ static const char *cmd_remote_rules(cmd_parms *cmd, void *_dcfg, const char *p1,
22662266
"Key and URI");
22672267
}
22682268

2269-
// FIXME: make it https only.
2270-
// if (strncasecmp(p1, "https", 5) != 0) {
2271-
if (strncasecmp(uri, "http", 4) != 0) {
2269+
if (strncasecmp(uri, "https", 5) != 0) {
22722270
return apr_psprintf(cmd->pool, "ModSecurity: Invalid URI:" \
2273-
" %s, expected an HTTPS address.", uri);
2271+
" '%s'. Expected HTTPS.", uri);
22742272
}
22752273

22762274
// FIXME: Should we handle more then one server at once?

apache2/msc_remote_rules.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,13 @@ int msc_remote_grab_content(apr_pool_t *mp, const char *uri, const char *key,
283283
headers_chunk = curl_slist_append(headers_chunk, header_key);
284284
}
285285

286+
/* Make it TLS 1.x only. */
287+
curl_easy_setopt(curl, CURLOPT_SSLVERSION, CURL_SSLVERSION_TLSv1);
288+
289+
/* those are the default options, but lets make sure */
290+
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1);
291+
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 1);
292+
286293
/* send all data to this function */
287294
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, msc_curl_write_memory_cb);
288295

apache2/msc_util.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2684,6 +2684,13 @@ int ip_tree_from_uri(TreeRoot **rtree, char *uri,
26842684
/* we pass our 'chunk' struct to the callback function */
26852685
curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)&chunk);
26862686

2687+
/* Make it TLS 1.x only. */
2688+
curl_easy_setopt(curl, CURLOPT_SSLVERSION, CURL_SSLVERSION_TLSv1);
2689+
2690+
/* those are the default options, but lets make sure */
2691+
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1);
2692+
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 1);
2693+
26872694
/* some servers don't like requests that are made without a user-agent
26882695
field, so we provide one */
26892696
curl_easy_setopt(curl, CURLOPT_USERAGENT, "ModSecurity");

apache2/re_operators.c

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,13 @@ static int msre_op_ipmatchFromFile_param_init(msre_rule *rule, char **error_msg)
195195
}
196196
filepath = fn;
197197

198-
if ((strlen(fn) > strlen("http://") && strncmp(fn, "http://", strlen("http://")) == 0) ||
199-
(strlen(fn) > strlen("https://") && strncmp(fn, "https://", strlen("https://")) == 0))
198+
if (strlen(fn) > strlen("http://") && strncmp(fn, "http://", strlen("http://")) == 0)
199+
{
200+
*error_msg = apr_psprintf(rule->ruleset->mp, "HTTPS address or file " \
201+
"path are expected for operator ipmatchFromFile \"%s\"", fn);
202+
return 0;
203+
}
204+
else if (strlen(fn) > strlen("https://") && strncmp(fn, "https://", strlen("https://")) == 0)
200205
{
201206
res = ip_tree_from_uri(&rtree, fn, rule->ruleset->mp, error_msg);
202207
if (res)
@@ -1251,10 +1256,14 @@ static int msre_op_pmFromFile_param_init(msre_rule *rule, char **error_msg) {
12511256
/* Add path of the rule filename for a relative phrase filename */
12521257
filepath = fn;
12531258

1254-
if ((strlen(fn) > strlen("http://") && strncmp(fn, "http://", strlen("http://")) == 0) ||
1255-
(strlen(fn) > strlen("https://") && strncmp(fn, "https://", strlen("https://")) == 0))
1259+
if (strlen(fn) > strlen("http://") && strncmp(fn, "http://", strlen("http://")) == 0)
1260+
{
1261+
*error_msg = apr_psprintf(rule->ruleset->mp, "HTTPS address or " \
1262+
"file path are expected for operator pmFromFile \"%s\"", fn);
1263+
return 0;
1264+
}
1265+
else if (strlen(fn) > strlen("https://") && strncmp(fn, "https://", strlen("https://")) == 0)
12561266
{
1257-
12581267
CURL *curl;
12591268
CURLcode res;
12601269

@@ -1309,6 +1318,13 @@ static int msre_op_pmFromFile_param_init(msre_rule *rule, char **error_msg) {
13091318
/* we pass our 'chunk' struct to the callback function */
13101319
curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)&chunk);
13111320

1321+
/* Make it TLS 1.x only. */
1322+
curl_easy_setopt(curl, CURLOPT_SSLVERSION, CURL_SSLVERSION_TLSv1);
1323+
1324+
/* those are the default options, but lets make sure */
1325+
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1);
1326+
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 1);
1327+
13121328
/* some servers don't like requests that are made without a user-agent
13131329
field, so we provide one */
13141330
curl_easy_setopt(curl, CURLOPT_USERAGENT, "ModSecurity");

tests/regression/misc/30-pmfromfile.t

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
SecDebugLog $ENV{DEBUG_LOG}
99
SecDebugLogLevel 9
1010
SecRequestBodyAccess On
11-
SecRule REQUEST_FILENAME "\@pmFromFile http://modsec.zimmerle.org/ip_reputation.txt?code=123" "id:'123',phase:2,log,pass,t:none"
11+
SecRule REQUEST_FILENAME "\@pmFromFile https://www.modsecurity.org/modsecurity-regression-test.txt" "id:'123',phase:2,log,pass,t:none"
1212
),
1313
match_log => {
1414
error => [ qr/ModSecurity: Warning. Matched phrase \"127.0.0.1\" at REQUEST_FILENAME./, 1],

0 commit comments

Comments
 (0)