Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/haproxy: Lua scripts are saved with dots in the middle of the filename which gets interpreted incorrectly by the lua loader #2265

Closed
sorano opened this issue Mar 10, 2021 · 4 comments
Assignees

Comments

@sorano
Copy link

sorano commented Mar 10, 2021

Important notices
Before you add a new report, we ask you kindly to acknowledge the following:

[x] I have read the contributing guide lines at https://github.com/opnsense/plugins/blob/master/CONTRIBUTING.md

[x] I have searched the existing issues and I'm convinced that mine is new.

[x] The title contains the plugin to which this issue belongs

Describe the bug
When adding lua scripts to HAProxy under Advanced - Lua Scripts the scripts get saved with a randomized filename and stored in /tmp/haproxy/lua/
The problem is that the filenames always contain a dot in the middle like:

5f568444c86536.89376113.lua
5f568c3ed29eb4.97842016.lua
5f57638f250b46.07710505.lua

The issue arise when you have a lua script that has dependencies on another lua script and tries to load that using the require function, normally it would look something like this in the lua code:

local json = require "json"
(this would try to load json.lua)

In our current case on OPNsense if we want to deploy our lua scripts from within the webui we will have to use the randomized filename of the script:

local json = require "5f57638f250b46.07710505"

And here comes the bug, the lua loader does some weird interpretation on the first dot in the filename and tries to load:
5f57638f250b46/07710505.lua (see first syntax test screenshot for better visualization)

To Reproduce

  1. Go to HAProxy - Advanced - Lua Scripts
  2. Add 2 scripts, one of which tries to load another.

I used http.lua from here: https://github.com/authelia/authelia/blob/master/internal/suites/example/compose/haproxy/http.lua
and json.lua from here: https://github.com/rxi/json.lua/blob/master/json.lua

Save and apply, dont enable them yet.

  1. Look at the generated filenames via cli in /tmp/haproxy/lua/ and get the generated filename of json.lua

  2. Go back to Lua scripts webui and edit http.lua and replace

local json = require "json"

with the generated filename
local json = require "generatedfilexxx.yyyy"

  1. Enable both scripts and test syntax. You will get Lua runtime error.

  2. Go back to cli and copy or rename the json.lua generated filename but replace the first dot with an underscore.

cp generatedfilexxx.yyyy.lua generatedfilexxx_yyyy.lua

  1. Go back to Lua scripts webui and edit http.lua and change to the filename with underscore:
    local json = require "generatedfilexxx_yyyy"

  2. Test syntax, you will still get runtime errors in the webui for a reason I dont fully understand BUT!

  3. Go back to cli and verify the staging config from cli:
    haproxy -c -f /usr/local/etc/haproxy.conf.staging

Configuration file is valid now that you use underscore and only one dot in the filename.
( If you would have verified from cli at step 5 (when filename is still with double dots) it is not valid even from cli )

Expected behavior
Well, expected would be that the lua loader interprets all dots in the filename correctly, but that feels like it's outside of OPNsense scope. So a reasonable workaround could be to not save the lua scripts with two dots in the filename. I don't know the reasoning behind generating random filenames. Preferbly I would see that the files get saved with the same name you use while creating the script in the webui.

For example, if I create this:

image

It would get saved as json.lua
This would be the best solution since it prevents the need of manually editing scripts to reflect the autogenerated filename.

If the above is not possible another workaround could be to save the autogenerated filename with an underscore instead of the first dot.

Screenshots
Webui test syntax lua error when using double dots:
image

Webui test syntax lua error when using underscore:
image

CLI test syntax valid config when using underscore (so same configuration file that the webui test above throws an error with):
image

I don't know why cli syntax test is valid when the webui is not when both are verified against the same config file.
If I'd guess I would say that the difference here is that cli test is run as root and webui probably ran as www so maybe it's a permission issue? Or maybe chroot related.

Additional context
This issue existed even before the os-haproxy 3.0 release.

Environment
OPNsense 21.1.3-amd64
FreeBSD 12.1-RELEASE-p14-HBSD
OpenSSL 1.1.1j 16 Feb 2021

os-haproxy 3.0

@fraenki
Copy link
Member

fraenki commented Mar 10, 2021

And here comes the bug, the lua loader does some weird interpretation on the first dot in the filename and tries to load: 5f57638f250b46/07710505.lua

Did you already search the HAProxy bugtracker and mailing list for this issue?
Does it work if you try to escape the dot using a backslash, i.e. local json = require "5f57638f250b46\.07710505"?

