From b5398abaf27c2e2c29518a7faf7b94c72cec0e62 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Thu, 13 Nov 2014 12:52:00 -0800 Subject: [PATCH] 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. --- apache2/apache2_config.c | 6 ++---- apache2/msc_remote_rules.c | 7 +++++++ apache2/msc_util.c | 7 +++++++ apache2/re_operators.c | 26 +++++++++++++++++++++----- tests/regression/misc/30-pmfromfile.t | 2 +- 5 files changed, 38 insertions(+), 10 deletions(-) diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index af35b3a1..0fa61c59 100644 --- a/apache2/apache2_config.c +++ b/apache2/apache2_config.c @@ -2266,11 +2266,9 @@ static const char *cmd_remote_rules(cmd_parms *cmd, void *_dcfg, const char *p1, "Key and URI"); } - // FIXME: make it https only. - // if (strncasecmp(p1, "https", 5) != 0) { - if (strncasecmp(uri, "http", 4) != 0) { + if (strncasecmp(uri, "https", 5) != 0) { return apr_psprintf(cmd->pool, "ModSecurity: Invalid URI:" \ - " %s, expected an HTTPS address.", uri); + " '%s'. Expected HTTPS.", uri); } // FIXME: Should we handle more then one server at once? diff --git a/apache2/msc_remote_rules.c b/apache2/msc_remote_rules.c index 25922977..224219cf 100644 --- a/apache2/msc_remote_rules.c +++ b/apache2/msc_remote_rules.c @@ -283,6 +283,13 @@ int msc_remote_grab_content(apr_pool_t *mp, const char *uri, const char *key, headers_chunk = curl_slist_append(headers_chunk, header_key); } + /* Make it TLS 1.x only. */ + curl_easy_setopt(curl, CURLOPT_SSLVERSION, CURL_SSLVERSION_TLSv1); + + /* those are the default options, but lets make sure */ + curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1); + curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 1); + /* send all data to this function */ curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, msc_curl_write_memory_cb); diff --git a/apache2/msc_util.c b/apache2/msc_util.c index 2bdf9424..20dd9334 100644 --- a/apache2/msc_util.c +++ b/apache2/msc_util.c @@ -2684,6 +2684,13 @@ int ip_tree_from_uri(TreeRoot **rtree, char *uri, /* we pass our 'chunk' struct to the callback function */ curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)&chunk); + /* Make it TLS 1.x only. */ + curl_easy_setopt(curl, CURLOPT_SSLVERSION, CURL_SSLVERSION_TLSv1); + + /* those are the default options, but lets make sure */ + curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1); + curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 1); + /* some servers don't like requests that are made without a user-agent field, so we provide one */ curl_easy_setopt(curl, CURLOPT_USERAGENT, "ModSecurity"); diff --git a/apache2/re_operators.c b/apache2/re_operators.c index c309e9b5..4d34eaa2 100644 --- a/apache2/re_operators.c +++ b/apache2/re_operators.c @@ -195,8 +195,13 @@ static int msre_op_ipmatchFromFile_param_init(msre_rule *rule, char **error_msg) } filepath = fn; - if ((strlen(fn) > strlen("http://") && strncmp(fn, "http://", strlen("http://")) == 0) || - (strlen(fn) > strlen("https://") && strncmp(fn, "https://", strlen("https://")) == 0)) + if (strlen(fn) > strlen("http://") && strncmp(fn, "http://", strlen("http://")) == 0) + { + *error_msg = apr_psprintf(rule->ruleset->mp, "HTTPS address or file " \ + "path are expected for operator ipmatchFromFile \"%s\"", fn); + return 0; + } + else if (strlen(fn) > strlen("https://") && strncmp(fn, "https://", strlen("https://")) == 0) { res = ip_tree_from_uri(&rtree, fn, rule->ruleset->mp, error_msg); if (res) @@ -1251,10 +1256,14 @@ static int msre_op_pmFromFile_param_init(msre_rule *rule, char **error_msg) { /* Add path of the rule filename for a relative phrase filename */ filepath = fn; - if ((strlen(fn) > strlen("http://") && strncmp(fn, "http://", strlen("http://")) == 0) || - (strlen(fn) > strlen("https://") && strncmp(fn, "https://", strlen("https://")) == 0)) + if (strlen(fn) > strlen("http://") && strncmp(fn, "http://", strlen("http://")) == 0) + { + *error_msg = apr_psprintf(rule->ruleset->mp, "HTTPS address or " \ + "file path are expected for operator pmFromFile \"%s\"", fn); + return 0; + } + else if (strlen(fn) > strlen("https://") && strncmp(fn, "https://", strlen("https://")) == 0) { - CURL *curl; CURLcode res; @@ -1309,6 +1318,13 @@ static int msre_op_pmFromFile_param_init(msre_rule *rule, char **error_msg) { /* we pass our 'chunk' struct to the callback function */ curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)&chunk); + /* Make it TLS 1.x only. */ + curl_easy_setopt(curl, CURLOPT_SSLVERSION, CURL_SSLVERSION_TLSv1); + + /* those are the default options, but lets make sure */ + curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1); + curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 1); + /* some servers don't like requests that are made without a user-agent field, so we provide one */ curl_easy_setopt(curl, CURLOPT_USERAGENT, "ModSecurity"); diff --git a/tests/regression/misc/30-pmfromfile.t b/tests/regression/misc/30-pmfromfile.t index a39ed107..db93581e 100644 --- a/tests/regression/misc/30-pmfromfile.t +++ b/tests/regression/misc/30-pmfromfile.t @@ -8,7 +8,7 @@ SecDebugLog $ENV{DEBUG_LOG} SecDebugLogLevel 9 SecRequestBodyAccess On - SecRule REQUEST_FILENAME "\@pmFromFile http://modsec.zimmerle.org/ip_reputation.txt?code=123" "id:'123',phase:2,log,pass,t:none" + SecRule REQUEST_FILENAME "\@pmFromFile https://www.modsecurity.org/modsecurity-regression-test.txt" "id:'123',phase:2,log,pass,t:none" ), match_log => { error => [ qr/ModSecurity: Warning. Matched phrase \"127.0.0.1\" at REQUEST_FILENAME./, 1],