security: ngx.req.set_header(): now we always escape bytes in header names and header values which are prohibited by RFC 7230.

See https://tools.ietf.org/html/rfc7230#section-3.2.6 for more details.

Signed-off-by: Yichun Zhang (agentzh) <[email protected]>

Author: zhuizhuhaomeng

README.markdown

@@ -4552,9 +4552,9 @@ or a Lua table holding the query arguments' key-value pairs, as in
  ngx.req.set_uri_args({ a = 3, b = "hello world" })
 ```
 
-In the former case, i.e., when the whole query-strng is provided directly,
+In the former case, i.e., when the whole query-string is provided directly,
 the input Lua string should already be well-formed with the URI encoding.
-For security considerations, his method will autoamticaly escape any control and
+For security considerations, this method will automatically escape any control and
 whitespace characters (ASCII code 0x00 ~ 0x32 and 0x7F) in the Lua string.
 
 In the latter case, this method will escape argument keys and values according to the URI escaping rule.
@@ -4883,6 +4883,11 @@ ngx.req.set_header
 
 Set the current request's request header named `header_name` to value `header_value`, overriding any existing ones.
 
+The input Lua string `header_name` and `header_value` should already be well-formed with the URI encoding.
+For security considerations, this method will automatically escape " ", """, "(", ")", ",", "/", ":", ";", "?",
+"<", "=", ">", "?", "@", "[", "]", "\", "{", "}", 0x00-0x1F, 0x7F-0xFF in `header_name` and automatically escape
+"0x00-0x08, 0x0A-0x0F, 0x7F in `header_value`.
+
 By default, all the subrequests subsequently initiated by [ngx.location.capture](#ngxlocationcapture) and [ngx.location.capture_multi](#ngxlocationcapture_multi) will inherit the new header.
 
 Here is an example of setting the `Content-Type` header:

doc/HttpLuaModule.wiki

@@ -4084,6 +4084,11 @@ The <code>__index</code> metamethod will not be added when the <code>raw</code>
 
 Set the current request's request header named <code>header_name</code> to value <code>header_value</code>, overriding any existing ones.
 
+The input Lua string `header_name` and `header_value` should already be well-formed with the URI encoding.
+For security considerations, this method will automatically escape " ", """, "(", ")", ",", "/", ":", ";", "?",
+"<", "=", ">", "?", "@", "[", "]", "\", "{", "}", 0x00-0x1F, 0x7F-0xFF in `header_name` and automatically escape
+"0x00-0x08, 0x0A-0x0F, 0x7F in `header_value`.
+
 By default, all the subrequests subsequently initiated by [[#ngx.location.capture|ngx.location.capture]] and [[#ngx.location.capture_multi|ngx.location.capture_multi]] will inherit the new header.
 
 Here is an example of setting the <code>Content-Type</code> header:

src/ngx_http_lua_headers_in.c

@@ -653,16 +653,18 @@ ngx_http_lua_set_input_header(ngx_http_request_t *r, ngx_str_t key,
 {
     ngx_http_lua_header_val_t         hv;
     ngx_http_lua_set_header_t        *handlers = ngx_http_lua_set_handlers;
-
+    ngx_int_t                         rc;
     ngx_uint_t                        i;
 
     dd("set header value: %.*s", (int) value.len, value.data);
 
-    if (ngx_http_lua_check_unsafe_string(r, key.data, key.len,
-                                         "header name") != NGX_OK
-        || ngx_http_lua_check_unsafe_string(r, value.data, value.len,
-                                            "header value") != NGX_OK)
-    {
+    rc = ngx_http_lua_copy_escaped_header(r, &key, 1);
+    if (rc != NGX_OK) {
+        return NGX_ERROR;
+    }
+
+    rc = ngx_http_lua_copy_escaped_header(r, &value, 0);
+    if (rc != NGX_OK) {
         return NGX_ERROR;
     }
 

src/ngx_http_lua_headers_out.c

@@ -491,11 +491,11 @@ ngx_http_lua_set_output_header(ngx_http_request_t *r, ngx_http_lua_ctx_t *ctx,
 
     dd("set header value: %.*s", (int) value.len, value.data);
 
-    if (ngx_http_lua_check_unsafe_string(r, key.data, key.len,
-                                         "header name") != NGX_OK
-        || ngx_http_lua_check_unsafe_string(r, value.data, value.len,
-                                            "header value") != NGX_OK)
-    {
+    if (ngx_http_lua_copy_escaped_header(r, &key, 1) != NGX_OK) {
+        return NGX_ERROR;
+    }
+
+    if (ngx_http_lua_copy_escaped_header(r, &value, 0) != NGX_OK) {
         return NGX_ERROR;
     }
 

src/ngx_http_lua_util.c

@@ -2008,8 +2008,52 @@ ngx_http_lua_escape_uri(u_char *dst, u_char *src, size_t size, ngx_uint_t type)
 
                     /* mail_auth is the same as memcached */
 
+                    /* " ", """, "(", ")", ",", "/", ":", ";", "?",
+                     * "<", "=", ">", "?", "@", "[", "]", "\", "{",
+                     * "}", %00-%1F, %7F-%FF
+                     */
+
+    static uint32_t   header_name[] = {
+        0xffffffff, /* 1111 1111 1111 1111  1111 1111 1111 1111 */
+
+                    /* ?>=< ;:98 7654 3210  /.-, +*)( '&%$ #"!  */
+        0xfc009305, /* 1111 1100 0000 0000  1001 0011 0000 0101 */
+
+                    /* _^]\ [ZYX WVUT SRQP  ONML KJIH GFED CBA@ */
+        0x38000001, /* 0011 1000 0000 0000  0000 0000 0000 0001 */
+
+                    /*  ~}| {zyx wvut srqp  onml kjih gfed cba` */
+        0xa8000000, /* 1010 1000 0000 0000  0000 0000 0000 0000 */
+
+        0xffffffff, /* 1111 1111 1111 1111  1111 1111 1111 1111 */
+        0xffffffff, /* 1111 1111 1111 1111  1111 1111 1111 1111 */
+        0xffffffff, /* 1111 1111 1111 1111  1111 1111 1111 1111 */
+        0xffffffff, /* 1111 1111 1111 1111  1111 1111 1111 1111 */
+    };
+
+                    /* "%00-%08, %0A-%0F, %7F */
+
+    static uint32_t   header_value[] = {
+        0xfffffdff, /* 1111 1111 1111 1111  1111 1101 1111 1111 */
+
+                    /* ?>=< ;:98 7654 3210  /.-, +*)( '&%$ #"!  */
+        0x00000000, /* 0000 0000 0000 0000  0000 0000 0000 0000 */
+
+                    /* _^]\ [ZYX WVUT SRQP  ONML KJIH GFED CBA@ */
+        0x00000000, /* 0000 0000 0000 0000  0000 0000 0000 0000 */
+
+                    /*  ~}| {zyx wvut srqp  onml kjih gfed cba` */
+        0x80000000, /* 1000 0000 0000 0000  0000 0000 0000 0000 */
+
+        0x00000000, /* 0000 0000 0000 0000  0000 0000 0000 0000 */
+        0x00000000, /* 0000 0000 0000 0000  0000 0000 0000 0000 */
+        0x00000000, /* 0000 0000 0000 0000  0000 0000 0000 0000 */
+        0x00000000, /* 0000 0000 0000 0000  0000 0000 0000 0000 */
+    };
+
     static uint32_t  *map[] =
