bug: Failed auth with body transformer not working

[deltaglow]

Current Behavior

I have in route key-auth and body-transformer plugins. When key-auth plugin fail ([lua] plugin.lua:1171: run_plugin(): key-auth exits with http status code 401) then body-transformer horibly crash with error

[error] 49#49: *325 failed to run body_filter_by_lua*: /usr/local/apisix/apisix/plugins/body-transformer.lua:220: attempt to index local 'conf' (a nil value)
stack traceback:
        /usr/local/apisix/apisix/plugins/body-transformer.lua:220: in function 'phase_func'
        /usr/local/apisix/apisix/plugin.lua:1203: in function 'common_phase'
        /usr/local/apisix/apisix/init.lua:806: in function 'http_body_filter_phase'
        body_filter_by_lua(nginx.conf:368):2: in main chunk

When I remove key-auth plugin or key-auth doesn't fail, it works as expected.

Expected Behavior

It should probably stop executing next plugins or at least body-transformer should have all data to not crash.

Error Logs

2024/09/26 09:09:26 [warn] 50#50: *14043 [lua] plugin.lua:1171: run_plugin(): key-auth exits with http status code 401, client: 172.68.213.37, server: _, request: "POST /example HTTP/2.0", host: "example.com"
2024/09/26 09:09:26 [error] 50#50: *14043 failed to run body_filter_by_lua*: /usr/local/apisix/apisix/plugins/body-transformer.lua:220: attempt to index local 'conf' (a nil value)
stack traceback:
        /usr/local/apisix/apisix/plugins/body-transformer.lua:220: in function 'phase_func'
        /usr/local/apisix/apisix/plugin.lua:1203: in function 'common_phase'
        /usr/local/apisix/apisix/init.lua:806: in function 'http_body_filter_phase'
        body_filter_by_lua(nginx.conf:368):2: in main chunk, client: 1.1.1.1, server: _, request: "POST /example HTTP/2.0", host: "example.com"
2024/09/26 09:09:26 [error] 50#50: *14043 failed to run body_filter_by_lua*: /usr/local/apisix/apisix/plugins/body-transformer.lua:220: attempt to index local 'conf' (a nil value)
stack traceback:
        /usr/local/apisix/apisix/plugins/body-transformer.lua:220: in function 'phase_func'
        /usr/local/apisix/apisix/plugin.lua:1203: in function 'common_phase'
        /usr/local/apisix/apisix/init.lua:806: in function 'http_body_filter_phase'
        body_filter_by_lua(nginx.conf:368):2: in main chunk, client: 1.1.1.1, server: _, request: "POST /example HTTP/2.0", host: "example.com"

Steps to Reproduce

1. Create route with plugins key-auth and body-transformer
2. Make request with failing auth

Environment

• APISIX version (run apisix version): 3.10.0

[shreemaan-abhishek]

this indeed looks like a bug, please share your apisix resource configurations

[deltaglow]

Consumer:

{
  "plugins": {
    "key-auth": {
      "key": "abcdef"
    }
  },
  "username": "consumer_user"
}

Route:

{
  "name": "Some route",
  "status": 1,
  "plugins": {
	"key-auth": {
		"_meta": {
			"disable": true
		},
		"hide_credentials": true
	},
    "body-transformer": {
      "request": {
        "input_format": "json",
        "template_is_base64": true,
        "template": "<base64 template>"
      }
    },
    "proxy-rewrite": {
      "regex_uri": [
        "^/some-route",
        "/other-route"
      ],
      "use_real_request_uri_unsafe": false
    }
  },
  "host": "host.example.com",
  "methods": [
    "GET",
    "POST"
  ],
  "uri": "/some-route",
  "upstream": {
    "scheme": "https",
    "type": "roundrobin",
    "nodes": [
      {
        "host": "other-host.example.com",
        "weight": 1,
        "priority": 0,
        "port": 443
      }
    ],
    "hash_on": "vars",
    "timeout": {
      "connect": 6,
      "read": 6,
      "send": 6
    },
    "keepalive_pool": {
      "idle_timeout": 60,
      "requests": 1000,
      "size": 320
    },
    "pass_host": "node"
  }
}

Request:

curl --request POST \
  --url 'https://host.example.com/some-route?apikey=abcdef' \
  --header 'Content-Type: application/json' \
  --data '{
	"attr1": "value 1",
	"attr2": "value 2"
}'

[ShaunMaher]

Hi.

