MEDIUM: http: restrict the HTTP version token to 1 digit as per RFC7230

While RFC2616 used to allow an undeterminate amount of digits for the
major and minor components of the HTTP version, RFC7230 has reduced
that to a single digit for each.

If a server can't properly parse the version string and falls back to 0.9,
it could then send a head-less response whose payload would be taken for
headers, which could confuse downstream agents.

Since there's no more reason for supporting a version scheme that was
never used, let's upgrade to the updated version of the standard. It is
still possible to enforce support for the old behaviour using options
accept-invalid-http-request and accept-invalid-http-response.

(cherry picked from commit 91852eb4280e5fbe63dcd9ea32c168d7516c6667)
This commit is contained in:
Willy Tarreau 2015-05-01 13:26:00 +02:00
parent 660418d9b3
commit 5564555b86
2 changed files with 46 additions and 4 deletions

View File

@ -3747,7 +3747,7 @@ no option accept-invalid-http-request
yes | yes | yes | no yes | yes | yes | no
Arguments : none Arguments : none
By default, HAProxy complies with RFC2616 in terms of message parsing. This By default, HAProxy complies with RFC7230 in terms of message parsing. This
means that invalid characters in header names are not permitted and cause an means that invalid characters in header names are not permitted and cause an
error to be returned to the client. This is the desired behaviour as such error to be returned to the client. This is the desired behaviour as such
forbidden characters are essentially used to build attacks exploiting server forbidden characters are essentially used to build attacks exploiting server
@ -3760,7 +3760,9 @@ no option accept-invalid-http-request
chars 0-31, 32 (space), 34 ('"'), 60 ('<'), 62 ('>'), 92 ('\'), 94 ('^'), 96 chars 0-31, 32 (space), 34 ('"'), 60 ('<'), 62 ('>'), 92 ('\'), 94 ('^'), 96
('`'), 123 ('{'), 124 ('|'), 125 ('}'), 127 (delete) and anything above are ('`'), 123 ('{'), 124 ('|'), 125 ('}'), 127 (delete) and anything above are
not allowed at all. Haproxy always blocks a number of them (0..32, 127). The not allowed at all. Haproxy always blocks a number of them (0..32, 127). The
remaining ones are blocked by default unless this option is enabled. remaining ones are blocked by default unless this option is enabled. This
option also relaxes the test on the HTTP version format, it allows multiple
digits for both the major and the minor version.
This option should never be enabled by default as it hides application bugs This option should never be enabled by default as it hides application bugs
and open security breaches. It should only be deployed after a problem has and open security breaches. It should only be deployed after a problem has
@ -3786,7 +3788,7 @@ no option accept-invalid-http-response
yes | no | yes | yes yes | no | yes | yes
Arguments : none Arguments : none
By default, HAProxy complies with RFC2616 in terms of message parsing. This By default, HAProxy complies with RFC7230 in terms of message parsing. This
means that invalid characters in header names are not permitted and cause an means that invalid characters in header names are not permitted and cause an
error to be returned to the client. This is the desired behaviour as such error to be returned to the client. This is the desired behaviour as such
forbidden characters are essentially used to build attacks exploiting server forbidden characters are essentially used to build attacks exploiting server
@ -3794,7 +3796,9 @@ no option accept-invalid-http-response
server will emit invalid header names for whatever reason (configuration, server will emit invalid header names for whatever reason (configuration,
implementation) and the issue will not be immediately fixed. In such a case, implementation) and the issue will not be immediately fixed. In such a case,
it is possible to relax HAProxy's header name parser to accept any character it is possible to relax HAProxy's header name parser to accept any character
even if that does not make sense, by specifying this option. even if that does not make sense, by specifying this option. This option also
relaxes the test on the HTTP version format, it allows multiple digits for
both the major and the minor version.
This option should never be enabled by default as it hides application bugs This option should never be enabled by default as it hides application bugs
and open security breaches. It should only be deployed after a problem has and open security breaches. It should only be deployed after a problem has

View File

@ -2853,6 +2853,25 @@ int http_wait_for_request(struct session *s, struct channel *req, int an_bit)
if (unlikely(msg->sl.rq.v_l == 0) && !http_upgrade_v09_to_v10(txn)) if (unlikely(msg->sl.rq.v_l == 0) && !http_upgrade_v09_to_v10(txn))
goto return_bad_req; goto return_bad_req;
/* RFC7230#2.6 has enforced the format of the HTTP version string to be
* exactly one digit "." one digit. This check may be disabled using
* option accept-invalid-http-request.
*/
if (!(s->fe->options2 & PR_O2_REQBUG_OK)) {
if (msg->sl.rq.v_l != 8) {
msg->err_pos = msg->sl.rq.v;
goto return_bad_req;
}
if (req->buf->p[msg->sl.rq.v + 4] != '/' ||
!isdigit((unsigned char)req->buf->p[msg->sl.rq.v + 5]) ||
req->buf->p[msg->sl.rq.v + 6] != '.' ||
!isdigit((unsigned char)req->buf->p[msg->sl.rq.v + 7])) {
msg->err_pos = msg->sl.rq.v + 4;
goto return_bad_req;
}
}
/* ... and check if the request is HTTP/1.1 or above */ /* ... and check if the request is HTTP/1.1 or above */
if ((msg->sl.rq.v_l == 8) && if ((msg->sl.rq.v_l == 8) &&
((req->buf->p[msg->sl.rq.v + 5] > '1') || ((req->buf->p[msg->sl.rq.v + 5] > '1') ||
@ -5870,6 +5889,25 @@ int http_wait_for_response(struct session *s, struct channel *rep, int an_bit)
if (objt_server(s->target)) if (objt_server(s->target))
objt_server(s->target)->counters.p.http.rsp[n]++; objt_server(s->target)->counters.p.http.rsp[n]++;
/* RFC7230#2.6 has enforced the format of the HTTP version string to be
* exactly one digit "." one digit. This check may be disabled using
* option accept-invalid-http-response.
*/
if (!(s->be->options2 & PR_O2_RSPBUG_OK)) {
if (msg->sl.st.v_l != 8) {
msg->err_pos = 0;
goto hdr_response_bad;
}
if (rep->buf->p[4] != '/' ||
!isdigit((unsigned char)rep->buf->p[5]) ||
rep->buf->p[6] != '.' ||
!isdigit((unsigned char)rep->buf->p[7])) {
msg->err_pos = 4;
goto hdr_response_bad;
}
}
/* check if the response is HTTP/1.1 or above */ /* check if the response is HTTP/1.1 or above */
if ((msg->sl.st.v_l == 8) && if ((msg->sl.st.v_l == 8) &&
((rep->buf->p[5] > '1') || ((rep->buf->p[5] > '1') ||