owasp-modsecurity
2.x standalone fails to link since ap_map_http_request_error() is not exported as APR function
To integrate the ModSecurity 2.x with HAProxy, I compiled the standalone/ . It compiles successfully, but when linked with SPOA published at https://github.com/haproxy/spoa-modsecurity/ link phase fails with:
LANG=C make MODSEC_INC=/usr/local/src/modsecurity/ModSecurity/INSTALL/include MODSEC_LIB=/usr/local/src/modsecurity/ModSecurity/INSTALL/lib APACHE2_INC=/usr/include/httpd APR_INC=/usr/include/apr-1 | tee log.build
cc -o modsecurity spoa.o modsec_wrapper.o /usr/local/src/modsecurity/ModSecurity/INSTALL/lib/standalone.a -lpthread -levent -levent_pthreads -lcurl -lapr-1 -laprutil-1 -lxml2 -lpcre -lpcre2-8 -lyajl
/usr/local/src/modsecurity/ModSecurity/INSTALL/lib/standalone.a(standalone_la-apache2_io.o): In function `read_request_body':
/usr/local/src/modsecurity/ModSecurity/standalone/../apache2/apache2_io.c:237: undefined reference to
`ap_map_http_request_error'
collect2: error: ld returned 1 exit status
make: *** [Makefile:43: modsecurity] Error 1
The function ap_map_http_request_error() is internal to httpd and not published to APR libraries.
The patch below fixes this, but I feel somewhat uncomfortable.
diff --git a/apache2/apache2_io.c b/apache2/apache2_io.c
index 8deeb01c..383439a3 100644
--- a/apache2/apache2_io.c
+++ b/apache2/apache2_io.c
@@ -175,6 +175,42 @@ apr_status_t input_filter(ap_filter_t *f, apr_bucket_brigade *bb_out,
return APR_SUCCESS;
}
+/* Ack. ap_map_http_request_error() is not an apr function, so it is
+ * not public in a library. Include it here.
+ */
+/*
+ * Map specific APR codes returned by the filter stack to HTTP error
+ * codes, or the default status code provided. Use it as follows:
+ *
+ * return ap_map_http_request_error(rv, HTTP_BAD_REQUEST);
+ *
+ * If the filter has already handled the error, AP_FILTER_ERROR will
+ * be returned, which is cleanly passed through.
+ *
+ * These mappings imply that the filter stack is reading from the
+ * downstream client, the proxy will map these codes differently.
+ */
+AP_DECLARE(int) ap_map_http_request_error(apr_status_t rv, int status)
+{
+ switch (rv) {
+ case AP_FILTER_ERROR:
+ return AP_FILTER_ERROR;
+
+ case APR_ENOSPC:
+ return HTTP_REQUEST_ENTITY_TOO_LARGE;
+
+ case APR_ENOTIMPL:
+ return HTTP_NOT_IMPLEMENTED;
+
+ case APR_TIMEUP:
+ case APR_ETIMEDOUT:
+ return HTTP_REQUEST_TIME_OUT;
+
+ default:
+ return status;
+ }
+}
+
/**
* Reads request body from a client.
*/
Hi @kabe-gh,
thanks for reporting this issue.
Unfortunately I was able to reproduce your issue. Beside of that, I ran into a few other issues too.
This quick patch fixes the mentioned problem:
diff --git a/apache2/apache2_io.c b/apache2/apache2_io.c
index 8deeb01c..25e7c425 100644
--- a/apache2/apache2_io.c
+++ b/apache2/apache2_io.c
@@ -28,6 +28,31 @@
static void dummy_free_func(void *data) {}
#endif
+#ifdef VERSION_NGINX
+/* from apache2/modules/http/http_filters.c file */
+AP_DECLARE(int) msc_ap_map_http_request_error(apr_status_t rv, int status)
+{
+ switch (rv) {
+ case AP_FILTER_ERROR:
+ return AP_FILTER_ERROR;
+
+ case APR_ENOSPC:
+ return HTTP_REQUEST_ENTITY_TOO_LARGE;
+
+ case APR_ENOTIMPL:
+ return HTTP_NOT_IMPLEMENTED;
+
+ case APR_TIMEUP:
+ case APR_ETIMEDOUT:
+ return HTTP_REQUEST_TIME_OUT;
+
+ default:
+ return status;
+ }
+}
+
+#endif
+
/**
* This request filter will forward the previously stored
* request body further down the chain (most likely to the
@@ -234,7 +259,11 @@ apr_status_t read_request_body(modsec_rec *msr, char **error_msg) {
*error_msg = apr_psprintf(msr->mp, "Error reading request body: %s", get_apr_error(msr->mp, rc));
break;
}
+#ifdef VERSION_NGINX
+ return msc_ap_map_http_request_error(rc, HTTP_BAD_REQUEST);
+#else
return ap_map_http_request_error(rc, HTTP_BAD_REQUEST);
+#endif
}
/* Loop through the buckets in the brigade in order
To be honest I don't like this solution, because we need to use macro VERSION_NGINX, but as I checked the source there are two other macros used:
BUILD_STANDALONE_MODULE
VERSION_STANDALONE
The first one is used only in build system (automake related files), the second one is used only in source (and standalone/standalone.vcxproj).
The only macro which is uses in both subsystem is the VERSION_NGINX. I think first we should clarify what macro is used for which functionality, then choose one and use that. (I don't want to use VERSION_NGINX because this does not describe the correct use case, for eg. you want to use as a standalone module through SPOA, not for Nginx.)
But as I mentioned I ran into other problems too. First, the default PCRE engine is now PCRE2, so I had to modify the Makefile in HAProxy's spoa's source:
diff --git a/Makefile b/Makefile
index a00bdd0..c7a8799 100644
--- a/Makefile
+++ b/Makefile
@@ -35,7 +35,7 @@ endif
CFLAGS += -g -Wall -pthread
INCS += -Iinclude -I$(MODSEC_INC) -I$(APACHE2_INC) -I$(APR_INC) -I$(LIBXML_INC) -I$(EVENT_INC)
-LIBS += -lpthread $(EVENT_LIB) -levent_pthreads -lcurl -lapr-1 -laprutil-1 -lxml2 -lpcre -lyajl
+LIBS += -lpthread $(EVENT_LIB) -levent_pthreads -lcurl -lapr-1 -laprutil-1 -lxml2 -lpcre2-8 -lyajl
OBJS = spoa.o modsec_wrapper.o
After this modification I still got these issues:
cc -o modsecurity spoa.o modsec_wrapper.o /home/airween/src/modsecurity-apache-standalone/INSTALL/lib/standalone.a -lpthread -levent -levent_pthreads -lcurl -lapr-1 -laprutil-1 -lxml2 -lpcre2-8 -lyajl
/usr/bin/ld: /home/airween/src/modsecurity-apache-standalone/INSTALL/lib/standalone.a(standalone_la-re_operators.o): in function `msre_op_fuzzy_hash_execute':
/home/airween/src/modsecurity-apache-standalone/standalone/../apache2/re_operators.c:4279:(.text+0xc5b): undefined reference to `fuzzy_hash_buf'
/usr/bin/ld: /home/airween/src/modsecurity-apache-standalone/standalone/../apache2/re_operators.c:4289:(.text+0xc78): undefined reference to `fuzzy_compare'
collect2: error: ld returned 1 exit status
make: *** [Makefile:43: modsecurity] Error 1
Summarize: thank you again for this report, I have to dive into the standalone module building.
@kabe-gh just FYI: I've added a PR to haproxy/spoa-modsecurity. And I hope I can fix this issue too soon - but the spoa module maintainers need to merge that PR, I guess.