* [PATCH v1 0/2] Improve OpenVPN configuration value processing @ 2021-11-16 15:14 Jussi Laakkonen 2021-11-16 15:14 ` [PATCH v1 1/2] vpn-provider: Support checking if provider setting key exists Jussi Laakkonen ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Jussi Laakkonen @ 2021-11-16 15:14 UTC (permalink / raw) To: connman Rework how OpenVPN processes the configuration values by introducing a more clear way to classify different types of options. This makes it possible to add options that may have a value or can be used without a value thus, resorting to default value set for the option. Also the --auth-user-pass and alike options having a special value are handled in cleaner way. Motivation for this was that the --comp-lzo option was not processed correctly with its value, and even though being listed as deprecated some OpenVPN providers use the value. Omitting this option even though it was required caused issues with both UDP and TCP VPNs: write to TUN/TAP : Invalid argument (code=22) after which OpenVPN silently restarts itself. Jussi Laakkonen (2): vpn-provider: Support checking if provider setting key exists. openvpn: Improve configuration value processing vpn/plugins/openvpn.c | 119 ++++++++++++++++++++++++++---------------- vpn/vpn-provider.c | 6 +++ vpn/vpn-provider.h | 2 + 3 files changed, 83 insertions(+), 44 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 1/2] vpn-provider: Support checking if provider setting key exists. 2021-11-16 15:14 [PATCH v1 0/2] Improve OpenVPN configuration value processing Jussi Laakkonen @ 2021-11-16 15:14 ` Jussi Laakkonen 2021-11-16 15:14 ` [PATCH v1 2/2] openvpn: Improve configuration value processing Jussi Laakkonen 2021-11-18 21:02 ` [PATCH v1 0/2] Improve OpenVPN " Daniel Wagner 2 siblings, 0 replies; 6+ messages in thread From: Jussi Laakkonen @ 2021-11-16 15:14 UTC (permalink / raw) To: connman Add a function to check if the setting key exists in the setting hash table. --- vpn/vpn-provider.c | 6 ++++++ vpn/vpn-provider.h | 2 ++ 2 files changed, 8 insertions(+) diff --git a/vpn/vpn-provider.c b/vpn/vpn-provider.c index 4bb6c24e..cc325967 100644 --- a/vpn/vpn-provider.c +++ b/vpn/vpn-provider.c @@ -2937,6 +2937,12 @@ bool vpn_provider_get_string_immutable(struct vpn_provider *provider, return setting->immutable; } +bool vpn_provider_setting_key_exists(struct vpn_provider *provider, + const char *key) +{ + return g_hash_table_contains(provider->setting_strings, key); +} + void vpn_provider_set_auth_error_limit(struct vpn_provider *provider, unsigned int limit) { diff --git a/vpn/vpn-provider.h b/vpn/vpn-provider.h index ee09e8ef..5d1455da 100644 --- a/vpn/vpn-provider.h +++ b/vpn/vpn-provider.h @@ -88,6 +88,8 @@ int vpn_provider_set_boolean(struct vpn_provider *provider, const char *key, bool force_change); bool vpn_provider_get_boolean(struct vpn_provider *provider, const char *key, bool default_value); +bool vpn_provider_setting_key_exists(struct vpn_provider *provider, + const char *key); void vpn_provider_set_auth_error_limit(struct vpn_provider *provider, unsigned int limit); -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v1 2/2] openvpn: Improve configuration value processing 2021-11-16 15:14 [PATCH v1 0/2] Improve OpenVPN configuration value processing Jussi Laakkonen 2021-11-16 15:14 ` [PATCH v1 1/2] vpn-provider: Support checking if provider setting key exists Jussi Laakkonen @ 2021-11-16 15:14 ` Jussi Laakkonen 2021-11-18 21:02 ` [PATCH v1 0/2] Improve OpenVPN " Daniel Wagner 2 siblings, 0 replies; 6+ messages in thread From: Jussi Laakkonen @ 2021-11-16 15:14 UTC (permalink / raw) To: connman Improve how the configuration options are processed in OpenVPN. Add a config option type that is providing better granularity in how the options should be processed when adding to the task, which eventually sets them to the binary. Drop the has_value option as unnecessary. This now allows to add string variables that have a default value when they are used alone by setting the "opt_type" to OPT_STRING. And adding the option without a value. Furthermore, support also setting an option to null for a string option if the option string value matches the special value set in the array. This is the case for example with --auth-user-pass where the value "-" for OpenVPN.AuthUserPass indicates that the password should be queried from VPN agent using the management interface. --- vpn/plugins/openvpn.c | 119 ++++++++++++++++++++++++++---------------- 1 file changed, 75 insertions(+), 44 deletions(-) diff --git a/vpn/plugins/openvpn.c b/vpn/plugins/openvpn.c index e1962942..761474de 100644 --- a/vpn/plugins/openvpn.c +++ b/vpn/plugins/openvpn.c @@ -56,36 +56,48 @@ static DBusConnection *connection; +enum opt_type { + OPT_NONE = 0, + OPT_STRING = 1, + OPT_BOOL = 2, +}; + struct { - const char *cm_opt; - const char *ov_opt; - char has_value; + const char *cm_opt; + const char *ov_opt; + const char *ov_opt_to_null; + enum opt_type opt_type; } ov_options[] = { - { "Host", "--remote", 1 }, - { "OpenVPN.CACert", "--ca", 1 }, - { "OpenVPN.Cert", "--cert", 1 }, - { "OpenVPN.Key", "--key", 1 }, - { "OpenVPN.MTU", "--tun-mtu", 1 }, - { "OpenVPN.NSCertType", "--ns-cert-type", 1 }, - { "OpenVPN.Proto", "--proto", 1 }, - { "OpenVPN.Port", "--port", 1 }, - { "OpenVPN.AuthUserPass", "--auth-user-pass", 1 }, - { "OpenVPN.AskPass", "--askpass", 1 }, - { "OpenVPN.AuthNoCache", "--auth-nocache", 0 }, - { "OpenVPN.TLSRemote", "--tls-remote", 1 }, - { "OpenVPN.TLSAuth", NULL, 1 }, - { "OpenVPN.TLSAuthDir", NULL, 1 }, - { "OpenVPN.TLSCipher", "--tls-cipher", 1}, - { "OpenVPN.Cipher", "--cipher", 1 }, - { "OpenVPN.Auth", "--auth", 1 }, - { "OpenVPN.CompLZO", "--comp-lzo", 0 }, - { "OpenVPN.RemoteCertTls", "--remote-cert-tls", 1 }, - { "OpenVPN.ConfigFile", "--config", 1 }, - { "OpenVPN.DeviceType", NULL, 1 }, - { "OpenVPN.Verb", "--verb", 1 }, - { "OpenVPN.Ping", "--ping", 1}, - { "OpenVPN.PingExit", "--ping-exit", 1}, - { "OpenVPN.RemapUsr1", "--remap-usr1", 1}, + { "Host", "--remote", NULL, OPT_STRING}, + { "OpenVPN.CACert", "--ca", NULL, OPT_STRING}, + { "OpenVPN.Cert", "--cert", NULL, OPT_STRING}, + { "OpenVPN.Key", "--key", NULL, OPT_STRING}, + { "OpenVPN.MTU", "--tun-mtu", NULL, OPT_STRING}, + { "OpenVPN.NSCertType", "--ns-cert-type", NULL, OPT_STRING}, + { "OpenVPN.Proto", "--proto", NULL, OPT_STRING}, + { "OpenVPN.Port", "--port", NULL, OPT_STRING}, + /* + * If the AuthUserPass option is "-", provide the input via management + * interface. To facilitate this set the option as NULL. + */ + { "OpenVPN.AuthUserPass", "--auth-user-pass", "-", OPT_STRING}, + { "OpenVPN.AskPass", "--askpass", NULL, OPT_STRING}, + { "OpenVPN.AuthNoCache", "--auth-nocache", NULL, OPT_BOOL}, + { "OpenVPN.TLSRemote", "--tls-remote", NULL, OPT_STRING}, + { "OpenVPN.TLSAuth", NULL, NULL, OPT_NONE}, + { "OpenVPN.TLSCipher", "--tls-cipher", NULL, OPT_STRING}, + { "OpenVPN.TLSAuthDir", NULL, NULL, OPT_NONE}, + { "OpenVPN.Cipher", "--cipher", NULL, OPT_STRING}, + { "OpenVPN.Auth", "--auth", NULL, OPT_STRING}, + /* Is set to adaptive by default if value is omitted */ + { "OpenVPN.CompLZO", "--comp-lzo", NULL, OPT_STRING}, + { "OpenVPN.RemoteCertTls", "--remote-cert-tls", NULL, OPT_STRING}, + { "OpenVPN.ConfigFile", "--config", NULL, OPT_STRING}, + { "OpenVPN.DeviceType", NULL, NULL, OPT_NONE}, + { "OpenVPN.Verb", "--verb", NULL, OPT_STRING}, + { "OpenVPN.Ping", "--ping", NULL, OPT_STRING}, + { "OpenVPN.PingExit", "--ping-exit", NULL, OPT_STRING}, + { "OpenVPN.RemapUsr1", "--remap-usr1", NULL, OPT_STRING}, }; struct ov_private_data { @@ -355,29 +367,48 @@ static int ov_save(struct vpn_provider *provider, GKeyFile *keyfile) static int task_append_config_data(struct vpn_provider *provider, struct connman_task *task) { - const char *option; int i; for (i = 0; i < (int)ARRAY_SIZE(ov_options); i++) { - if (!ov_options[i].ov_opt) - continue; + const char *ov_opt = ov_options[i].ov_opt; + const char *cm_opt = ov_options[i].cm_opt; + const char *option = NULL; - option = vpn_provider_get_string(provider, - ov_options[i].cm_opt); - if (!option) + switch (ov_options[i].opt_type) { + case OPT_NONE: continue; - /* - * If the AuthUserPass option is "-", provide the input - * via management interface - */ - if (!strcmp(ov_options[i].cm_opt, "OpenVPN.AuthUserPass") && - !strcmp(option, "-")) - option = NULL; + case OPT_STRING: + if (!ov_opt) + continue; + + option = vpn_provider_get_string(provider, cm_opt); + /* + * A string option may be used alone without a value + * in which case the default value is used by OpenVPN. + */ + if (!option && !vpn_provider_setting_key_exists( + provider, ov_opt)) + continue; + + const char *opt_to_null = ov_options[i].ov_opt_to_null; + if (opt_to_null && !g_strcmp0(option, opt_to_null)) + option = NULL; + + break; + + case OPT_BOOL: + if (!ov_opt) + continue; + + /* Ignore the boolean toggle if option is disabled. */ + if (!vpn_provider_get_boolean(provider, cm_opt, false)) + continue; + + break; + } - if (connman_task_add_argument(task, - ov_options[i].ov_opt, - ov_options[i].has_value ? option : NULL) < 0) + if (connman_task_add_argument(task, ov_opt, option)) return -EIO; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1 0/2] Improve OpenVPN configuration value processing 2021-11-16 15:14 [PATCH v1 0/2] Improve OpenVPN configuration value processing Jussi Laakkonen 2021-11-16 15:14 ` [PATCH v1 1/2] vpn-provider: Support checking if provider setting key exists Jussi Laakkonen 2021-11-16 15:14 ` [PATCH v1 2/2] openvpn: Improve configuration value processing Jussi Laakkonen @ 2021-11-18 21:02 ` Daniel Wagner 2021-11-19 8:09 ` Jussi Laakkonen 2 siblings, 1 reply; 6+ messages in thread From: Daniel Wagner @ 2021-11-18 21:02 UTC (permalink / raw) To: Jussi Laakkonen; +Cc: connman Hi Jussi, On Tue, Nov 16, 2021 at 05:14:15PM +0200, Jussi Laakkonen wrote: > Rework how OpenVPN processes the configuration values by introducing a > more clear way to classify different types of options. This makes it possible > to add options that may have a value or can be used without a value thus, > resorting to default value set for the option. Also the --auth-user-pass > and alike options having a special value are handled in cleaner way. > > Motivation for this was that the --comp-lzo option was not processed > correctly with its value, and even though being listed as deprecated some > OpenVPN providers use the value. Omitting this option even though it was > required caused issues with both UDP and TCP VPNs: > write to TUN/TAP : Invalid argument (code=22) > after which OpenVPN silently restarts itself. Both patches applied after fixing up a warning: vpn/plugins/openvpn.c:394:25: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] 394 | const char *opt_to_null = ov_options[i].ov_opt_to_null; | ^~~~~ Daniel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 0/2] Improve OpenVPN configuration value processing 2021-11-18 21:02 ` [PATCH v1 0/2] Improve OpenVPN " Daniel Wagner @ 2021-11-19 8:09 ` Jussi Laakkonen 2021-11-19 8:30 ` Daniel Wagner 0 siblings, 1 reply; 6+ messages in thread From: Jussi Laakkonen @ 2021-11-19 8:09 UTC (permalink / raw) To: Daniel Wagner; +Cc: connman Hi Daniel, On 11/18/21 11:02 PM, Daniel Wagner wrote: > Hi Jussi, > > On Tue, Nov 16, 2021 at 05:14:15PM +0200, Jussi Laakkonen wrote: >> Rework how OpenVPN processes the configuration values by introducing a >> more clear way to classify different types of options. This makes it possible >> to add options that may have a value or can be used without a value thus, >> resorting to default value set for the option. Also the --auth-user-pass >> and alike options having a special value are handled in cleaner way. >> >> Motivation for this was that the --comp-lzo option was not processed >> correctly with its value, and even though being listed as deprecated some >> OpenVPN providers use the value. Omitting this option even though it was >> required caused issues with both UDP and TCP VPNs: >> write to TUN/TAP : Invalid argument (code=22) >> after which OpenVPN silently restarts itself. > > Both patches applied after fixing up a warning: > Thanks. > vpn/plugins/openvpn.c:394:25: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] > 394 | const char *opt_to_null = ov_options[i].ov_opt_to_null; > | ^~~~~ > Oops, sorry. My build env did not complain about that so I guess I overlooked that bit. Cheers, Jussi ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 0/2] Improve OpenVPN configuration value processing 2021-11-19 8:09 ` Jussi Laakkonen @ 2021-11-19 8:30 ` Daniel Wagner 0 siblings, 0 replies; 6+ messages in thread From: Daniel Wagner @ 2021-11-19 8:30 UTC (permalink / raw) To: Jussi Laakkonen; +Cc: connman Hi Jussi, On Fri, Nov 19, 2021 at 10:09:27AM +0200, Jussi Laakkonen wrote: > Oops, sorry. My build env did not complain about that so I guess I > overlooked that bit. No worries. My normal build didn't catch it either. I have a commit hook which builds any patch I add. This one triggered. Cheers, Daniel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-19 8:30 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-16 15:14 [PATCH v1 0/2] Improve OpenVPN configuration value processing Jussi Laakkonen 2021-11-16 15:14 ` [PATCH v1 1/2] vpn-provider: Support checking if provider setting key exists Jussi Laakkonen 2021-11-16 15:14 ` [PATCH v1 2/2] openvpn: Improve configuration value processing Jussi Laakkonen 2021-11-18 21:02 ` [PATCH v1 0/2] Improve OpenVPN " Daniel Wagner 2021-11-19 8:09 ` Jussi Laakkonen 2021-11-19 8:30 ` Daniel Wagner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).