static locations longer than 255 characters lead to unexpected behaviors

[Max Laverse]

Description
Defining locations with paths longer than 255 characters leads to unexpected behaviors with some or almost all locations being ignored.

Probable Root Cause
When Nginx starts, it copies the locations read from the configuration into a tree. The locations are initially stored in a structure allowing large strings, but once copied in ngx_http_create_locations_tree(), the variable holding the length of the location is cast to an u_char. For static locations having a long path, the original size is eventually altered leading to a misrepresentation of the data and unexpected results.

How to reproduce
Using the following configuration:

daemon off;
master_process off;
error_log /dev/stderr debug;

events {
  worker_connections 4096;
}

http {
  server {
    server_name localhost;

    location = /43210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321 {
      return 200;
    }

    location = /543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321 {
      return 200;
    }

    location = /6543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321 {
      return 200;
    }

    location / {
      return 500;
    }
  }
}

The expected result would be 200 if you call any of the first 3 URLs. The actual result in my environment is 200, 200, 500.

If I add this additional block:

location = /76543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210987654321 {
  return 200;
}

all URLs now returns 500.

[Maxim Dounin]

Thanks for the report.

Indeed, the code does not expect a location prefix to be longer than 255 bytes, and handles this incorrectly. The most simple fix would be to use u_short instead. Patch:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1673665947 -10800
#      Sat Jan 14 06:12:27 2023 +0300
# Node ID 369a97def1983d4e4dbed8d1b0692cdca485e091
# Parent  07b0bee87f32be91a33210bc06973e07c4c1dac9
Fixed handling of very long locations (ticket #2435).

Previously, location prefix length in ngx_http_location_tree_node_t was
stored as "u_char", and therefore location prefixes longer than 255 bytes
were handled incorrectly.

Fix is to use "u_short" instead.  With "u_short", prefixes up to 65535 bytes
can be safely used, and this isn't reachable due to NGX_CONF_BUFFER, which
is 4096 bytes.

diff --git a/src/http/ngx_http.c b/src/http/ngx_http.c
--- a/src/http/ngx_http.c
+++ b/src/http/ngx_http.c
@@ -1130,7 +1130,7 @@ ngx_http_create_locations_tree(ngx_conf_
     node->auto_redirect = (u_char) ((lq->exact && lq->exact->auto_redirect)
                            || (lq->inclusive && lq->inclusive->auto_redirect));
 
-    node->len = (u_char) len;
+    node->len = (u_short) len;
     ngx_memcpy(node->name, &lq->name->data[prefix], len);
 
     ngx_queue_split(locations, q, &tail);
diff --git a/src/http/ngx_http_core_module.h b/src/http/ngx_http_core_module.h
--- a/src/http/ngx_http_core_module.h
+++ b/src/http/ngx_http_core_module.h
@@ -463,8 +463,8 @@ struct ngx_http_location_tree_node_s {
     ngx_http_core_loc_conf_t        *exact;
     ngx_http_core_loc_conf_t        *inclusive;
 
+    u_short                          len;
     u_char                           auto_redirect;
-    u_char                           len;
     u_char                           name[1];
 };
 

Review and testing appreciated.

[Max Laverse]

Thanks for the quick reply.

I don't know if more changes are needed to fix a similar problem in different conditions, but those two changes make sense to me. I confirmed with my test case and additional debugging log lines that it fixes the issue I was seeing. I also tried to increase the location's size close to 4096 which worked without trouble until I eventually caused a configuration loading error "too long parameter", and that's expected according to your previous message.

[Maxim Dounin]

Thanks for testing. Could you please share some details about the use case where you've encountered the issue? It's somewhat uncommon to use locations with such long prefixes, and it would be interesting to know how these are used in practice.

[Max Laverse]

Sure. We're using Nginx within our Ingress Controllers instance for Kubernetes clusters. It's not the version provided by nginx.com, but the one from the Kubernetes project: ​https://github.com/kubernetes/ingress-nginx

The Ingress Controller offers an external authentication feature based on the ngx_http_auth_request_module. When enabled, this feature exposes the authentication service through an internal location block under the same server{}. The path of the location is generated by the controller, and it's the base64 value of the path you actually want to protect.

In our case, we encountered this issue with a regexp that contained a lot of different patterns: "/base-path/(pattern1|pattern2|pattern3|...)". With a long regular expression and base64 encoding on top of it + some prefixes and suffixes added by the controller, you eventually end up with this internal authentication location being longer than 255 characters.

[Maxim Dounin]

Thanks for the details.

Just for the record, I've submitted the patch for review:

​https://mailman.nginx.org/pipermail/nginx-devel/2023-January/VHNST4UCT7ARWHKIQ4CDFSBHDPULUFHN.html

[Maxim Dounin <mdounin@…>]

In 8121:4eb1383f6432/nginx:

Fixed handling of very long locations (ticket #2435).

Previously, location prefix length in ngx_http_location_tree_node_t was
stored as "u_char", and therefore location prefixes longer than 255 bytes
were handled incorrectly.

Fix is to use "u_short" instead. With "u_short", prefixes up to 65535 bytes
can be safely used, and this isn't reachable due to NGX_CONF_BUFFER, which
is 4096 bytes.

[Maxim Dounin]

Fix committed, thanks for reporting this.