From c21c6bc0be2a4467402bc2d8718859dedb10b676 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. --- 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 495529ec9a..022a90fec1 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 6d7446e52f5bfe379c2b1f54f9244b33fd236e32 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. --- 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 70d8ff29cb..44bf51a11f 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 c213d342b6..6d78ab161c 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 3e3124cacb..ab924e69a5 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" /*****************************************************************************/ @@ -55,7 +56,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 6998c5f129f70d676698de07667d6ee0334839ce 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. --- 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 8d20b9363b4461c7e86f2310a2fbfe88a09d5284 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. --- 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 8f96d3cb0c40ef19ab6524a39eeae9b03da1b4f3 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 --- 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 fd8c8ffe0d674beed69c910b7566cabdea8896ad 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` --- 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; }