HTTP: Fix mistakes and unclarities on maxline and max_resp_len params

Also rename internal structure fields iobuf(len) to readbuf(len) for clarity

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13960)

diff --git a/crypto/http/http_client.c b/crypto/http/http_client.c
index b4dfcb81519e8623082756590d461667446e6400..6e75725f07242b65954aa77e2743c3522098ab73 100644 (file)
--- a/crypto/http/http_client.c
+++ b/crypto/http/http_client.c
@@ -42,8 +42,8 @@
 
 struct ossl_http_req_ctx_st {
     int state;                  /* Current I/O state */
-    unsigned char *iobuf;       /* Line buffer */
-    int iobuflen;               /* Line buffer length */
+    unsigned char *readbuf;     /* Buffer for reading response by line */
+    int readbuflen;             /* Buffer length, equals maxline */
     BIO *wbio;                  /* BIO to send request to */
     BIO *rbio;                  /* BIO to read response from */
     BIO *mem;                   /* Memory BIO response is built into */
@@ -57,9 +57,6 @@ struct ossl_http_req_ctx_st {
     char *redirection_url;      /* Location given with HTTP status 301/302 */
 };
 
-#define HTTP_DEFAULT_MAX_LINE_LENGTH (4 * 1024)
-#define HTTP_DEFAULT_MAX_RESP_LEN (100 * 1024)
-
 /* HTTP states */
 
 #define OHS_NOREAD          0x1000 /* If set no reading should be performed */
@@ -92,12 +89,12 @@ OSSL_HTTP_REQ_CTX *OSSL_HTTP_REQ_CTX_new(BIO *wbio, BIO *rbio,
     if ((rctx = OPENSSL_zalloc(sizeof(*rctx))) == NULL)
         return NULL;
     rctx->state = OHS_ERROR;
-    rctx->iobuflen = maxline > 0 ? maxline : HTTP_DEFAULT_MAX_LINE_LENGTH;
-    rctx->iobuf = OPENSSL_malloc(rctx->iobuflen);
+    rctx->readbuflen = maxline > 0 ? maxline : HTTP_DEFAULT_MAX_LINE_LENGTH;
+    rctx->readbuf = OPENSSL_malloc(rctx->readbuflen);
     rctx->wbio = wbio;
     rctx->rbio = rbio;
     rctx->mem = BIO_new(BIO_s_mem());
-    if (rctx->iobuf == NULL || rctx->mem == NULL) {
+    if (rctx->readbuf == NULL || rctx->mem == NULL) {
         OSSL_HTTP_REQ_CTX_free(rctx);
         return NULL;
     }
@@ -115,7 +112,7 @@ void OSSL_HTTP_REQ_CTX_free(OSSL_HTTP_REQ_CTX *rctx)
     if (rctx == NULL)
         return;
     BIO_free(rctx->mem); /* this may indirectly call ERR_clear_error() */
-    OPENSSL_free(rctx->iobuf);
+    OPENSSL_free(rctx->readbuf);
     OPENSSL_free(rctx);
 }
 
@@ -409,7 +406,8 @@ static int check_set_resp_len(OSSL_HTTP_REQ_CTX *rctx, unsigned long len)
                        "length=%lu, max=%lu", len, rctx->max_resp_len);
     if (rctx->resp_len != 0 && rctx->resp_len != len)
         ERR_raise_data(ERR_LIB_HTTP, HTTP_R_INCONSISTENT_CONTENT_LENGTH,
-                       "length=%lu, before=%lu", len, rctx->resp_len);
+                       "ASN.1 length=%lu, Content-Length=%lu",
+                       len, rctx->resp_len);
     rctx->resp_len = len;
     return 1;
 }