I have done some digging and maybe I have something to add. I'm very new to Lua and APISIX so, I might be way off track.

The fact that run_plugin(): key-auth exits with http status code 401 ends up in the logs, implies we are getting to this line of APISIX code:

apisix/apisix/plugin.lua

Line 1174 in a91e79a

core.log.warn(plugins[i].name, " exits with http status code ", code)

A few lines later, it core.response.exit(code, body), which I think means that the client request should be closed (and maybe it is). It doesn't seem to stop lower priority plugins (body-transformer, plugin code I have written) from running though. I'm not sure if this is intentional (to let other plugins do stuff in the event of an auth failure) or a bug.

Until someone that knows what they are doing can have a look at this, I have a dodgy workaround.

• Create a new plugin
• Give it a priority of 1081 (one higher than the default priority of the body-transformer plugin)
• Give it a body_filter function with the following code:

function _M.body_filter(conf, ctx)
  if ngx.status == 401 then
    for i, plugin in ipairs(ctx.plugins) do
      if plugin.name and plugin.name == "body-transformer" then
        core.log.error(plugin_name .. ":body_filter(): ctx.plugin[" .. i .. "]: " .. core.json.encode(ctx.plugins[i], true) .. ".")
        core.log.error(plugin_name .. ":body_filter(): this is the body-transformer!  Disabling functions.")
        plugin.rewrite = nil
        plugin.body_filter = nil
      end
    end
  end
end

This uses the ctx variable to enumerate all plugins enabled for this request. It finds the body-transformer plugin and replaces it's rewrite and body_filter functions with nil, effectively preventing them from running.

Complete plugin:

-- When using the key-auth and body-transformer plugins together, a failed
--  authentication can result in the body-transformer plugin failing (unhandled
--  error) and the client recieving an empty response.
-- This plugin is a work around that, if authentication has failed, disables the
--  rewrite and body_filter functions in the body-transformer plugin (just for 
--  the current request).  If the functions are disabled, they can't fail.

local plugin_name = "exit-on-auth-failure"
local core = require("apisix.core")
local ngx = ngx

local schema = {
  type = "object",
  properties = { }
}

local _M = {
  version = 0.1,
  -- body-transformer priority: 1080.  This plugin must have a higher priority
  priority = 1081,
  name = plugin_name,
  schema = schema,
  scope = "global",
}

function _M.body_filter(conf, ctx)
  if ngx.status == 401 then
    for i, plugin in ipairs(ctx.plugins) do
      if plugin.name and plugin.name == "body-transformer" then
        core.log.error(plugin_name .. ":body_filter(): ctx.plugin[" .. i .. "]: " .. core.json.encode(ctx.plugins[i], true) .. ".")
        core.log.error(plugin_name .. ":body_filter(): this is the body-transformer!  Disabling functions.")
        plugin.rewrite = nil
        plugin.body_filter = nil
      end
    end
  end
end

return _M

Put the above in a file called exit-on-auth-failure.lua in the same location as the other APISIX plugins, add it to the list of plugins in apisix.yaml, add it to the list of plugins on any route that has key-auth and body-transformer enabled. Workaround done.

[shreemaan-abhishek]

@ShaunMaher, the body-transformer plugin's execution priority is lower than key-auth and when key-auth fails, the rewrite phase is aborted, this means the body-transformer plugin's rewrite function is not executed. Due to this, ctx.body_transformer_conf ends up being uninitialized:

apisix/apisix/plugins/body-transformer.lua

Line 194 in 4bbb7cc

ctx.body_transformer_conf = conf

But during body_filter phase, body-transformer's body_filter function is called and APISIX panics because ctx.body_transformer_conf is nil:

apisix/apisix/plugins/body-transformer.lua

Lines 219 to 220 in 4bbb7cc

	local conf = ctx.body_transformer_conf
	if conf.response then

.

This can be fixed just by adding a nil check.

[ShaunMaher]

Hi @shreemaan-abhishek

Thanks for the feedback.

One thing I still don't quite understand though is whether or not the body_filter phase should run at all if the authentication failed. I can see why you might want it to (e.g. rewrite the error message before it goes back to the client). It seems like it should be something that should be made extremely clear in the doco though. If, for example, you're using serverless-*-function to take some action, assuming that key-auth won't let that action happen if the authentication failed, you're in for trouble.

I'm happy to come up with a PR that updates the code in body-transformer and the doco if you like.

Cheers.
Shaun.