Assorted basic/shared/home/import hardening fixes flagged by kres (#42950)

This commit is contained in:
Lennart Poettering
2026-07-14 23:31:53 +02:00
committed by GitHub
9 changed files with 240 additions and 40 deletions
+19 -5
View File
@@ -726,7 +726,11 @@ static int write_to_journal(
if (LOG_PRI(level) > log_target_max_level[LOG_TARGET_JOURNAL])
return 0;
iovec_len = MIN(6 + log_context_num_fields() * 3, IOVEC_MAX);
if (!MUL_SAFE(&iovec_len, log_context_num_fields(), 3) ||
!ADD_SAFE(&iovec_len, iovec_len, 6))
iovec_len = IOVEC_MAX;
else
iovec_len = MIN(iovec_len, IOVEC_MAX);
iovec = newa(struct iovec, iovec_len);
log_do_header(header, sizeof(header), level, error, file, line, func, object_field, object, extra_field, extra);
@@ -1024,7 +1028,11 @@ int log_struct_internal(
int r;
bool fallback = false;
iovec_len = MIN(17 + log_context_num_fields() * 3, IOVEC_MAX);
if (!MUL_SAFE(&iovec_len, log_context_num_fields(), 3) ||
!ADD_SAFE(&iovec_len, iovec_len, 17))
iovec_len = IOVEC_MAX;
else
iovec_len = MIN(iovec_len, IOVEC_MAX);
iovec = newa(struct iovec, iovec_len);
/* If the journal is available do structured logging.
@@ -1123,15 +1131,21 @@ int log_struct_iovec_internal(
char header[LINE_MAX];
struct iovec *iovec;
size_t n = 0, iovec_len;
size_t n = 0, iovec_len, n_context_iovec, n_input_iovec_expanded;
iovec_len = MIN(1 + n_input_iovec * 2 + log_context_num_fields() * 3, IOVEC_MAX);
if (!MUL_SAFE(&n_input_iovec_expanded, n_input_iovec, 2) ||
!MUL_SAFE(&n_context_iovec, log_context_num_fields(), 3) ||
!ADD_SAFE(&iovec_len, 1, n_input_iovec_expanded) ||
!ADD_SAFE(&iovec_len, iovec_len, n_context_iovec))
iovec_len = IOVEC_MAX;
else
iovec_len = MIN(iovec_len, IOVEC_MAX);
iovec = newa(struct iovec, iovec_len);
log_do_header(header, sizeof(header), level, error, file, line, func, NULL, NULL, NULL, NULL);
iovec[n++] = IOVEC_MAKE_STRING(header);
for (size_t i = 0; i < n_input_iovec; i++) {
for (size_t i = 0; i < n_input_iovec && n + 2 <= iovec_len; i++) {
iovec[n++] = input_iovec[i];
iovec[n++] = IOVEC_MAKE_STRING("\n");
}
+49 -10
View File
@@ -357,6 +357,33 @@ static char *ascii_ellipsize_mem(const char *s, size_t old_length, size_t new_le
return t;
}
/* Walk backwards from 'end' (exclusive) to the start of the last complete UTF-8 character, without
* descending below 'start', validate it and return a pointer to its first byte, optionally decoding it
* into *ret_c. Returns NULL on a truncated or otherwise malformed sequence. */
static const char* find_previous_unichar(const char *start, const char *end, char32_t *ret_c) {
const char *p;
int r;
assert(start);
assert(end);
assert(end > start);
for (p = end; p > start; ) {
p--;
if (((uint8_t) *p & 0xc0) != 0x80) /* Found a non-continuation byte, i.e. a character start. */
break;
}
r = utf8_encoded_valid_unichar(p, end - p);
if (r < 0 || p + r != end)
return NULL;
if (ret_c)
assert_se(utf8_encoded_to_unichar(p, ret_c) == r);
return p;
}
char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigned percent) {
size_t x, k, len, len2;
const char *i, *j;
@@ -400,11 +427,13 @@ char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigne
continue; /* ANSI sequences don't take up any space in output */
}
char32_t c;
r = utf8_encoded_to_unichar(i, &c);
r = utf8_encoded_valid_unichar(i, s + old_length - i);
if (r < 0)
return NULL;
char32_t c;
assert_se(utf8_encoded_to_unichar(i, &c) == r);
int w = unichar_iswide(c) ? 2 : 1;
if (k + w > x)
break;
@@ -431,9 +460,9 @@ char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigne
continue;
}
tt = utf8_prev_char(t);
r = utf8_encoded_to_unichar(tt, &c);
if (r < 0)
/* Find the previous complete UTF-8 character inside the retained suffix. */
tt = find_previous_unichar(i, t, &c);
if (!tt)
return NULL;
w = unichar_iswide(c) ? 2 : 1;
@@ -451,11 +480,21 @@ char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigne
if (k >= new_length) {
/* Make space for ellipsis, if required and possible. We know that the edge character is not
* part of an ANSI sequence (because then we'd skip it). If the last character we looked at
* was wide, we don't need to make space. */
if (j < s + old_length)
j = utf8_next_char(j);
else if (i > s)
i = utf8_prev_char(i);
* was wide, we don't need to make space.
* Move the edge by one complete UTF-8 character within the input slice. */
if (j < s + old_length) {
r = utf8_encoded_valid_unichar(j, s + old_length - j);
if (r < 0)
return NULL;
j += r;
} else if (i > s) {
const char *tt = find_previous_unichar(s, i, NULL);
if (!tt)
return NULL;
i = tt;
}
}
len = i - s;
+71 -18
View File
@@ -3702,7 +3702,7 @@ int home_passwd_luks(
const PasswordCache *cache, /* the passwords acquired via PKCS#11/FIDO2 security tokens */
char **effective_passwords /* new passwords */) {
size_t volume_key_size, max_key_slots, n_effective;
size_t volume_key_size, max_key_slots, n_effective, n_occupied = 0;
_cleanup_(erase_and_freep) void *volume_key = NULL;
struct crypt_pbkdf_type good_pbkdf, minimal_pbkdf;
const char *type;
@@ -3742,44 +3742,97 @@ int home_passwd_luks(
n_effective = strv_length(effective_passwords);
for (size_t i = 0; i < max_key_slots; i++)
switch (sym_crypt_keyslot_status(setup->crypt_device, i)) {
case CRYPT_SLOT_INACTIVE:
break;
case CRYPT_SLOT_ACTIVE:
case CRYPT_SLOT_ACTIVE_LAST:
case CRYPT_SLOT_UNBOUND:
n_occupied++;
break;
default:
return log_error_errno(SYNTHETIC_ERRNO(EIO),
"Failed to determine state of LUKS key slot %zu.", i);
}
if (n_effective > max_key_slots - n_occupied)
return log_error_errno(SYNTHETIC_ERRNO(ENOSPC),
"Not enough free LUKS key slots (%zu required, %zu available).",
n_effective, max_key_slots - n_occupied);
build_good_pbkdf(&good_pbkdf, h);
build_minimal_pbkdf(&minimal_pbkdf, h);
for (size_t i = 0; i < max_key_slots; i++) {
r = sym_crypt_keyslot_destroy(setup->crypt_device, i);
if (r < 0 && !IN_SET(r, -ENOENT, -EINVAL)) /* Returns EINVAL or ENOENT if there's no key in this slot already */
return log_error_errno(r, "Failed to destroy LUKS password: %m");
/* Rotate the LUKS key slots in two passes, so that at every on-disk state at least one valid key slot
* exists for each password the user currently holds. First add all new passwords into free key slots
* (purely additive, hence non-destructive), and only once that fully succeeded destroy the old slots.
* If an add fails (e.g. the argon2 PBKDF, which is intentionally memory hungry, hits OOM), we roll
* back the slots added so far and return with the pre-existing slots untouched. The reverse order
* (destroy a slot first, then add its replacement at the same index) would, on such a failure, leave
* the slot destroyed with no replacement, permanently locking the user out. */
if (i >= n_effective) {
if (r >= 0)
log_info("Destroyed LUKS key slot %zu.", i);
continue;
}
_cleanup_free_ int *new_slots = new(int, n_effective);
if (!new_slots)
return log_oom();
size_t n_added = 0;
for (size_t i = 0; i < n_effective; i++) {
if (password_cache_contains(cache, effective_passwords[i])) { /* Is this a FIDO2 or PKCS#11 password? */
log_debug("Using minimal PBKDF for slot %zu", i);
log_debug("Using minimal PBKDF for new key slot.");
r = sym_crypt_set_pbkdf_type(setup->crypt_device, &minimal_pbkdf);
} else {
log_debug("Using good PBKDF for slot %zu", i);
log_debug("Using good PBKDF for new key slot.");
r = sym_crypt_set_pbkdf_type(setup->crypt_device, &good_pbkdf);
}
if (r < 0)
return log_error_errno(r, "Failed to tweak PBKDF for slot %zu: %m", i);
if (r < 0) {
log_error_errno(r, "Failed to tweak PBKDF for new key slot: %m");
goto rollback;
}
r = sym_crypt_keyslot_add_by_volume_key(
setup->crypt_device,
i,
CRYPT_ANY_SLOT,
volume_key,
volume_key_size,
effective_passwords[i],
strlen(effective_passwords[i]));
if (r < 0)
return log_error_errno(r, "Failed to set up LUKS password: %m");
if (r < 0) {
log_error_errno(r, "Failed to set up LUKS password: %m");
goto rollback;
}
log_info("Updated LUKS key slot %zu.", i);
new_slots[n_added++] = r;
log_info("Added LUKS key slot %i.", r);
}
/* All replacement slots are committed now; drop every slot that isn't one we just added. */
for (size_t i = 0; i < max_key_slots; i++) {
bool keep = false;
for (size_t j = 0; j < n_added; j++)
if (new_slots[j] == (int) i) {
keep = true;
break;
}
if (keep)
continue;
r = sym_crypt_keyslot_destroy(setup->crypt_device, i);
if (r < 0 && !IN_SET(r, -ENOENT, -EINVAL)) /* Returns EINVAL or ENOENT if there's no key in this slot already */
return log_error_errno(r, "Failed to destroy LUKS password: %m");
if (r >= 0)
log_info("Destroyed LUKS key slot %zu.", i);
}
return 1;
rollback:
/* Undo the slots added in this call, so that the header is left exactly as we found it. */
for (size_t i = 0; i < n_added; i++)
(void) sym_crypt_keyslot_destroy(setup->crypt_device, new_slots[i]);
return r;
}
int home_lock_luks(UserRecord *h, HomeSetup *setup) {
+1 -1
View File
@@ -1293,7 +1293,7 @@ static void oci_pull_job_on_finished_layer(PullJob *j) {
goto finish;
}
assert(set_remove(i->active_layer_jobs, j) == j);
assert_se(set_remove(i->active_layer_jobs, j) == j);
r = oci_pull_work(i);
if (r <= 0)
+2 -2
View File
@@ -959,7 +959,7 @@ static int fd_copy_fifo(
AT_SYMLINK_NOFOLLOW) < 0)
r = -errno;
if (fchmodat(dt, to, st->st_mode & 07777, 0) < 0)
if (fchmodat(dt, to, st->st_mode & 07777, AT_SYMLINK_NOFOLLOW) < 0)
r = -errno;
(void) utimensat(dt, to, (struct timespec[]) { st->st_atim, st->st_mtim }, AT_SYMLINK_NOFOLLOW);
@@ -1012,7 +1012,7 @@ static int fd_copy_node(
AT_SYMLINK_NOFOLLOW) < 0)
r = -errno;
if (fchmodat(dt, to, st->st_mode & 07777, 0) < 0)
if (fchmodat(dt, to, st->st_mode & 07777, AT_SYMLINK_NOFOLLOW) < 0)
r = -errno;
(void) utimensat(dt, to, (struct timespec[]) { st->st_atim, st->st_mtim }, AT_SYMLINK_NOFOLLOW);
+6 -4
View File
@@ -1309,15 +1309,17 @@ static int dissect_image(
start = sym_blkid_partition_get_start(pp);
if (start < 0)
return errno_or_else(EIO);
assert((uint64_t) start < UINT64_MAX/512);
if ((uint64_t) start >= UINT64_MAX/512)
return log_debug_errno(SYNTHETIC_ERRNO(EOVERFLOW),
"Partition start LBA too large to convert to a byte offset, refusing.");
errno = 0;
size = sym_blkid_partition_get_size(pp);
if (size < 0)
return errno_or_else(EIO);
assert((uint64_t) size < UINT64_MAX/512);
if ((uint64_t) size >= UINT64_MAX/512)
return log_debug_errno(SYNTHETIC_ERRNO(EOVERFLOW),
"Partition size in LBA too large to convert to a byte offset, refusing.");
/* While probing we need the non-diskseq device node name to access the thing, hence mask off
* DISSECT_IMAGE_DISKSEQ_DEVNODE. */
+64
View File
@@ -3,6 +3,7 @@
#include <linux/fsverity.h>
#include <sys/file.h>
#include <sys/ioctl.h>
#include <sys/wait.h>
#include <sys/xattr.h>
#include <unistd.h>
@@ -308,6 +309,69 @@ TEST(copy_tree_at_symlink) {
fd = safe_close(fd);
}
TEST(copy_tree_fifo_chmod_symlink_race) {
_cleanup_(rm_rf_physical_and_freep) char *srcp = NULL, *dstp = NULL;
_cleanup_close_ int src = -EBADF, dst = -EBADF;
_cleanup_close_pair_ int ready[2] = EBADF_PAIR, stop[2] = EBADF_PAIR;
struct stat st;
int status;
pid_t pid;
char c;
if (!slow_tests_enabled())
return (void) log_tests_skipped("slow tests are disabled");
ASSERT_OK(src = mkdtemp_open(NULL, 0, &srcp));
ASSERT_OK(dst = mkdtemp_open(NULL, 0, &dstp));
ASSERT_OK_ERRNO(mkfifoat(src, "fifo", 0777));
ASSERT_OK_ERRNO(fchmodat(src, "fifo", 0777, 0));
ASSERT_OK(write_string_file_at(dst, "victim", "victim", WRITE_STRING_FILE_CREATE));
ASSERT_OK_ERRNO(fchmodat(dst, "victim", 0600, 0));
ASSERT_OK_ERRNO(pipe2(ready, O_CLOEXEC));
ASSERT_OK_ERRNO(pipe2(stop, O_CLOEXEC|O_NONBLOCK));
pid = ASSERT_OK_ERRNO(fork());
if (pid == 0) {
ready[0] = safe_close(ready[0]);
stop[1] = safe_close(stop[1]);
assert_se(write(ready[1], &(const char) { 'x' }, 1) == 1);
for (;;) {
char stop_char;
ssize_t n = read(stop[0], &stop_char, 1);
if (n >= 0)
_exit(EXIT_SUCCESS);
(void) unlinkat(dst, "fifo", 0);
(void) symlinkat("victim", dst, "fifo");
}
}
ready[1] = safe_close(ready[1]);
stop[0] = safe_close(stop[0]);
ASSERT_OK_EQ_ERRNO(read(ready[0], &c, 1), 1);
bool changed = false;
for (unsigned i = 0; i < 20000; i++) {
(void) copy_tree_at(src, "fifo", dst, "fifo", UID_INVALID, GID_INVALID, COPY_REPLACE, NULL, NULL);
ASSERT_OK_ERRNO(fstatat(dst, "victim", &st, 0));
if ((st.st_mode & 0777) != 0600) {
changed = true;
break;
}
}
ASSERT_OK_EQ_ERRNO(write(stop[1], &(const char) { 'x' }, 1), 1);
ASSERT_OK_ERRNO(waitpid(pid, &status, 0));
ASSERT_TRUE(WIFEXITED(status));
ASSERT_FALSE(changed);
}
TEST_RET(copy_bytes) {
_cleanup_close_pair_ int pipefd[2] = EBADF_PAIR;
_cleanup_close_ int infd = -EBADF;
+6
View File
@@ -75,6 +75,12 @@ TEST(ellipsize_mem) {
test_ellipsize_mem_one(s, l, k);
}
TEST(ellipsize_mem_truncated_utf8) {
static const char s[] = { 'a', '\xf0', '\x80', '\x80', '\x80', 'b' };
ASSERT_NULL(ellipsize_mem(s, 2, 1, 50));
}
static void test_ellipsize_one(const char *p) {
_cleanup_free_ char *t = NULL;
t = ellipsize(p, columns(), 70);
+22
View File
@@ -145,6 +145,28 @@ TEST(log_format_iovec) {
LOG_ITEM("FOOFOO=%hu-%llu", (unsigned short) 4, (long long unsigned) 3));
}
TEST(log_struct_iovec_many_fields) {
LogTarget old_target = log_get_target();
struct iovec iovec[1024];
log_set_target(LOG_TARGET_JOURNAL);
log_open();
if (log_on_console()) {
log_set_target(old_target);
log_open();
return (void) log_tests_skipped("journal socket is not available");
}
iovec[0] = IOVEC_MAKE_STRING("MESSAGE=many fields");
for (size_t i = 1; i < ELEMENTSOF(iovec); i++)
iovec[i] = IOVEC_MAKE_STRING("FIELD=value");
ASSERT_OK(log_struct_iovec(LOG_INFO, iovec, ELEMENTSOF(iovec)));
log_set_target(old_target);
log_open();
}
static void test_log_struct(void) {
log_struct(LOG_INFO,
"MESSAGE=Waldo PID="PID_FMT" (no errno)", getpid_cached(),