dasharo/systemd
0
0
Fork 0
mirror of https://github.com/Dasharo/systemd.git synced 2026-03-06 15:02:31 -08:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #31425 from YHNdnzj/fd-util-followup

Browse Source 
fd-util: introduce fd_verify_safe_flags
This commit is contained in:
Yu Watanabe
2024-02-22 08:29:11 +09:00
committed by GitHub
parent 46be112082 f5cbe31374
commit 0584e2aa7c
7 changed files with 115 additions and 123 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
src/basic/fd-util.c
View File

@@ -913,6 +913,36 @@ int fd_is_opath(int fd) {
return FLAGS_SET(r, O_PATH);
}
int fd_verify_safe_flags(int fd) {
int flags, unexpected_flags;
/* Check if an extrinsic fd is safe to work on (by a privileged service). This ensures that clients
* can't trick a privileged service into giving access to a file the client doesn't already have
* access to (especially via something like O_PATH).
*
* O_NOFOLLOW: For some reason the kernel will return this flag from fcntl; it doesn't go away
* immediately after open(). It should have no effect whatsoever to an already-opened FD,
* and since we refuse O_PATH it should be safe.
*
* RAW_O_LARGEFILE: glibc secretly sets this and neglects to hide it from us if we call fcntl.
* See comment in missing_fcntl.h for more details about this.
*/
assert(fd >= 0);
flags = fcntl(fd, F_GETFL);
if (flags < 0)
return -errno;
unexpected_flags = flags & ~(O_ACCMODE|O_NOFOLLOW|RAW_O_LARGEFILE);
if (unexpected_flags != 0)
return log_debug_errno(SYNTHETIC_ERRNO(EREMOTEIO),
"Unexpected flags set for extrinsic fd: 0%o",
(unsigned) unexpected_flags);
return 0;
}
int read_nr_open(void) {
_cleanup_free_ char *nr_open = NULL;
int r;

17
src/basic/fd-util.h
View File

@@ -22,20 +22,6 @@
#define EBADF_PAIR { -EBADF, -EBADF }
#define EBADF_TRIPLET { -EBADF, -EBADF, -EBADF }
/* Flags that are safe to have set on an FD given to a privileged service to operate on.
* This ensures that clients can't trick a privileged service into giving access to a file the client
* doesn't already have access to (especially via something like O_PATH).
*
* O_NOFOLLOW: For some reason the kernel will return this flag from fcntl; it doesn't go away immediately
* after open(). It should have no effect whatsoever to an already-opened FD, but if it does
* it's decreasing the risk to a privileged service since it disables symlink following.
*
* RAW_O_LARGEFILE: glibc secretly sets this and neglects to hide it from us if we call fcntl. See comment
* in missing_fcntl.h for more details about this.
*/
#define SAFE_FD_FLAGS (O_ACCMODE|O_NOFOLLOW|RAW_O_LARGEFILE)
#define UNSAFE_FD_FLAGS(flags) ((unsigned)(flags) & ~SAFE_FD_FLAGS)
int close_nointr(int fd);
int safe_close(int fd);
void safe_close_pair(int p[static 2]);
@@ -126,7 +112,10 @@ static inline int make_null_stdio(void) {
int fd_reopen(int fd, int flags);
int fd_reopen_condition(int fd, int flags, int mask, int *ret_new_fd);
int fd_is_opath(int fd);
int fd_verify_safe_flags(int fd);
int read_nr_open(void);
int fd_get_diskseq(int fd, uint64_t *ret);

18
src/home/homed-bus.c
View File

@@ -89,7 +89,7 @@ int bus_message_read_blobs(sd_bus_message *m, Hashmap **ret, sd_bus_error *error
_cleanup_free_ char *filename = NULL;
_cleanup_close_ int fd = -EBADF;
const char *_filename = NULL;
int _fd, flags;
int _fd;
r = sd_bus_message_read(m, "{sh}", &_filename, &_fd);
if (r < 0)
@@ -111,18 +111,14 @@ int bus_message_read_blobs(sd_bus_message *m, Hashmap **ret, sd_bus_error *error
r = fd_verify_regular(fd);
if (r < 0)
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "FD for %s is not a regular file", filename);
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "FD for '%s' is not a regular file", filename);
flags = fcntl(fd, F_GETFL);
if (flags < 0)
return -errno;
/* Refuse fds w/ unexpected flags set. In particular, we don't want to permit O_PATH FDs, since
* those don't actually guarantee that the client has access to the file. */
if (UNSAFE_FD_FLAGS(flags) != 0)
r = fd_verify_safe_flags(fd);
if (r == -EREMOTEIO)
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS,
"FD for %s has unexpected flags set: 0%o",
filename, UNSAFE_FD_FLAGS(flags));
"FD for '%s' has unexpected flags set", filename);
if (r < 0)
return r;
r = hashmap_put(blobs, filename, FD_TO_PTR(fd));
if (r < 0)

