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

luci-app-openvpn: add editing field for .up and .down scripts #3595

Closed
wants to merge 2 commits into from

Conversation

walterav1984
Copy link

@walterav1984 walterav1984 commented Feb 1, 2020

These changes allow a seperate 'ovpnprofile.up/ovpnprofile.down' file/script besides the existing 'ovpnprofile.auth' file and the existing 'ovpnprofile.ovpn'&'ovpnprofile.auth' edit fields.

Review requested:
It creates 'ovpnprofile.up/ovpnprofile.down' which are executable, because of a quick/dirty hack without further understanding of the lua languange.

Please backport these simple changes to 19.07.x because its 'luci-app-openvpn' can than be configured in very advanced ways just from the webinterface.

Signed-off-by: Walter Sonius walterav1984@gmail.com

This changes allow a seperate 'ovpnprofile.up/ovpnprofile.down' file/script besides the existing 'ovpnprofile.auth' file and edit field. However it creates 'ovpnprofile.up/ovpnprofile.down' which are not executable. If someone can add the right 'chmod +x' via luci, than please edit/correct this PR and commit it?
@feckert
Copy link
Member

feckert commented Feb 3, 2020

I have already submitted a similar patch functionality openwrt/openwrt#1596
I think this is more generic.
This is pending upstream for a core developer reviewer.

@walterav1984
Copy link
Author

walterav1984 commented Feb 3, 2020

That openwrt/openwrt#1596 PR is looking more advanced, but not 'luci' exposed?

Although this PR is incomplete when completed, it might add alot of functionality in a very similar existing way already done for .auth files in the 19.07 release.

Does your hotplug mechanism support multiple/different up/down scripts for each seperate openvpn account/process at the same time? If yours will get merge, can it be exposed into the luci-app-openvpn for easy editing?

@feckert feckert added the WIP pull request the author is still working on label Feb 5, 2020
@feckert
Copy link
Member

feckert commented Feb 5, 2020

Does your hotplug mechanism support multiple/different up/down scripts for each seperate openvpn account/process at the same time? If yours will get merge, can it be exposed into the luci-app-openvpn for easy editing?

Yes that is possible but for now only up and down will use the OpenWrt hotplug in this pullrequest.
You could move your special script into /etc/hotplug.d/openvpn and this get called on openvpn up down events. That script place should be used for other package in OpenWrt that want to get informed on openvpn events.
For example:

  • Send a notifification that some has connected
  • Enable an LED if an connection is up or down

Or you could paste your script into /etc/openvpn.user this is also called but does not be related to other packages this file belongs to the openvpn package and should be edit by the user for special cases.

Another possibility is if you add your own script or command to the config option up/down, then the OpenWrt hotplug.d subsystem is not called only your configured script or command are executed.

@feckert feckert added the feature pull request adding a new feature label Feb 5, 2020
@feckert feckert changed the title luci-app-openvpn: add editing field for .up & .down scripts WIP luci-app-openvpn: add editing field for .up and .down scripts Feb 5, 2020
This edit makes the 'openvpnprofile.up' & 'openvpnprofile.down' scripts executable when created by luci-app-openvpn webform editing field. 

Warning this hack has been made without any real knowledge or understanding of the lua programming language, please correct if wrong!
@walterav1984
Copy link
Author

walterav1984 commented Jun 22, 2020

Hi @feckert your PR was recently added 8fe9940 into openwrt instead of luci. However it's still unclear for me if you can have seperate up/down scripts for each vpn account and if these up/down scripts can be edited by l(uci) webinterface?

If so I may close this PR?

@feckert
Copy link
Member

feckert commented Jun 22, 2020

My solution is a generic solution and so was added to the openwrt main repository
You could add your up/down scripts into /etc/hotplug.d/openvpn.
On each up/down event the scripts are called.
In this callback scripts you could decide on which instance you want to react.

