fix(p8h): restore daemon --super refusal, race-free secret-file check, remaining log escapes
- server_module_gate: refuse client-chosen ownership against the ORIGINAL config so an explicit --super is still refused under an operator --no-super veto (the veto must not turn a refusal into an accept). - credentials: open-then-fstat the exact secret inode, require current-user ownership and no group/other bits, but continue to allow process-substitution FIFOs; removes the stat->fopen TOCTOU. - file.c preallocate + protocol.c send-string debug logs escape attacker paths. - usage/RSYNC_COMPAT updated for --old-args no-op and secret-file rules.
This commit is contained in:
+2
-2
@@ -246,8 +246,8 @@ void print_usage(void) {
|
||||
printf(" -T, --temp-dir <dir> Scratch dir for temp files before atomic install\n");
|
||||
printf(" --fastsync-server-path <path>\n");
|
||||
printf(" Path to fastsync-server on remote (default: fastsync-server)\n");
|
||||
printf(
|
||||
" --old-args Disable safe SSH command argument quoting (legacy compatibility)\n");
|
||||
printf(" --old-args Accepted for rsync CLI compatibility; no effect (the\n");
|
||||
printf(" remote server path is always safely quoted now)\n");
|
||||
printf(" -M, --remote-option=OPT Append OPT to the REMOTE server invocation over SSH\n");
|
||||
printf(" (repeatable; each value is single-quote-escaped on the remote\n");
|
||||
printf(" command line; empty values and values with control characters\n");
|
||||
|
||||
+5
-4
@@ -257,10 +257,11 @@ static const char* server_module_gate(const Config* config, void* context) {
|
||||
standalone/SSH server has a single operator-authorized root and keeps
|
||||
honoring these. */
|
||||
if (!module->client_owner) {
|
||||
/* Ownership: refuse the whole transfer up front (a clear failure). Evaluated
|
||||
against the effective copy (so an operator --no-super has already
|
||||
neutralized an explicit --super), exactly as before. */
|
||||
if (identity_ownership_requested(&effective)) {
|
||||
/* Ownership: refuse the whole transfer up front (a clear failure).
|
||||
Evaluated against the ORIGINAL config so an explicit --super is refused
|
||||
even when an operator --no-super veto already forced the effective copy
|
||||
to OFF (the veto must not silently convert a refusal into an accept). */
|
||||
if (identity_ownership_requested(config)) {
|
||||
log_message(LOG_LEVEL_ERROR,
|
||||
"daemon module '%s' refuses client-chosen ownership/super-user activities "
|
||||
"(no `client owner = yes` opt-in); refusing",
|
||||
|
||||
+42
-27
@@ -2,6 +2,7 @@
|
||||
#include "utils.h"
|
||||
#include <ctype.h>
|
||||
#include <errno.h>
|
||||
#include <fcntl.h>
|
||||
#include <openssl/evp.h>
|
||||
#include <stdarg.h>
|
||||
#include <stdint.h>
|
||||
@@ -9,6 +10,7 @@
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
#include <sys/stat.h>
|
||||
#include <unistd.h>
|
||||
|
||||
/* One store entry: a username and its password's SHA-256 hex digest. The
|
||||
* plaintext password never appears here (and never on the daemon host). */
|
||||
@@ -36,21 +38,44 @@ static bool is_comment_char(char c) {
|
||||
return c == '#' || c == ';';
|
||||
}
|
||||
|
||||
/* A --password-file / --early-input carries plaintext or credential material
|
||||
* and must not be accessible to group or other, mirroring the TLS private-key
|
||||
* check in transport_tls.c. Reject any group/other permission bit (including
|
||||
* execute) with a clear error. A stat failure is left for the caller's fopen
|
||||
* to report, so a missing file keeps its existing "cannot open" message. */
|
||||
static bool secret_file_is_private(const char* path, char* err, size_t err_size) {
|
||||
struct stat st;
|
||||
if (stat(path, &st) != 0)
|
||||
return true;
|
||||
if (!S_ISREG(st.st_mode) || (st.st_mode & (S_IRWXG | S_IRWXO)) != 0) {
|
||||
set_error(err, err_size,
|
||||
"refusing to read secret file '%s': permissions must be owner-only (0600)", path);
|
||||
return false;
|
||||
/* Open a --password-file / --early-input after verifying the EXACT inode we
|
||||
* will read: it must be owned by the effective user and grant no group/other
|
||||
* permission bit (mode 0600), mirroring the TLS private-key check. We open by
|
||||
* path and then fstat the resulting fd (rather than stat()ing the path first
|
||||
* and reopening it), so the permission decision is made on the same inode that
|
||||
* is read and cannot be raced by swapping the path between check and open.
|
||||
* The path may be a process-substitution pipe (`<(...)` -> /dev/fd/N), so
|
||||
* regular files and FIFOs are accepted when the ownership/mode checks pass.
|
||||
*
|
||||
* Returns a FILE* the caller must fclose, or NULL with `err` filled. */
|
||||
static FILE* secret_file_open(const char* path, char* err, size_t err_size) {
|
||||
int fd = open(path, O_RDONLY | O_CLOEXEC);
|
||||
if (fd < 0) {
|
||||
set_error(err, err_size, "cannot open secret file '%s': %s", path, strerror(errno));
|
||||
return NULL;
|
||||
}
|
||||
return true;
|
||||
struct stat st;
|
||||
if (fstat(fd, &st) != 0) {
|
||||
set_error(err, err_size, "cannot stat secret file '%s': %s", path, strerror(errno));
|
||||
close(fd);
|
||||
return NULL;
|
||||
}
|
||||
bool is_readable_kind = S_ISREG(st.st_mode) || S_ISFIFO(st.st_mode);
|
||||
if (!is_readable_kind || st.st_uid != geteuid() || (st.st_mode & (S_IRWXG | S_IRWXO)) != 0) {
|
||||
set_error(err, err_size,
|
||||
"refusing to read secret file '%s': it must be owned by the current user and "
|
||||
"owner-only (0600), not accessible to group/other",
|
||||
path);
|
||||
close(fd);
|
||||
return NULL;
|
||||
}
|
||||
FILE* fp = fdopen(fd, "r");
|
||||
if (!fp) {
|
||||
set_error(err, err_size, "cannot read secret file '%s': %s", path, strerror(errno));
|
||||
close(fd);
|
||||
return NULL;
|
||||
}
|
||||
return fp;
|
||||
}
|
||||
|
||||
/* Trim leading/trailing ASCII space and tab in place; returns the new start. */
|
||||
@@ -142,14 +167,8 @@ static CredentialStore* load_store_file(const char* path, char* err, size_t err_
|
||||
if (!path)
|
||||
return store;
|
||||
|
||||
if (!secret_file_is_private(path, err, err_size)) {
|
||||
credentials_free(store);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
FILE* fp = fopen(path, "r");
|
||||
FILE* fp = secret_file_open(path, err, err_size);
|
||||
if (!fp) {
|
||||
set_error(err, err_size, "cannot open credential file '%s': %s", path, strerror(errno));
|
||||
credentials_free(store);
|
||||
return NULL;
|
||||
}
|
||||
@@ -328,13 +347,9 @@ int credentials_read_secret_file(const char* path, char** user_out, char** passw
|
||||
set_error(err, err_size, "no --password-file path");
|
||||
return -1;
|
||||
}
|
||||
if (!secret_file_is_private(path, err, err_size))
|
||||
FILE* fp = secret_file_open(path, err, err_size);
|
||||
if (!fp)
|
||||
return -1;
|
||||
FILE* fp = fopen(path, "r");
|
||||
if (!fp) {
|
||||
set_error(err, err_size, "cannot open password file '%s': %s", path, strerror(errno));
|
||||
return -1;
|
||||
}
|
||||
|
||||
int line_no = 0;
|
||||
char line[CREDENTIAL_MAX_LINE + 2];
|
||||
|
||||
+12
-6
@@ -930,9 +930,12 @@ static bool file_to_disk_secure_impl(const char* path, const void* data,
|
||||
int prealloc_rc = 0;
|
||||
if (preallocate && !sparse && data_size > 0) {
|
||||
prealloc_rc = preallocate_fd(fd, data_size);
|
||||
if (prealloc_rc != 0)
|
||||
log_message(LOG_LEVEL_ERROR, "preallocate failed for '%s' (%s); transfer aborted", path,
|
||||
strerror(prealloc_rc));
|
||||
if (prealloc_rc != 0) {
|
||||
char* escaped_path = output_escape(path, log_get_8_bit_output());
|
||||
log_message(LOG_LEVEL_ERROR, "preallocate failed for '%s' (%s); transfer aborted",
|
||||
escaped_path ? escaped_path : "<allocation failed>", strerror(prealloc_rc));
|
||||
free(escaped_path);
|
||||
}
|
||||
}
|
||||
if (prealloc_rc == 0) {
|
||||
/* posix_fallocate does not guarantee the fd's file offset is left
|
||||
@@ -1037,9 +1040,12 @@ static bool file_to_disk_secure_impl(const char* path, const void* data,
|
||||
int prealloc_rc = 0;
|
||||
if (preallocate && !sparse && data_size > 0) {
|
||||
prealloc_rc = preallocate_fd(fd, data_size);
|
||||
if (prealloc_rc != 0)
|
||||
log_message(LOG_LEVEL_ERROR, "preallocate failed for '%s' (%s); transfer aborted", path,
|
||||
strerror(prealloc_rc));
|
||||
if (prealloc_rc != 0) {
|
||||
char* escaped_path = output_escape(path, log_get_8_bit_output());
|
||||
log_message(LOG_LEVEL_ERROR, "preallocate failed for '%s' (%s); transfer aborted",
|
||||
escaped_path ? escaped_path : "<allocation failed>", strerror(prealloc_rc));
|
||||
free(escaped_path);
|
||||
}
|
||||
}
|
||||
if (prealloc_rc == 0) {
|
||||
lseek(fd, 0, SEEK_SET);
|
||||
|
||||
@@ -430,10 +430,14 @@ static bool protocol_send_str_impl(ProtocolSession* session, const char* data, b
|
||||
return false;
|
||||
if (!protocol_send_n_data(session, data, size))
|
||||
return false;
|
||||
if (redact)
|
||||
if (redact) {
|
||||
log_debug_message(LOG_DEBUG_PROTO, "Send String: <redacted>");
|
||||
else
|
||||
log_debug_message(LOG_DEBUG_PROTO, "Send String: %s", data);
|
||||
} else {
|
||||
char* escaped_data = output_escape(data, log_get_8_bit_output());
|
||||
log_debug_message(LOG_DEBUG_PROTO, "Send String: %s",
|
||||
escaped_data ? escaped_data : "<allocation failed>");
|
||||
free(escaped_data);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user