@@ -434,7 +432,7 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx)
     rctx->redirection_url = NULL;
  next_io:
     if ((rctx->state & OHS_NOREAD) == 0) {
-        n = BIO_read(rctx->rbio, rctx->iobuf, rctx->iobuflen);
+        n = BIO_read(rctx->rbio, rctx->readbuf, rctx->readbuflen);
         if (n <= 0) {
             if (BIO_should_retry(rctx->rbio))
                 return -1;
@@ -442,7 +440,7 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx)
         }
 
         /* Write data to memory BIO */
-        if (BIO_write(rctx->mem, rctx->iobuf, n) != n)
+        if (BIO_write(rctx->mem, rctx->readbuf, n) != n)
             return 0;
     }
 
@@ -513,13 +511,13 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx)
          */
         n = BIO_get_mem_data(rctx->mem, &p);
         if (n <= 0 || memchr(p, '\n', n) == 0) {
-            if (n >= rctx->iobuflen) {
+            if (n >= rctx->readbuflen) {
                 rctx->state = OHS_ERROR;
                 return 0;
             }
             goto next_io;
         }
-        n = BIO_gets(rctx->mem, (char *)rctx->iobuf, rctx->iobuflen);
+        n = BIO_gets(rctx->mem, (char *)rctx->readbuf, rctx->readbuflen);
 
         if (n <= 0) {
             if (BIO_should_retry(rctx->mem))
@@ -529,7 +527,7 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx)
         }
 
         /* Don't allow excessive lines */
-        if (n == rctx->iobuflen) {
+        if (n == rctx->readbuflen) {
             ERR_raise(ERR_LIB_HTTP, HTTP_R_RESPONSE_LINE_TOO_LONG);
             rctx->state = OHS_ERROR;
             return 0;
@@ -537,7 +535,7 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx)
 
         /* First line */
         if (rctx->state == OHS_FIRSTLINE) {
-            switch (parse_http_line1((char *)rctx->iobuf)) {
+            switch (parse_http_line1((char *)rctx->readbuf)) {
             case HTTP_STATUS_CODE_OK:
                 rctx->state = OHS_HEADERS;
                 goto next_line;
@@ -555,7 +553,7 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx)
                 return 0;
             }
         }
-        key = (char *)rctx->iobuf;
+        key = (char *)rctx->readbuf;
         value = strchr(key, ':');
         if (value != NULL) {
             *(value++) = '\0';
@@ -596,8 +594,8 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx)
             }
         }
 
