From 44d0bb474e37c287fb8152908a62baff6e44a434 Mon Sep 17 00:00:00 2001 From: Jareth Gomes Date: Thu, 30 Jan 2025 06:59:08 +0000 Subject: [PATCH 1/4] Changes default config LAPI URL to prevent 404ing This is more or less a bandage fix and doesn't fix the underlying problem within the module. --- config/crowdsec-apache2-bouncer.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/crowdsec-apache2-bouncer.conf b/config/crowdsec-apache2-bouncer.conf index e1eed4b..88c0b32 100644 --- a/config/crowdsec-apache2-bouncer.conf +++ b/config/crowdsec-apache2-bouncer.conf @@ -1,5 +1,5 @@ ## Basic configuration -CrowdsecURL http://127.0.0.1:8080/ +CrowdsecURL http://127.0.0.1:8080 # Make sure there is no trailing forward slash CrowdsecAPIKey $API_KEY # Behavior if we can't reach (or timeout) LAPI From d697c35250fb51bf759c89fa655b41ce5a92ce4d Mon Sep 17 00:00:00 2001 From: Jareth Gomes Date: Thu, 30 Jan 2025 07:29:32 +0000 Subject: [PATCH 2/4] Fixes double slash URLs to properly report back a 404 error --- mod_crowdsec.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/mod_crowdsec.c b/mod_crowdsec.c index 65681ec..37970b8 100644 --- a/mod_crowdsec.c +++ b/mod_crowdsec.c @@ -422,7 +422,12 @@ static int crowdsec_proxy(request_rec * r, const char **response) status = ap_run_sub_req(rr); - if (HTTP_NOT_FOUND == status) { + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r, + "crowdsec: function call status is '%d' (response status is " + "'%d') from url: %s", + status, rr->status, rr->filename); + + if (HTTP_NOT_FOUND == status || (!status && HTTP_NOT_FOUND == rr->status)) { ap_log_rerror(APLOG_MARK, APLOG_ERR, APR_SUCCESS, r, "crowdsec: we received a 404 Not Found when speaking " "to the crowdsec service '%s', you might be pointing at " From 61729dd5bce7fff80d9449c0184c2db9e9a71a10 Mon Sep 17 00:00:00 2001 From: Jareth Gomes Date: Wed, 5 Feb 2025 21:20:00 -0600 Subject: [PATCH 3/4] Changes conditions to use apache OK status constant --- mod_crowdsec.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mod_crowdsec.c b/mod_crowdsec.c index 37970b8..a635891 100644 --- a/mod_crowdsec.c +++ b/mod_crowdsec.c @@ -427,7 +427,8 @@ static int crowdsec_proxy(request_rec * r, const char **response) "'%d') from url: %s", status, rr->status, rr->filename); - if (HTTP_NOT_FOUND == status || (!status && HTTP_NOT_FOUND == rr->status)) { + if (HTTP_NOT_FOUND == status || + (status == OK && HTTP_NOT_FOUND == rr->status)) { ap_log_rerror(APLOG_MARK, APLOG_ERR, APR_SUCCESS, r, "crowdsec: we received a 404 Not Found when speaking " "to the crowdsec service '%s', you might be pointing at " @@ -437,7 +438,7 @@ static int crowdsec_proxy(request_rec * r, const char **response) return HTTP_INTERNAL_SERVER_ERROR; } - else if ((status)) { + else if (status != OK) { crowdsec_config_rec *conf = (crowdsec_config_rec *) ap_get_module_config(r->per_dir_config, From 247d0bc1631c21ecd9eaa45c1fdea37cdd13301a Mon Sep 17 00:00:00 2001 From: Jareth Gomes Date: Wed, 5 Feb 2025 22:27:51 -0600 Subject: [PATCH 4/4] Adds validation when Crowdsec LAPI URL is set This simply parses the URL string to ensure the prefix contains a scheme and an authority, following the RFC 3986 standard. If the scheme and authority cannot be parsed due to the passed string not following the RFC standard, Apache will fail to start and print an error to logs. The path, query, and fragment are subsequently ignored and a warning gets printed if anything besides the string "/" is found after the authority. --- config/crowdsec-apache2-bouncer.conf | 6 +- mod_crowdsec.c | 95 ++++++++++++++++++++++++++-- 2 files changed, 93 insertions(+), 8 deletions(-) diff --git a/config/crowdsec-apache2-bouncer.conf b/config/crowdsec-apache2-bouncer.conf index 88c0b32..d47c51f 100644 --- a/config/crowdsec-apache2-bouncer.conf +++ b/config/crowdsec-apache2-bouncer.conf @@ -1,5 +1,9 @@ ## Basic configuration -CrowdsecURL http://127.0.0.1:8080 # Make sure there is no trailing forward slash + +# Must be of the form http:// +# Anything after the authority (path, query, or fragment) in URL will be +# ignored and a warning will be printed to logs +CrowdsecURL http://127.0.0.1:8080 CrowdsecAPIKey $API_KEY # Behavior if we can't reach (or timeout) LAPI diff --git a/mod_crowdsec.c b/mod_crowdsec.c index a635891..a71b3b7 100644 --- a/mod_crowdsec.c +++ b/mod_crowdsec.c @@ -101,10 +101,16 @@ module AP_MODULE_DECLARE_DATA crowdsec_module; +struct url { + const char *scheme; + const char *authority; + const char *path; +}; + typedef struct { /* the url of the crowdsec service */ - const char *url; + struct url *url; /* the API key of the crowdsec service */ const char *key; /* shared obect cache mutex */ @@ -368,11 +374,11 @@ static int crowdsec_proxy(request_rec * r, const char **response) * filter that reads and parses the response from the API. */ - const char *target = apr_pstrcat(r->pool, sconf->url, - "/v1/decisions?ip=", - ap_escape_urlencoded(r->pool, - r->useragent_ip), - NULL); + char *api_path = "/v1/decisions?ip="; + + const char *target = apr_pstrcat( + r->pool, sconf->url->scheme, "://", sconf->url->authority, api_path, + ap_escape_urlencoded(r->pool, r->useragent_ip), NULL); ap_log_rerror(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r, "crowdsec: looking up IP '%s' at url: %s", @@ -865,6 +871,64 @@ static const char *set_crowdsec_location(cmd_parms *cmd, void *dconf, return NULL; } +// Note: url *must* be null-terminated +struct url *find_base_lapi_url(apr_pool_t *pool, const char *url, + const char **err) +{ + char *scheme = NULL; + char *authority = NULL; + apr_size_t authority_start_idx = 0; + char *path = NULL; + + for (apr_size_t i = 0; url[i] != '\0'; i++) { + if (!scheme) { + if (url[i] == ':') { + scheme = apr_pstrndup(pool, url, i); + if (apr_cstr_casecmpn(url + i + 1, "//", 2)) { + *err = + "invalid lapi base url: \"//\" after scheme not found"; + return NULL; + } + authority_start_idx = i + 3; + i += 2; + } + continue; + } + if (!authority) { + if (url[i] == '/') { + authority = apr_pstrndup(pool, url + authority_start_idx, + i - authority_start_idx); + i--; + } + continue; + } + if (!path) { + path = apr_pstrdup(pool, url + i); + break; + } + } + + if (!scheme) { + *err = "invalid lapi base url: scheme is missing"; + return NULL; + } + + if (!authority) { + if (url[authority_start_idx] == '\0') { + *err = "invalid lapi base url: authority is missing"; + return NULL; + } + authority = apr_pstrdup(pool, url + authority_start_idx); + } + + struct url *res = apr_palloc(pool, sizeof(struct url)); + res->scheme = scheme; + res->authority = authority; + res->path = path; + + return res; +} + static const char *set_crowdsec_url(cmd_parms * cmd, void *dconf, const char *url) { @@ -872,7 +936,24 @@ static const char *set_crowdsec_url(cmd_parms * cmd, void *dconf, ap_get_module_config(cmd->server->module_config, &crowdsec_module); - sconf->url = url; + const char *err = NULL; + struct url *u = find_base_lapi_url(cmd->temp_pool, url, &err); + if (err) { + return err; + } + + ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, sconf, + "scheme: \"%s\", authority: \"%s\", path: \"%s\"", u->scheme, + u->authority, u->path); + if (u->path) { + if (apr_strnatcmp(u->path, "/")) { + ap_log_error(APLOG_MARK, APLOG_WARNING, APR_SUCCESS, sconf, + "lapi url: path (\"%s\") was found and will be ignored", + u->path); + } + } + + sconf->url = u; sconf->url_set = 1; return NULL;