Require the use of CRLF in chunked message body (#12149)

* Require the use of CRLF in chunked message body

* Fix docs

Author: maskit

doc/admin-guide/files/records.yaml.en.rst

@@ -1028,6 +1028,15 @@ allow-plain
    for details about chunked trailers. By default, this option is enabled
    and therefore |TS| will drop chunked trailers.
 
+.. ts:cv:: CONFIG proxy.config.http.strict_chunk_parsing INT 1
+   :reloadable:
+   :overridable:
+
+   Specifies whether |TS| strictly checks errors in chunked message body.
+   If enabled (``1``), |TS| returns 400 Bad Request if chunked message body is
+   not compliant with RFC 9112. If disabled (``0``),  |TS| allows using LF as
+   a line terminator.
+
 .. ts:cv:: CONFIG proxy.config.http.send_http11_requests INT 1
    :reloadable:
    :overridable:

doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst

@@ -112,6 +112,7 @@ TSOverridableConfigKey Value                                              Config
 :enumerator:`TS_CONFIG_HTTP_CACHE_WHEN_TO_REVALIDATE`                   :ts:cv:`proxy.config.http.cache.when_to_revalidate`
 :enumerator:`TS_CONFIG_HTTP_CHUNKING_ENABLED`                           :ts:cv:`proxy.config.http.chunking_enabled`
 :enumerator:`TS_CONFIG_HTTP_CHUNKING_SIZE`                              :ts:cv:`proxy.config.http.chunking.size`
+:enumerator:`TS_CONFIG_HTTP_STRICT_CHUNK_PARSING`                       :ts:cv:`proxy.config.http.strict_chunk_parsing`
 :enumerator:`TS_CONFIG_HTTP_DROP_CHUNKED_TRAILERS`                      :ts:cv:`proxy.config.http.drop_chunked_trailers`
 :enumerator:`TS_CONFIG_HTTP_CONNECT_ATTEMPTS_MAX_RETRIES_DOWN_SERVER`   :ts:cv:`proxy.config.http.connect_attempts_max_retries_down_server`
 :enumerator:`TS_CONFIG_HTTP_CONNECT_ATTEMPTS_MAX_RETRIES`               :ts:cv:`proxy.config.http.connect_attempts_max_retries`

doc/developer-guide/api/types/TSOverridableConfigKey.en.rst

@@ -91,6 +91,7 @@ Enumeration Members
 .. enumerator:: TS_CONFIG_NET_SOCK_PACKET_TOS_OUT
 .. enumerator:: TS_CONFIG_HTTP_INSERT_AGE_IN_RESPONSE
 .. enumerator:: TS_CONFIG_HTTP_CHUNKING_SIZE
+.. enumerator:: TS_CONFIG_HTTP_STRICT_CHUNK_PARSING
 .. enumerator:: TS_CONFIG_HTTP_DROP_CHUNKED_TRAILERS
 .. enumerator:: TS_CONFIG_HTTP_FLOW_CONTROL_ENABLED
 .. enumerator:: TS_CONFIG_HTTP_FLOW_CONTROL_LOW_WATER_MARK

include/proxy/http/HttpConfig.h

@@ -655,6 +655,7 @@ struct OverridableHttpConfigParams {
 
   MgmtInt  http_chunking_size         = 4096; ///< Maximum chunk size for chunked output.
   MgmtByte http_drop_chunked_trailers = 1;    ///< Whether to drop chunked trailers.
+  MgmtByte http_strict_chunk_parsing  = 1;    ///< Whether to parse chunked body strictly.
   MgmtInt  flow_high_water_mark       = 0;    ///< Flow control high water mark.
   MgmtInt  flow_low_water_mark        = 0;    ///< Flow control low water mark.
 

include/proxy/http/HttpTunnel.h

@@ -111,6 +111,8 @@ struct ChunkedHandler {
    */
   bool drop_chunked_trailers = false;
 
+  bool strict_chunk_parsing = true;
+
   bool truncation = false;
 
   /** The number of bytes to skip from the reader because they are not body bytes.
@@ -127,8 +129,10 @@ struct ChunkedHandler {
   int          last_server_event    = VC_EVENT_NONE;
 
   // Chunked header size parsing info.
-  int running_sum = 0;
-  int num_digits  = 0;
+  int  running_sum = 0;
+  int  num_digits  = 0;
+  int  num_cr      = 0;
+  bool prev_is_cr  = false;
 
   /// @name Output data.
   //@{
@@ -143,8 +147,8 @@ struct ChunkedHandler {
   //@}
   ChunkedHandler();
 
-  void init(IOBufferReader *buffer_in, HttpTunnelProducer *p, bool drop_chunked_trailers);
-  void init_by_action(IOBufferReader *buffer_in, Action action, bool drop_chunked_trailers);
+  void init(IOBufferReader *buffer_in, HttpTunnelProducer *p, bool drop_chunked_trailers, bool strict_parsing);
+  void init_by_action(IOBufferReader *buffer_in, Action action, bool drop_chunked_trailers, bool strict_parsing);
   void clear();
 
   /// Set the max chunk @a size.
@@ -389,6 +393,7 @@ class HttpTunnel : public Continuation
 
   /// A named variable for the @a drop_chunked_trailers parameter to @a set_producer_chunking_action.
   static constexpr bool DROP_CHUNKED_TRAILERS = true;
+  static constexpr bool PARSE_CHUNK_STRICTLY  = true;
 
   /** Designate chunking behavior to the producer.
    *
@@ -399,9 +404,10 @@ class HttpTunnel : public Continuation
    * @param[in] drop_chunked_trailers If @c true, chunked trailers are filtered
    *   out. Logically speaking, this is only applicable when proxying chunked
    *   content, thus only when @a action is @c TCA_PASSTHRU_CHUNKED_CONTENT.
+   * @param[in] parse_chunk_strictly If @c true, no parse error will be allowed
    */
   void set_producer_chunking_action(HttpTunnelProducer *p, int64_t skip_bytes, TunnelChunkingAction_t action,
-                                    bool drop_chunked_trailers);
+                                    bool drop_chunked_trailers, bool parse_chunk_strictly);
   /// Set the maximum (preferred) chunk @a size of chunked output for @a producer.
   void set_producer_chunking_size(HttpTunnelProducer *producer, int64_t size);
 
@@ -480,6 +486,9 @@ class HttpTunnel : public Continuation
   /// Corresponds to proxy.config.http.drop_chunked_trailers having a value of 1.
   bool http_drop_chunked_trailers = false;
 
+  /// Corresponds to proxy.config.http.strict_chunk_parsing having a value of 1.
+  bool http_strict_chunk_parsing = false;
+
   /** The number of body bytes processed in this last execution of the tunnel.
    *
    * This accounting is used to determine how many bytes to copy into the body

include/ts/apidefs.h.in

@@ -893,6 +893,7 @@ enum TSOverridableConfigKey {
   TS_CONFIG_HTTP_NO_DNS_JUST_FORWARD_TO_PARENT,
   TS_CONFIG_HTTP_CACHE_IGNORE_QUERY,
   TS_CONFIG_HTTP_DROP_CHUNKED_TRAILERS,
+  TS_CONFIG_HTTP_STRICT_CHUNK_PARSING,
   TS_CONFIG_LAST_ENTRY,
 };
 

plugins/lua/ts_lua_http_config.cc

@@ -149,6 +149,7 @@ typedef enum {
   TS_LUA_CONFIG_NET_DEFAULT_INACTIVITY_TIMEOUT                = TS_CONFIG_NET_DEFAULT_INACTIVITY_TIMEOUT,
   TS_LUA_CONFIG_HTTP_NO_DNS_JUST_FORWARD_TO_PARENT            = TS_CONFIG_HTTP_NO_DNS_JUST_FORWARD_TO_PARENT,
   TS_LUA_CONFIG_HTTP_CACHE_IGNORE_QUERY                       = TS_CONFIG_HTTP_CACHE_IGNORE_QUERY,
+  TS_LUA_CONFIG_HTTP_STRICT_CHUNK_PARSING                     = TS_CONFIG_HTTP_STRICT_CHUNK_PARSING,
   TS_LUA_CONFIG_LAST_ENTRY                                    = TS_CONFIG_LAST_ENTRY,
 } TSLuaOverridableConfigKey;
 
@@ -289,6 +290,7 @@ ts_lua_var_item ts_lua_http_config_vars[] = {
   TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_NET_DEFAULT_INACTIVITY_TIMEOUT),
   TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_NO_DNS_JUST_FORWARD_TO_PARENT),
   TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_CACHE_IGNORE_QUERY),
+  TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_STRICT_CHUNK_PARSING),
   TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_LAST_ENTRY),
 };
 

src/api/InkAPI.cc

@@ -7136,6 +7136,9 @@ _conf_to_memberp(TSOverridableConfigKey conf, OverridableHttpConfigParams *overr
   case TS_CONFIG_HTTP_DROP_CHUNKED_TRAILERS:
     ret = _memberp_to_generic(&overridableHttpConfig->http_drop_chunked_trailers, conv);
     break;
+  case TS_CONFIG_HTTP_STRICT_CHUNK_PARSING:
+    ret = _memberp_to_generic(&overridableHttpConfig->http_strict_chunk_parsing, conv);
+    break;
   case TS_CONFIG_HTTP_FLOW_CONTROL_ENABLED:
     ret = _memberp_to_generic(&overridableHttpConfig->flow_control_enabled, conv);
     break;

src/api/InkAPITest.cc

@@ -8715,7 +8715,8 @@ std::array<std::string_view, TS_CONFIG_LAST_ENTRY> SDK_Overridable_Configs = {
    "proxy.config.body_factory.response_suppression_mode", "proxy.config.http.parent_proxy.enable_parent_timeout_markdowns",
    "proxy.config.http.parent_proxy.disable_parent_markdowns", "proxy.config.net.default_inactivity_timeout",
    "proxy.config.http.no_dns_just_forward_to_parent", "proxy.config.http.cache.ignore_query",
-   "proxy.config.http.drop_chunked_trailers", }
+   "proxy.config.http.drop_chunked_trailers", "proxy.config.http.strict_chunk_parsing",
+   }
 };
 
 extern ClassAllocator<HttpSM> httpSMAllocator;

src/proxy/FetchSM.cc

@@ -228,7 +228,8 @@ FetchSM::check_chunked()
 
     if (resp_is_chunked && (fetch_flags & TS_FETCH_FLAGS_DECHUNK)) {
       ChunkedHandler *ch = &chunked_handler;
-      ch->init_by_action(resp_reader, ChunkedHandler::ACTION_DECHUNK, HttpTunnel::DROP_CHUNKED_TRAILERS);
+      ch->init_by_action(resp_reader, ChunkedHandler::ACTION_DECHUNK, HttpTunnel::DROP_CHUNKED_TRAILERS,
+                         HttpTunnel::PARSE_CHUNK_STRICTLY);
       ch->dechunked_reader = ch->dechunked_buffer->alloc_reader();
       ch->state            = ChunkedHandler::CHUNK_READ_SIZE;
       resp_reader->dealloc();