4
src/journal/fuzz-journald-native-fd.c
View File

@@ -30,13 +30,13 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
.uid = geteuid(),
.gid = getegid(),
};
server_process_native_file(s, sealed_fd, &ucred, tv, label, label_len);
(void) server_process_native_file(s, sealed_fd, &ucred, tv, label, label_len);
unsealed_fd = mkostemp_safe(name);
assert_se(unsealed_fd >= 0);
assert_se(write(unsealed_fd, data, size) == (ssize_t) size);
assert_se(lseek(unsealed_fd, 0, SEEK_SET) == 0);
server_process_native_file(s, unsealed_fd, &ucred, tv, label, label_len);
(void) server_process_native_file(s, unsealed_fd, &ucred, tv, label, label_len);
return 0;
}

165
src/journal/journald-native.c
View File

@@ -326,7 +326,7 @@ void server_process_native_message(
} while (r == 0);
}
void server_process_native_file(
int server_process_native_file(
Server *s,
int fd,
const struct ucred *ucred,
@@ -342,32 +342,25 @@ void server_process_native_file(
assert(s);
assert(fd >= 0);
if (fstat(fd, &st) < 0) {
log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT, "Failed to stat passed file, ignoring: %m");
return;
}
if (fstat(fd, &st) < 0)
return log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT,
"Failed to stat passed file: %m");
r = stat_verify_regular(&st);
if (r < 0) {
log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT, "File passed is not regular, ignoring: %m");
return;
}
if (r < 0)
return log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
"File passed is not regular, ignoring message: %m");
if (st.st_size <= 0)
return;
return 0;
int flags = fcntl(fd, F_GETFL);
if (flags < 0) {
log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT, "Failed to get flags of passed file, ignoring: %m");
return;
}
if (UNSAFE_FD_FLAGS(flags) != 0) {
log_ratelimit_error(JOURNAL_LOG_RATELIMIT,
"Unexpected flags of passed memory fd (0%o), ignoring message: %m",
UNSAFE_FD_FLAGS(flags));
return;
}
r = fd_verify_safe_flags(fd);
if (r == -EREMOTEIO)
return log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
"Unexpected flags of passed memory fd, ignoring message.");
if (r < 0)
return log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
"Failed to get flags of passed file: %m");
/* If it's a memfd, check if it is sealed. If so, we can just mmap it and use it, and do not need to
* copy the data out. */
@@ -381,34 +374,26 @@ void server_process_native_file(
* path. */
r = fd_get_path(fd, &k);
if (r < 0) {
log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
"readlink(/proc/self/fd/%i) failed: %m", fd);
return;
}
if (r < 0)
return log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
"Failed to get path of passed fd: %m");
e = PATH_STARTSWITH_SET(k, "/dev/shm/", "/tmp/", "/var/tmp/");
if (!e) {
log_ratelimit_error(JOURNAL_LOG_RATELIMIT,
"Received file outside of allowed directories. Refusing.");
return;
}
if (!e)
return log_ratelimit_error_errno(SYNTHETIC_ERRNO(EPERM), JOURNAL_LOG_RATELIMIT,
"Received file outside of allowed directories, refusing.");
if (!filename_is_valid(e)) {
log_ratelimit_error(JOURNAL_LOG_RATELIMIT,
"Received file in subdirectory of allowed directories. Refusing.");
return;
}
if (!filename_is_valid(e))
return log_ratelimit_error_errno(SYNTHETIC_ERRNO(EPERM), JOURNAL_LOG_RATELIMIT,
"Received file in subdirectory of allowed directories, refusing.");
}
/* When !sealed, set a lower memory limit. We have to read the file, effectively doubling memory
* use. */
if (st.st_size > ENTRY_SIZE_MAX / (sealed ? 1 : 2)) {
log_ratelimit_error(JOURNAL_LOG_RATELIMIT,
"File passed too large (%"PRIu64" bytes). Ignoring.",
(uint64_t) st.st_size);
return;
}
if (st.st_size > ENTRY_SIZE_MAX / (sealed ? 1 : 2))
return log_ratelimit_error_errno(SYNTHETIC_ERRNO(EFBIG), JOURNAL_LOG_RATELIMIT,
"File passed too large (%"PRIu64" bytes), refusing.",
(uint64_t) st.st_size);
if (sealed) {
void *p;
@@ -419,62 +404,54 @@ void server_process_native_file(
ps = PAGE_ALIGN(st.st_size);
assert(ps < SIZE_MAX);
p = mmap(NULL, ps, PROT_READ, MAP_PRIVATE, fd, 0);
if (p == MAP_FAILED) {
log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT,
"Failed to map memfd, ignoring: %m");
return;
}
if (p == MAP_FAILED)
return log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT,
"Failed to map memfd: %m");
server_process_native_message(s, p, st.st_size, ucred, tv, label, label_len);
assert_se(munmap(p, ps) >= 0);
} else {
_cleanup_free_ void *p = NULL;
struct statvfs vfs;
ssize_t n;
if (fstatvfs(fd, &vfs) < 0) {
log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT,
"Failed to stat file system of passed file, not processing it: %m");
return;
}
/* Refuse operating on file systems that have mandatory locking enabled, see:
*
* https://github.com/systemd/systemd/issues/1822
*/
if (vfs.f_flag & ST_MANDLOCK) {
log_ratelimit_error(JOURNAL_LOG_RATELIMIT,
"Received file descriptor from file system with mandatory locking enabled, not processing it.");
return;
}
/* Make the fd non-blocking. On regular files this has the effect of bypassing mandatory
* locking. Of course, this should normally not be necessary given the check above, but let's
* better be safe than sorry, after all NFS is pretty confusing regarding file system flags,
* and we better don't trust it, and so is SMB. */
r = fd_nonblock(fd, true);
if (r < 0) {
log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
"Failed to make fd non-blocking, not processing it: %m");
return;
}
/* The file is not sealed, we can't map the file here, since clients might then truncate it
* and trigger a SIGBUS for us. So let's stupidly read it. */
p = malloc(st.st_size);
if (!p) {
log_oom();
return;
}
n = pread(fd, p, st.st_size, 0);
if (n < 0)
log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT,
"Failed to read file, ignoring: %m");
else if (n > 0)
server_process_native_message(s, p, n, ucred, tv, label, label_len);
return 0;
}
_cleanup_free_ void *p = NULL;
struct statvfs vfs;
ssize_t n;
if (fstatvfs(fd, &vfs) < 0)
return log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT,
"Failed to stat file system of passed file: %m");
/* Refuse operating on file systems that have mandatory locking enabled.
* See also: https://github.com/systemd/systemd/issues/1822 */
if (FLAGS_SET(vfs.f_flag, ST_MANDLOCK))
return log_ratelimit_error_errno(SYNTHETIC_ERRNO(EPERM), JOURNAL_LOG_RATELIMIT,
"Received file descriptor from file system with mandatory locking enabled, not processing it.");
/* Make the fd non-blocking. On regular files this has the effect of bypassing mandatory
* locking. Of course, this should normally not be necessary given the check above, but let's
* better be safe than sorry, after all NFS is pretty confusing regarding file system flags,
* and we better don't trust it, and so is SMB. */
r = fd_nonblock(fd, true);
if (r < 0)
return log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
"Failed to make fd non-blocking: %m");
/* The file is not sealed, we can't map the file here, since clients might then truncate it
* and trigger a SIGBUS for us. So let's stupidly read it. */
p = malloc(st.st_size);
if (!p)
return log_oom();
n = pread(fd, p, st.st_size, 0);
if (n < 0)
return log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT,
"Failed to read file: %m");
if (n > 0)
server_process_native_message(s, p, n, ucred, tv, label, label_len);
return 0;
}
int server_open_native_socket(Server *s, const char *native_socket) {

2
src/journal/journald-native.h
View File

@@ -12,7 +12,7 @@ void server_process_native_message(
const char *label,
size_t label_len);
void server_process_native_file(
int server_process_native_file(
Server *s,
int fd,
const struct ucred *ucred,

2
src/journal/journald-server.c
View File

@@ -1538,7 +1538,7 @@ int server_process_datagram(
if (n > 0 && n_fds == 0)
server_process_native_message(s, s->buffer, n, ucred, tv, label, label_len);
else if (n == 0 && n_fds == 1)
server_process_native_file(s, fds[0], ucred, tv, label, label_len);
(void) server_process_native_file(s, fds[0], ucred, tv, label, label_len);
else if (n_fds > 0)
log_ratelimit_warning(JOURNAL_LOG_RATELIMIT,
"Got too many file descriptors via native socket. Ignoring.");