From 7f8aba2f44e9ca65b8a95987fa6c46020e1bdd6d Mon Sep 17 00:00:00 2001 From: Alexandr Nedvedicky Date: Fri, 8 Mar 2024 11:21:18 +0100 Subject: [PATCH] Limit the number of http headers when receiving the http response Change introduces a default limit on HTTP headers we expect to receive from server to 256. If limit is exceeded http client library indicates HTTP_R_RESPONSE_TOO_MANY_HDRLINES error. Application can use OSSL_HTTP_REQ_CTX_set_max_response_hdr_lines() to change default. Setting limit to 0 implies no limit (current behavior). Fixes #22264 Reviewed-by: Matt Caswell Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/23781) --- CHANGES.md | 8 +++++ crypto/err/openssl.txt | 1 + crypto/http/http_client.c | 23 +++++++++++++ crypto/http/http_err.c | 17 ++++++--- doc/man3/OSSL_HTTP_REQ_CTX.pod | 11 +++++- include/crypto/httperr.h | 5 ++- include/openssl/http.h | 4 +++ include/openssl/httperr.h | 3 +- test/http_test.c | 63 ++++++++++++++++++++++++++++++++++ util/libcrypto.num | 1 + 10 files changed, 128 insertions(+), 8 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index ac6b7525bf..ddb2ba56a2 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -155,6 +155,14 @@ OpenSSL 3.3 *Hugo Landau* + * New limit on HTTP response headers is introduced to HTTP client. The + default limit is set to 256 header lines. If limit is exceeded the + response processing stops with error HTTP_R_RESPONSE_TOO_MANY_HDRLINES. + Application may call OSSL_HTTP_REQ_CTX_set_max_response_hdr_lines(3) + to change the default. Setting the value to 0 disables the limit. + + *Alexandr Nedvedicky* + OpenSSL 3.2 ----------- diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index 22b5de9a7a..1607ad835f 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -837,6 +837,7 @@ HTTP_R_REDIRECTION_FROM_HTTPS_TO_HTTP:112:redirection from https to http HTTP_R_REDIRECTION_NOT_ENABLED:116:redirection not enabled HTTP_R_RESPONSE_LINE_TOO_LONG:113:response line too long HTTP_R_RESPONSE_PARSE_ERROR:104:response parse error +HTTP_R_RESPONSE_TOO_MANY_HDRLINES:130:response too many hdrlines HTTP_R_RETRY_TIMEOUT:129:retry timeout HTTP_R_SERVER_CANCELED_CONNECTION:127:server canceled connection HTTP_R_SOCK_NOT_SUPPORTED:122:sock not supported diff --git a/crypto/http/http_client.c b/crypto/http/http_client.c index acc32769a8..ebcaa8a111 100644 --- a/crypto/http/http_client.c +++ b/crypto/http/http_client.c @@ -67,6 +67,7 @@ struct ossl_http_req_ctx_st { time_t max_time; /* Maximum end time of current transfer, or 0 */ time_t max_total_time; /* Maximum end time of total transfer, or 0 */ char *redirection_url; /* Location obtained from HTTP status 301/302 */ + size_t max_hdr_lines; /* Max. number of http hdr lines, or 0 */ }; /* HTTP states */ @@ -106,6 +107,7 @@ OSSL_HTTP_REQ_CTX *OSSL_HTTP_REQ_CTX_new(BIO *wbio, BIO *rbio, int buf_size) rctx->buf = OPENSSL_malloc(rctx->buf_size); rctx->wbio = wbio; rctx->rbio = rbio; + rctx->max_hdr_lines = OSSL_HTTP_DEFAULT_MAX_RESP_HDR_LINES; if (rctx->buf == NULL) { OPENSSL_free(rctx); return NULL; @@ -355,6 +357,16 @@ int OSSL_HTTP_REQ_CTX_set1_req(OSSL_HTTP_REQ_CTX *rctx, const char *content_type return res; } +void OSSL_HTTP_REQ_CTX_set_max_response_hdr_lines(OSSL_HTTP_REQ_CTX *rctx, + size_t count) +{ + if (rctx == NULL) { + ERR_raise(ERR_LIB_HTTP, ERR_R_PASSED_NULL_PARAMETER); + return; + } + rctx->max_hdr_lines = count; +} + static int add1_headers(OSSL_HTTP_REQ_CTX *rctx, const STACK_OF(CONF_VALUE) *headers, const char *host) { @@ -537,6 +549,7 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx) size_t resp_len; const unsigned char *p; char *buf, *key, *value, *line_end = NULL; + size_t resp_hdr_lines = 0; if (rctx == NULL) { ERR_raise(ERR_LIB_HTTP, ERR_R_PASSED_NULL_PARAMETER); @@ -682,6 +695,14 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx) return 0; } + resp_hdr_lines++; + if (rctx->max_hdr_lines != 0 && rctx->max_hdr_lines < resp_hdr_lines) { + ERR_raise(ERR_LIB_HTTP, HTTP_R_RESPONSE_TOO_MANY_HDRLINES); + OSSL_TRACE(HTTP, "Received too many headers\n"); + rctx->state = OHS_ERROR; + return 0; + } + /* Don't allow excessive lines */ if (n == rctx->buf_size) { ERR_raise(ERR_LIB_HTTP, HTTP_R_RESPONSE_LINE_TOO_LONG); @@ -786,6 +807,8 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx) if (OSSL_TRACE_ENABLED(HTTP)) OSSL_TRACE(HTTP, "]\n"); + resp_hdr_lines = 0; + if (rctx->keep_alive != 0 /* do not let server initiate keep_alive */ && !found_keep_alive /* otherwise there is no change */) { if (rctx->keep_alive == 2) { diff --git a/crypto/http/http_err.c b/crypto/http/http_err.c index 332ad926d3..22c2b40e61 100644 --- a/crypto/http/http_err.c +++ b/crypto/http/http_err.c @@ -1,6 +1,6 @@ /* * Generated by util/mkerr.pl DO NOT EDIT - * Copyright 1995-2021 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 1995-2024 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the Apache License 2.0 (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -12,7 +12,9 @@ #include #include "crypto/httperr.h" -#ifndef OPENSSL_NO_ERR +#ifndef OPENSSL_NO_HTTP + +# ifndef OPENSSL_NO_ERR static const ERR_STRING_DATA HTTP_str_reasons[] = { {ERR_PACK(ERR_LIB_HTTP, 0, HTTP_R_ASN1_LEN_EXCEEDS_MAX_RESP_LEN), @@ -55,6 +57,8 @@ static const ERR_STRING_DATA HTTP_str_reasons[] = { "response line too long"}, {ERR_PACK(ERR_LIB_HTTP, 0, HTTP_R_RESPONSE_PARSE_ERROR), "response parse error"}, + {ERR_PACK(ERR_LIB_HTTP, 0, HTTP_R_RESPONSE_TOO_MANY_HDRLINES), + "response too many hdrlines"}, {ERR_PACK(ERR_LIB_HTTP, 0, HTTP_R_RETRY_TIMEOUT), "retry timeout"}, {ERR_PACK(ERR_LIB_HTTP, 0, HTTP_R_SERVER_CANCELED_CONNECTION), "server canceled connection"}, @@ -70,13 +74,16 @@ static const ERR_STRING_DATA HTTP_str_reasons[] = { {0, NULL} }; -#endif +# endif int ossl_err_load_HTTP_strings(void) { -#ifndef OPENSSL_NO_ERR +# ifndef OPENSSL_NO_ERR if (ERR_reason_error_string(HTTP_str_reasons[0].error) == NULL) ERR_load_strings_const(HTTP_str_reasons); -#endif +# endif return 1; } +#else +NON_EMPTY_TRANSLATION_UNIT +#endif diff --git a/doc/man3/OSSL_HTTP_REQ_CTX.pod b/doc/man3/OSSL_HTTP_REQ_CTX.pod index f74fcb35ce..e80673ea49 100644 --- a/doc/man3/OSSL_HTTP_REQ_CTX.pod +++ b/doc/man3/OSSL_HTTP_REQ_CTX.pod @@ -15,7 +15,8 @@ OSSL_HTTP_REQ_CTX_exchange, OSSL_HTTP_REQ_CTX_get0_mem_bio, OSSL_HTTP_REQ_CTX_get_resp_len, OSSL_HTTP_REQ_CTX_set_max_response_length, -OSSL_HTTP_is_alive +OSSL_HTTP_is_alive, +OSSL_HTTP_REQ_CTX_set_max_response_hdr_lines - HTTP client low-level functions =head1 SYNOPSIS @@ -50,6 +51,9 @@ OSSL_HTTP_is_alive int OSSL_HTTP_is_alive(const OSSL_HTTP_REQ_CTX *rctx); + void OSSL_HTTP_REQ_CTX_set_max_response_hdr_lines(OSSL_HTTP_REQ_CTX *rctx, + size_t count); + =head1 DESCRIPTION B is a context structure for an HTTP request and response, @@ -191,6 +195,11 @@ In case the client application keeps I but the connection then dies for any reason at the server side, it will notice this obtaining an I/O error when trying to send the next request via I. +The OSSL_HTTP_REQ_CTX_set_max_response_hdr_lines() function changes the limit +for the number of HTTP headers which can be received in a response. The default +value is 256. If the number of HTTP headers in a response exceeds the limit, +then the HTTP_R_RESPONSE_TOO_MANY_HDRLINES error is indicated. + =head1 WARNINGS The server's response may be unexpected if the hostname that was used to diff --git a/include/crypto/httperr.h b/include/crypto/httperr.h index 969df17b83..827d61a235 100644 --- a/include/crypto/httperr.h +++ b/include/crypto/httperr.h @@ -1,6 +1,6 @@ /* * Generated by util/mkerr.pl DO NOT EDIT - * Copyright 2020-2021 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 2020-2024 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the Apache License 2.0 (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -19,7 +19,10 @@ extern "C" { # endif +# ifndef OPENSSL_NO_HTTP + int ossl_err_load_HTTP_strings(void); +# endif # ifdef __cplusplus } diff --git a/include/openssl/http.h b/include/openssl/http.h index a3cbf15f5a..4f58652cc1 100644 --- a/include/openssl/http.h +++ b/include/openssl/http.h @@ -37,6 +37,8 @@ extern "C" { #define OSSL_HTTP_DEFAULT_MAX_LINE_LEN (4 * 1024) #define OSSL_HTTP_DEFAULT_MAX_RESP_LEN (100 * 1024) +#define OSSL_HTTP_DEFAULT_MAX_RESP_HDR_LINES 256 + /* Low-level HTTP API */ OSSL_HTTP_REQ_CTX *OSSL_HTTP_REQ_CTX_new(BIO *wbio, BIO *rbio, int buf_size); @@ -105,6 +107,8 @@ int OSSL_HTTP_parse_url(const char *url, int *pssl, char **puser, char **phost, const char *OSSL_HTTP_adapt_proxy(const char *proxy, const char *no_proxy, const char *server, int use_ssl); +void OSSL_HTTP_REQ_CTX_set_max_response_hdr_lines(OSSL_HTTP_REQ_CTX *rctx, + size_t count); # endif /* !defined(OPENSSL_NO_HTTP) */ # ifdef __cplusplus diff --git a/include/openssl/httperr.h b/include/openssl/httperr.h index ee08959203..ae7f00cac0 100644 --- a/include/openssl/httperr.h +++ b/include/openssl/httperr.h @@ -1,6 +1,6 @@ /* * Generated by util/mkerr.pl DO NOT EDIT - * Copyright 1995-2021 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 1995-2024 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the Apache License 2.0 (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -44,6 +44,7 @@ # define HTTP_R_REDIRECTION_NOT_ENABLED 116 # define HTTP_R_RESPONSE_LINE_TOO_LONG 113 # define HTTP_R_RESPONSE_PARSE_ERROR 104 +# define HTTP_R_RESPONSE_TOO_MANY_HDRLINES 130 # define HTTP_R_RETRY_TIMEOUT 129 # define HTTP_R_SERVER_CANCELED_CONNECTION 127 # define HTTP_R_SOCK_NOT_SUPPORTED 122 diff --git a/test/http_test.c b/test/http_test.c index 3f70f6223a..63783179f4 100644 --- a/test/http_test.c +++ b/test/http_test.c @@ -416,6 +416,66 @@ static int test_http_keep_alive_1_require_no(void) return test_http_keep_alive('1', 2, 0); } +static int test_http_resp_hdr_limit(size_t limit) +{ + BIO *wbio = BIO_new(BIO_s_mem()); + BIO *rbio = BIO_new(BIO_s_mem()); + BIO *mem = NULL; + server_args mock_args = { NULL, NULL, NULL, '0', 0 }; + int res = 0; + OSSL_HTTP_REQ_CTX *rctx = NULL; + + if (TEST_ptr(wbio) == 0 || TEST_ptr(rbio) == 0) + goto err; + + mock_args.txt = text1; + mock_args.content_type = "text/plain"; + mock_args.version = '1'; + mock_args.out = rbio; + mock_args.content_type = "text/plain"; + + BIO_set_callback_ex(wbio, http_bio_cb_ex); + BIO_set_callback_arg(wbio, (char *)&mock_args); + + rctx = OSSL_HTTP_REQ_CTX_new(wbio, rbio, 8192); + if (TEST_ptr(rctx) == 0) + goto err; + + if (!TEST_true(OSSL_HTTP_REQ_CTX_set_request_line(rctx, 0 /* GET */, + NULL, NULL, RPATH))) + goto err; + + OSSL_HTTP_REQ_CTX_set_max_response_hdr_lines(rctx, limit); + mem = OSSL_HTTP_REQ_CTX_exchange(rctx); + + if (limit == 1) + res = TEST_ptr_null(mem); + else + res = TEST_ptr(mem); + + err: + BIO_free(wbio); + BIO_free(rbio); + OSSL_HTTP_REQ_CTX_free(rctx); + + return res; +} + +static int test_hdr_resp_hdr_limit_none(void) +{ + return test_http_resp_hdr_limit(0); +} + +static int test_hdr_resp_hdr_limit_short(void) +{ + return (test_http_resp_hdr_limit(1)); +} + +static int test_hdr_resp_hdr_limit_256(void) +{ + return test_http_resp_hdr_limit(256); +} + void cleanup_tests(void) { X509_free(x509); @@ -452,5 +512,8 @@ int setup_tests(void) ADD_TEST(test_http_keep_alive_1_require_yes); ADD_TEST(test_http_keep_alive_0_require_no); ADD_TEST(test_http_keep_alive_1_require_no); + ADD_TEST(test_hdr_resp_hdr_limit_none); + ADD_TEST(test_hdr_resp_hdr_limit_short); + ADD_TEST(test_hdr_resp_hdr_limit_256); return 1; } diff --git a/util/libcrypto.num b/util/libcrypto.num index 7f646cbba9..89a211d2f6 100644 --- a/util/libcrypto.num +++ b/util/libcrypto.num @@ -5547,3 +5547,4 @@ ERR_pop ? 3_3_0 EXIST::FUNCTION: X509_STORE_get1_objects ? 3_3_0 EXIST::FUNCTION: OPENSSL_LH_set_thunks ? 3_3_0 EXIST::FUNCTION: OPENSSL_LH_doall_arg_thunk ? 3_3_0 EXIST::FUNCTION: +OSSL_HTTP_REQ_CTX_set_max_response_hdr_lines ? 3_3_0 EXIST::FUNCTION:HTTP -- 2.39.2