I have also added the possibility that on each up/down event the file /etc/openvpn.user is called. This file is also saved during firmware upgrade.

The next step is to add a possibility for editing this file in the LuCI.

@jow-
Copy link
Contributor

jow- commented Jun 22, 2020

Note that the 01-user script is currently called with env -i so it won't have access to the environment variables provided by the OpenVPN daemon which might or might not be an issue (@feckert).

@feckert
Copy link
Member

feckert commented Jun 24, 2020

@jow- Due to the change in openvpn, we also have to remove the up and down from the option list. Since that doesn't make sense anymore if I configure openvpn via uci in the LuCI.

Note that the 01-user script is currently called with env -i so it won't have access to the environment variables provided by the OpenVPN daemon which might or might not be an issue (@feckert).

If the hotplug scripts replace the up and down then we should not delete the env with -i. Then we get all call option from openvpn. This can be an advantage or a disadvantage when we think about security. Any script in openvpn can read the data and do something with it. But since openwrt is a single user system I think this is negligible.

Independently of this I would say we have to make it possible for the user to edit the file openvpn.user in the LuCI. This is also done by the mwan3.

@walterav1984
Copy link
Author

walterav1984 commented Jun 24, 2020

Now I understand this is a more generic way of making "openwrt" itself aware of the up/down status of a VPN connection and let the system respond to it instead of leaving it to "openvpn daemon" which may need extra user privileges on some OS to perform these tasks itself(script_security).

... environment variables provided by the OpenVPN daemon ...

These variables are essential in setup since every openvpn-profile may receive different settings from each openvpnserver which each different openvpnprofile.up/down scripts individually respond to.

I cannot say what is the "best/secure" way to setup the system but doing it by openvpn deamon is part of openvpn its feature set and relative easy to implement for me with (l)uci via this current PR.

The /etc/openvpn.user you suggest for these settings does it behave like /etc/openvpn/openvpn-profile-x.user and /etc/opevpn/openvpn-profile-y.user like the different .conf/.auth profiles right now and has to be made (yet) configurable by (l)uci which is still missing?

@feckert
Copy link
Member

feckert commented Jun 24, 2020

The /etc/openvpn.user you suggest for these settings does it behave like /etc/openvpn/openvpn-profile-x.user and /etc/opevpn/openvpn-profile-y.user like the different .conf/.auth profiles right now and has to be made (yet) configurable by (l)uci which is still missing?

You could move this scripts /etc/openvpn/openvpn-profile-x.user or /etc/openvpn/openvpn-profile-y.user to /etc/hotplug.d/openvpn. On each up/down event alle files in this directory are called/executed. I think the /etc/openvpn.user is a quick and dirty possibility to test your script. But for now we get no openvpn env in the openvpn.user script. I test this with a logger command.

@feckert feckert self-requested a review June 29, 2020 20:12
@walterav1984
Copy link
Author

walterav1984 commented Mar 25, 2021

...But for now we get no openvpn env in the openvpn.user script. I test this with a logger command.

I guess this is fixed by: jollaman999/openwrt@57664a1

How to expose those user/up/down scripts via luci, should that be part of the openvpn package or some sort of "luci-app-hotplug-userscripts" example package?

@feckert
Copy link
Member

feckert commented Apr 6, 2021

@walterav1984 From my point of view, the script should be stored under directory /etc/hotplug.d/openvpn.
All scripts that are stored there are called on an up/down event off an openvpn tunnel.
Maybe we should put a 00-user script that calls the editable /etc/openvpn.user script.
This can then be changed via the LuCI.
Have a look at this in mwan3.
https://github.com/openwrt/packages/blob/master/net/mwan3/files/etc/hotplug.d/iface/16-mwan3-user
and
https://github.com/openwrt/packages/blob/master/net/mwan3/files/etc/mwan3.user

@feckert feckert closed this Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature pull request adding a new feature WIP pull request the author is still working on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants