Certain parts of the code like args_add_*() are trival and self contained.
Move them to the beginning and together.

It's inconsistent and not needed.

Modify and extend the set of helper functions to append
command line arguments.

We should be consistend about when we consider a key present and when
it is missing. Incidentally, for certain properties we treat an empty
value like it is missing/unset.

This check seems arbitrary and is done inconsistently. At least,
use a particular function nmovpn_arg_is_set() which embodies the
requirement and which is also used by export() in the same context.

nmovpn_arg_is_set() expresses the concept of whether a data key is present,
whether it should be considered as set or unset. That is, in many cases an
empty value is treated like a missing/unset key.

This concept is not only revevant during export, but everytime we access
the setting, like when initializing the GUI or when passing the argument
to openvpn.

Hence, move it to shared/utils.h.

By looking at the source code, it seems that openvpn accepts extra-certs
for every configuration type. On the other hand, the manual page groups
the option under TLS Mode, indicating that this only makes sense for
TLS. In the GUI and during export, handle extra-certs only for TLS
connection. When starting openvpn, pass it one whenever it's present
in the connection.

The same butten is responsible for both for the tls-auth and tls-crypt option.
Maybe, the tooltip should completley change, depending on the mode selected
above. However, that is more complicated and not necessarily better. Swapping
the tooltip could also be confusing. So, go the easy way, and describe both
options in the same tooltip.

and add :. This makes it consistent with the other options
in the same table.

They are not only literally identical, they are also strongly related.
That is, the XML blob's name is really identical to the corresponding
command line option.

Reordering #define has no visible effect.

Reordering the properties like in @advanced_keys has effects in
the order in which we find keys. Optimally we would have frequently
used keys as first. But in reality, if the order of the search list
matters performance we, we should instead use a lookup table. So,
asciibetically sort oder is suitable also here and looks consistent
in source code.

NM waits at most 60 seconds for the connection to be established:
lower the connect timeout if there are multiple remotes, so that we
try at least 3 of them.

https://bugzilla.gnome.org/show_bug.cgi?id=792252

https://bugzilla.gnome.org/show_bug.cgi?id=792252

After leaving the main loop on SIGTERM, we want to tear down the
NMVpnServicePlugin instance. This should also unexports the service on
D-Bus and ensure that no more requests are accepted -- as we wouldn't
handle them anyway.

Note that afterwards we still want to iterate the main loop waiting for
processes to terminate.

This reverts commit 9f459770.
The crash that 9f459770 tried to address is actually a bug
in libnm, where NMVpnServicePlugin would not unexport the D-Bus
service. This must be fixed by libnm instead [1]. Note that overall
the crash is of limited priority, because we are already in the process
of shutting down.

[1] https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=929f36c56f3b22a917066c5cb7bfc938630f9e8c

- ensure that we started terminating the processes that we are
  waiting that they terminate. It's not clear that we already called
  pids_pending_send_sigterm() at this point. Also add a is_terminating
  variable, to initiate shutdown precisely once.

- while pids_pending_send_sigterm() already schedules a 2 seconds
  timeout to send a SIGKILL if SIGTERM doesn't work, there is
  no guarantee that pids_pending_wait_for_processes() will always
  complete. Also schedule a 3 seconds timeout. If within that time
  the processes didn't all terminate, we don't wait any longer.

- don't pass a GMainLoop to pids_pending_wait_for_processes().
  The entire mechanism uses g_timeout_add(), which can only work
  with the current main context. It cannot work to pass a loop
  for another context. Hence, we don't need the loop either.

Don't just exit(), but just return from the main function.
This way, we get a chance to cleanup all resources properly.

Beniamino Galvani authored Jan 15, 2018 and Thomas Haller committed Jan 15, 2018

