bug: module apisix.utils.redis - Redis connection reuse issue

[jakubzieba]

Current Behavior

When we use multiple rate-limiting plugins that share the same Redis address but connect to different databases or use different credentials (username/password), connection reuse can lead to getting a connection with the wrong database or user/password.

For example, assuming I configure two such limit-count plugins on a routes,

"limit-count": {
    "count": 1,
    "time_window": 30,
    "rejected_code": 429,
    "key": "remote_addr",
    "policy": "redis",
    "redis_host": "redis1.local",
    "redis_port": 6379,
    "redis_database": 1
}

"limit-count": {
    "count": 1,
    "time_window": 30,
    "rejected_code": 429,
    "key": "remote_addr",
    "policy": "redis",
    "redis_host": "redis1.local",
    "redis_port": 6379,
    "redis_database": 2
}

I will randomly get connections from the pool that use the wrong database. The core issue here is that authentication and database selection are executed only during the initial connect, after which the connection is placed back into the pool.

To fix this problem, we could re-authenticate and re-select the database whenever we detect that the connection's current credentials or database don't match the required ones. Alternatively – which in my opinion is a better approach – we can introduce separate connection pools for such connections. We can achieve this by implementing a function that generates a pool name based on the username, password, and database number,

local function get_pool_name(conf)
    local database = conf.redis_database or 0
    local connection_string = conf.redis_host .. ":" .. conf.redis_port .. "/" .. database
    if conf.redis_username and conf.redis_username ~= "" then
        local password = conf.redis_password or ""
        local hashed_credentials = ngx.md5(conf.redis_username .. ":" .. password)
        return hashed_credentials .. "@" .. connection_string
    else
        return connection_string
    end
end

and then use it in sock_opts.

local sock_opts = {
        ...
        pool = get_pool_name(conf)
    }

Expected Behavior

Redis commands must be executed using a connection configured with the appropriate user, password, and database ID, in accordance with the plugin's configuration.

Error Logs

No response

Steps to Reproduce

No description needed.

Environment

It's irrelevant.

[Baoyuantop]

Thanks for the report @jakubzieba . Could you please provide your specific routes and plugin configurations?

In the current implementation, apisix.utils.redis only runs AUTH and SELECT when get_reused_times() == 0. The Redis adapters for limit-count, limit-req, and limit-conn then put the socket back into the keepalive pool. At the same time, the sock_opts passed to red:connect(...) only contains the SSL options and does not set an explicit pool name that includes the database / username / password. So two configurations with the same Redis host and port but different databases or credentials can indeed reuse a connection with the wrong Redis logical context.

[jakubzieba]

We found this error while testing a staging configuration using the limit-count plugin before deploying it to production. We are using Docker / Docker Compose to run APISIX and Redis.
This can be easily reproduced by setting nginx.worker_processes to 1

nginx_config:
  worker_processes: 1

and configuring two routes as follows:

{
  "name": "route-1",
  "uri": "/route-1",
  "upstream": {
    "type": "roundrobin",
    "nodes": {
      "web1:80": 1
    }
  },
  "plugins": {
    "proxy-rewrite": {
      "uri": "/icas-jwt-auth.php"
    },
    "limit-count": {
      "rejected_code": 429,
      "policy": "redis",
      "rejected_msg": "too many requests",
      "redis_host": "redis-master",
      "redis_port": 56379,
      "redis_database": 1,
      "count": 10,
      "time_window": 60,
      "key": "remote_addr"
    }
  }
}

{
  "name": "route-2",
  "uri": "/route-2",
  "upstream": {
    "type": "roundrobin",
    "nodes": {
      "web1:80": 1
    }
  },
  "plugins": {
    "proxy-rewrite": {
      "uri": "/icas-jwt-auth.php"
    },
    "limit-count": {
      "rejected_code": 429,
      "policy": "redis",
      "rejected_msg": "too many requests",
      "redis_host": "redis-master",
      "redis_port": 56379,
      "redis_database": 2,
      "count": 10,
      "time_window": 60,
      "key": "remote_addr"
    }
  }
}

Once running, simply send a request to /route-1, wait 2–3 seconds, and then send a request to /route-2. You will then see that in the Redis database with index 1, two keys are created—for both route-1 and route-2—even though route-2 should be in database 2.

When we first noticed this, we had more of these similar routes (with different database ids). We were sending requests parallelly to random routes and noticed that the limits were not working properly. This led us to look into the source code, which is how we discovered this connection pool issue. After applying the fix I submitted above, everything started working as expected.

[hanzhenfang]

I also reproduced this issue with APISIX 3.16.0-debian and Redis 7-alpine.

Environment

• APISIX image: apache/apisix:3.16.0-debian
• Redis image: redis:7-alpine
• Deployment mode: traditional mode with etcd
• Redis address used by both routes: redis:6379

Reproduction Setup

I created two routes with the same Redis address but different Redis databases.

Route 1:

{
  "uri": "/redis-db1",
  "plugins": {
    "limit-count": {
      "policy": "redis",
      "count": 1000,
      "time_window": 60,
      "key": "same-user",
      "key_type": "constant",
      "redis_host": "redis",
      "redis_port": 6379,
      "redis_database": 1,
      "redis_keepalive_pool": 1,
      "redis_keepalive_timeout": 60000
    }
  },
  "upstream": {
    "type": "roundrobin",
    "nodes": {
      "mock-ai-1:1980": 1
    }
  }
}

Route 2:

{
  "uri": "/redis-db2",
  "plugins": {
    "limit-count": {
      "policy": "redis",
      "count": 1000,
      "time_window": 60,
      "key": "same-user",
      "key_type": "constant",
      "redis_host": "redis",
      "redis_port": 6379,
      "redis_database": 2,
      "redis_keepalive_pool": 1,
      "redis_keepalive_timeout": 60000
    }
  },
  "upstream": {
    "type": "roundrobin",
    "nodes": {
      "mock-ai-1:1980": 1
    }
  }
}

Then I flushed DB 1 and DB 2:

redis-cli -n 1 FLUSHDB
redis-cli -n 2 FLUSHDB

Then I sent alternating requests:

for i in {1..50}; do
  curl -s -o /dev/null http://127.0.0.1:9080/redis-db1
  curl -s -o /dev/null http://127.0.0.1:9080/redis-db2
done

The upstream returned 404, but that is unrelated to this issue. The X-RateLimit-* headers were present, so the limit-count plugin had already executed and accessed Redis.

Expected Behavior

DB 1 should only contain keys for /redis-db1.

DB 2 should only contain keys for /redis-db2.

Expected:

DB 1:
  plugin-limit-count/apisix/routes/redis-db1:...:same-user

DB 2:
  plugin-limit-count/apisix/routes/redis-db2:...:same-user

Actual Behavior

Both DB 1 and DB 2 contained keys from both routes.

DB 1:
  plugin-limit-count/apisix/routes/redis-db1:1802751831:same-user
  plugin-limit-count/apisix/routes/redis-db2:3762521015:same-user

DB 2:
  plugin-limit-count/apisix/routes/redis-db1:1802751831:same-user
  plugin-limit-count/apisix/routes/redis-db2:3762521015:same-user

The values also confirmed that both route counters were updated in both databases:

DB 1:
  redis-db1 key = 962
  redis-db2 key = 964

DB 2:
  redis-db1 key = 988
  redis-db2 key = 986

This means requests for /redis-db1 and /redis-db2 crossed Redis database boundaries.