diff --git a/NEWS b/NEWS index 06c01d1c9c..b5fba3cb99 100644 --- a/NEWS +++ b/NEWS @@ -31,6 +31,12 @@ CHANGES WITH 260 in spe: rather than the first, to keep these options coherent with other unit settings. + * Support for non-system users and groups in udev rules and + systemd-networkd configuration has been restored, but is deprecated + and discouraged. systemd-udevd and systemd-networkd will emit + warnings when non-system accounts are used. This support will be + removed in a future release. + Changes in other components: * New options SYSTEMD_COLORS=auto-16, SYSTEMD_COLORS=auto-256, and @@ -669,13 +675,13 @@ 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. - Similarly, systemd-networkd refuses User=/Group= settings with a - non-system user/group specified in .netdev files for Tun/Tap + Similarly, systemd-networkd will warn about User=/Group= settings + with a non-system user/group specified in .netdev files for Tun/Tap interfaces. * systemd-cryptenroll, systemd-repart and systemd-creds no longer diff --git a/catalog/systemd.catalog.in b/catalog/systemd.catalog.in index 8172a6128a..473edf2f07 100644 --- a/catalog/systemd.catalog.in +++ b/catalog/systemd.catalog.in @@ -981,6 +981,24 @@ Defined-By: systemd Support: %SUPPORT_URL% Documentation: man:systemd(1) -It has been attempted to break an ordering cycle between units for which jobs -have been enqueued as part of a transaction, but this was not successful. The -transaction will fail. +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) systemd-networkd(8) + +The ownership of a device managed by systemd-udevd or systemd-networkd 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/man/systemd.netdev.xml b/man/systemd.netdev.xml index 415feea046..6a84b7a648 100644 --- a/man/systemd.netdev.xml +++ b/man/systemd.netdev.xml @@ -2019,16 +2019,18 @@ Ports=eth2 User= - User to grant access to the /dev/net/tun device. The specified - user must be a system user. + User to grant access to the /dev/net/tun device. + Note that the user must be resolvable during early boot. Using non-system users + is deprecated. Group= - Group to grant access to the /dev/net/tun device. The specified - group must be a system group. + Group to grant access to the /dev/net/tun device. + Note that the group must be resolvable during early boot. Using non-system groups + is deprecated. diff --git a/src/network/netdev/netdev.h b/src/network/netdev/netdev.h index afc252480c..be8a05b70d 100644 --- a/src/network/netdev/netdev.h +++ b/src/network/netdev/netdev.h @@ -258,6 +258,16 @@ const struct ConfigPerfItem* network_netdev_gperf_lookup(const char *str, GPERF_ /* Macros which append INTERFACE= to the message */ +#define log_netdev_syntax(netdev, level, message_id, fmt, ...) \ + ({ \ + const NetDev *_n = (netdev); \ + const char *_ifname = _n ? _n->ifname : NULL; \ + log_struct(level, \ + LOG_MESSAGE(fmt, __VA_ARGS__), \ + LOG_MESSAGE_ID(message_id), \ + LOG_ITEM("INTERFACE=%s", strempty(_ifname))); \ + }) + #define log_netdev_full_errno_zerook(netdev, level, error, ...) \ ({ \ const NetDev *_n = (netdev); \ diff --git a/src/network/netdev/tuntap.c b/src/network/netdev/tuntap.c index 1288458819..25195680f6 100644 --- a/src/network/netdev/tuntap.c +++ b/src/network/netdev/tuntap.c @@ -6,6 +6,8 @@ #include #include +#include "sd-messages.h" + #include "alloc-util.h" #include "daemon-util.h" #include "fd-util.h" @@ -14,6 +16,7 @@ #include "socket-util.h" #include "string-util.h" #include "tuntap.h" +#include "uid-classification.h" #include "user-record.h" #include "user-util.h" #include "userdb.h" @@ -236,27 +239,43 @@ static int tuntap_verify(NetDev *netdev, const char *filename) { if (t->user_name) { _cleanup_(user_record_unrefp) UserRecord *ur = NULL; - r = userdb_by_name(t->user_name, &USERDB_MATCH_ROOT_AND_SYSTEM, + r = userdb_by_name(t->user_name, /* match = */ NULL, USERDB_SUPPRESS_SHADOW | USERDB_PARSE_NUMERIC, &ur); if (r < 0) log_netdev_warning_errno(netdev, r, "Cannot resolve user name '%s', ignoring: %s", t->user_name, STRERROR_USER(r)); - else + else { + if (!uid_is_system(ur->uid)) + log_netdev_syntax(netdev, LOG_WARNING, + SD_MESSAGE_SYSTEM_ACCOUNT_REQUIRED_STR, + "User '%s' configured as owner is not a system user. " + "Support for device node ownership by non-system accounts is deprecated and will be removed in the future.", + t->user_name); + t->uid = ur->uid; + } } if (t->group_name) { _cleanup_(group_record_unrefp) GroupRecord *gr = NULL; - r = groupdb_by_name(t->group_name, &USERDB_MATCH_ROOT_AND_SYSTEM, + r = groupdb_by_name(t->group_name, /* match = */ NULL, USERDB_SUPPRESS_SHADOW | USERDB_PARSE_NUMERIC, &gr); if (r < 0) log_netdev_warning_errno(netdev, r, "Cannot resolve group name '%s', ignoring: %s", t->group_name, STRERROR_GROUP(r)); - else + else { + if (!gid_is_system(gr->gid)) + log_netdev_syntax(netdev, LOG_WARNING, + SD_MESSAGE_SYSTEM_ACCOUNT_REQUIRED_STR, + "Group '%s' configured as owner is not a system group. " + "Support for device node ownership by non-system accounts is deprecated and will be removed in the future.", + t->group_name); + t->gid = gr->gid; + } } return 0; diff --git a/src/systemd/sd-messages.h b/src/systemd/sd-messages.h index 6dc7dd527a..5465db8b13 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 6213a7e58f..06686e3715 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 66f34a3e36..66ab257be8 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.'