2.x standalone fails to link since ap_map_http_request_error() is not exported as APR function

[kabe-gh]

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.
  */

[airween]

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.

[airween]

@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.