-        /* Look for blank line: end of headers */
-        for (p = rctx->iobuf; *p != '\0'; p++) {
+        /* Look for blank line indicating end of headers */
+        for (p = rctx->readbuf; *p != '\0'; p++) {
             if (*p != '\r' && *p != '\n')
                 break;
         }

diff --git a/doc/man3/OSSL_HTTP_REQ_CTX.pod b/doc/man3/OSSL_HTTP_REQ_CTX.pod
index 3955359978c9ce2050656bbe4e7ffa65cfb9702e..c0c3170edd74724e1381a312a0ea4c93d09eb48c 100644 (file)
--- a/doc/man3/OSSL_HTTP_REQ_CTX.pod
+++ b/doc/man3/OSSL_HTTP_REQ_CTX.pod
@@ -56,10 +56,13 @@ should be preferred.
 OSSL_HTTP_REQ_CTX_new() allocates a new HTTP request context structure,
 which gets populated with the B<BIO> to send the request to (I<wbio>),
 the B<BIO> to read the response from (I<rbio>, which may be equal to I<wbio>),
+the maximum expected response header line length (I<maxline>, where any
+zero or less indicates using the B<HTTP_DEFAULT_MAX_LINE_LENGTH> of 4KiB;
+this length is also used as the number of content bytes read at a time),
 the request method (I<method_POST>, which may be 1 to indicate that the C<POST>
 method is to be used, or 0 to indicate that the C<GET> method is to be used),
-the maximum expected response header length (I<max_resp_len>,
-where any zero or less indicates the default of 4KiB),
+the maximum allowed response content length (I<max_resp_len>, where 0 means
+that the B<HTTP_DEFAULT_MAX_RESP_LEN> is used, which currently is 100 KiB),
 a response timeout measure in seconds (I<timeout>,
 where 0 indicates no timeout, i.e., waiting indefinitely),
 the expected MIME content type of the response (I<expected_content_type>,
@@ -106,13 +109,16 @@ OSSL_HTTP_REQ_CTX_sendreq_d2i() calls OSSL_HTTP_REQ_CTX_nbio(), possibly
 several times until a timeout is reached, and DER decodes the received
 response using the ASN.1 template I<it>.
 
-OSSL_HTTP_REQ_CTX_set_max_response_length() sets the maximum response length
-for I<rctx> to I<len>. If the response exceeds this length an error occurs.
-If not set a default value of 100k is used.
-
 OSSL_HTTP_REQ_CTX_get0_mem_bio() returns the internal memory B<BIO>.  This can
 be used to affect the HTTP request text.  I<Use with caution!>
 
+OSSL_HTTP_REQ_CTX_set_max_response_length() sets the maximum allowed
+response content length for I<rctx> to I<len>. If not set or I<len> is 0
+then the B<HTTP_DEFAULT_MAX_RESP_LEN> is used, which currently is 100 KiB.
+If the C<Content-Length> header is present and exceeds this value or
+the content is an ASN.1 encoded structure with a length exceeding this value
+or both length indications are present but disagree then an error occurs.
+
 =head1 WARNINGS
 
 The server's response may be unexpected if the hostname that was used to

diff --git a/doc/man3/OSSL_HTTP_transfer.pod b/doc/man3/OSSL_HTTP_transfer.pod
index dda59201cf54d113ec79868669673da7e368af86..33ae25685bd3cae0c7ce5d62a20b61d36761b832 100644 (file)
--- a/doc/man3/OSSL_HTTP_transfer.pod
+++ b/doc/man3/OSSL_HTTP_transfer.pod
@@ -123,9 +123,10 @@ while using a proxy for HTTPS connections requires a suitable callback function
 such as OSSL_HTTP_proxy_connect(), described below.
 
 The I<maxline> parameter specifies the response header maximum line length,
-where 0 indicates the default value, which currently is 4k.
+where a value <= 0 indicates using the B<HTTP_DEFAULT_MAX_LINE_LENGTH> of 4KiB.
+This length is also used as the number of content bytes read at a time.
 The I<max_resp_len> parameter specifies the maximum response length,
-where 0 indicates the default value, which currently is 100k.
+where 0 indicates the B<HTTP_DEFAULT_MAX_RESP_LEN>, which currently is 100 KiB.
 
 An ASN.1-encoded response is expected by OSSL_HTTP_get_asn1() and
 OSSL_HTTP_post_asn1(), while for OSSL_HTTP_get() or OSSL_HTTP_transfer()

diff --git a/include/openssl/http.h b/include/openssl/http.h
index b35302289fbfae995cfa917dbfaa230309e0396e..8f9e8c4cfaea61f03ceef1d8045d80e52612c459 100644 (file)
--- a/include/openssl/http.h
+++ b/include/openssl/http.h
@@ -35,6 +35,9 @@ typedef BIO *(*OSSL_HTTP_bio_cb_t)(BIO *bio, void *arg, int connect, int detail)
 # define OPENSSL_HTTP_PROXY "HTTP_PROXY"
 # define OPENSSL_HTTPS_PROXY "HTTPS_PROXY"
 
+#define HTTP_DEFAULT_MAX_LINE_LENGTH (4 * 1024)
+#define HTTP_DEFAULT_MAX_RESP_LEN (100 * 1024)
+
 OSSL_HTTP_REQ_CTX *OSSL_HTTP_REQ_CTX_new(BIO *wbio, BIO *rbio,
                                          int method_GET, int maxline,
                                          unsigned long max_resp_len,