BUG/MINOR: stats: Be more strict on what is a valid request to the stats applet
First of all, only GET, HEAD and POST methods are now allowed. Others will be
rejected with the status code STAT_STATUS_IVAL (invalid request). Then, for the
legacy HTTP, only POST requests with a content-length are allowed. Now, chunked
encoded requests are also considered as invalid because the chunk formatting
will interfere with the parsing of POST parameters. In HTX, It is not a problem
because data are unchunked.
This patch must be backported to 1.9. For prior versions too, but HTX part must
be removed. The patch introducing the status code STAT_STATUS_IVAL must also be
backported.
diff --git a/src/proto_http.c b/src/proto_http.c
index 7bf0e20..4190313 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -1311,22 +1311,27 @@
}
}
- /* Was the status page requested with a POST ? */
- if (unlikely(txn->meth == HTTP_METH_POST && txn->req.body_len > 0)) {
+ if (txn->meth == HTTP_METH_GET || txn->meth == HTTP_METH_HEAD)
+ appctx->st0 = STAT_HTTP_HEAD;
+ else if (txn->meth == HTTP_METH_POST && (msg->flags & HTTP_MSGF_CNT_LEN)) {
if (appctx->ctx.stats.flags & STAT_ADMIN) {
/* we'll need the request body, possibly after sending 100-continue */
- if (msg->msg_state < HTTP_MSG_CHUNK_SIZE)
+ if (msg->msg_state < HTTP_MSG_DATA)
req->analysers |= AN_REQ_HTTP_BODY;
appctx->st0 = STAT_HTTP_POST;
}
else {
+ /* POST without admin level */
+ appctx->ctx.stats.flags &= ~STAT_CHUNKED;
appctx->ctx.stats.st_code = STAT_STATUS_DENY;
appctx->st0 = STAT_HTTP_LAST;
}
}
else {
- /* So it was another method (GET/HEAD) */
- appctx->st0 = STAT_HTTP_HEAD;
+ /* Unsupported method or chunked POST */
+ appctx->ctx.stats.flags &= ~STAT_CHUNKED;
+ appctx->ctx.stats.st_code = STAT_STATUS_IVAL;
+ appctx->st0 = STAT_HTTP_LAST;
}
s->task->nice = -32; /* small boost for HTTP statistics */
diff --git a/src/proto_htx.c b/src/proto_htx.c
index 255199e..a65297d 100644
--- a/src/proto_htx.c
+++ b/src/proto_htx.c
@@ -4952,8 +4952,9 @@
}
}
- /* Was the status page requested with a POST ? */
- if (unlikely(txn->meth == HTTP_METH_POST)) {
+ if (txn->meth == HTTP_METH_GET || txn->meth == HTTP_METH_HEAD)
+ appctx->st0 = STAT_HTTP_HEAD;
+ else if (txn->meth == HTTP_METH_POST) {
if (appctx->ctx.stats.flags & STAT_ADMIN) {
/* we'll need the request body, possibly after sending 100-continue */
if (msg->msg_state < HTTP_MSG_DATA)
@@ -4961,14 +4962,17 @@
appctx->st0 = STAT_HTTP_POST;
}
else {
+ /* POST without admin level */
appctx->ctx.stats.flags &= ~STAT_CHUNKED;
appctx->ctx.stats.st_code = STAT_STATUS_DENY;
appctx->st0 = STAT_HTTP_LAST;
}
}
else {
- /* So it was another method (GET/HEAD) */
- appctx->st0 = STAT_HTTP_HEAD;
+ /* Unsupported method */
+ appctx->ctx.stats.flags &= ~STAT_CHUNKED;
+ appctx->ctx.stats.st_code = STAT_STATUS_IVAL;
+ appctx->st0 = STAT_HTTP_LAST;
}
s->task->nice = -32; /* small boost for HTTP statistics */