-        { uri, args, uri_component, html, refresh, memcached, memcached };
+        { uri, args, uri_component, html, refresh, memcached, memcached,
+          header_name, header_value };
 
     escape = map[type];
 
@@ -4328,4 +4372,38 @@ ngx_http_lua_escape_log(u_char *dst, u_char *src, size_t size)
 }
 
 
+ngx_int_t
+ngx_http_lua_copy_escaped_header(ngx_http_request_t *r,
+    ngx_str_t *dst, int is_name)
+{
+    size_t       escape;
+    size_t       len;
+    u_char      *data;
+    int          type;
+
+    type = is_name
+        ? NGX_HTTP_LUA_ESCAPE_HEADER_NAME : NGX_HTTP_LUA_ESCAPE_HEADER_VALUE;
+
+    data = dst->data;
+    len = dst->len;
+
+    escape = ngx_http_lua_escape_uri(NULL, data, len, type);
+    if (escape > 0) {
+        /*
+         * we allocate space for the trailling '\0' char here because nginx
+         * header values must be null-terminated
+         */
+        dst->data = ngx_palloc(r->pool, len + 2 * escape + 1);
+        if (dst->data == NULL) {
+            return NGX_ERROR;
+        }
+
+        ngx_http_lua_escape_uri(dst->data, data, len, type);
+        dst->len = len + 2 * escape;
+        dst->data[dst->len] = '\0';
+    }
+
+    return NGX_OK;
+}
+
 /* vi:set ft=c ts=4 sw=4 et fdm=marker: */

src/ngx_http_lua_util.h

@@ -27,6 +27,9 @@
 #   define NGX_HTTP_SWITCHING_PROTOCOLS 101
 #endif
 