Preferbly I would see that the files get saved with the same name you use while creating the script in the webui.

Changing the filename might break them for other people. But we could probably introduce a new setting to let the user switch between autogenerated filenames and user-specified filenames.

@fraenki fraenki self-assigned this Mar 10, 2021
@sorano
Copy link
Author

sorano commented Mar 12, 2021

Thanks for your reply @fraenki

And here comes the bug, the lua loader does some weird interpretation on the first dot in the filename and tries to load: 5f57638f250b46/07710505.lua

Does it work if you try to escape the dot using a backslash, i.e. local json = require "5f57638f250b46\.07710505"?

Sorry, I forgot to mention that in my initial report. I had already tried to escape the dot with a backslash and that throws a Lua error aswell:
image
However, looking at that error more specifically now with fresh eyes I believe I must use another syntax for the escape sequence in Lua so I will take a second shot and see if I can get it to escape properly.

Did you already search the HAProxy bugtracker and mailinglist for this issue?

I did not look for bugs in HAProxy as from my understanding it is more of a Lua issue than a HAProxy issue.

See here for what I based that assumption on:
https://www.lua.org/manual/5.1/manual.html#pdf-package.loaders

"the searcher will change each interrogation mark in the template by filename, which is the module name with each dot replaced by a "directory separator" (such as "/" in Unix)"

However, I'm no Lua expert so I could very well be wrong.

Preferbly I would see that the files get saved with the same name you use while creating the script in the webui.

Changing the filename might break them for other people. But we could probably introduce a new setting to let the user switch between autogenerated filenames and user-specified filenames.

Understood.
Introducing a new setting that allows user-specified names would be a good way to solve it, since by not having to bother with editing the scripts in other ways it makes it easier for those who use public Lua scripts. Basically just copy and paste into the webui with the proper name and you're done.

@sorano
Copy link
Author

sorano commented Mar 12, 2021

So while I was looking for a solution to escape properly I randomly stumbled across this:
https://github.com/kernelsauce/turbo/blob/master/turbo/escape.lua

Take a look at how they load JSON.lua:
local json = require('turbo.3rdparty.JSON')

Translating literally to (you can see the file in their repo):
turbo/3rdparty/JSON.lua

So that makes me further convinced that using two dots in the filename is not a good idea and that it's related to Lua and not HAProxy.

And regarding escaping, well it seems like Lua cannot escape the dot hehe...
http://www.lua.org/manual/5.1/manual.html#2.1

The following strings denote other tokens:

 +     -     *     /     %     ^     #
 ==    ~=    <=    >=    <     >     =
 (     )     {     }     [     ]
 ;     :     ,     .     ..    ...

@fraenki fraenki changed the title [HAProxy] Lua scripts are saved with dots in the middle of the filename which gets interpreted incorrectly by the lua loader net/haproxy: Lua scripts are saved with dots in the middle of the filename which gets interpreted incorrectly by the lua loader Jul 11, 2021
fraenki added a commit to fraenki/plugins that referenced this issue Oct 31, 2021
@fraenki
Copy link
Member

fraenki commented Oct 31, 2021

@sorano Sorry for the long silence. If you're still interesting in this, I've created a fix to address this issue. You can test it with the following command on an up-to-date OPNsense installation:

opnsense-patch -c plugins 719b4b00

The patch contains a number of improvements:

  • Add a new option "filename scheme" to Lua scripts. This way you may choose to use the name for filenames. It will then be saved as myluascript.lua instead of random.id.lua. The default is still to use the random ID (this prevents issues when using the same name for multiple Lua scripts).
  • Add a new option "preload" to Lua scripts. This is required to avoid the following HAProxy warning: Trying to register service 'lua.hello_world' more than once. This error will be shown when adding a Lua script in the GUI and using it in another Lua script with the "require" function. The new option allows to disable automatic preloading of all Lua scripts. Scripts with preloading disabled can only be used after "require"'ing them.
  • Add option lua-prepend-path /tmp/haproxy/lua/?.lua to haproxy.conf. This is required for the "require" function to work as expected, otherwise it would be unable to find the specified Lua script.

Steps to test this:

  • Add a Lua script "helloworld", set "filename scheme" to "name" and disable preloading.
  • Add another Lua script "testlua" and include the other script: local luatest = require "helloworld"
  • Run HAProxy config test

@fraenki fraenki closed this as completed in 719b4b0 Nov 3, 2021
@opnsense opnsense locked as resolved and limited conversation to collaborators Dec 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants