OpenVPN / openvpn3-linux

OpenVPN 3 Linux client
GNU Affero General Public License v3.0
573 stars 152 forks source link

DBus - some net.openvpn.v3.sessions.StatusChange signals are sent as unicast messages #54

Closed moogpwns closed 3 years ago

moogpwns commented 3 years ago

I'm writing a simple applet for the Cinnamon DE using the DBus API. Basic functionality is to open & close connections and notify on connection drops.

I've noticed that some net.openvpn.v3.sessions.StatusChange signals include a destination value, so DBus treats them as unicast messages and prevents my subscribers from receiving the signals.

In particular, the following signals (from dbus-monitor output) are fired when a connection is closed, but are not routed to my client:

signal time=1625847646.846790 sender=:1.891 -> destination=:1.892 serial=2834 path=/net/openvpn/v3/sessions/052850e7s915fs483esb3e7s3afb389a2e49; interface=net.openvpn.v3.sessions; member=StatusChange
   uint32 3
   uint32 28
   string "Session closed"
signal time=1625847646.848262 sender=:1.891 -> destination=:1.892 serial=2839 path=/net/openvpn/v3/sessions/052850e7s915fs483esb3e7s3afb389a2e49; interface=net.openvpn.v3.sessions; member=StatusChange
   uint32 3
   uint32 19
   string ""

The major/minor codes correspond to SESSION : PROC_KILLED and SESSION : SESS_REMOVED.

Further details on this SO question.

Any chance the destination could be removed from these (and potentially other) signals, to make them available to other interested listeners?

dsommers commented 3 years ago

First of all, this sounds like a cool idea - and that's part of why OpenVPN 3 Linux was designed around the D-Bus service.

The eaves-drop possibility is not something I'm very found of. If you consider a multi-user system where more users are logged in; I want to avoid leaking too much data between user sessions - but there are some issues here currently, so it is not fully working as intended.

But you should be able to "subscribe" to Log, StatusChange and AttentionRequired signals from your code. There is one message which is broadcasted, which only provides bare minimum information, which is the SessionManagerEvent. This is sent by the main net.openvpn.v3.sessions service.

That signal will be sent out when a session is created or destroyed (stopped) and contains only a D-Bus path and UID of the owner of the session. This can then be used to attach a signal handler to new sessions.

To get a list of all sessions already running, call the net.openvpn.v3.sessions.FetchAvailableSessions method, which returns an array of session D-Bus paths currently available.

All these interfaces are going to go through some refactoring and I will most likely add a new method in the session objects which must be called first to receive these signals. This is to tighten the possible data leakage even more. I hope to get into that in one of the next coming releases (v14_beta took way too long time to get out though).

moogpwns commented 3 years ago

Yeh I've never used DBus before this project. Quite nice to work with once you get your head around it. Great job on your DBus docs btw, they helped me a lot :+1:

Fair point about data leakage on multi-user systems. You could maybe use the session DBus instance for sensitive signals, but I guess that adds a layer of complexity to the architecture.

So yes I am receiving the SessionManagerEvent signals, but I wasn't sure if they were suitable. As you say, they contain only minimal info so it felt like a bit of a workaround. Can I assume then that a SessionManagerEvent with one of the following StatusMajor values will only be used for the scenario given?

CONFIG (1)     -> Session Created
CONNECTION (2) -> Session Destroyed

Regarding the other functionality, I'm using net.openvpn.v3.configuration.FetchAvailableConfigs to build the list of connections when the applet initialises, then fetching the name property of each individual config object and passing those to net.openvpn.v3.sessions.LookupConfigName to check if any are already connected.

I can create sessions from the applet using net.openvpn.v3.sessions.NewTunnel and then Ready, Connect, Disconnect, etc. on the newly-created session object.

I'm getting connection successful confirmation from the net.openvpn.v3.session.StatusChange signals. The only missing piece for the basic functionality I need is detecting connection destroyed. I'll try the above and let you know.

moogpwns commented 3 years ago

Sorry, I've just re-read the documentation you linked to and it clearly states that the status code in the SessionManagerEvent signal is not a StatusMajor code at all, but rather an EventType specifically for the scenarios you described.

Seems a lot less like a workaround now :)

dsommers commented 3 years ago

Fair point about data leakage on multi-user systems. You could maybe use the session DBus instance for sensitive signals, but I guess that adds a layer of complexity to the architecture.

That's less trivial than it sounds like, as you will then need a proxy between the net.openvpn.v3.sessions service and the local unprivileged users's session bus. D-Bus session busses are isolated within a single user session and openvpn3-service-sessionmgr runs under a different user with not much privileges at all - only the system bus can be used in this case.

Other than that, it sounds like you've done quite a good job ... and you've figured out the important detail about SessionManagerEvent's not being the same as StatusChange events. Ideally, the SessionManagerEvent shouldn't be needed, but there are some other ugliness around the glib2 D-Bus implementation which doesn't allow the final StatusChange event to pass before shutting down. This will also be improved in a later release, but that is a lot more refactoring so it will happen in a release after the current release series are considered fully stable.

moogpwns commented 3 years ago

Closing as this is the expected behaviour.