[modsecurity.conf-recommended] align processing on request & response for json

[fzipi]

I think this issue might be relevant in this context also: corazawaf/coraza#1086.

Adapting @YvesZelros's issue here:

The modsecurity.conf-recommended enable JSON request body processor but don't parse JSON on response.
I propose align request & response configuration by adding application/json on SecResponseBodyMimeType:
SecResponseBodyMimeType text/plain text/html text/xml application/json

What do you think?

[marcstern]

It highly depends on the rules. For instance, if you only checl for stack traces, there are very few chances to find some in JSON (compared to HTML).
If you look for credit card numbers, it makes sense.
As we enabled XML, it looks logical to allow JSON as well (or to remove XML).

[YvesZelros]

@marcstern I don't agree that we have few chances to find stack traces on JSON. Why do you thinks that ? It's really depending of backend.

As exemple I do a quick test with an Java application based on quarkus by dropping a table to simulate an sql error and the response is on json.

Header
Content-Type: application/json; charset=utf-8
Content:

{
    "details": "Error id 9652978e-4d41-490f-8818-4321bf1e54f2-1, org.hibernate.exception.SQLGrammarException: JDBC exception executing SQL [select * from Application a1_0 fetch first ? rows only] [ERROR: relation \"application\" does not exist",
"stack": "org.hibernate.exception.SQLGrammarException: JDBC exception executing SQL [select * from Application a1_0 fetch first ? rows only] [ERROR: relation \"application\" does not exist\n  Position: 556] [n/a]\n\tat org.hibernate.exception.internal.SQLStateConversionDelegate.convert(SQLStateConversionDelegate.java:91)\n\tat org.hibernate.exception.internal.StandardSQLExceptionConverter.convert(StandardSQLExceptionConverter.java:58)\n\tat org.hibernate.engine.jdbc.spi.SqlExceptionHelper.convert(SqlExceptionHelper.java:108)\n\tat org.hibernate.engine.jdbc.spi.SqlExceptionHelper.convert(SqlExceptionHelper.java:94)\n\tat org.hibernate.sql.results.jdbc.internal.DeferredResultSetAccess.executeQuery(DeferredResultSetAccess.java:265)\n\tat org.hibernate.sql.results.jdbc.internal.DeferredResultSetAccess.getResultSet(DeferredResultSetAccess.java:167)\n\tat org.hibernate.sql.results.jdbc.internal.JdbcValuesResultSetImpl.advanceNext(JdbcValuesResultSetImpl.java:218)\n\tat org.hibernate.sql.results.jdbc.internal.JdbcValuesResultSetImpl.processNext(JdbcValuesResultSetImpl.java:98)\n\tat org.hibernate.sql.results.jdbc.internal.AbstractJdbcValues.next(AbstractJdbcValues.java:19)\n\tat org.hibernate.sql.results.internal.RowProcessingStateStandardImpl.next(RowProcessingStateStandardImpl.java:66)\n\tat org.hibernate.sql.results.spi.ListResultsConsumer.consume(ListResultsConsumer.java:202)\n\tat org.hibernate.sql.results.spi.ListResultsConsumer.consume(ListResultsConsumer.java:33)\n\tat org.hibernate.sql.exec.internal.JdbcSelectExecutorStandardImpl.doExecuteQuery(JdbcSelectExecutorStandardImpl.java:209)\n\tat org.hibernate.sql.exec.internal.JdbcSelectExecutorStandardImpl.executeQuery(JdbcSelectExecutorStandardImpl.java:83)\n\tat org.hibernate.sql.exec.spi.JdbcSelectExecutor.list(JdbcSelectExecutor.java:76)\n\tat org.hibernate.sql.exec.spi.JdbcSelectExecutor.list(JdbcSelectExecutor.java:65)\n\tat org.hibernate.query.sqm.internal.ConcreteSqmSelectQueryPlan.lambda$new$2(ConcreteSqmSelectQueryPlan.java:137)\n\tat org.hibernate.query.sqm.internal.ConcreteSqmSelectQueryPlan.withCacheableSqmInterpretation(ConcreteSqmSelectQueryPlan.java:362)\n\tat org.hibernate.query.sqm.internal.ConcreteSqmSelectQueryPlan.performList(ConcreteSqmSelectQueryPlan.java:303)\n\tat org.hibernate.query.sqm.internal.QuerySqmImpl.doList(QuerySqmImpl.java:509)\n\tat org.hibernate.query.spi.AbstractSelectionQuery.list(AbstractSelectionQuery.java:427)\n\tat org.hibernate.query.Query.getResultList(Query.java:120)\n\tat io.quarkus.hibernate.orm.panache.common.runtime.CommonPanacheQueryImpl.firstResult(CommonPanacheQueryImpl.java:328)\n\tat io.quarkus.hibernate.orm.panache.common.runtime.CommonPanacheQueryImpl.firstResultOptional(CommonPanacheQueryImpl.java:334)\n\tat io.quarkus.hibernate.orm.panache.runtime.PanacheQueryImpl.firstResultOptional(PanacheQueryImpl.java:164)\n\tat io.quarkus.arc.impl.AroundInvokeInvocationContext$NextAroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:97)\n\tat io.quarkus.security.runtime.interceptor.SecurityHandler.handle(SecurityHandler.java:27)\n\tat io.quarkus.security.runtime.interceptor.AuthenticatedInterceptor.intercept(AuthenticatedInterceptor.java:29)\n\tat io.quarkus.security.runtime.interceptor.AuthenticatedInterceptor_Bean.intercept(Unknown Source)\n\tat io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:42)\n\tat io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:70)\n\tat io.quarkus.arc.impl.AroundInvokeInvocationContext$NextAroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:97)\n\tat io.quarkus.security.runtime.interceptor.SecurityHandler.handle(SecurityHandler.java:27)\n\tat io.quarkus.security.runtime.interceptor.RolesAllowedInterceptor.intercept(RolesAllowedInterceptor.java:29)\n\tat io.quarkus.security.runtime.interceptor.RolesAllowedInterceptor_Bean.intercept(Unknown Source)\n\tat io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:42)\n\tat io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:70)\n\tat io.quarkus.arc.impl.AroundInvokeInvocationContext$NextAroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:97)\n\tat io.quarkus.resteasy.reactive.server.runtime.StandardSecurityCheckInterceptor.intercept(StandardSecurityCheckInterceptor.java:44)\n\tat io.quarkus.resteasy.reactive.server.runtime.StandardSecurityCheckInterceptor_AuthenticatedInterceptor_Bean.intercept(Unknown Source)\n\tat io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:42)\n\tat io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:70)\n\tat io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:62)\n\tat io.quarkus.resteasy.reactive.server.runtime.StandardSecurityCheckInterceptor.intercept(StandardSecurityCheckInterceptor.java:44)\n\tat io.quarkus.resteasy.reactive.server.runtime.StandardSecurityCheckInterceptor_RolesAllowedInterceptor_Bean.intercept(Unknown Source)\n\tat io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:42)\n\tat io.quarkus.arc.impl.AroundInvokeInvocationContext.perform(AroundInvokeInvocationContext.java:30)\n\tat io.quarkus.arc.impl.InvocationContexts.performAroundInvoke(InvocationContexts.java:27)\n\tat com.zel.operator.platform.client.contollers.AppConfigController_Subclass.getApplication(Unknown Source)\n\tat com.zel.operator.platform.client.contollers.AppConfigController$quarkusrestinvoker$getApplication_39feb0863fd9bf4265d6532f1487bb1c6071446b.invoke(Unknown Source)\n\tat org.jboss.resteasy.reactive.server.handlers.InvocationHandler.handle(InvocationHandler.java:29)\n\tat io.quarkus.resteasy.reactive.server.runtime.QuarkusResteasyReactiveRequestContext.invokeHandler(QuarkusResteasyReactiveRequestContext.java:141)\n\tat org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:147)\n\tat io.quarkus.vertx.core.runtime.VertxCoreRecorder$14.runWith(VertxCoreRecorder.java:599)\n\tat org.jboss.threads.EnhancedQueueExecutor$Task.doRunWith(EnhancedQueueExecutor.java:2516)\n\tat org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2495)\n\tat org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1521)\n\tat org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:11)\n\tat org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:11)\n\tat io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)\n\tat java.base/java.lang.Thread.run(Thread.java:1583)\nCaused by: org.postgresql.util.PSQLException: ERROR: relation \"application\" does not exist\n  Position: 556\n\tat org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2725)\n\tat org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:2412)\n\tat org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:371)\n\tat org.postgresql.jdbc.PgStatement.executeInternal(PgStatement.java:502)\n\tat org.postgresql.jdbc.PgStatement.execute(PgStatement.java:419)\n\tat org.postgresql.jdbc.PgPreparedStatement.executeWithFlags(PgPreparedStatement.java:194)\n\tat org.postgresql.jdbc.PgPreparedStatement.executeQuery(PgPreparedStatement.java:137)\n\tat io.agroal.pool.wrapper.PreparedStatementWrapper.executeQuery(PreparedStatementWrapper.java:80)\n\tat org.hibernate.sql.results.jdbc.internal.DeferredResultSetAccess.executeQuery(DeferredResultSetAccess.java:246)\n\t... 62 more"
}

An second test with a Node application =>

Header
Content-Type: application/json; charset=utf-8
Content:

{"statusCode":500,"message":"Internal server error"}

Here we don't have the stack on the error, but it's really depending how error handler is implemented on the application.

[theseion]

We should consider the potential impact on performance. Many web servers today serve APIs with JSON responses. These responses can be huge. While it's probably a good idea to turn the setting on, users should be made aware of the potential downside.

[fzipi]

Well,

As we enabled XML, it looks logical to allow JSON as well (or to remove XML).

I would say having enabled XML in the middle of 2010s might have been a good choice. Nowadays, probably it is not. This is something we have discussed in the CRS team about having a stripped down version of the rules supporting only json instead...