+#define NGX_HTTP_LUA_ESCAPE_HEADER_NAME  7
+
+#define NGX_HTTP_LUA_ESCAPE_HEADER_VALUE  8
 
 /* key in Lua vm registry for all the "ngx.ctx" tables */
 #define ngx_http_lua_ctx_tables_key  "ngx_lua_ctx_tables"
@@ -169,6 +172,9 @@ void ngx_http_lua_unescape_uri(u_char **dst, u_char **src, size_t size,
 uintptr_t ngx_http_lua_escape_uri(u_char *dst, u_char *src,
     size_t size, ngx_uint_t type);
 
+ngx_int_t ngx_http_lua_copy_escaped_header(ngx_http_request_t *r,
+    ngx_str_t *dst, int is_name);
+
 void ngx_http_lua_inject_req_api(ngx_log_t *log, lua_State *L);
 
 void ngx_http_lua_process_args_option(ngx_http_request_t *r,

t/016-resp-header.t

@@ -8,7 +8,7 @@ use Test::Nginx::Socket::Lua;
 
 repeat_each(2);
 
-plan tests => repeat_each() * (blocks() * 3 + 80);
+plan tests => repeat_each() * (blocks() * 3 + 77);
 
 #no_diff();
 no_long_string();
@@ -51,7 +51,7 @@ Content-Type: text/html
 
 
 
-=== TEST 3: set response content-type header
+=== TEST 3: set response content-length header
 --- config
     location /read {
         content_by_lua '
@@ -1976,14 +1976,12 @@ Content-Type: application/json
     }
 --- request
 GET /t
---- error_code: 500
 --- response_headers
-header:
+header: value%0Dfoo:bar%0Abar:foo
 foo:
 bar:
---- error_log
-unsafe byte "0xd" in header value "value\x0Dfoo:bar\x0Abar:foo"
-failed to set header
+--- no_error_log
+[error]
 
 
 
@@ -1997,14 +1995,12 @@ failed to set header
     }
 --- request
 GET /t
---- error_code: 500
 --- response_headers
-header:
+header: value%0Afoo:bar%0Dbar:foo
 foo:
 bar:
---- error_log
-unsafe byte "0xa" in header value "value\x0Afoo:bar\x0Dbar:foo"
-failed to set header
+--- no_error_log
+[error]
 
 
 
@@ -2018,14 +2014,13 @@ failed to set header
     }
 --- request
 GET /t
---- error_code: 500
 --- response_headers
+header%3A%20value%0Dfoo%3Abar%0Abar%3Afoo: xx
 header:
 foo:
 bar:
---- error_log
-unsafe byte "0xd" in header name "header: value\x0Dfoo:bar\x0Abar:foo"
-failed to set header
+--- no_error_log
+[error]
 
 
 
@@ -2039,14 +2034,13 @@ failed to set header
     }
 --- request
 GET /t
---- error_code: 500
 --- response_headers
+header%3A%20value%0Afoo%3Abar%0Dbar%3Afoo: xx
 header:
 foo:
 bar:
---- error_log
-unsafe byte "0xa" in header name "header: value\x0Afoo:bar\x0Dbar:foo"
-failed to set header
+--- no_error_log
+[error]
 
 
 
@@ -2060,14 +2054,13 @@ failed to set header
     }
 --- request
 GET /t
---- error_code: 500
 --- response_headers
+%0Dheader%3A%20value%0Dfoo%3Abar%0Abar%3Afoo: xx
 header:
 foo:
 bar:
---- error_log
-unsafe byte "0xd" in header name "\x0Dheader: value\x0Dfoo:bar\x0Abar:foo"
-failed to set header
+--- no_error_log
+[error]
 
 
 
@@ -2081,14 +2074,13 @@ failed to set header
     }
 --- request
 GET /t
---- error_code: 500
 --- response_headers
+%0Aheader%3A%20value%0Afoo%3Abar%0Dbar%3Afoo: xx
 header:
 foo:
 bar:
---- error_log
-unsafe byte "0xa" in header name "\x0Aheader: value\x0Afoo:bar\x0Dbar:foo"
-failed to set header
+--- no_error_log
+[error]
 
 
 
@@ -2105,11 +2097,10 @@ failed to set header
     }
 --- request
 GET /t
---- error_code: 500
 --- response_headers
-foo:
 xx:
 xxx:
---- error_log
-unsafe byte "0xa" in header value "foo\x0Axx:bar"
-failed to set header
+--- raw_response_headers_like chomp
+foo: foo%0Axx:bar\r\nfoo: bar%0Dxxx:foo\r\n
+--- no_error_log
+[error]