From d27fcd07541ae6f524115d5b0f36e14673135ca3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 10 Dec 2019 10:01:24 +0100 Subject: [PATCH 1/6] build/meson: allow configuring default for main.auth-polkit setting We always build PolicyKit support, because it merely depends on some D-Bus calls. However, there are two things to configure: - the default value for main.auth-polkit in NetworkManager.conf. This is now called "-Dconfig_auth_polkit_default=$VAL". - whether to install the policy file. This is called "-Dpolkit=$VAL". These settings are mostly independent, so add "config_auth_polkit_default" to make the default explicitly configurable. (cherry picked from commit c21c6bc0be2a4467402bc2d8718859dedb10b676) --- contrib/fedora/rpm/NetworkManager.spec | 1 + meson.build | 14 ++++++++------ meson_options.txt | 1 + 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index d4a802f74c..d7e3612dc4 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/contrib/fedora/rpm/NetworkManager.spec @@ -585,6 +585,7 @@ This tool is still experimental. %endif -Dselinux=true \ -Dpolkit=true \ + -Dconfig_auth_polkit_default=true \ -Dpolkit_agent=true \ -Dmodify_system=true \ -Dconcheck=true \ diff --git a/meson.build b/meson.build index 13a5ae1648..bde86bae79 100644 --- a/meson.build +++ b/meson.build @@ -464,8 +464,11 @@ if enable_polkit polkit_gobject_policydir = dependency('polkit-gobject-1').get_pkgconfig_variable('policydir', define_variable: ['prefix', nm_prefix]) endif -config_default_main_auth_polkit = enable_polkit.to_string() -config_h.set_quoted('NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT', config_default_main_auth_polkit) +config_auth_polkit_default = get_option('config_auth_polkit_default') +if config_auth_polkit_default == 'default' + config_auth_polkit_default = (enable_polkit ? 'true' : 'false') +endif +config_h.set_quoted('NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT', config_auth_polkit_default) enable_modify_system = get_option('modify_system') @@ -784,7 +787,7 @@ data_conf = configuration_data() data_conf.set('DISTRO_NETWORK_SERVICE', (enable_ifcfg_rh ? 'network.service' : '')) data_conf.set('NM_CONFIG_DEFAULT_LOGGING_AUDIT_TEXT', config_default_logging_audit) data_conf.set('NM_CONFIG_DEFAULT_LOGGING_BACKEND_TEXT', config_logging_backend_default) -data_conf.set('NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT_TEXT', config_default_main_auth_polkit) +data_conf.set('NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT_TEXT', config_auth_polkit_default) data_conf.set('NM_CONFIG_DEFAULT_MAIN_DHCP', config_dhcp_default) data_conf.set('NM_CONFIG_DEFAULT_MAIN_RC_MANAGER', config_dns_rc_manager_default) data_conf.set('NM_MAJOR_VERSION', nm_major_version) @@ -935,10 +938,9 @@ output += ' nmplugindir: ' + nm_plugindir + '\n' output += '\nPlatform:\n' output += ' session tracking: ' + ','.join(session_trackers) + '\n' output += ' suspend/resume: ' + suspend_resume + '\n' -output += ' policykit: ' + enable_polkit.to_string() +output += ' policykit: ' + enable_polkit.to_string() + ' (default: ' + config_auth_polkit_default + ')' if enable_polkit - modify = (enable_modify_system ? 'permissive' : 'restrictive') - output += ' (' + modify + ' modify.system) (default: main.auth-polkit=true)' + output += ' (' + (enable_modify_system ? 'permissive' : 'restrictive') + ' modify.system)' endif output += '\n' output += ' polkit agent: ' + enable_polkit_agent.to_string() + '\n' diff --git a/meson_options.txt b/meson_options.txt index ffe2faa8de..4f4f0d5c5a 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -14,6 +14,7 @@ option('session_tracking_consolekit', type: 'boolean', value: true, description: option('session_tracking', type: 'combo', choices: ['systemd', 'elogind', 'no'], value: 'systemd', description: 'Compatibility option to choose one session tracking module') option('suspend_resume', type: 'combo', choices: ['upower', 'systemd', 'elogind', 'consolekit', 'auto'], value: 'auto', description: 'Build NetworkManager with specific suspend/resume support') option('polkit', type: 'boolean', value: true, description: 'User auth-polkit configuration option.') +option('config_auth_polkit_default', type: 'combo', choices: ['default', 'true', 'false'], value: 'default', description: 'Default value for configuration main.auth-polkit.') option('modify_system', type: 'boolean', value: false, description: 'Allow users to modify system connections') option('polkit_agent', type: 'boolean', value: false, description: 'enable polkit agent for clients') option('selinux', type: 'boolean', value: true, description: 'Build with SELinux') From a0a5b0b2f51a01b651270a4de72c6d7e80d47a84 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 10 Dec 2019 08:51:03 +0100 Subject: [PATCH 2/6] core: add main.auth-polkit option "root-only" We always build with PolicyKit support enabled, because it has no additional dependencies, beside some D-Bus calls. However, in NetworkManager.conf the user could configure "main.auth-polkit" to disable PolicyKit. However, previously it would only allow to disable PolicyKit while granting access to all users. I think it's useful to have an option that disables PolicyKit and grants access only to root. I think we should not go too far in implementing our own authorization mechanisms beside PolicyKit (e.g. you cannot disable PolicyKit and grant access based on group membership of the user). However, disabling PolicyKit can be useful sometimes, and it's simple to implement a "root-only" setup. Note one change is that when NetworkManager now runs without a D-Bus connection (in initrd), it would deny all non-root requests. Previously it would grant access. I think there should be little difference in practice, because if we have no D-Bus we also don't have any requests to authenticate. (cherry picked from commit 6d7446e52f5bfe379c2b1f54f9244b33fd236e32) --- configure.ac | 12 ++++---- man/NetworkManager.conf.xml | 7 +++-- meson_options.txt | 2 +- src/main.c | 5 +-- src/nm-auth-manager.c | 61 ++++++++++++++++++++++++------------- src/nm-auth-manager.h | 3 +- src/nm-config-data.c | 46 ++++++++++++++++++++++++++++ src/nm-config-data.h | 24 +++++++++++++++ src/nm-config.h | 1 - 9 files changed, 125 insertions(+), 36 deletions(-) diff --git a/configure.ac b/configure.ac index 3fbcd86512..2dc18bc15f 100644 --- a/configure.ac +++ b/configure.ac @@ -614,18 +614,18 @@ AM_CONDITIONAL(WITH_JSON_VALIDATION, test "${enable_json_validation}" != "no") # default configuration for main.auth-polkit. User can always enable/disable polkit # authorization via config. AC_ARG_ENABLE(polkit, - AS_HELP_STRING([--enable-polkit=yes|no], + AS_HELP_STRING([--enable-polkit=yes|no|root-only], [set default value for auth-polkit configuration option. This value can be overwritten by NM configuration. 'disabled' is an alias for 'no']), [enable_polkit=${enableval}], [enable_polkit=yes]) -if (test "${enable_polkit}" != "no" -a "${enable_polkit}" != "disabled"); then +if test "${enable_polkit}" == "root-only" ; then + enable_polkit='root-only' +elif test "${enable_polkit}" != "no" -a "${enable_polkit}" != "disabled" ; then enable_polkit=true - AC_DEFINE(NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT, "true", [The default value of the auth-polkit configuration option]) - AC_SUBST(NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT_TEXT, true) else enable_polkit=false - AC_DEFINE(NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT, "false", [The default value of the auth-polkit configuration option]) - AC_SUBST(NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT_TEXT, false) fi +AC_DEFINE_UNQUOTED(NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT, "$enable_polkit", [The default value of the auth-polkit configuration option]) +AC_SUBST(NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT_TEXT, "$enable_polkit") PKG_CHECK_MODULES(POLKIT, [polkit-agent-1 >= 0.97], [have_pk_agent=yes],[have_pk_agent=no]) AC_ARG_ENABLE(polkit-agent, diff --git a/man/NetworkManager.conf.xml b/man/NetworkManager.conf.xml index cdf55ed62e..c0c05147a0 100644 --- a/man/NetworkManager.conf.xml +++ b/man/NetworkManager.conf.xml @@ -158,8 +158,11 @@ plugins-=remove-me auth-polkit Whether the system uses PolicyKit for authorization. - If false, all requests will be allowed. If - true, non-root requests are authorized using PolicyKit. + If true, non-root requests are authorized using PolicyKit. + Requests from root (user ID zero) are always granted without asking PolicyKit. + If false, all requests will be allowed and PolicyKit is + not used. If set to root-only PolicyKit is not used and + all requests except root are denied. The default value is &NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT_TEXT;. diff --git a/meson_options.txt b/meson_options.txt index 4f4f0d5c5a..041d9bfc38 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -14,7 +14,7 @@ option('session_tracking_consolekit', type: 'boolean', value: true, description: option('session_tracking', type: 'combo', choices: ['systemd', 'elogind', 'no'], value: 'systemd', description: 'Compatibility option to choose one session tracking module') option('suspend_resume', type: 'combo', choices: ['upower', 'systemd', 'elogind', 'consolekit', 'auto'], value: 'auto', description: 'Build NetworkManager with specific suspend/resume support') option('polkit', type: 'boolean', value: true, description: 'User auth-polkit configuration option.') -option('config_auth_polkit_default', type: 'combo', choices: ['default', 'true', 'false'], value: 'default', description: 'Default value for configuration main.auth-polkit.') +option('config_auth_polkit_default', type: 'combo', choices: ['default', 'true', 'false', 'root-only'], value: 'default', description: 'Default value for configuration main.auth-polkit.') option('modify_system', type: 'boolean', value: false, description: 'Allow users to modify system connections') option('polkit_agent', type: 'boolean', value: false, description: 'enable polkit agent for clients') option('selinux', type: 'boolean', value: true, description: 'Build with SELinux') diff --git a/src/main.c b/src/main.c index 24157b18c6..b1466c1852 100644 --- a/src/main.c +++ b/src/main.c @@ -410,10 +410,7 @@ main (int argc, char *argv[]) NM_UTILS_KEEP_ALIVE (config, nm_netns_get (), "NMConfig-depends-on-NMNetns"); - nm_auth_manager_setup (nm_config_data_get_value_boolean (nm_config_get_data_orig (config), - NM_CONFIG_KEYFILE_GROUP_MAIN, - NM_CONFIG_KEYFILE_KEY_MAIN_AUTH_POLKIT, - NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT_BOOL)); + nm_auth_manager_setup (nm_config_data_get_main_auth_polkit (nm_config_get_data_orig (config))); manager = nm_manager_setup (); diff --git a/src/nm-auth-manager.c b/src/nm-auth-manager.c index 244342c035..3f248aeeec 100644 --- a/src/nm-auth-manager.c +++ b/src/nm-auth-manager.c @@ -42,7 +42,7 @@ typedef struct { guint changed_signal_id; bool disposing:1; bool shutting_down:1; - bool polkit_enabled_construct_only:1; + NMAuthPolkitMode auth_polkit_mode:3; } NMAuthManagerPrivate; struct _NMAuthManager { @@ -118,6 +118,7 @@ struct _NMAuthManagerCallId { gpointer user_data; guint64 call_numid; guint idle_id; + bool idle_is_authorized:1; }; #define cancellation_id_to_str_a(call_numid) \ @@ -256,9 +257,10 @@ static gboolean _call_on_idle (gpointer user_data) { NMAuthManagerCallId *call_id = user_data; - gboolean is_authorized = TRUE; + gboolean is_authorized; gboolean is_challenge = FALSE; + is_authorized = call_id->idle_is_authorized; call_id->idle_id = 0; _LOG2T (call_id, "completed: authorized=%d, challenge=%d (simulated)", @@ -312,22 +314,25 @@ nm_auth_manager_check_authorization (NMAuthManager *self, call_id = g_slice_new (NMAuthManagerCallId); *call_id = (NMAuthManagerCallId) { - .self = g_object_ref (self), - .callback = callback, - .user_data = user_data, - .call_numid = ++priv->call_numid_counter, + .self = g_object_ref (self), + .callback = callback, + .user_data = user_data, + .call_numid = ++priv->call_numid_counter, + .idle_is_authorized = TRUE, }; c_list_link_tail (&priv->calls_lst_head, &call_id->calls_lst); - if (!priv->dbus_connection) { - _LOG2T (call_id, "CheckAuthorization(%s), subject=%s (succeeding due to polkit authorization disabled)", action_id, nm_auth_subject_to_string (subject, subject_buf, sizeof (subject_buf))); - call_id->idle_id = g_idle_add (_call_on_idle, call_id); - } else if (nm_auth_subject_is_internal (subject)) { + if (nm_auth_subject_is_internal (subject)) { _LOG2T (call_id, "CheckAuthorization(%s), subject=%s (succeeding for internal request)", action_id, nm_auth_subject_to_string (subject, subject_buf, sizeof (subject_buf))); call_id->idle_id = g_idle_add (_call_on_idle, call_id); } else if (nm_auth_subject_get_unix_process_uid (subject) == 0) { _LOG2T (call_id, "CheckAuthorization(%s), subject=%s (succeeding for root)", action_id, nm_auth_subject_to_string (subject, subject_buf, sizeof (subject_buf))); call_id->idle_id = g_idle_add (_call_on_idle, call_id); + } else if (priv->auth_polkit_mode != NM_AUTH_POLKIT_MODE_USE_POLKIT) { + _LOG2T (call_id, "CheckAuthorization(%s), subject=%s (PolicyKit disabled and always %s authorization to non-root user)", action_id, nm_auth_subject_to_string (subject, subject_buf, sizeof (subject_buf)), + priv->auth_polkit_mode == NM_AUTH_POLKIT_MODE_ALLOW_ALL ? "grant" : "deny"); + call_id->idle_is_authorized = (priv->auth_polkit_mode == NM_AUTH_POLKIT_MODE_ALLOW_ALL); + call_id->idle_id = g_idle_add (_call_on_idle, call_id); } else { GVariant *parameters; GVariantBuilder builder; @@ -461,11 +466,17 @@ static void set_property (GObject *object, guint prop_id, const GValue *value, GParamSpec *pspec) { NMAuthManagerPrivate *priv = NM_AUTH_MANAGER_GET_PRIVATE ((NMAuthManager *) object); + int v_int; switch (prop_id) { case PROP_POLKIT_ENABLED: /* construct-only */ - priv->polkit_enabled_construct_only = !!g_value_get_boolean (value); + v_int = g_value_get_int (value); + g_return_if_fail (NM_IN_SET (v_int, NM_AUTH_POLKIT_MODE_ROOT_ONLY, + NM_AUTH_POLKIT_MODE_ALLOW_ALL, + NM_AUTH_POLKIT_MODE_USE_POLKIT)); + priv->auth_polkit_mode = v_int; + nm_assert (priv->auth_polkit_mode == v_int); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); @@ -481,6 +492,7 @@ nm_auth_manager_init (NMAuthManager *self) NMAuthManagerPrivate *priv = NM_AUTH_MANAGER_GET_PRIVATE (self); c_list_init (&priv->calls_lst_head); + priv->auth_polkit_mode = NM_AUTH_POLKIT_MODE_ROOT_ONLY; } static void @@ -493,8 +505,11 @@ constructed (GObject *object) G_OBJECT_CLASS (nm_auth_manager_parent_class)->constructed (object); - if (!priv->polkit_enabled_construct_only) { - create_message = "polkit disabled"; + if (priv->auth_polkit_mode != NM_AUTH_POLKIT_MODE_USE_POLKIT) { + if (priv->auth_polkit_mode == NM_AUTH_POLKIT_MODE_ROOT_ONLY) + create_message = "polkit disabled, root-only"; + else + create_message = "polkit disabled, allow-all"; goto out; } @@ -503,7 +518,8 @@ constructed (GObject *object) if (!priv->dbus_connection) { /* This warrants an info level message. */ logl = LOGL_INFO; - create_message = "D-Bus connection not available. Polkit is disabled and all requests are authenticated."; + create_message = "D-Bus connection not available. Polkit is disabled and only root will be authorized."; + priv->auth_polkit_mode = NM_AUTH_POLKIT_MODE_ROOT_ONLY; goto out; } @@ -527,14 +543,17 @@ out: } NMAuthManager * -nm_auth_manager_setup (gboolean polkit_enabled) +nm_auth_manager_setup (NMAuthPolkitMode auth_polkit_mode) { NMAuthManager *self; g_return_val_if_fail (!singleton_instance, singleton_instance); + nm_assert (NM_IN_SET (auth_polkit_mode, NM_AUTH_POLKIT_MODE_ROOT_ONLY, + NM_AUTH_POLKIT_MODE_ALLOW_ALL, + NM_AUTH_POLKIT_MODE_USE_POLKIT)); self = g_object_new (NM_TYPE_AUTH_MANAGER, - NM_AUTH_MANAGER_POLKIT_ENABLED, polkit_enabled, + NM_AUTH_MANAGER_POLKIT_ENABLED, (int) auth_polkit_mode, NULL); _LOGD ("set instance"); @@ -579,11 +598,11 @@ nm_auth_manager_class_init (NMAuthManagerClass *klass) object_class->dispose = dispose; obj_properties[PROP_POLKIT_ENABLED] = - g_param_spec_boolean (NM_AUTH_MANAGER_POLKIT_ENABLED, "", "", - FALSE, - G_PARAM_WRITABLE | - G_PARAM_CONSTRUCT_ONLY | - G_PARAM_STATIC_STRINGS); + g_param_spec_int (NM_AUTH_MANAGER_POLKIT_ENABLED, "", "", + NM_AUTH_POLKIT_MODE_ROOT_ONLY, NM_AUTH_POLKIT_MODE_USE_POLKIT, NM_AUTH_POLKIT_MODE_USE_POLKIT, + G_PARAM_WRITABLE | + G_PARAM_CONSTRUCT_ONLY | + G_PARAM_STATIC_STRINGS); g_object_class_install_properties (object_class, _PROPERTY_ENUMS_LAST, obj_properties); diff --git a/src/nm-auth-manager.h b/src/nm-auth-manager.h index 083f98aaaa..33e3bb2ccd 100644 --- a/src/nm-auth-manager.h +++ b/src/nm-auth-manager.h @@ -7,6 +7,7 @@ #define NM_AUTH_MANAGER_H #include "nm-auth-subject.h" +#include "nm-config-data.h" /*****************************************************************************/ @@ -49,7 +50,7 @@ typedef struct _NMAuthManagerClass NMAuthManagerClass; GType nm_auth_manager_get_type (void); -NMAuthManager *nm_auth_manager_setup (gboolean polkit_enabled); +NMAuthManager *nm_auth_manager_setup (NMAuthPolkitMode auth_polkit_mode); NMAuthManager *nm_auth_manager_get (void); void nm_auth_manager_force_shutdown (NMAuthManager *self); diff --git a/src/nm-config-data.c b/src/nm-config-data.c index b5868b7b1a..8f53304aa6 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -386,6 +386,52 @@ _nm_config_data_get_keyfile_user (const NMConfigData *self) /*****************************************************************************/ +static NMAuthPolkitMode +nm_auth_polkit_mode_from_string (const char *str) +{ + int as_bool; + + if (!str) + return NM_AUTH_POLKIT_MODE_UNKNOWN; + + if (nm_streq (str, "root-only")) + return NM_AUTH_POLKIT_MODE_ROOT_ONLY; + + as_bool = _nm_utils_ascii_str_to_bool (str, -1); + if (as_bool != -1) { + return as_bool + ? NM_AUTH_POLKIT_MODE_USE_POLKIT + : NM_AUTH_POLKIT_MODE_ALLOW_ALL; + } + + return NM_AUTH_POLKIT_MODE_UNKNOWN; +} + +NMAuthPolkitMode +nm_config_data_get_main_auth_polkit (const NMConfigData *self) +{ + NMAuthPolkitMode auth_polkit_mode; + const char *str; + + str = nm_config_data_get_value (self, + NM_CONFIG_KEYFILE_GROUP_MAIN, + NM_CONFIG_KEYFILE_KEY_MAIN_AUTH_POLKIT, + NM_CONFIG_GET_VALUE_STRIP + | NM_CONFIG_GET_VALUE_NO_EMPTY); + auth_polkit_mode = nm_auth_polkit_mode_from_string (str); + if (auth_polkit_mode == NM_AUTH_POLKIT_MODE_UNKNOWN) { + auth_polkit_mode = nm_auth_polkit_mode_from_string (NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT); + if (auth_polkit_mode == NM_AUTH_POLKIT_MODE_UNKNOWN) { + nm_assert_not_reached (); + auth_polkit_mode = NM_AUTH_POLKIT_MODE_ROOT_ONLY; + } + } + + return auth_polkit_mode; +} + +/*****************************************************************************/ + /** * nm_config_data_get_groups: * @self: the #NMConfigData instance diff --git a/src/nm-config-data.h b/src/nm-config-data.h index a6f03902d5..f83ab92874 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -6,6 +6,28 @@ #ifndef NM_CONFIG_DATA_H #define NM_CONFIG_DATA_H +/*****************************************************************************/ + +typedef enum { + + /* an invalid mode. */ + NM_AUTH_POLKIT_MODE_UNKNOWN, + + /* don't use PolicyKit, but only allow root user (uid 0). */ + NM_AUTH_POLKIT_MODE_ROOT_ONLY, + + /* don't use PolicyKit, but allow all requests. */ + NM_AUTH_POLKIT_MODE_ALLOW_ALL, + + /* use PolicyKit to authorize requests. Root user (uid 0) always + * gets a free pass, without consulting PolicyKit. If PolicyKit is not + * running, authorization will fail for non root users. */ + NM_AUTH_POLKIT_MODE_USE_POLKIT, + +} NMAuthPolkitMode; + +/*****************************************************************************/ + #define NM_TYPE_CONFIG_DATA (nm_config_data_get_type ()) #define NM_CONFIG_DATA(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_CONFIG_DATA, NMConfigData)) #define NM_CONFIG_DATA_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_CONFIG_DATA, NMConfigDataClass)) @@ -132,6 +154,8 @@ const char *nm_config_data_get_connectivity_response (const NMConfigData *config int nm_config_data_get_autoconnect_retries_default (const NMConfigData *config_data); +NMAuthPolkitMode nm_config_data_get_main_auth_polkit (const NMConfigData *config_data); + const char *const*nm_config_data_get_no_auto_default (const NMConfigData *config_data); gboolean nm_config_data_get_no_auto_default_for_device (const NMConfigData *self, NMDevice *device); diff --git a/src/nm-config.h b/src/nm-config.h index e3e658c702..d9460ebb46 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -219,7 +219,6 @@ extern char *_nm_config_match_env; #define NM_CONFIG_DEVICE_STATE_DIR ""NMRUNDIR"/devices" -#define NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT_BOOL (nm_streq (""NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT, "true")) #define NM_CONFIG_DEFAULT_LOGGING_AUDIT_BOOL (nm_streq (""NM_CONFIG_DEFAULT_LOGGING_AUDIT, "true")) typedef enum { From 62c9d8c109074a9179a0303150ab353286464348 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 10 Dec 2019 08:44:07 +0100 Subject: [PATCH 3/6] config: return GPtrArray with warnings from internal read_entire_config() function The underlying GPtrArray that we use to construct the list of warnings is more useful than the strv array. For the internal function, don't let it return the strv array but instead take (and fill) the warnings as GPtrArray. There is no difference in practice, because also previously we would always create an empty GPtrArray. (cherry picked from commit 6998c5f129f70d676698de07667d6ee0334839ce) --- src/nm-config.c | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/src/nm-config.c b/src/nm-config.c index 9da2fe0a48..7d5be95c61 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -1210,7 +1210,7 @@ read_entire_config (const NMConfigCmdLineOptions *cli, const char *system_config_dir, char **out_config_main_file, char **out_config_description, - char ***out_warnings, + GPtrArray *warnings, GError **error) { gs_unref_keyfile GKeyFile *keyfile = NULL; @@ -1220,14 +1220,13 @@ read_entire_config (const NMConfigCmdLineOptions *cli, guint i; gs_free char *o_config_main_file = NULL; const char *run_config_dir = ""; - gs_unref_ptrarray GPtrArray *warnings = NULL; - g_return_val_if_fail (config_dir, NULL); - g_return_val_if_fail (system_config_dir, NULL); - g_return_val_if_fail (!out_config_main_file || !*out_config_main_file, FALSE); - g_return_val_if_fail (!out_config_description || !*out_config_description, NULL); - g_return_val_if_fail (!error || !*error, FALSE); - g_return_val_if_fail (out_warnings && !*out_warnings, FALSE); + nm_assert (config_dir); + nm_assert (system_config_dir); + nm_assert (!out_config_main_file || !*out_config_main_file); + nm_assert (!out_config_description || !*out_config_description); + nm_assert (!error || !*error); + nm_assert (warnings); if ( (""RUN_CONFIG_DIR)[0] == '/' && !nm_streq (RUN_CONFIG_DIR, system_config_dir) @@ -1236,7 +1235,6 @@ read_entire_config (const NMConfigCmdLineOptions *cli, /* create a default configuration file. */ keyfile = nm_config_create_keyfile (); - warnings = g_ptr_array_new_with_free_func (g_free); system_confs = _get_config_dir_files (system_config_dir); confs = _get_config_dir_files (config_dir); @@ -1326,10 +1324,6 @@ read_entire_config (const NMConfigCmdLineOptions *cli, } NM_SET_OUT (out_config_main_file, g_steal_pointer (&o_config_main_file)); - g_ptr_array_add (warnings, NULL); - *out_warnings = (char **) g_ptr_array_free (warnings, warnings->len == 1); - g_steal_pointer (&warnings); - return g_steal_pointer (&keyfile); } @@ -2572,7 +2566,7 @@ nm_config_reload (NMConfig *self, NMConfigChangeFlags reload_flags, gboolean emi char *config_description = NULL; gs_strfreev char **no_auto_default = NULL; gboolean intern_config_needs_rewrite; - gs_strfreev char **warnings = NULL; + gs_unref_ptrarray GPtrArray *warnings = NULL; guint i; g_return_if_fail (NM_IS_CONFIG (self)); @@ -2589,6 +2583,8 @@ nm_config_reload (NMConfig *self, NMConfigChangeFlags reload_flags, gboolean emi return; } + warnings = g_ptr_array_new_with_free_func (g_free); + /* pass on the original command line options. This means, that * options specified at command line cannot ever be reloaded from * file. That seems desirable. @@ -2598,7 +2594,7 @@ nm_config_reload (NMConfig *self, NMConfigChangeFlags reload_flags, gboolean emi priv->system_config_dir, &config_main_file, &config_description, - &warnings, + warnings, &error); if (!keyfile) { _LOGE ("Failed to reload the configuration: %s", error->message); @@ -2607,9 +2603,9 @@ nm_config_reload (NMConfig *self, NMConfigChangeFlags reload_flags, gboolean emi return; } - if (emit_warnings && warnings) { - for (i = 0; warnings[i]; i++) - _LOGW ("%s", warnings[i]); + if (emit_warnings) { + for (i = 0; i < warnings->len; i++) + _LOGW ("%s", (const char *) warnings->pdata[i]); } no_auto_default = no_auto_default_from_file (priv->no_auto_default_file); @@ -2779,7 +2775,7 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) gs_free char *config_main_file = NULL; gs_free char *config_description = NULL; gs_strfreev char **no_auto_default = NULL; - gs_strfreev char **warnings = NULL; + gs_unref_ptrarray GPtrArray *warnings = NULL; gs_free char *configure_and_quit = NULL; gboolean intern_config_needs_rewrite; const char *s; @@ -2806,12 +2802,14 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) else priv->intern_config_file = g_strdup (DEFAULT_INTERN_CONFIG_FILE); + warnings = g_ptr_array_new_with_free_func (g_free); + keyfile = read_entire_config (&priv->cli, priv->config_dir, priv->system_config_dir, &config_main_file, &config_description, - &warnings, + warnings, error); if (!keyfile) return FALSE; @@ -2858,7 +2856,10 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) keyfile_intern); priv->config_data = g_object_ref (priv->config_data_orig); - priv->warnings = g_steal_pointer (&warnings); + if (warnings->len > 0) { + g_ptr_array_add (warnings, NULL); + priv->warnings = (char **) g_ptr_array_free (g_steal_pointer (&warnings), FALSE); + } return TRUE; } From e7e0909d445169b0e34df31a2d2fa1055b40f4ba Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 11 Dec 2019 11:35:11 +0100 Subject: [PATCH 4/6] config: add nm_config_data_get_warnings() to get additional warnings about wrong configuration No additional warnings are implemented yet. (cherry picked from commit 8d20b9363b4461c7e86f2310a2fbfe88a09d5284) --- src/nm-config-data.c | 8 ++++++++ src/nm-config-data.h | 3 +++ src/nm-config.c | 14 +++++++++----- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 8f53304aa6..d8146457a3 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -1598,6 +1598,14 @@ nm_config_data_diff (NMConfigData *old_data, NMConfigData *new_data) /*****************************************************************************/ +void +nm_config_data_get_warnings (const NMConfigData *self, + GPtrArray *warnings) +{ +} + +/*****************************************************************************/ + static void get_property (GObject *object, guint prop_id, diff --git a/src/nm-config-data.h b/src/nm-config-data.h index f83ab92874..2a3f2a89eb 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -234,6 +234,9 @@ void nm_global_dns_config_free (NMGlobalDnsConfig *dns_config); NMGlobalDnsConfig *nm_global_dns_config_from_dbus (const GValue *value, GError **error); void nm_global_dns_config_to_dbus (const NMGlobalDnsConfig *dns_config, GValue *value); +void nm_config_data_get_warnings (const NMConfigData *self, + GPtrArray *warnings); + /* private accessors */ GKeyFile *_nm_config_data_get_keyfile (const NMConfigData *self); GKeyFile *_nm_config_data_get_keyfile_user (const NMConfigData *self); diff --git a/src/nm-config.c b/src/nm-config.c index 7d5be95c61..a7bb350302 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -2603,11 +2603,6 @@ nm_config_reload (NMConfig *self, NMConfigChangeFlags reload_flags, gboolean emi return; } - if (emit_warnings) { - for (i = 0; i < warnings->len; i++) - _LOGW ("%s", (const char *) warnings->pdata[i]); - } - no_auto_default = no_auto_default_from_file (priv->no_auto_default_file); keyfile_intern = intern_config_read (priv->intern_config_file, @@ -2624,6 +2619,13 @@ nm_config_reload (NMConfig *self, NMConfigChangeFlags reload_flags, gboolean emi (const char *const*) no_auto_default, keyfile, keyfile_intern); + + if (emit_warnings) { + nm_config_data_get_warnings (priv->config_data_orig, warnings); + for (i = 0; i < warnings->len; i++) + _LOGW ("%s", (const char *) warnings->pdata[i]); + } + g_free (config_main_file); g_free (config_description); g_key_file_unref (keyfile); @@ -2855,6 +2857,8 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) keyfile, keyfile_intern); + nm_config_data_get_warnings (priv->config_data_orig, warnings); + priv->config_data = g_object_ref (priv->config_data_orig); if (warnings->len > 0) { g_ptr_array_add (warnings, NULL); From 74d0571cb4de7ec1fdca50d20a844b52a2dd9d31 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 11 Dec 2019 11:40:22 +0100 Subject: [PATCH 5/6] config: emit warning about invalid main.auth-polkit setting (cherry picked from commit 8f96d3cb0c40ef19ab6524a39eeae9b03da1b4f3) --- src/nm-config-data.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index d8146457a3..c787aa98ac 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -407,8 +407,9 @@ nm_auth_polkit_mode_from_string (const char *str) return NM_AUTH_POLKIT_MODE_UNKNOWN; } -NMAuthPolkitMode -nm_config_data_get_main_auth_polkit (const NMConfigData *self) +static NMAuthPolkitMode +_config_data_get_main_auth_polkit (const NMConfigData *self, + gboolean *out_invalid_config) { NMAuthPolkitMode auth_polkit_mode; const char *str; @@ -420,16 +421,24 @@ nm_config_data_get_main_auth_polkit (const NMConfigData *self) | NM_CONFIG_GET_VALUE_NO_EMPTY); auth_polkit_mode = nm_auth_polkit_mode_from_string (str); if (auth_polkit_mode == NM_AUTH_POLKIT_MODE_UNKNOWN) { + NM_SET_OUT (out_invalid_config, (str != NULL)); auth_polkit_mode = nm_auth_polkit_mode_from_string (NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT); if (auth_polkit_mode == NM_AUTH_POLKIT_MODE_UNKNOWN) { nm_assert_not_reached (); auth_polkit_mode = NM_AUTH_POLKIT_MODE_ROOT_ONLY; } - } + } else + NM_SET_OUT (out_invalid_config, FALSE); return auth_polkit_mode; } +NMAuthPolkitMode +nm_config_data_get_main_auth_polkit (const NMConfigData *self) +{ + return _config_data_get_main_auth_polkit (self, NULL); +} + /*****************************************************************************/ /** @@ -1602,6 +1611,18 @@ void nm_config_data_get_warnings (const NMConfigData *self, GPtrArray *warnings) { + gboolean invalid; + + nm_assert (NM_IS_CONFIG_DATA (self)); + nm_assert (warnings); + + _config_data_get_main_auth_polkit (self, &invalid); + if (invalid) { + g_ptr_array_add (warnings, + g_strdup_printf ("invalid setting for %s.%s (should be one of \"true\", \"false\", \"root-only\")", + NM_CONFIG_KEYFILE_GROUP_MAIN, + NM_CONFIG_KEYFILE_KEY_MAIN_AUTH_POLKIT)); + } } /*****************************************************************************/ From 096da3a046597a27189a202862b2425d5bc7ea07 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 11 Dec 2019 11:48:25 +0100 Subject: [PATCH 6/6] config: print config warnings during `NetworkManager --print-config` (cherry picked from commit fd8c8ffe0d674beed69c910b7566cabdea8896ad) --- src/main.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/main.c b/src/main.c index b1466c1852..262b64841f 100644 --- a/src/main.c +++ b/src/main.c @@ -154,6 +154,7 @@ print_config (NMConfigCmdLineOptions *config_cli) gs_unref_object NMConfig *config = NULL; gs_free_error GError *error = NULL; NMConfigData *config_data; + const char *const*warnings; nm_logging_setup ("OFF", "ALL", NULL, NULL); @@ -166,6 +167,13 @@ print_config (NMConfigCmdLineOptions *config_cli) config_data = nm_config_get_data (config); fprintf (stdout, "# NetworkManager configuration: %s\n", nm_config_data_get_config_description (config_data)); nm_config_data_log (config_data, "", "", nm_config_get_no_auto_default_file (config), stdout); + + warnings = nm_config_get_warnings (config); + if (warnings && warnings[0]) + fprintf (stdout, "\n"); + for ( ; warnings && warnings[0]; warnings++) + fprintf (stdout, "# WARNING: %s\n", warnings[0]); + return 0; }