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.'