Allow restrictions based on custom Authorization Header matching logic

[sladkani]

In commit e1205b7 ("Allow restrictions based on Authorization header"), we added restriction match type that tests the Auth Header of the websocket upgrade request matches a regex.

We'd like to augment that approach for cases where the wstunnel server needs to perform some computation on the presented Auth Header value, or perform some custom matching logic.
Examples would be computing a hash, performing jwt validation, comparing to a dynamic set of values, etc.

There could be numerous different custom match implementations. So instead of suggesting additional tailored MatchConfig types, lets allow the admin to pass a lua script that implements his custom match logic.
The script is processed and a global function named 'auth_validate' is invoked, given the Auth Header value as a parameter. The function must return true/false; true iff access is granted.

example lua script:

--
local match = string.match

function auth_validate(auth)
    if match(auth, "^Basic aGk6dGhlcmU=$") then
        return true
    end
    return false
end
--

Any failure loading the script or invoking the lua function is considered an authentication failure.

[erebe]

Hello,

Thank you for the PR and the feature is nice, but sadly I doubt it is going to be merged into the project.
From my point of view, this is more the work of a real reverse proxy than wstunnel itself. Especially as there is no users/account notion in the project.

Also from a technical standpoint there are 2 limitations with the current PR:

• All the code of the server is async, but the code of this handler is blocking, which would lead to blocking a whole thread/other connections while reading file from disk, compiling the script and executing the function.
  It seems there is an async variant at least for calling the function https://docs.rs/mlua/latest/mlua/struct.Function.html#method.call_async

std::fs::read_to_string
lua.load(&script).exec()
validate_fn.call(auth_val)

• Which lead to the 2nd point, reading and compiling the script should be cached and not done on every request. With the cache refreshed if the file change on disk. It would avoid having this blocking/starving connections issue also.

[sladkani]

Thanks @erebe !

There are several design benefits performing the authentication in wstunnel server itself and not in a reverse proxy.
Assuming the technical issues you described are fixed, will you consider merging this?

Re your 2nd point, can you please suggest the proper code location where the scripts needs to be read and cached?
Should the cache be global? Embedded in some other context? What should be the cache key? the script filename?

[erebe]

Would you mind describing the benefits ?

I don't want to give you false hope, so I am going to tell that even with the changes, the PR is not going to be merged. But the code is pretty isolated, so I don't think you will have trouble to maintain a fork with your patch.

Regarding the code location for the cache, you should compile the lua code while parsing the config file
https://github.com/erebe/wstunnel/blob/main/wstunnel/src/restrictions/mod.rs#L18
You can extend the AuthorizationScript object with an optional field that is ignored by serde to hold the compiled code.

and add a hook for the file to be watched for change in
https://github.com/erebe/wstunnel/blob/main/wstunnel/src/restrictions/config_reloader.rs#L70
https://github.com/erebe/wstunnel/blob/main/wstunnel/src/restrictions/config_reloader.rs#L112

[sladkani]

Would you mind describing the benefits ?

Sure.
In a system where there are numerous dynamic different wstunnel servers (each having its own authentication configuration), implementing authentications in the reverse-proxy means the reverse-proxy needs to be aware of every instance's own configuration, i.e. needs to be dynamically configured for every new/removed wstunnel instance and its credentials.
That's an implementation burden.
(for forwarding purposes, the reverse-proxy does not need to be aware of actual wstunnel instances, it may use a fixed rule for performing the correct fanning).

I don't want to give you false hope, so I am going to tell that even with the changes, the PR is not going to be merged. But the code is pretty isolated, so I don't think you will have trouble to maintain a fork with your patch.

NP, thanks for the heads-up.

BTW, instead of a generic scripting facility for wstunnel server authentication, how about the more-scoped solution presented here? Do you think this is a better merge candidate?

sladkani@d1bb4bb

Thanks!

[erebe]

Thanks for the explanation 👍

how about the more-scoped solution presented here

What kind of logic do you have in mind ?

[sladkani]

how about the more-scoped solution presented here

What kind of logic do you have in mind ?

like this one:

+      # This match applies if the Authorization Header carries a bearer token whose hash matches a value
+      # given in hex-digest format.
+      # If present, Authentication Header must exists, must carry a Bearer token, and its hash must match.
+      # - !BearerHash ["Sha256", "5994471abb01112afcc18159f6cc74b4f511b99806da59b3caf5a9c173cacfc5"]

implemented in this commit
sladkani@d1bb4bb

WDYT?

[erebe]

I prefer the lua version to be honest 🙈

Having a BearerHash is too specific to be of use out of your use case.

[sladkani]

I prefer the lua version to be honest 🙈

Having a BearerHash is too specific to be of use out of your use case.

Definitely. This is exactly why initially submitted a generic solution: "instead of suggesting additional tailored MatchConfig types, lets allow the admin to pass a lua script that implements his custom match logic".

Anyway, we're left with the following options:

1. maintain my fork with several tailored MatchConfig matchers
2. fix the Lua implementation according to your input, and have this merged. Overall, this provides the ability to customize authentications in setups where reverse-proxy is undesired/unavailable, and IMHO requiring low maintainer's effort: the tailored logic is in the user's script, not in the app rust code
3. an alternative generic/semi-generic solution you can think of, i'm willing to implement

Let me know.
Thanks!

[erebe]

Yeah, the lua version is more powerful/flexible in your case.

Without traction beside on your part, I am not going to merge the PR. But if some other people manifest, I may re-open the question.

If I were you, if your needs stop at having a hash of the JWT. I would go with the custom matcher and maintaining a fork. This part of the code does not move/evolve much, so I doubt it will be a big toil to maintain the patch.

If you plan to have more complex matcher, I would go into refining the patch with the lua interpreter to avoid the blocking in the critical part. As explained, as long as you can reload/modify the config file when you update the lua file, you can compile the lua code outside of the critical path when the config file is being parsed.

[sladkani]

Thanks @erebe, will go with a fork then.