system/systemd
system/systemd
1
0
Fork 0
mirror of https://github.com/systemd/systemd synced 2024-10-15 04:24:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

env-util: replace unsetenv_erase() by new getenv_steal_erase() helper

Browse source 
The new helper combines a bunch of steps every invocation of
unsetenv_erase() did so far: getenv() + strdup() + unsetenv_erase().
Let's unify this into one helper that is harder to use incorrectly. It's
in inspired by TAKE_PTR() in a way: get the env var out and invalidate
where it was before.
This commit is contained in:
Lennart Poettering 2022-02-19 00:08:39 +01:00 committed by Yu Watanabe
parent 5cf84d2545
commit e99ca14741
9 changed files with 91 additions and 85 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

29
src/basic/env-util.c
View file

@ -868,19 +868,36 @@ int getenv_path_list(const char *name, char ***ret_paths) {
return 1;
}
int unsetenv_erase(const char *name) {
char *p;
int getenv_steal_erase(const char *name, char **ret) {
_cleanup_(erase_and_freep) char *a = NULL;
char *e;
assert(name);
p = getenv(name);
if (!p)
return 0;
/* Reads an environment variable, makes a copy of it, erases its memory in the environment block and removes
* it from there. Usecase: reading passwords from the env block (which is a bad idea, but useful for
* testing, and given that people are likely going to misuse this, be thorough) */
string_erase(p);
e = getenv(name);
if (!e) {
if (ret)
*ret = NULL;
return 0;
}
if (ret) {
a = strdup(e);
if (!a)
return -ENOMEM;
}
string_erase(e);
if (unsetenv(name) < 0)
return -errno;
if (ret)
*ret = TAKE_PTR(a);
return 1;
}

2
src/basic/env-util.h
View file

@ -70,3 +70,5 @@ int setenv_systemd_exec_pid(bool update_only);
int getenv_path_list(const char *name, char ***ret_paths);
int unsetenv_erase(const char *name);
int getenv_steal_erase(const char *name, char **ret);

15
src/cryptenroll/cryptenroll-password.c
View file

@ -17,20 +17,13 @@ int enroll_password(
_cleanup_free_ char *error = NULL;
const char *node;
int r, keyslot;
char *e;
assert_se(node = crypt_get_device_name(cd));
e = getenv("NEWPASSWORD");
if (e) {
new_password = strdup(e);
if (!new_password)
return log_oom();
assert_se(unsetenv_erase("NEWPASSWORD") >= 0);
} else {
r = getenv_steal_erase("NEWPASSWORD", &new_password);
if (r < 0)
return log_error_errno(r, "Failed to acquire password from environment: %m");
if (r == 0) {
_cleanup_free_ char *disk_path = NULL;
unsigned i = 5;
const char *id;

20
src/cryptenroll/cryptenroll.c
View file

@ -409,8 +409,8 @@ static int prepare_luks(
size_t *ret_volume_key_size) {
_cleanup_(crypt_freep) struct crypt_device *cd = NULL;
_cleanup_(erase_and_freep) char *envpw = NULL;
_cleanup_(erase_and_freep) void *vk = NULL;
char *e = NULL;
size_t vks;
int r;
@ -445,23 +445,17 @@ static int prepare_luks(
if (!vk)
return log_oom();
e = getenv("PASSWORD");
if (e) {
_cleanup_(erase_and_freep) char *password = NULL;
password = strdup(e);
if (!password)
return log_oom();
assert_se(unsetenv_erase("PASSWORD") >= 0);
r = getenv_steal_erase("PASSWORD", &envpw);
if (r < 0)
return log_error_errno(r, "Failed to acquire password from environment: %m");
if (r > 0) {
r = crypt_volume_key_get(
cd,
CRYPT_ANY_SLOT,
vk,
&vks,
password,
strlen(password));
envpw,
strlen(envpw));
if (r < 0)
return log_error_errno(r, "Password from environment variable $PASSWORD did not work.");
} else {

12
src/cryptsetup/cryptsetup-fido2.c
View file

@ -30,12 +30,12 @@ int acquire_fido2_key(
size_t *ret_decrypted_key_size,
AskPasswordFlags ask_password_flags) {
_cleanup_(erase_and_freep) char *envpw = NULL;
_cleanup_strv_free_erase_ char **pins = NULL;
_cleanup_free_ void *loaded_salt = NULL;
bool device_exists = false;
const char *salt;
size_t salt_size;
char *e;
int r;
ask_password_flags |= ASK_PASSWORD_PUSH_CACHE | ASK_PASSWORD_ACCEPT_CACHED;
@ -66,13 +66,13 @@ int acquire_fido2_key(
salt = loaded_salt;
}
e = getenv("PIN");
if (e) {
pins = strv_new(e);
r = getenv_steal_erase("PIN", &envpw);
if (r < 0)
return log_error_errno(r, "Failed to acquire password from environment: %m");
if (r > 0) {
pins = strv_new(envpw);
if (!pins)
return log_oom();
assert_se(unsetenv_erase("PIN") >= 0);
}
for (;;) {

15
src/cryptsetup/cryptsetup.c
View file

@ -818,20 +818,19 @@ static bool libcryptsetup_plugins_support(void) {
#if HAVE_LIBCRYPTSETUP_PLUGINS
static int acquire_pins_from_env_variable(char ***ret_pins) {
char *e;
_cleanup_(erase_and_freep) char *envpin = NULL;
_cleanup_strv_free_erase_ char **pins = NULL;
int r;
assert(ret_pins);
e = getenv("PIN");
if (e) {
pins = strv_new(e);
r = getenv_steal_erase("PIN", &envpin);
if (r < 0)
return log_error_errno(r, "Failed to acquire PIN from environment: %m");
if (r > 0) {
pins = strv_new(envpin);
if (!pins)
return log_oom();
string_erase(e);
if (unsetenv("PIN") < 0)
return log_error_errno(errno, "Failed to unset $PIN: %m");
}
*ret_pins = TAKE_PTR(pins);

55
src/home/homectl.c
View file

@ -201,24 +201,25 @@ static int acquire_existing_password(
AskPasswordFlags flags) {
_cleanup_(strv_free_erasep) char **password = NULL;
_cleanup_(erase_and_freep) char *envpw = NULL;
_cleanup_free_ char *question = NULL;
char *e;
int r;
assert(user_name);
assert(hr);
e = getenv("PASSWORD");
if (e) {
r = getenv_steal_erase("PASSWORD", &envpw);
if (r < 0)
return log_error_errno(r, "Failed to acquire password from environment: %m");
if (r > 0) {
/* People really shouldn't use environment variables for passing passwords. We support this
* only for testing purposes, and do not document the behaviour, so that people won't
* actually use this outside of testing. */
r = user_record_set_password(hr, STRV_MAKE(e), true);
r = user_record_set_password(hr, STRV_MAKE(envpw), true);
if (r < 0)
return log_error_errno(r, "Failed to store password: %m");
assert_se(unsetenv_erase("PASSWORD") >= 0);
return 1;
}
@ -261,24 +262,25 @@ static int acquire_recovery_key(
AskPasswordFlags flags) {
_cleanup_(strv_free_erasep) char **recovery_key = NULL;
_cleanup_(erase_and_freep) char *envpw = NULL;
_cleanup_free_ char *question = NULL;
char *e;
int r;
assert(user_name);
assert(hr);
e = getenv("RECOVERY_KEY");
if (e) {
r = getenv_steal_erase("PASSWORD", &envpw);
if (r < 0)
return log_error_errno(r, "Failed to acquire password from environment: %m");
if (r > 0) {
/* People really shouldn't use environment variables for passing secrets. We support this
* only for testing purposes, and do not document the behaviour, so that people won't
* actually use this outside of testing. */
r = user_record_set_password(hr, STRV_MAKE(e), true); /* recovery keys are stored in the record exactly like regular passwords! */
r = user_record_set_password(hr, STRV_MAKE(envpw), true); /* recovery keys are stored in the record exactly like regular passwords! */
if (r < 0)
return log_error_errno(r, "Failed to store recovery key: %m");
assert_se(unsetenv_erase("RECOVERY_KEY") >= 0);
return 1;
}
@ -318,20 +320,21 @@ static int acquire_token_pin(
AskPasswordFlags flags) {
_cleanup_(strv_free_erasep) char **pin = NULL;
_cleanup_(erase_and_freep) char *envpin = NULL;
_cleanup_free_ char *question = NULL;
char *e;
int r;
assert(user_name);
assert(hr);
e = getenv("PIN");
if (e) {
r = user_record_set_token_pin(hr, STRV_MAKE(e), false);
r = getenv_steal_erase("PIN", &envpin);
if (r < 0)
return log_error_errno(r, "Failed to acquire PIN from environment: %m");
if (r > 0) {
r = user_record_set_token_pin(hr, STRV_MAKE(envpin), false);
if (r < 0)
return log_error_errno(r, "Failed to store token PIN: %m");
assert_se(unsetenv_erase("PIN") >= 0);
return 1;
}
@ -1150,33 +1153,25 @@ static int acquire_new_password(
bool suggest,
char **ret) {
_cleanup_(erase_and_freep) char *envpw = NULL;
unsigned i = 5;
char *e;
int r;
assert(user_name);
assert(hr);
e = getenv("NEWPASSWORD");
if (e) {
_cleanup_(erase_and_freep) char *copy = NULL;
r = getenv_steal_erase("NEWPASSWORD", &envpw);
if (r < 0)
return log_error_errno(r, "Failed to acquire password from environment: %m");
if (r > 0) {
/* As above, this is not for use, just for testing */
if (ret) {
copy = strdup(e);
if (!copy)
return log_oom();
}
r = user_record_set_password(hr, STRV_MAKE(e), /* prepend = */ true);
r = user_record_set_password(hr, STRV_MAKE(envpw), /* prepend = */ true);
if (r < 0)
return log_error_errno(r, "Failed to store password: %m");
assert_se(unsetenv_erase("NEWPASSWORD") >= 0);
if (ret)
*ret = TAKE_PTR(copy);
*ret = TAKE_PTR(envpw);
return 0;
}

12
src/shared/pkcs11-util.c
View file

@ -275,15 +275,17 @@ int pkcs11_token_login(
for (unsigned tries = 0; tries < 3; tries++) {
_cleanup_strv_free_erase_ char **passwords = NULL;
char **i, *e;
_cleanup_(erase_and_freep) char *envpin = NULL;
char **i;
e = getenv("PIN");
if (e) {
passwords = strv_new(e);
r = getenv_steal_erase("PIN", &envpin);
if (r < 0)
return log_error_errno(r, "Failed to acquire PIN from environment: %m");
if (r > 0) {
passwords = strv_new(envpin);
if (!passwords)
return log_oom();
assert_se(unsetenv_erase("PIN") >= 0);
} else if (headless)
return log_error_errno(SYNTHETIC_ERRNO(ENOPKG), "PIN querying disabled via 'headless' option. Use the 'PIN' environment variable.");
else {

16
src/test/test-env-util.c
View file

@ -406,17 +406,17 @@ TEST(setenv_systemd_exec_pid) {
assert_se(set_unset_env("SYSTEMD_EXEC_PID", saved, 1) >= 0);
}
TEST(unsetenv_erase) {
TEST(getenv_steal_erase) {
int r;
r = safe_fork("(sd-unsetenverase)", FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL);
r = safe_fork("(sd-getenvstealerase)", FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL);
if (r == 0) {
_cleanup_strv_free_ char **l = NULL;
char **e;
/* child */
assert_se(unsetenv_erase("thisenvvardefinitelywontexist") == 0);
assert_se(getenv_steal_erase("thisenvvardefinitelywontexist", NULL) == 0);
l = strv_new("FOO=BAR", "QUUX=PIFF", "ONE=TWO", "A=B");
assert_se(strv_length(l) == 4);
@ -424,7 +424,7 @@ TEST(unsetenv_erase) {
environ = l;
STRV_FOREACH(e, environ) {
_cleanup_free_ char *n = NULL;
_cleanup_free_ char *n = NULL, *copy1 = NULL, *copy2 = NULL;
char *eq;
eq = strchr(*e, '=');
@ -434,9 +434,13 @@ TEST(unsetenv_erase) {
n = strndup(*e, eq - *e);
assert_se(n);
assert_se(streq_ptr(getenv(n), eq + 1));
copy1 = strdup(eq + 1);
assert_se(copy1);
assert_se(streq_ptr(getenv(n), copy1));
assert_se(getenv(n) == eq + 1);
assert_se(unsetenv_erase(n) > 0);
assert_se(getenv_steal_erase(n, &copy2) > 0);
assert_se(streq_ptr(copy1, copy2));
assert_se(isempty(eq + 1));
assert_se(!getenv(n));
}