From 5c05a339c6665e3a35f6000a46dcd1da80fcdced Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 9 Feb 2026 12:02:02 +0100 Subject: [PATCH] udev-rules: downgrade error about non-system user/group in OWNER=/GROUP= This reverts (in sprit) commit f5cdf9515aceca2e91f9a33b74267e0cf5a5b7e8, "udev-rules: ignore non-system user/group in OWNER=/GROUP=". The original change was done to clean up a situation where we added a new group, but the group could already have been used for some other purposes, and now the some unexpected entity would own the device. Unfortunately, this check doesn't really address the issue, since the existing account might as well be a system account, which might be equally bad. In addition, this change is a big compatiblity break, causing existing rules to stop working. Since quite a lot of systems have local configuration to assign devices to users for various purposes, this is very noticable to users. In a way, the original change to add a new group was the compat break, and follow-up patch to cahnge the rule parsing evolved a small compat break into a much bigger one. There is merit to the change though, since device nodes shouldn't be owned by users and groups and different mechanisms should be used instead. To avoid breaking users systems, and since the original goal cannot be achieved by this patch, let's downgrade this to a warning to guide users towards different solutions. --- NEWS | 4 +-- catalog/systemd.catalog.in | 18 ++++++++++ src/systemd/sd-messages.h | 3 ++ src/udev/udev-rules.c | 56 ++++++++++++++++++++++++++++--- test/units/TEST-17-UDEV.verify.sh | 13 ++++--- 5 files changed, 84 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index 06c01d1c9cc..6ce07c82cb5 100644 --- a/NEWS +++ b/NEWS @@ -669,8 +669,8 @@ CHANGES WITH 258: an incompatible change of sorts, since per-user services will typically not be available for such PAM sessions of system users. - * systemd-udevd ignores OWNER=/GROUP= settings with a non-system - user/group specified in udev rules files, to avoid device nodes being + * systemd-udevd warns about OWNER=/GROUP= settings with a non-system + user/group specified in udev rules files. Device nodes should not be owned by a non-system user/group. It is recommended to check udev rules files with 'udevadm verify' and/or 'udevadm test' commands if the specified user/group in OWNER=/GROUP= are valid. diff --git a/catalog/systemd.catalog.in b/catalog/systemd.catalog.in index 2d926d8caee..911254ceb92 100644 --- a/catalog/systemd.catalog.in +++ b/catalog/systemd.catalog.in @@ -984,3 +984,21 @@ Documentation: man:systemd(1) An unsuccessful attempt has been made to break an ordering cycle between units for which jobs have been enqueued as part of a transaction. The transaction will fail. + +-- 3405205d368e49feb5ab3925fee13874 +Subject: Non-system user or group used for device ownership +Defined-By: systemd +Support: %SUPPORT_URL% +Documentation: man:systemd(1) systemd-udevd(8) + +The ownership of a device managed by systemd-udevd is assigned to a "regular" +(non-system) user or group. This is currently allowed for compatibility, but is +deprecated and discouraged. Ownership of a device node grants the privileges to +change ACLs, the group, access mode, or set labels or extended attributes, +which creates a conflict of management, because both udev and the user are in +power to change these attributes. In addition, device nodes appear early in +boot, while regular users may appear only later. + +For devices managed by systemd-udevd, it is instead recommended to use the +"uaccess"/"xaccess" mechanisms to grant limited and temporary access to device +nodes, see sd-login(8). diff --git a/src/systemd/sd-messages.h b/src/systemd/sd-messages.h index 6dc7dd527a0..5465db8b131 100644 --- a/src/systemd/sd-messages.h +++ b/src/systemd/sd-messages.h @@ -303,6 +303,9 @@ _SD_BEGIN_DECLARATIONS; #define SD_MESSAGE_CANT_BREAK_ORDERING_CYCLE SD_ID128_MAKE(b3,11,2d,da,d1,90,45,53,8c,76,68,5b,a5,91,8a,80) #define SD_MESSAGE_CANT_BREAK_ORDERING_CYCLE_STR SD_ID128_MAKE_STR(b3,11,2d,da,d1,90,45,53,8c,76,68,5b,a5,91,8a,80) +#define SD_MESSAGE_SYSTEM_ACCOUNT_REQUIRED SD_ID128_MAKE(34,05,20,5d,36,8e,49,fe,b5,ab,39,25,fe,e1,38,74) +#define SD_MESSAGE_SYSTEM_ACCOUNT_REQUIRED_STR SD_ID128_MAKE_STR(34,05,20,5d,36,8e,49,fe,b5,ab,39,25,fe,e1,38,74) + _SD_END_DECLARATIONS; #endif diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 6213a7e58f7..06686e3715e 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -5,6 +5,7 @@ #include #include "sd-json.h" +#include "sd-messages.h" #include "alloc-util.h" #include "architecture.h" @@ -50,6 +51,7 @@ #include "udev-trace.h" #include "udev-util.h" #include "udev-worker.h" +#include "uid-classification.h" #include "user-record.h" #include "user-util.h" #include "userdb.h" @@ -226,6 +228,26 @@ static bool token_is_for_parents(UdevRuleToken *token) { /*** Logging helpers ***/ +#define log_udev_event_syntax(event, token, level, message_id, fmt, ...) \ + ({ \ + UdevEvent *_event = (event); \ + UdevRuleToken *_token = ASSERT_PTR(token); \ + int _level = (level); \ + sd_device *_d = token_is_for_parents(_token) ? _event->dev_parent : _event->dev; \ + const char *_sysname = NULL; \ + \ + if (_d && log_get_max_level() >= LOG_PRI(_level)) \ + (void) sd_device_get_sysname(_d, &_sysname); \ + log_struct(_level, \ + LOG_MESSAGE("%s:%u %s:" fmt, \ + strna(_token->rule_line->rule_file ? _token->rule_line->rule_file->filename : NULL), \ + _token->rule_line->line_number, \ + _token->token_str, \ + __VA_ARGS__), \ + LOG_MESSAGE_ID(message_id), \ + LOG_ITEM("DEVICE=%s", strempty(_sysname))); \ + }) + #define _log_udev_rule_file_full(device, device_u, file, file_u, line_nr, level, level_u, error, fmt, ...) \ ({ \ int level_u = (level); \ @@ -509,13 +531,19 @@ static int rule_resolve_user(UdevRuleLine *rule_line, const char *name, uid_t *r } _cleanup_(user_record_unrefp) UserRecord *ur = NULL; - r = userdb_by_name(name, &USERDB_MATCH_ROOT_AND_SYSTEM, + r = userdb_by_name(name, /* match= */ NULL, USERDB_SUPPRESS_SHADOW | USERDB_PARSE_NUMERIC | USERDB_SYNTHESIZE_NUMERIC, &ur); if (r < 0) return log_line_error_errno(rule_line, r, "Failed to resolve user '%s', ignoring: %s", name, STRERROR_USER(r)); + if (!uid_is_system(ur->uid)) + log_struct(LOG_WARNING, + LOG_MESSAGE("%s:%u User %s configured to own a device node is not a system user. " + "Support for device node ownership by non-system accounts is deprecated and will be removed in the future.", + rule_line->rule_file->filename, rule_line->line_number, name), + LOG_MESSAGE_ID(SD_MESSAGE_SYSTEM_ACCOUNT_REQUIRED_STR)); _cleanup_free_ char *n = strdup(name); if (!n) @@ -544,13 +572,19 @@ static int rule_resolve_group(UdevRuleLine *rule_line, const char *name, gid_t * } _cleanup_(group_record_unrefp) GroupRecord *gr = NULL; - r = groupdb_by_name(name, &USERDB_MATCH_ROOT_AND_SYSTEM, + r = groupdb_by_name(name, /* match= */ NULL, USERDB_SUPPRESS_SHADOW | USERDB_PARSE_NUMERIC | USERDB_SYNTHESIZE_NUMERIC, &gr); if (r < 0) return log_line_error_errno(rule_line, r, "Failed to resolve group '%s', ignoring: %s", name, STRERROR_GROUP(r)); + if (!gid_is_system(gr->gid)) + log_struct(LOG_WARNING, + LOG_MESSAGE("%s:%u Group %s configured to own a device node is not a system group. " + "Support for device node ownership by non-system accounts is deprecated and will be removed in the future.", + rule_line->rule_file->filename, rule_line->line_number, name), + LOG_MESSAGE_ID(SD_MESSAGE_SYSTEM_ACCOUNT_REQUIRED_STR)); _cleanup_free_ char *n = strdup(name); if (!n) @@ -2674,7 +2708,7 @@ static int udev_rule_apply_token_to_event( return true; _cleanup_(user_record_unrefp) UserRecord *ur = NULL; - r = userdb_by_name(owner, &USERDB_MATCH_ROOT_AND_SYSTEM, + r = userdb_by_name(owner, /* match= */ NULL, USERDB_SUPPRESS_SHADOW | USERDB_PARSE_NUMERIC | USERDB_SYNTHESIZE_NUMERIC, &ur); if (r < 0) @@ -2682,6 +2716,13 @@ static int udev_rule_apply_token_to_event( "Failed to resolve user \"%s\", ignoring: %s", owner, STRERROR_USER(r)); else { + if (!uid_is_system(ur->uid)) + log_udev_event_syntax(event, token, LOG_WARNING, + SD_MESSAGE_SYSTEM_ACCOUNT_REQUIRED_STR, + "User %s configured to own a device node is not a system user. " + "Support for device node ownership by non-system accounts is deprecated and will be removed in the future.", + owner); + event->uid = ur->uid; log_event_debug(event, token, "Set owner: %s("UID_FMT")", owner, event->uid); } @@ -2700,7 +2741,7 @@ static int udev_rule_apply_token_to_event( return true; _cleanup_(group_record_unrefp) GroupRecord *gr = NULL; - r = groupdb_by_name(group, &USERDB_MATCH_ROOT_AND_SYSTEM, + r = groupdb_by_name(group, /* match= */ NULL, USERDB_SUPPRESS_SHADOW | USERDB_PARSE_NUMERIC | USERDB_SYNTHESIZE_NUMERIC, &gr); if (r < 0) @@ -2708,6 +2749,13 @@ static int udev_rule_apply_token_to_event( "Failed to resolve group \"%s\", ignoring: %s", group, STRERROR_GROUP(r)); else { + if (!gid_is_system(gr->gid)) + log_udev_event_syntax(event, token, LOG_WARNING, + SD_MESSAGE_SYSTEM_ACCOUNT_REQUIRED_STR, + "Group %s configured to own a device node is not a system group. " + "Support for device node ownership by non-system accounts is deprecated and will be removed in the future.", + group); + event->gid = gr->gid; log_event_debug(event, token, "Set group: %s("GID_FMT")", group, event->gid); } diff --git a/test/units/TEST-17-UDEV.verify.sh b/test/units/TEST-17-UDEV.verify.sh index 66f34a3e36f..66ab257be8a 100755 --- a/test/units/TEST-17-UDEV.verify.sh +++ b/test/units/TEST-17-UDEV.verify.sh @@ -315,8 +315,10 @@ if ! getent passwd 12345 >/dev/null; then fi # regular user if getent passwd testuser >/dev/null; then - test_syntax_error 'OWNER="testuser"' "Failed to resolve user 'testuser', ignoring: Not a system user" - test_syntax_error "OWNER=\"$(id -u testuser)\"" "Failed to resolve user '$(id -u testuser)', ignoring: Not a system user" + echo 'OWNER="testuser"' >"${rules}" + udevadm verify "${rules}" + echo "OWNER=\"$(id -u testuser)\"" >"${rules}" + udevadm verify "${rules}" fi test_syntax_error 'GROUP{a}="b"' 'Invalid attribute for GROUP.' test_syntax_error 'GROUP-="b"' 'Invalid operator for GROUP.' @@ -347,8 +349,11 @@ if ! getent group 12345 >/dev/null; then fi # regular group if getent group testuser >/dev/null; then - test_syntax_error 'GROUP="testuser"' "Failed to resolve group 'testuser', ignoring: Not a system group" - test_syntax_error "GROUP=\"$(id -g testuser)\"" "Failed to resolve group '$(id -g testuser)', ignoring: Not a system group" + echo 'GROUP="testuser"' >"${rules}" + udevadm verify "${rules}" + + echo "GROUP=\"$(id -g testuser)\"" >"${rules}" + udevadm verify "${rules}" fi test_syntax_error 'MODE{a}="b"' 'Invalid attribute for MODE.' test_syntax_error 'MODE-="b"' 'Invalid operator for MODE.' -- 2.47.3