pids_pending_wait_for_processes() runs the main loop, where queued
D-Bus events could be processed. Ensure the plugin is still
alive. Fixes the following:

 nm-openvpn-serv[7900]: g_dbus_method_invocation_take_error: assertion 'error != NULL' failed
 NetworkManager[4823]: nm-openvpn[7900] <info>  openvpn[7912]: send SIGKILL

 #0  nm_vpn_service_plugin_disconnect (plugin=0x91b8e0, err=err@entry=0x7ffce3dc80c8) at libnm/nm-vpn-service-plugin.c:236
 #1  0x00007fe1a361aae6 in impl_vpn_service_plugin_disconnect (plugin=<optimized out>, context=0x92f6c0, user_data=<optimized out>) at libnm/nm-vpn-service-plugin.c:857
 #2  0x00007fe1a01b8d1e in ffi_call_unix64 () from target:/lib64/libffi.so.6
 #3  0x00007fe1a01b868f in ffi_call () from target:/lib64/libffi.so.6
 #4  0x00007fe1a1570f39 in g_cclosure_marshal_generic () from target:/lib64/libgobject-2.0.so.0
 #5  0x00007fe1a35f59fe in _nm_dbus_method_meta_marshal (closure=<optimized out>, return_value=0x7ffce3dc84d0, n_param_values=<optimized out>, param_values=<optimized out>,
     invocation_hint=<optimized out>, marshal_data=<optimized out>) at libnm/nm-dbus-helpers.c:203
 #6  0x00007fe1a157073d in g_closure_invoke () from target:/lib64/libgobject-2.0.so.0
 #7  0x00007fe1a15834de in signal_emit_unlocked_R () from target:/lib64/libgobject-2.0.so.0
 #8  0x00007fe1a158b270 in g_signal_emitv () from target:/lib64/libgobject-2.0.so.0
 #9  0x00007fe1a3683034 in _nmdbus_vpn_plugin_skeleton_handle_method_call (connection=<optimized out>, sender=<optimized out>, object_path=<optimized out>,
     interface_name=0x7fe18c012b80 "org.freedesktop.NetworkManager.VPN.Plugin", method_name=0x7fe18c00f7b0 "Disconnect", parameters=<optimized out>, invocation=0x92f6c0, user_data=0x916e20)
     at introspection/org.freedesktop.NetworkManager.VPN.Plugin.c:2946
 #10 0x00007fe1a18890d7 in g_dbus_interface_method_dispatch_helper () from target:/lib64/libgio-2.0.so.0
 #11 0x00007fe1a18713dc in call_in_idle_cb () from target:/lib64/libgio-2.0.so.0
 #12 0x00007fe1a1294597 in g_idle_dispatch () from target:/lib64/libglib-2.0.so.0
 #13 0x00007fe1a1297bb7 in g_main_context_dispatch () from target:/lib64/libglib-2.0.so.0
 #14 0x00007fe1a1297f60 in g_main_context_iterate.isra () from target:/lib64/libglib-2.0.so.0
 #15 0x00007fe1a1297fec in g_main_context_iteration () from target:/lib64/libglib-2.0.so.0
 #16 0x0000000000403645 in pids_pending_wait_for_processes (main_loop=0x933470) at src/nm-openvpn-service.c:417

https://bugzilla.gnome.org/show_bug.cgi?id=792252

Otherwise, ",," appears to be a valid remoted.

Fixes: 3c5c7efb

https://bugzilla.gnome.org/show_bug.cgi?id=792252

Fixes: 3c5c7efb

https://bugzilla.gnome.org/show_bug.cgi?id=792252

https://bugzilla.gnome.org/show_bug.cgi?id=790612

The connection is perfectly valid without the "username" key.
Note that the VPN settings support a data setting
NM_OPENVPN_KEY_USERNAME ("username"), but NMSettingVpn also has a
NM_SETTING_VPN_USER_NAME ("user-name") setting.

So, NM_OPENVPN_KEY_USERNAME has preference over
NM_SETTING_VPN_USER_NAME. But even if the username is entirely missing
in the setting, NetworkManager will populate it with the user-name
of the user activating the connection.

It is wrong to require a user name in the GUI.

There are two places where we request secrets. Their logging differs,
which is confusing.

I want to see whether the server calls interactive connect.

- don't use strlen() for checking for empty string.
- replace strtol() with _nm_utils_ascii_str_to_int64()
- drop some asserts. Instead of asserting against having a valid
  widget, just call the GTK function. In case the assert is violated,
  GTK triggers a g_return_*() assertion, which is checked anyway
  and is a more graceful failure then dumping core.
- construct the widget names via nm_sprintf_buf(). It's stack-allocated
  and required less lines of code.

In the past, the properties plugin contained the non-gtk part and the
GTK gui. Hence, several GTK related parts were in "properties/auth-helpers.c".

That is no longer the case. The GTK part of the plugin is in
"properties/nm-openvpn-editor.c".

There is no reason for this split. Moving related code to different
files doesn't make it simpler but more complicated.

Move the GTK code back to "properties/nm-openvpn-editor.c"

No code was changed, except necessary adjustments.

Compatibility tweaks should be done at one place, by "nm-default.h".