From: Dr. David von Oheimb Date: Wed, 15 Jan 2025 19:13:00 +0000 (+0100) Subject: http_client.c: fix OSSL_HTTP_REQ_CTX_nbio() to return content on non-fatal HTTP statu... X-Git-Tag: openssl-3.5.0-alpha1~626 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=64b478419aeff9e4771a5ac9640715da3d70eba9;p=thirdparty%2Fopenssl.git http_client.c: fix OSSL_HTTP_REQ_CTX_nbio() to return content on non-fatal HTTP status code >=400 Reviewed-by: Dmitry Belyavskiy Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/25541) --- diff --git a/crypto/http/http_client.c b/crypto/http/http_client.c index e23160fe3f6..95dabc8ec54 100644 --- a/crypto/http/http_client.c +++ b/crypto/http/http_client.c @@ -35,6 +35,7 @@ #define HTTP_STATUS_CODE_OK 200 #define HTTP_STATUS_CODE_MOVED_PERMANENTLY 301 #define HTTP_STATUS_CODE_FOUND 302 +#define HTTP_STATUS_CODES_NONFATAL_ERROR 400 /* Stateful HTTP request code, supporting blocking and non-blocking I/O */ @@ -431,7 +432,7 @@ static OSSL_HTTP_REQ_CTX *http_req_ctx_new(int free_wbio, BIO *wbio, BIO *rbio, static int parse_http_line1(char *line, int *found_keep_alive) { - int i, retcode, err; + int i, retcode; char *code, *reason, *end; if (!CHECK_AND_SKIP_PREFIX(line, HTTP_PREFIX_VERSION)) @@ -487,14 +488,11 @@ static int parse_http_line1(char *line, int *found_keep_alive) case HTTP_STATUS_CODE_FOUND: return retcode; default: - err = HTTP_R_RECEIVED_ERROR; - if (retcode < 400) - err = HTTP_R_STATUS_CODE_UNSUPPORTED; - if (*reason == '\0') - ERR_raise_data(ERR_LIB_HTTP, err, "code=%s", code); - else - ERR_raise_data(ERR_LIB_HTTP, err, "code=%s, reason=%s", code, - reason); + if (retcode < HTTP_STATUS_CODES_NONFATAL_ERROR) { + ERR_raise_data(ERR_LIB_HTTP, HTTP_R_STATUS_CODE_UNSUPPORTED, "code=%s", code); + if (*reason != '\0') + ERR_add_error_data(2, ", reason=", reason); + } /* must return content normally if status >= 400 */ return retcode; } @@ -752,7 +750,8 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx) /* First line in response header */ if (rctx->state == OHS_FIRSTLINE) { - switch (parse_http_line1(buf, &found_keep_alive)) { + i = parse_http_line1(buf, &found_keep_alive); + switch (i) { case HTTP_STATUS_CODE_OK: rctx->state = OHS_HEADERS; goto next_line; @@ -766,8 +765,10 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx) /* redirection is not supported/recommended for POST */ /* fall through */ default: - rctx->state = OHS_HEADERS_ERROR; - goto next_line; /* continue parsing and reporting header */ + /* must return content if status >= 400 */ + rctx->state = i < HTTP_STATUS_CODES_NONFATAL_ERROR + ? OHS_HEADERS_ERROR : OHS_HEADERS; + goto next_line; /* continue parsing, also on HTTP error */ } } key = buf; @@ -864,6 +865,8 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx) got_text ? "text" : "ASN.1"); goto next_line; } + /* discard response content when trace not enabled */ + (void)BIO_reset(rctx->rbio); return 0; } diff --git a/test/http_test.c b/test/http_test.c index eca2a929221..67fed9a05ca 100644 --- a/test/http_test.c +++ b/test/http_test.c @@ -15,6 +15,10 @@ #include "testutil.h" +#define HTTP_STATUS_CODE_OK 200 +#define HTTP_STATUS_CODES_FATAL_ERROR 399 +#define HTTP_STATUS_CODES_NONFATAL_ERROR 400 + static const ASN1_ITEM *x509_it = NULL; static X509 *x509 = NULL; #define RPATH "/path" @@ -32,6 +36,8 @@ typedef struct { * For POST, copy request headers+body from mem BIO |in| as response to |out|. * For GET, redirect to RPATH unless already there, else use |content_type| and * respond with |txt| if not NULL, else with |rsp| of ASN1 type |it|. + * Take the status code suggsted by the client via special prefix of the path. + * On fatal status, respond with empty content. * Response hdr has HTTP version 1.|version| and |keep_alive| (unless implicit). */ static int mock_http_server(BIO *in, BIO *out, char version, int keep_alive, @@ -40,15 +46,27 @@ static int mock_http_server(BIO *in, BIO *out, char version, int keep_alive, { const char *req, *path; long count = BIO_get_mem_data(in, (unsigned char **)&req); - const char *hdr = (char *)req; + const char *hdr = (char *)req, *suggested_status; + char status[4] = "200"; int len; int is_get = count >= 4 && CHECK_AND_SKIP_PREFIX(hdr, "GET "); - /* first line should contain "(GET|POST) HTTP/1.x" */ + /* first line should contain "(GET|POST) (/)?/ HTTP/1.x" */ if (!is_get && !(TEST_true(count >= 5 && CHECK_AND_SKIP_PREFIX(hdr, "POST ")))) return 0; + /* get any status code string to be returned suggested by test client */ + if (*hdr == '/') { + suggested_status = ++hdr; + while (*hdr >= '0' && *hdr <= '9') + hdr++; + if (hdr == suggested_status + sizeof(status) - 1) + strncpy(status, suggested_status, sizeof(status) - 1); + else + hdr = suggested_status - 1; + } + path = hdr; hdr = strchr(hdr, ' '); if (hdr == NULL) @@ -74,12 +92,19 @@ static int mock_http_server(BIO *in, BIO *out, char version, int keep_alive, "Location: %s\r\n\r\n", version, RPATH) > 0; /* same server */ } - if (BIO_printf(out, "HTTP/1.%c 200 OK\r\n", version) <= 0) + if (BIO_printf(out, "HTTP/1.%c %s %s\r\n", version, status, + /* mock some reason string: */ + strcmp(status, "200") == 0 ? "OK" : + strcmp(status, "400") >= 0 ? "error" : "fatal") <= 0) return 0; if ((version == '0') == keep_alive) /* otherwise, default */ if (BIO_printf(out, "Connection: %s\r\n", version == '0' ? "keep-alive" : "close") <= 0) return 0; + + if (strcmp(status, "399") == 0) /* HTTP_STATUS_CODES_FATAL_ERROR */ + return BIO_puts(out, "\r\n") == 2; /* empty content */ + if (is_get) { /* construct new header and body */ if (txt != NULL) len = strlen(txt); @@ -122,7 +147,7 @@ static long http_bio_cb_ex(BIO *bio, int oper, const char *argp, size_t len, #define DOCTYPE_HTML "\n" /* do_get > 1 used for testing redirection */ -static int test_http_method(int do_get, int do_txt) +static int test_http_method(int do_get, int do_txt, int suggested_status) { BIO *wbio = BIO_new(BIO_s_mem()); BIO *rbio = BIO_new(BIO_s_mem()); @@ -134,7 +159,7 @@ static int test_http_method(int do_get, int do_txt) int res = 0; int real_server = do_txt && 0; /* remove "&& 0" for using real server */ - snprintf(path, sizeof(path), "%s", + snprintf(path, sizeof(path), "/%d%s", suggested_status, do_get > 1 ? "/will-be-redirected" : RPATH); if (do_txt) { content_type = "text/plain"; @@ -176,6 +201,10 @@ static int test_http_method(int do_get, int do_txt) req, content_type, !do_txt /* expect_asn1 */, OSSL_HTTP_DEFAULT_MAX_RESP_LEN, 0 /* timeout */, 0 /* keep_alive */); + if (!TEST_int_eq(suggested_status == HTTP_STATUS_CODES_FATAL_ERROR, rsp == NULL)) + goto err; + if (suggested_status == HTTP_STATUS_CODES_FATAL_ERROR) + res = 1; if (rsp != NULL) { if (do_get && real_server) { char rtext[sizeof(DOCTYPE_HTML)]; @@ -362,32 +391,52 @@ static int test_http_url_invalid_path(void) static int test_http_get_txt(void) { - return test_http_method(1 /* GET */, 1); + return test_http_method(1 /* GET */, 1, HTTP_STATUS_CODE_OK); } static int test_http_get_txt_redirected(void) { - return test_http_method(2 /* GET with redirection */, 1); + return test_http_method(2 /* GET with redirection */, 1, HTTP_STATUS_CODE_OK); +} + +static int test_http_get_txt_fatal_status(void) +{ + return test_http_method(1 /* GET */, 1, HTTP_STATUS_CODES_FATAL_ERROR); +} + +static int test_http_get_txt_error_status(void) +{ + return test_http_method(1 /* GET */, 1, HTTP_STATUS_CODES_NONFATAL_ERROR); } static int test_http_post_txt(void) { - return test_http_method(0 /* POST */, 1); + return test_http_method(0 /* POST */, 1, HTTP_STATUS_CODE_OK); } static int test_http_get_x509(void) { - return test_http_method(1 /* GET */, 0); + return test_http_method(1 /* GET */, 0, HTTP_STATUS_CODE_OK); } static int test_http_get_x509_redirected(void) { - return test_http_method(2 /* GET with redirection */, 0); + return test_http_method(2 /* GET with redirection */, 0, HTTP_STATUS_CODE_OK); } static int test_http_post_x509(void) { - return test_http_method(0 /* POST */, 0); + return test_http_method(0 /* POST */, 0, HTTP_STATUS_CODE_OK); +} + +static int test_http_post_x509_fatal_status(void) +{ + return test_http_method(0 /* POST */, 0, HTTP_STATUS_CODES_FATAL_ERROR); +} + +static int test_http_post_x509_error_status(void) +{ + return test_http_method(0 /* POST */, 0, HTTP_STATUS_CODES_NONFATAL_ERROR); } static int test_http_keep_alive_0_no_no(void) @@ -520,10 +569,15 @@ int setup_tests(void) ADD_TEST(test_http_get_txt); ADD_TEST(test_http_get_txt_redirected); + ADD_TEST(test_http_get_txt_fatal_status); + ADD_TEST(test_http_get_txt_error_status); ADD_TEST(test_http_post_txt); ADD_TEST(test_http_get_x509); ADD_TEST(test_http_get_x509_redirected); ADD_TEST(test_http_post_x509); + ADD_TEST(test_http_post_x509_fatal_status); + ADD_TEST(test_http_post_x509_error_status); + ADD_TEST(test_http_keep_alive_0_no_no); ADD_TEST(test_http_keep_alive_1_no_no); ADD_TEST(test_http_keep_alive_0_prefer_yes);