system/systemd
system/systemd
1
0
Fork 0
mirror of https://github.com/systemd/systemd synced 2024-10-02 22:37:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

shared/specifier: fix %u/%U/%g/%G when called as unprivileged user

Browse source 
We would resolve those specifiers to the calling user/group. This is mostly OK
when done in the manager, because the manager generally operates as root
in system mode, and a non-root in user mode. It would still be wrong if
called with --test though. But in systemctl, this would be generally wrong,
since we can call 'systemctl --system' as a normal user, either for testing
or even for actual operation with '--root=…'.

When operating in --global mode, %u/%U/%g/%G should return an error.

The information whether we're operating in system mode, user mode, or global
mode is passed as the data pointer to specifier_group_name(), specifier_user_name(),
specifier_group_id(), specifier_user_id(). We can't use userdata, because
it's already used for other things.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2022-03-09 22:29:19 +01:00
parent 4a84db4c0c
commit 172e9cc3ee
11 changed files with 150 additions and 99 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/core/unit-printf.c
View file

@ -183,7 +183,7 @@ int unit_name_printf(const Unit *u, const char* format, char **ret) {
COMMON_SYSTEM_SPECIFIERS,
COMMON_CREDS_SPECIFIERS,
COMMON_CREDS_SPECIFIERS(u->manager->unit_file_scope),
{}
};
@ -253,7 +253,7 @@ int unit_full_printf_full(const Unit *u, const char *format, size_t max_length,
COMMON_SYSTEM_SPECIFIERS,
COMMON_CREDS_SPECIFIERS,
COMMON_CREDS_SPECIFIERS(u->manager->unit_file_scope),
COMMON_TMP_SPECIFIERS,
{}

9
src/shared/install-printf.c
View file

@ -97,7 +97,12 @@ static int specifier_last_component(char specifier, const void *data, const char
return 0;
}
int install_name_printf(const UnitFileInstallInfo *i, const char *format, const char *root, char **ret) {
int install_name_printf(
UnitFileScope scope,
const UnitFileInstallInfo *i,
const char *format,
const char *root,
char **ret) {
/* This is similar to unit_name_printf() */
const Specifier table[] = {
@ -109,7 +114,7 @@ int install_name_printf(const UnitFileInstallInfo *i, const char *format, const
COMMON_SYSTEM_SPECIFIERS,
COMMON_CREDS_SPECIFIERS,
COMMON_CREDS_SPECIFIERS(scope),
{}
};

7
src/shared/install-printf.h
View file

@ -4,4 +4,9 @@
#include "install.h"
#include "unit-name.h"
int install_name_printf(const UnitFileInstallInfo *i, const char *format, const char *root, char **ret);
int install_name_printf(
UnitFileScope scope,
const UnitFileInstallInfo *i,
const char *format,
const char *root,
char **ret);

9
src/shared/install.c
View file

@ -1157,7 +1157,7 @@ static int config_parse_also(
if (r == 0)
break;
r = install_name_printf(info, word, info->root, &printed);
r = install_name_printf(ctx->scope, info, word, info->root, &printed);
if (r < 0)
return log_syntax(unit, LOG_WARNING, filename, line, r,
"Failed to resolve unit name in Also=\"%s\": %m", word);
@ -1188,6 +1188,7 @@ static int config_parse_default_instance(
void *data,
void *userdata) {
InstallContext *ctx = ASSERT_PTR(data);
UnitFileInstallInfo *info = ASSERT_PTR(userdata);
_cleanup_free_ char *printed = NULL;
int r;
@ -1205,7 +1206,7 @@ static int config_parse_default_instance(
return log_syntax(unit, LOG_WARNING, filename, line, 0,
"DefaultInstance= only makes sense for template units, ignoring.");
r = install_name_printf(info, rvalue, info->root, &printed);
r = install_name_printf(ctx->scope, info, rvalue, info->root, &printed);
if (r < 0)
return log_syntax(unit, LOG_WARNING, filename, line, r,
"Failed to resolve instance name in DefaultInstance=\"%s\": %m", rvalue);
@ -1772,7 +1773,7 @@ static int install_info_symlink_alias(
STRV_FOREACH(s, info->aliases) {
_cleanup_free_ char *alias_path = NULL, *dst = NULL, *dst_updated = NULL;
q = install_name_printf(info, *s, info->root, &dst);
q = install_name_printf(scope, info, *s, info->root, &dst);
if (q < 0) {
unit_file_changes_add(changes, n_changes, q, *s, NULL);
return q;
@ -1858,7 +1859,7 @@ static int install_info_symlink_wants(
STRV_FOREACH(s, list) {
_cleanup_free_ char *path = NULL, *dst = NULL;
q = install_name_printf(info, *s, info->root, &dst);
q = install_name_printf(scope, info, *s, info->root, &dst);
if (q < 0) {
unit_file_changes_add(changes, n_changes, q, *s, NULL);
return q;

44
src/shared/specifier.c
View file

@ -22,6 +22,7 @@
#include "specifier.h"
#include "string-util.h"
#include "strv.h"
#include "unit-file.h"
#include "user-util.h"
/*
@ -309,11 +310,15 @@ int specifier_os_image_version(char specifier, const void *data, const char *roo
}
int specifier_group_name(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
UnitFileScope scope = PTR_TO_INT(data);
char *t;
assert(ret);
t = gid_to_name(getgid());
if (scope == UNIT_FILE_GLOBAL)
return -EINVAL;
t = gid_to_name(scope == UNIT_FILE_USER ? getgid() : 0);
if (!t)
return -ENOMEM;
@ -322,27 +327,42 @@ int specifier_group_name(char specifier, const void *data, const char *root, con
}
int specifier_group_id(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
UnitFileScope scope = PTR_TO_INT(data);
gid_t gid;
assert(ret);
if (asprintf(ret, UID_FMT, getgid()) < 0)
if (scope == UNIT_FILE_GLOBAL)
return -EINVAL;
gid = scope == UNIT_FILE_USER ? getgid() : 0;
if (asprintf(ret, UID_FMT, gid) < 0)
return -ENOMEM;
return 0;
}
int specifier_user_name(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
UnitFileScope scope = PTR_TO_INT(data);
uid_t uid;
char *t;
assert(ret);
/* If we are UID 0 (root), this will not result in NSS, otherwise it might. This is good, as we want to be able
* to run this in PID 1, where our user ID is 0, but where NSS lookups are not allowed.
if (scope == UNIT_FILE_GLOBAL)
return -EINVAL;
* We don't use getusername_malloc() here, because we don't want to look at $USER, to remain consistent with
* specifer_user_id() below.
uid = scope == UNIT_FILE_USER ? getuid() : 0;
/* If we are UID 0 (root), this will not result in NSS, otherwise it might. This is good, as we want
* to be able to run this in PID 1, where our user ID is 0, but where NSS lookups are not allowed.
* We don't use getusername_malloc() here, because we don't want to look at $USER, to remain
* consistent with specifer_user_id() below.
*/
t = uid_to_name(getuid());
t = uid_to_name(uid);
if (!t)
return -ENOMEM;
@ -351,9 +371,17 @@ int specifier_user_name(char specifier, const void *data, const char *root, cons
}
int specifier_user_id(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
UnitFileScope scope = PTR_TO_INT(data);
uid_t uid;
assert(ret);
if (asprintf(ret, UID_FMT, getuid()) < 0)
if (scope == UNIT_FILE_GLOBAL)
return -EINVAL;
uid = scope == UNIT_FILE_USER ? getuid() : 0;
if (asprintf(ret, UID_FMT, uid) < 0)
return -ENOMEM;
return 0;

10
src/shared/specifier.h
View file

@ -84,11 +84,11 @@ int specifier_var_tmp_dir(char specifier, const void *data, const char *root, co
{ 'w', specifier_os_version_id, NULL }, \
{ 'W', specifier_os_variant_id, NULL }
#define COMMON_CREDS_SPECIFIERS \
{ 'g', specifier_group_name, NULL }, \
{ 'G', specifier_group_id, NULL }, \
{ 'u', specifier_user_name, NULL }, \
{ 'U', specifier_user_id, NULL }
#define COMMON_CREDS_SPECIFIERS(scope) \
{ 'g', specifier_group_name, INT_TO_PTR(scope) }, \
{ 'G', specifier_group_id, INT_TO_PTR(scope) }, \
{ 'u', specifier_user_name, INT_TO_PTR(scope) }, \
{ 'U', specifier_user_id, INT_TO_PTR(scope) }
#define COMMON_TMP_SPECIFIERS \
{ 'T', specifier_tmp_dir, NULL }, \

91
src/test/test-load-fragment.c
View file

@ -510,59 +510,74 @@ TEST(install_printf, .sd_booted = true) {
assert_se(user = uid_to_name(getuid()));
assert_se(asprintf(&uid, UID_FMT, getuid()) >= 0);
#define expect(src, pattern, result) \
#define expect(scope, src, pattern, result) \
do { \
_cleanup_free_ char *t = NULL; \
_cleanup_free_ char \
*d1 = strdup(i.name), \
*d2 = strdup(i.path); \
assert_se(install_name_printf(&src, pattern, NULL, &t) >= 0 || !result); \
_cleanup_free_ char *t = NULL, \
*d1 = ASSERT_PTR(strdup(i.name)), \
*d2 = ASSERT_PTR(strdup(i.path)); \
int r = install_name_printf(scope, &src, pattern, NULL, &t); \
assert_se(result ? r >= 0 : r < 0); \
memzero(i.name, strlen(i.name)); \
memzero(i.path, strlen(i.path)); \
assert_se(d1 && d2); \
if (result) { \
printf("%s\n", t); \
assert_se(streq(t, result)); \
} else assert_se(t == NULL); \
} else \
assert_se(!t); \
strcpy(i.name, d1); \
strcpy(i.path, d2); \
} while (false)
expect(i, "%n", "name.service");
expect(i, "%N", "name");
expect(i, "%p", "name");
expect(i, "%i", "");
expect(i, "%j", "name");
expect(i, "%g", group);
expect(i, "%G", gid);
expect(i, "%u", user);
expect(i, "%U", uid);
expect(UNIT_FILE_SYSTEM, i, "%n", "name.service");
expect(UNIT_FILE_SYSTEM, i, "%N", "name");
expect(UNIT_FILE_SYSTEM, i, "%p", "name");
expect(UNIT_FILE_SYSTEM, i, "%i", "");
expect(UNIT_FILE_SYSTEM, i, "%j", "name");
expect(UNIT_FILE_SYSTEM, i, "%g", "root");
expect(UNIT_FILE_SYSTEM, i, "%G", "0");
expect(UNIT_FILE_SYSTEM, i, "%u", "root");
expect(UNIT_FILE_SYSTEM, i, "%U", "0");
expect(i, "%m", mid);
expect(i, "%b", bid);
expect(i, "%H", host);
expect(UNIT_FILE_SYSTEM, i, "%m", mid);
expect(UNIT_FILE_SYSTEM, i, "%b", bid);
expect(UNIT_FILE_SYSTEM, i, "%H", host);
expect(i2, "%g", group);
expect(i2, "%G", gid);
expect(i2, "%u", user);
expect(i2, "%U", uid);
expect(UNIT_FILE_SYSTEM, i2, "%g", "root");
expect(UNIT_FILE_SYSTEM, i2, "%G", "0");
expect(UNIT_FILE_SYSTEM, i2, "%u", "root");
expect(UNIT_FILE_SYSTEM, i2, "%U", "0");
expect(i3, "%n", "name@inst.service");
expect(i3, "%N", "name@inst");
expect(i3, "%p", "name");
expect(i3, "%g", group);
expect(i3, "%G", gid);
expect(i3, "%u", user);
expect(i3, "%U", uid);
expect(UNIT_FILE_USER, i2, "%g", group);
expect(UNIT_FILE_USER, i2, "%G", gid);
expect(UNIT_FILE_USER, i2, "%u", user);
expect(UNIT_FILE_USER, i2, "%U", uid);
expect(i3, "%m", mid);
expect(i3, "%b", bid);
expect(i3, "%H", host);
/* gcc-12.0.1-0.9.fc36.x86_64 insist that streq(…, NULL) is called,
* even though the call is inside of a conditional where the pointer is checked. :( */
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wnonnull"
expect(UNIT_FILE_GLOBAL, i2, "%g", NULL);
expect(UNIT_FILE_GLOBAL, i2, "%G", NULL);
expect(UNIT_FILE_GLOBAL, i2, "%u", NULL);
expect(UNIT_FILE_GLOBAL, i2, "%U", NULL);
#pragma GCC diagnostic pop
expect(i4, "%g", group);
expect(i4, "%G", gid);
expect(i4, "%u", user);
expect(i4, "%U", uid);
expect(UNIT_FILE_SYSTEM, i3, "%n", "name@inst.service");
expect(UNIT_FILE_SYSTEM, i3, "%N", "name@inst");
expect(UNIT_FILE_SYSTEM, i3, "%p", "name");
expect(UNIT_FILE_USER, i3, "%g", group);
expect(UNIT_FILE_USER, i3, "%G", gid);
expect(UNIT_FILE_USER, i3, "%u", user);
expect(UNIT_FILE_USER, i3, "%U", uid);
expect(UNIT_FILE_SYSTEM, i3, "%m", mid);
expect(UNIT_FILE_SYSTEM, i3, "%b", bid);
expect(UNIT_FILE_SYSTEM, i3, "%H", host);
expect(UNIT_FILE_USER, i4, "%g", group);
expect(UNIT_FILE_USER, i4, "%G", gid);
expect(UNIT_FILE_USER, i4, "%u", user);
expect(UNIT_FILE_USER, i4, "%U", uid);
}
static uint64_t make_cap(int cap) {

3
src/test/test-specifier.c
View file

@ -8,6 +8,7 @@
#include "string-util.h"
#include "strv.h"
#include "tests.h"
#include "unit-file.h"
static void test_specifier_escape_one(const char *a, const char *b) {
_cleanup_free_ char *x = NULL;
@ -46,7 +47,7 @@ TEST(specifier_escape_strv) {
static const Specifier specifier_table[] = {
COMMON_SYSTEM_SPECIFIERS,
COMMON_CREDS_SPECIFIERS,
COMMON_CREDS_SPECIFIERS(UNIT_FILE_USER),
{ 'h', specifier_user_home, NULL },
COMMON_TMP_SPECIFIERS,

53
src/tmpfiles/tmpfiles.c
View file

@ -204,31 +204,6 @@ STATIC_DESTRUCTOR_REGISTER(arg_image, freep);
static int specifier_machine_id_safe(char specifier, const void *data, const char *root, const void *userdata, char **ret);
static int specifier_directory(char specifier, const void *data, const char *root, const void *userdata, char **ret);
static const Specifier specifier_table[] = {
{ 'a', specifier_architecture, NULL },
{ 'b', specifier_boot_id, NULL },
{ 'B', specifier_os_build_id, NULL },
{ 'H', specifier_host_name, NULL },
{ 'l', specifier_short_host_name, NULL },
{ 'm', specifier_machine_id_safe, NULL },
{ 'o', specifier_os_id, NULL },
{ 'v', specifier_kernel_release, NULL },
{ 'w', specifier_os_version_id, NULL },
{ 'W', specifier_os_variant_id, NULL },
{ 'h', specifier_user_home, NULL },
{ 'C', specifier_directory, UINT_TO_PTR(DIRECTORY_CACHE) },
{ 'L', specifier_directory, UINT_TO_PTR(DIRECTORY_LOGS) },
{ 'S', specifier_directory, UINT_TO_PTR(DIRECTORY_STATE) },
{ 't', specifier_directory, UINT_TO_PTR(DIRECTORY_RUNTIME) },
COMMON_CREDS_SPECIFIERS,
COMMON_TMP_SPECIFIERS,
{}
};
static int specifier_machine_id_safe(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
int r;
@ -2737,7 +2712,7 @@ static bool should_include_path(const char *path) {
return false;
}
static int specifier_expansion_from_arg(Item *i) {
static int specifier_expansion_from_arg(const Specifier *specifier_table, Item *i) {
int r;
assert(i);
@ -2944,6 +2919,30 @@ static int parse_line(
assert(line >= 1);
assert(buffer);
const Specifier specifier_table[] = {
{ 'a', specifier_architecture, NULL },
{ 'b', specifier_boot_id, NULL },
{ 'B', specifier_os_build_id, NULL },
{ 'H', specifier_host_name, NULL },
{ 'l', specifier_short_host_name, NULL },
{ 'm', specifier_machine_id_safe, NULL },
{ 'o', specifier_os_id, NULL },
{ 'v', specifier_kernel_release, NULL },
{ 'w', specifier_os_version_id, NULL },
{ 'W', specifier_os_variant_id, NULL },
{ 'h', specifier_user_home, NULL },
{ 'C', specifier_directory, UINT_TO_PTR(DIRECTORY_CACHE) },
{ 'L', specifier_directory, UINT_TO_PTR(DIRECTORY_LOGS) },
{ 'S', specifier_directory, UINT_TO_PTR(DIRECTORY_STATE) },
{ 't', specifier_directory, UINT_TO_PTR(DIRECTORY_RUNTIME) },
COMMON_CREDS_SPECIFIERS(arg_user ? UNIT_FILE_USER : UNIT_FILE_SYSTEM),
COMMON_TMP_SPECIFIERS,
{}
};
r = extract_many_words(
&buffer,
NULL,
@ -3148,7 +3147,7 @@ static int parse_line(
if (!should_include_path(i.path))
return 0;
r = specifier_expansion_from_arg(&i);
r = specifier_expansion_from_arg(specifier_table, &i);
if (r == -ENXIO)
return log_unresolvable_specifier(fname, line);
if (r < 0) {

11
test/test-systemctl-enable.sh
View file

@ -500,9 +500,10 @@ check_alias t '' && { echo "Expected failure" >&2; exit 1; }
check_alias T '' && { echo "Expected failure" >&2; exit 1; }
check_alias V '' && { echo "Expected failure" >&2; exit 1; }
# FIXME: we use the calling user instead of root :(
check_alias g root || :
check_alias G 0 || :
check_alias g root
check_alias G 0
check_alias u root
check_alias U 0
check_alias i ""
@ -521,10 +522,6 @@ check_alias N 'some-some-link6@'
check_alias p 'some-some-link6'
# FIXME: we use the calling user instead of root :(
check_alias u root || :
check_alias U 0 || :
check_alias v "$(uname -r)"
check_alias % '%' && { echo "Expected failure because % is not legal in unit name" >&2; exit 1; }

8
test/test-systemd-tmpfiles.py
View file

@ -98,13 +98,13 @@ def test_valid_specifiers(*, user):
test_content('f {} - - - - %b', '{}'.format(id128.get_boot().hex), user=user)
test_content('f {} - - - - %H', '{}'.format(socket.gethostname()), user=user)
test_content('f {} - - - - %v', '{}'.format(os.uname().release), user=user)
test_content('f {} - - - - %U', '{}'.format(os.getuid()), user=user)
test_content('f {} - - - - %G', '{}'.format(os.getgid()), user=user)
test_content('f {} - - - - %U', '{}'.format(os.getuid() if user else 0), user=user)
test_content('f {} - - - - %G', '{}'.format(os.getgid() if user else 0), user=user)
puser = pwd.getpwuid(os.getuid())
puser = pwd.getpwuid(os.getuid() if user else 0)
test_content('f {} - - - - %u', '{}'.format(puser.pw_name), user=user)
pgroup = grp.getgrgid(os.getgid())
pgroup = grp.getgrgid(os.getgid() if user else 0)
test_content('f {} - - - - %g', '{}'.format(pgroup.gr_name), user=user)
# Note that %h is the only specifier in which we look the environment,