diff --git a/contrib/fwupd.spec.in b/contrib/fwupd.spec.in index 42ea2024a..b011292b1 100644 --- a/contrib/fwupd.spec.in +++ b/contrib/fwupd.spec.in @@ -326,9 +326,6 @@ for fn in /etc/fwupd/remotes.d/*.conf; do fi done -# ensure this is private -chmod 0660 /etc/fwupd/redfish.conf - %preun %systemd_preun fwupd.service diff --git a/libfwupd/fwupd-enums.c b/libfwupd/fwupd-enums.c index b46533e74..46df3d261 100644 --- a/libfwupd/fwupd-enums.c +++ b/libfwupd/fwupd-enums.c @@ -440,6 +440,8 @@ fwupd_plugin_flag_to_string(FwupdPluginFlags plugin_flag) return "unknown"; if (plugin_flag == FWUPD_PLUGIN_FLAG_AUTH_REQUIRED) return "auth-required"; + if (plugin_flag == FWUPD_PLUGIN_FLAG_SECURE_CONFIG) + return "secure-config"; return NULL; } @@ -484,6 +486,8 @@ fwupd_plugin_flag_from_string(const gchar *plugin_flag) return FWUPD_PLUGIN_FLAG_KERNEL_TOO_OLD; if (g_strcmp0(plugin_flag, "auth-required") == 0) return FWUPD_PLUGIN_FLAG_AUTH_REQUIRED; + if (g_strcmp0(plugin_flag, "secure-config") == 0) + return FWUPD_PLUGIN_FLAG_SECURE_CONFIG; return FWUPD_DEVICE_FLAG_UNKNOWN; } diff --git a/libfwupd/fwupd-enums.h b/libfwupd/fwupd-enums.h index 2828294bf..3502cf831 100644 --- a/libfwupd/fwupd-enums.h +++ b/libfwupd/fwupd-enums.h @@ -854,6 +854,15 @@ typedef enum { * Since: 1.6.2 */ #define FWUPD_PLUGIN_FLAG_AUTH_REQUIRED (1u << 12) +/** + * FWUPD_PLUGIN_FLAG_SECURE_CONFIG: + * + * The plugin requires the config file to be saved with permissions that only allow the root user + * to read. + * + * Since: 1.8.5 + */ +#define FWUPD_PLUGIN_FLAG_SECURE_CONFIG (1u << 13) /** * FWUPD_PLUGIN_FLAG_UNKNOWN: * diff --git a/libfwupd/fwupd-self-test.c b/libfwupd/fwupd-self-test.c index 649db344a..379e8e22b 100644 --- a/libfwupd/fwupd-self-test.c +++ b/libfwupd/fwupd-self-test.c @@ -170,7 +170,7 @@ fwupd_enums_func(void) g_assert_cmpstr(tmp, !=, NULL); g_assert_cmpint(fwupd_device_problem_from_string(tmp), ==, i); } - for (guint64 i = 1; i <= FWUPD_PLUGIN_FLAG_AUTH_REQUIRED; i *= 2) { + for (guint64 i = 1; i <= FWUPD_PLUGIN_FLAG_SECURE_CONFIG; i *= 2) { const gchar *tmp = fwupd_plugin_flag_to_string(i); if (tmp == NULL) g_warning("missing plugin flag 0x%x", (guint)i); diff --git a/libfwupdplugin/README.md b/libfwupdplugin/README.md index 5f288cbc4..857d17d22 100644 --- a/libfwupdplugin/README.md +++ b/libfwupdplugin/README.md @@ -98,3 +98,4 @@ Remember: Plugins should be upstream! ## 1.8.5 * `fu_volume_new_esp_default()`: Use `fu_context_get_esp_volumes()` instead. +* `fu_plugin_set_secure_config_value()`: Set `FWUPD_PLUGIN_FLAG_SECURE_CONFIG` and use `fu_plugin_set_config_value()` diff --git a/libfwupdplugin/fu-plugin.c b/libfwupdplugin/fu-plugin.c index b431f6d41..4feee4c05 100644 --- a/libfwupdplugin/fu-plugin.c +++ b/libfwupdplugin/fu-plugin.c @@ -84,6 +84,9 @@ typedef gboolean (*FuPluginFlaggedDeviceFunc)(FuPlugin *self, GError **error); typedef gboolean (*FuPluginDeviceArrayFunc)(FuPlugin *self, GPtrArray *devices, GError **error); +#define FU_PLUGIN_FILE_MODE_NONSECURE 0644 +#define FU_PLUGIN_FILE_MODE_SECURE 0640 + /** * fu_plugin_is_open: * @self: a #FuPlugin @@ -299,6 +302,14 @@ fu_plugin_guess_name_from_fn(const gchar *filename) return name; } +static gchar * +fu_plugin_get_config_filename(FuPlugin *self) +{ + g_autofree gchar *conf_dir = fu_path_from_kind(FU_PATH_KIND_SYSCONFDIR_PKG); + g_autofree gchar *conf_file = g_strdup_printf("%s.conf", fu_plugin_get_name(self)); + return g_build_filename(conf_dir, conf_file, NULL); +} + /** * fu_plugin_open: * @self: a #FuPlugin @@ -355,6 +366,23 @@ fu_plugin_open(FuPlugin *self, const gchar *filename, GError **error) fu_plugin_set_name(self, str); } + /* ensure the configure file is set to the correct permission */ + if (fu_plugin_has_flag(self, FWUPD_PLUGIN_FLAG_SECURE_CONFIG)) { + g_autofree gchar *conf_path = fu_plugin_get_config_filename(self); + if (g_file_test(conf_path, G_FILE_TEST_EXISTS)) { + gint rc = g_chmod(conf_path, FU_PLUGIN_FILE_MODE_SECURE); + if (rc != 0) { + g_set_error(error, + G_IO_ERROR, + G_IO_ERROR_FAILED, + "failed to change permission of %s", + filename); + fu_plugin_add_flag(self, FWUPD_PLUGIN_FLAG_FAILED_OPEN); + return FALSE; + } + } + } + /* optional */ if (vfuncs->load != NULL) { FuContext *ctx = fu_plugin_get_context(self); @@ -502,14 +530,6 @@ fu_plugin_config_monitor_changed_cb(GFileMonitor *monitor, g_signal_emit(self, signals[SIGNAL_CONFIG_CHANGED], 0); } -static gchar * -fu_plugin_get_config_filename(FuPlugin *self) -{ - g_autofree gchar *conf_dir = fu_path_from_kind(FU_PATH_KIND_SYSCONFDIR_PKG); - g_autofree gchar *conf_file = g_strdup_printf("%s.conf", fu_plugin_get_name(self)); - return g_build_filename(conf_dir, conf_file, NULL); -} - /** * fu_plugin_device_add: * @self: a #FuPlugin @@ -2388,36 +2408,6 @@ fu_plugin_security_attr_new(FuPlugin *self, const gchar *appstream_id) return g_steal_pointer(&attr); } -/** - * fu_plugin_set_config_value: - * @self: a #FuPlugin - * @key: a settings key - * @value: (nullable): a settings value - * @error: (nullable): optional return location for an error - * - * Sets a plugin config value. - * - * Returns: %TRUE for success - * - * Since: 1.7.0 - **/ -gboolean -fu_plugin_set_config_value(FuPlugin *self, const gchar *key, const gchar *value, GError **error) -{ - g_autofree gchar *conf_path = fu_plugin_get_config_filename(self); - g_autoptr(GKeyFile) keyfile = NULL; - - g_return_val_if_fail(FU_IS_PLUGIN(self), FALSE); - g_return_val_if_fail(key != NULL, FALSE); - g_return_val_if_fail(error == NULL || *error == NULL, FALSE); - - keyfile = g_key_file_new(); - if (!g_key_file_load_from_file(keyfile, conf_path, G_KEY_FILE_KEEP_COMMENTS, error)) - return FALSE; - g_key_file_set_string(keyfile, fu_plugin_get_name(self), key, value); - return g_key_file_save_to_file(keyfile, conf_path, error); -} - #if !GLIB_CHECK_VERSION(2, 66, 0) #define G_FILE_SET_CONTENTS_CONSISTENT 0 @@ -2458,25 +2448,12 @@ g_file_set_contents_full(const gchar *filename, } #endif -/** - * fu_plugin_set_secure_config_value: - * @self: a #FuPlugin - * @key: a settings key - * @value: (nullable): a settings value - * @error: (nullable): optional return location for an error - * - * Sets a plugin config file value and updates file so that non-privileged users - * cannot read it. - * - * Returns: %TRUE for success - * - * Since: 1.7.4 - **/ -gboolean -fu_plugin_set_secure_config_value(FuPlugin *self, - const gchar *key, - const gchar *value, - GError **error) +static gboolean +fu_plugin_set_config_value_internal(FuPlugin *self, + const gchar *key, + const gchar *value, + guint32 mode, + GError **error) { g_autofree gchar *conf_path = fu_plugin_get_config_filename(self); g_autofree gchar *data = NULL; @@ -2499,10 +2476,75 @@ fu_plugin_set_secure_config_value(FuPlugin *self, data, -1, G_FILE_SET_CONTENTS_CONSISTENT, - 0660, + mode, error); } +/** + * fu_plugin_set_secure_config_value: + * @self: a #FuPlugin + * @key: a settings key + * @value: (nullable): a settings value + * @error: (nullable): optional return location for an error + * + * Sets a plugin config file value and updates file so that non-privileged users + * cannot read it. + * + * This function is deprecated, and you should use `FWUPD_PLUGIN_FLAG_SECURE_CONFIG` and + * fu_plugin_set_config_value() instead. + * + * Returns: %TRUE for success + * + * Since: 1.7.4 + **/ +gboolean +fu_plugin_set_secure_config_value(FuPlugin *self, + const gchar *key, + const gchar *value, + GError **error) +{ + g_return_val_if_fail(FU_IS_PLUGIN(self), FALSE); + g_return_val_if_fail(key != NULL, FALSE); + g_return_val_if_fail(error == NULL || *error == NULL, FALSE); + fu_plugin_add_flag(self, FWUPD_PLUGIN_FLAG_SECURE_CONFIG); + return fu_plugin_set_config_value(self, key, value, error); +} + +/** + * fu_plugin_set_config_value: + * @self: a #FuPlugin + * @key: a settings key + * @value: (nullable): a settings value + * @error: (nullable): optional return location for an error + * + * Sets a plugin config value. + * + * Returns: %TRUE for success + * + * Since: 1.7.0 + **/ +gboolean +fu_plugin_set_config_value(FuPlugin *self, const gchar *key, const gchar *value, GError **error) +{ + g_return_val_if_fail(FU_IS_PLUGIN(self), FALSE); + g_return_val_if_fail(key != NULL, FALSE); + g_return_val_if_fail(error == NULL || *error == NULL, FALSE); + + /* when removing fu_plugin_set_secure_config_value() just remove this instead */ + if (fu_plugin_has_flag(self, FWUPD_PLUGIN_FLAG_SECURE_CONFIG)) { + return fu_plugin_set_config_value_internal(self, + key, + value, + FU_PLUGIN_FILE_MODE_SECURE, + error); + } + return fu_plugin_set_config_value_internal(self, + key, + value, + FU_PLUGIN_FILE_MODE_NONSECURE, + error); +} + /** * fu_plugin_get_config_value_boolean: * @self: a #FuPlugin diff --git a/libfwupdplugin/fu-plugin.h b/libfwupdplugin/fu-plugin.h index 1221ee049..cfe18f1c6 100644 --- a/libfwupdplugin/fu-plugin.h +++ b/libfwupdplugin/fu-plugin.h @@ -460,6 +460,7 @@ void fu_plugin_add_report_metadata(FuPlugin *self, const gchar *key, const gchar *value); gchar * fu_plugin_get_config_value(FuPlugin *self, const gchar *key); +G_DEPRECATED_FOR(fu_plugin_set_config_value) gboolean fu_plugin_set_secure_config_value(FuPlugin *self, const gchar *key, diff --git a/libfwupdplugin/fu-self-test.c b/libfwupdplugin/fu-self-test.c index aaf49c172..ab2d6550e 100644 --- a/libfwupdplugin/fu-self-test.c +++ b/libfwupdplugin/fu-self-test.c @@ -721,8 +721,9 @@ fu_plugin_config_func(void) g_assert_cmpstr(value, ==, "True"); g_assert_true(fu_plugin_get_config_value_boolean(plugin, "Key")); - /* check it is private, i.e. only readable by the user/group */ - ret = fu_plugin_set_secure_config_value(plugin, "Key", "False", &error); + /* write it private, i.e. only readable by the user/group */ + fu_plugin_add_flag(plugin, FWUPD_PLUGIN_FLAG_SECURE_CONFIG); + ret = fu_plugin_set_config_value(plugin, "Key", "False", &error); g_assert_no_error(error); g_assert_true(ret); rc = g_stat(fn, &statbuf); diff --git a/plugins/redfish/fu-plugin-redfish.c b/plugins/redfish/fu-plugin-redfish.c index ef6618c65..7004ceb67 100644 --- a/plugins/redfish/fu-plugin-redfish.c +++ b/plugins/redfish/fu-plugin-redfish.c @@ -63,7 +63,7 @@ fu_plugin_redfish_change_expired(FuPlugin *plugin, GError **error) uri = fu_plugin_get_config_value(plugin, "UserUri"); if (uri == NULL) { uri = g_strdup("/redfish/v1/AccountService/Accounts/2"); - if (!fu_plugin_set_secure_config_value(plugin, "UserUri", uri, error)) + if (!fu_plugin_set_config_value(plugin, "UserUri", uri, error)) return FALSE; } @@ -83,7 +83,7 @@ fu_plugin_redfish_change_expired(FuPlugin *plugin, GError **error) fu_redfish_backend_set_password(priv->backend, password_new); /* success */ - return fu_plugin_set_secure_config_value(plugin, "Password", password_new, error); + return fu_plugin_set_config_value(plugin, "Password", password_new, error); } static gboolean @@ -352,11 +352,11 @@ fu_redfish_plugin_ipmi_create_user(FuPlugin *plugin, GError **error) fu_redfish_backend_set_password(priv->backend, password_new); /* success */ - if (!fu_plugin_set_secure_config_value(plugin, "UserUri", uri, error)) + if (!fu_plugin_set_config_value(plugin, "UserUri", uri, error)) return FALSE; - if (!fu_plugin_set_secure_config_value(plugin, "Username", username_fwupd, error)) + if (!fu_plugin_set_config_value(plugin, "Username", username_fwupd, error)) return FALSE; - if (!fu_plugin_set_secure_config_value(plugin, "Password", password_new, error)) + if (!fu_plugin_set_config_value(plugin, "Password", password_new, error)) return FALSE; return TRUE; @@ -594,6 +594,7 @@ fu_plugin_redfish_init(FuPlugin *plugin) FuPluginData *priv = fu_plugin_alloc_data(plugin, sizeof(FuPluginData)); priv->backend = fu_redfish_backend_new(ctx); fu_plugin_add_firmware_gtype(plugin, NULL, FU_TYPE_REDFISH_SMBIOS); + fu_plugin_add_flag(plugin, FWUPD_PLUGIN_FLAG_SECURE_CONFIG); } static void diff --git a/src/fu-util-common.c b/src/fu-util-common.c index b63ef7939..29d49ada2 100644 --- a/src/fu-util-common.c +++ b/src/fu-util-common.c @@ -1690,6 +1690,10 @@ fu_util_plugin_flag_to_string(FwupdPluginFlags plugin_flag) /* TRANSLATORS: user needs to run a command */ return _("Authentication details are required"); } + if (plugin_flag == FWUPD_PLUGIN_FLAG_SECURE_CONFIG) { + /* TRANSLATORS: no peeking */ + return _("Configuration is only readable by the system administrator"); + } if (plugin_flag == FWUPD_PLUGIN_FLAG_EFIVAR_NOT_MOUNTED) { /* TRANSLATORS: the user is using Gentoo/Arch and has screwed something up */ return _("Required efivarfs filesystem was not found"); @@ -1731,6 +1735,7 @@ fu_util_plugin_flag_to_cli_text(FwupdPluginFlags plugin_flag) case FWUPD_PLUGIN_FLAG_CAPSULES_UNSUPPORTED: case FWUPD_PLUGIN_FLAG_UNLOCK_REQUIRED: case FWUPD_PLUGIN_FLAG_AUTH_REQUIRED: + case FWUPD_PLUGIN_FLAG_SECURE_CONFIG: case FWUPD_PLUGIN_FLAG_EFIVAR_NOT_MOUNTED: case FWUPD_PLUGIN_FLAG_ESP_NOT_FOUND: case FWUPD_PLUGIN_FLAG_KERNEL_TOO_OLD: