Opened 7 years ago

Closed 6 years ago

#338 closed Bug / Defect (wontfix)

down-root plugin does not adhere to environment contract specified for scripts

Reported by: gw Owned by: David Sommerseth
Priority: minor Milestone:
Component: plug-ins / plug-in API Version: OpenVPN 2.3.2 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords: plugin
Cc:

Description

According to the openvpn man page, certain environment variables will be available to a program/script given as a parameter to --up, --down, and others. However, when running a program using the down-root plugin, some environment variables are not available. Notably the "script_type" variable is absent (it should have "down" as its value).

Its not clear if this should be set in the openvpn code or the plugin code (probably openvpn so it applies to all plugins).

Change History (2)

comment:1 Changed 7 years ago by Samuli Seppänen

Component: Generic / unclassifiedplug-ins / plug-in API
Owner: set to David Sommerseth
Priority: majorminor
Status: newassigned

comment:2 Changed 6 years ago by David Sommerseth

Resolution: wontfix
Status: assignedclosed

This behavioural change between --down and using the down-root plugin is expected based on the current implementation. It is actually a feature and not a bug.

The reason is the design of the down-root plugin and the plug-in interface. When OpenVPN starts, it loads the down-root plugin and initialises it in the openvpn_plugin_func_v1() function, looking for the OPENVPN_PLUGIN_UP flag. In this part it forks out a process with root privileges (as the OpenVPN core has not yet dropped privileges) and basically tells this child to wait until further notice. This child has been provided with the environment variables of the init stage as this point. Soon afterwards the OpenVPN core drops root privileges, which basically is the down-root parent process.

When OpenVPN shuts down, towards the end it calls openvpn_plugin_func_v1() with OPENVPN_PLUGIN_DOWN set. As this is the parent process, it can only tell the client to "kick of the script". But it cannot modify the environment tables of the already forked out process, as there exists no mechanism to transport these variables to the child process before run_script() is called.

The quick-fix here is to have a separate down script which knows/understands when it is being run.

A "proper" fix would be to redesign the down-root plug-in to properly transport the environment variables from the parent to the child process once OPENVPN_PLUGIN_DOWN is run.

Given that this ticket has been open for over a year with no responses or any other reports regarding this issue, I've decided to close this one as "wontfix" for now. There exists an easy work-around. To solve this, it should be done in a proper and clean way, which basically means more heavy work on the down-root plug-in. The benefit of fixing this now is quite low, given that we've not seen this issue catching our attention much at all.

If you disagree, feel free to re-open it, but it will be put in a queue for feature requests instead. A better approach to get this feature included will actually be to provide patches with a good solution. I can happily review patches fixing this issue and you'll get the credits in the change log.

Note: See TracTickets for help on using tickets.