Merge feat/p5-remote-option: --remote-option, --trust-sender
# Conflicts: # RSYNC_COMPAT.md # src/client/client_cli.c # src/client/client_send.c # src/shared/transport_ssh.c # src/shared/transport_ssh.h # tests/integration/test_ssh.py # tests/test_client_cli.c # tests/test_transport_ssh.c
This commit is contained in:
@@ -175,6 +175,40 @@ static int set_nonneg_int_option(int* dest, const char* value, const char* optio
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* Forward decl: config_add_pattern is defined below, but the --remote-option
|
||||
* helper above needs it. */
|
||||
static int config_add_pattern(char*** patterns, int* count, const char* value, const char* optname);
|
||||
|
||||
/* Validate and append one --remote-option=OPT value. OPT is forwarded to the
|
||||
* remote server invocation (over SSH) by appending it to the remote command
|
||||
* line, so it must be a single safe shell word: it must be non-empty and must
|
||||
* contain no control characters that could break the single-quoted command
|
||||
* word ssh_build_remote_command wraps it in (newline/CR and other ASCII
|
||||
* control chars are rejected up front). Ordinary shell metacharacters
|
||||
* (; & | ` $ () etc.) need not be rejected because they are neutralized by the
|
||||
* single-quoting boundary, but rejecting control characters keeps the
|
||||
* quoting scheme airtight regardless of the remote shell. Returns 0 on
|
||||
* success, -1 on a rejected value. */
|
||||
static int config_add_remote_option(Config* config, const char* value, const char* optname) {
|
||||
if (!value || value[0] == '\0') {
|
||||
log_message(LOG_LEVEL_ERROR, "%s requires a non-empty option value", optname);
|
||||
return -1;
|
||||
}
|
||||
for (const unsigned char* p = (const unsigned char*)value; *p; p++) {
|
||||
if (*p < 0x20 || *p == 0x7f) {
|
||||
log_message(LOG_LEVEL_ERROR,
|
||||
"%s value contains a control character that could break the remote shell "
|
||||
"quoting; rejecting",
|
||||
optname);
|
||||
return -1;
|
||||
}
|
||||
}
|
||||
if (config_add_pattern(&config->remote_options, &config->remote_option_count, value, optname) !=
|
||||
0)
|
||||
return -1;
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* Validate and append one --compare-dest/--copy-dest/--link-dest directory.
|
||||
* The path is interpreted on the receiver relative to the destination root,
|
||||
* so it must be a non-empty relative path with no "." / ".." components (an
|
||||
@@ -606,6 +640,11 @@ static const OptionEntry OPTION_TABLE[] = {
|
||||
{"--xattrs", "-X", OPT_FLAG, offsetof(Config, preserve_xattrs)},
|
||||
{"--acls", "-A", OPT_FLAG, offsetof(Config, preserve_acls)},
|
||||
{"--fake-super", NULL, OPT_FLAG, offsetof(Config, fake_super)},
|
||||
/* Long-form-only: rsync's -M short form of --remote-option is INTENTIONALLY
|
||||
* unavailable because -M already means metadata mode in FastSync (a
|
||||
* documented divergence; see RSYNC_COMPAT.md). --trust-sender is a local
|
||||
* receiver policy and never travels to the remote peer. */
|
||||
{"--trust-sender", NULL, OPT_FLAG, offsetof(Config, trust_sender)},
|
||||
};
|
||||
|
||||
/* Only boolean options with no required argument are safe to negate. */
|
||||
@@ -1190,6 +1229,16 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
|
||||
}
|
||||
if (set_sockopts_option(config, argv[++i]) != 0)
|
||||
return -1;
|
||||
} else if (strncmp(argv[i], "--remote-option=", 16) == 0) {
|
||||
if (config_add_remote_option(config, argv[i] + 16, "--remote-option") != 0)
|
||||
return -1;
|
||||
} else if (opt_is(argv[i], "--remote-option", NULL)) {
|
||||
if (i + 1 >= argc) {
|
||||
log_message(LOG_LEVEL_ERROR, "missing argument for --remote-option");
|
||||
return -1;
|
||||
}
|
||||
if (config_add_remote_option(config, argv[++i], "--remote-option") != 0)
|
||||
return -1;
|
||||
} else if (strncmp(argv[i], "--compare-dest=", 15) == 0) {
|
||||
if (set_basis_dest_option(config, BASIS_DEST_COMPARE, argv[i] + 15, "--compare-dest") != 0)
|
||||
return -1;
|
||||
|
||||
@@ -368,7 +368,8 @@ static Client* connect_transfer_client(const Config* config) {
|
||||
}
|
||||
return client_connect_ssh(config->ssh_destination, config->ssh_port,
|
||||
config->fastsync_server_path, config->old_args, config->rsh_command,
|
||||
config->blocking_io);
|
||||
config->blocking_io, config->remote_options,
|
||||
config->remote_option_count);
|
||||
}
|
||||
|
||||
Client* client = client_create();
|
||||
|
||||
@@ -200,6 +200,16 @@ void print_usage(void) {
|
||||
printf(" Path to fastsync-server on remote (default: fastsync-server)\n");
|
||||
printf(
|
||||
" --old-args Disable safe SSH command argument quoting (legacy compatibility)\n");
|
||||
printf(" --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");
|
||||
printf(" are rejected). Long form only: rsync's -M short form is NOT\n");
|
||||
printf(" available because -M already means metadata preservation in\n");
|
||||
printf(" FastSync (documented divergence)\n");
|
||||
printf(" --trust-sender Trust the remote sender's file list: the receiver skips its\n");
|
||||
printf(" own up-front path-traversal/containment re-validation of the\n");
|
||||
printf(" incoming file list (fewer checks, faster, potentially unsafe).\n");
|
||||
printf(" Local receiver policy: never sent to the peer, off by default\n");
|
||||
printf(" -l, --links Copy symlinks as symlinks\n");
|
||||
printf(" --copy-links Transform symlinks into referent files\n");
|
||||
printf(" --safe-links Skip symlinks that point outside transfer tree\n");
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
#include "chunk.h"
|
||||
#include "config.h"
|
||||
#include "delay_updates.h"
|
||||
#include "file.h"
|
||||
#include "file_receive.h"
|
||||
#include "log.h"
|
||||
#include "metadata.h"
|
||||
@@ -92,7 +93,11 @@ static bool receiver_process_batch(Config* config, int file_descriptor) {
|
||||
send_status(file_descriptor, STATUS_ERROR);
|
||||
return false;
|
||||
}
|
||||
if (!utils_valid_batch_path(check_path)) {
|
||||
/* --trust-sender: accept a ``..``/absolute check path (a trusted sender's
|
||||
odd-but-legit entry) and defer containment to the secure stat below;
|
||||
an empty path is still always rejected. */
|
||||
if (check_path[0] == '\0' ||
|
||||
(!file_get_trust_sender() && !utils_valid_batch_path(check_path))) {
|
||||
free(check_path);
|
||||
send_status(file_descriptor, STATUS_ERROR);
|
||||
return false;
|
||||
|
||||
@@ -22,6 +22,7 @@
|
||||
static char* authorized_root;
|
||||
static int authorized_root_fd = -1;
|
||||
static bool allow_delete;
|
||||
static bool trust_sender;
|
||||
static bool allow_unauthenticated;
|
||||
static const char* required_client_cn;
|
||||
|
||||
@@ -220,6 +221,15 @@ void handler(int file_descriptor) {
|
||||
fd-walk reads a stable value during the whole transfer (and never bleeds
|
||||
across the per-connection forked processes). */
|
||||
file_set_keep_dirlinks(config->keep_dirlinks);
|
||||
/* --trust-sender is a LOCAL receiver policy: it never crosses the wire (so a
|
||||
wire peer can never enable it). The standalone server only honours it when
|
||||
its own CLI was started with --trust-sender (the client forwards that switch
|
||||
into the remote argv via --remote-option=--trust-sender; the server then
|
||||
parses it here and applies the policy below). Set before any multithreaded
|
||||
receiver/writer threads are spawned so the fd-walk reads a stable value
|
||||
during the whole transfer, and never bleeds across the per-connection
|
||||
forked processes. Off by default. */
|
||||
file_set_trust_sender(trust_sender);
|
||||
if (config->use_multithreading) {
|
||||
Queue* q = queue_create(100, file_destroy);
|
||||
if (q == NULL) {
|
||||
@@ -345,6 +355,7 @@ static void print_server_usage(void) {
|
||||
printf(" -4, --ipv4 Bind an IPv4 socket (default)\n");
|
||||
printf(" -6, --ipv6 Bind an IPv6 socket\n");
|
||||
printf(" --allow-delete Permit manifest deletion\n");
|
||||
printf(" --trust-sender Trust the remote sender's file list\n");
|
||||
printf(" --allow-unauthenticated Allow plaintext/anonymous network clients\n");
|
||||
printf(" -v, --verbose Enable debug logging\n");
|
||||
printf(" --help Show this help\n");
|
||||
@@ -397,6 +408,8 @@ int main(int argc, char* argv[]) {
|
||||
bind_family = AF_INET6;
|
||||
} else if (strcmp(argv[i], "--allow-delete") == 0) {
|
||||
allow_delete = true;
|
||||
} else if (strcmp(argv[i], "--trust-sender") == 0) {
|
||||
trust_sender = true;
|
||||
} else if (strcmp(argv[i], "--allow-unauthenticated") == 0) {
|
||||
allow_unauthenticated = true;
|
||||
} else if (strcmp(argv[i], "-p") == 0 && i + 1 < argc) {
|
||||
|
||||
@@ -125,6 +125,8 @@ static void config_set_defaults(Config* config) {
|
||||
config->outbuf = OUTBUF_BLOCK;
|
||||
config->old_args = false;
|
||||
config->temp_dir = NULL;
|
||||
config->remote_options = NULL;
|
||||
config->remote_option_count = 0;
|
||||
config->basis_dirs = NULL;
|
||||
config->basis_count = 0;
|
||||
config->partial_dir = NULL;
|
||||
@@ -166,6 +168,7 @@ static void config_set_defaults(Config* config) {
|
||||
config->open_noatime = false;
|
||||
config->use_xattrs = false;
|
||||
config->fake_super = false;
|
||||
config->trust_sender = false;
|
||||
}
|
||||
|
||||
static bool valid_wire_bool(int value) {
|
||||
@@ -507,6 +510,13 @@ void config_delete(Config* config) {
|
||||
file_list_destroy((FileListSet*)config->files_from_set);
|
||||
free(config->rsh_command);
|
||||
free(config->temp_dir);
|
||||
if (config->remote_options) {
|
||||
for (int i = 0; i < config->remote_option_count; i++)
|
||||
free(config->remote_options[i]);
|
||||
free(config->remote_options);
|
||||
}
|
||||
config->remote_options = NULL;
|
||||
config->remote_option_count = 0;
|
||||
for (int i = 0; i < config->basis_count; i++) {
|
||||
free(config->basis_dirs[i].path);
|
||||
config->basis_dirs[i].path = NULL;
|
||||
|
||||
+44
-1
@@ -266,6 +266,14 @@ typedef struct Config {
|
||||
int outbuf;
|
||||
bool old_args;
|
||||
char* temp_dir;
|
||||
/* --remote-option=OPT (Phase 5, long form only): one or more extra command-line
|
||||
* options to append to the REMOTE server invocation over SSH. CLIENT-ONLY:
|
||||
* they are composed into the remote command line by ssh_build_remote_command()
|
||||
* (each valid word is shell-escaped with the same quoting boundary as the
|
||||
* server path), and are NEVER serialized into the binary config frame. They
|
||||
* do NOT cross the wire and are never parsed on the receiver process. */
|
||||
char** remote_options;
|
||||
int remote_option_count;
|
||||
/* Alternate basis directories, ordered by command-line appearance. Each
|
||||
* entry's type selects compare/copy/link behavior on an exact match. These
|
||||
* cross the wire so the receiver can consult them; they are interpreted
|
||||
@@ -393,9 +401,44 @@ typedef struct Config {
|
||||
* a reserved user.fastsync.stat xattr recording the source uid/gid/mode/mtime
|
||||
* so a later privileged restore could re-apply them. Crosses the wire. */
|
||||
bool fake_super;
|
||||
|
||||
// Phase 5: --trust-sender
|
||||
/* Long-form-only, receiver-local policy. rsync's --trust-sender tells the
|
||||
* receiving side to trust that the sender already produced a sane file list,
|
||||
* relaxing the receiver's own up-front re-validation of every incoming path.
|
||||
* In FastSync the receiver normally double-checks each transmitted file-list
|
||||
* entry (empty / ".." path-traversal rejection) and refuses to materialize a
|
||||
* symlink whose target could escape the receive root. When trust_sender is
|
||||
* set, those redundant list-level re-checks are SKIPPED: the receiving side
|
||||
* trusts the sender's list instead of re-validating it (fewer checks, faster,
|
||||
* potentially unsafe, matching rsync). It is a LOCAL receiver policy and is
|
||||
* NEVER serialized into the config frame (it exists only on the process that
|
||||
* actually receives the file list). Even under trust_sender the low-level
|
||||
* fd-relative confinement primitives (file_open_secure_parent, the O_NOFOLLOW
|
||||
* parent walk, leaf/destination confinement) are deliberately KEPT as a hard
|
||||
* floor, so a hostile sender still cannot write or link outside the
|
||||
* authorized root (see the phase-5 notes in RSYNC_COMPAT.md). Off by
|
||||
* default; only relaxes validation when explicitly requested. */
|
||||
bool trust_sender;
|
||||
} Config;
|
||||
|
||||
#define PROTOCOL_VERSION "2.13.0"
|
||||
/* Phase 5 (remote-option wave): 2.13.0 -> 2.14.0.
|
||||
*
|
||||
* WHY the bump, grounded in the wire: the binary config-frame layout is
|
||||
* UNCHANGED by this wave (neither --remote-option nor --trust-sender adds a
|
||||
* serialized field; see the field comments above). --remote-option is
|
||||
* forwarded to the remote server over the SSH remote-command line
|
||||
* (ssh_build_remote_command) and --trust-sender is a purely local receiver
|
||||
* policy, so there is no new frame byte to negotiate. The bump is still the
|
||||
* correct release marker for Phase 5 because the client-to-server INVOCATION
|
||||
* surface changed: a client that composes remote-options expects a server that
|
||||
* knows how to honor them, and the only safe way to express "this feature set
|
||||
* is one coordinated release" is the strict same-version handshake FastSync
|
||||
* already performs for every release. A 2.14 client against a 2.13 server
|
||||
* fails the version check cleanly up front (rather than the remote server
|
||||
* rejecting an unfamiliar forwarded argv at a confusing later point), which is
|
||||
* exactly what the lockstep convention of this project requires. */
|
||||
#define PROTOCOL_VERSION "2.14.0"
|
||||
#define DEFAULT_CHUNK_SIZE (10 * 1024 * 1024)
|
||||
/* Upper bound on total basis-dir entries (rsync caps --link-dest at 20). */
|
||||
#define MAX_BASIS_DIRS 64
|
||||
|
||||
+27
-1
@@ -363,6 +363,23 @@ bool file_get_keep_dirlinks(void) {
|
||||
return file_keep_dirlinks;
|
||||
}
|
||||
|
||||
/* --trust-sender (Phase 5) receiver process-wide policy: when set, the receiver
|
||||
* trusts the sender's file list and skips its own redundant up-front re-
|
||||
* validation (empty/".." path rejection, escaping-symlink-target containment).
|
||||
* Kept OFF by default; the server's per-connection handler sets it once from the
|
||||
* received config before any receiver/writer threads start (each connection is
|
||||
* its own forked process, so this per-process value never bleeds across
|
||||
* connections). */
|
||||
static bool file_trust_sender = false;
|
||||
|
||||
void file_set_trust_sender(bool enable) {
|
||||
file_trust_sender = enable;
|
||||
}
|
||||
|
||||
bool file_get_trust_sender(void) {
|
||||
return file_trust_sender;
|
||||
}
|
||||
|
||||
/* True when `target` is a lexical symlink target that can never escape the
|
||||
* receive root once created beneath it: relative (not absolute) and containing
|
||||
* no ".." path component. Used by --munge-links' sender-side containment: an
|
||||
@@ -425,7 +442,16 @@ char* file_symlink_munge(const char* target) {
|
||||
* false) so a malicious sender can never materialize a symlink that points
|
||||
* outside the receive root. */
|
||||
bool file_symlink_at_secure(const char* path, const char* target) {
|
||||
if (!path || !target || has_path_traversal(path) || !file_symlink_target_contained(target))
|
||||
/* The link itself (`path`) is always kept below the authorized root. The
|
||||
TARGET may point anywhere: normally only a contained (relative, ".."-free)
|
||||
target is permitted so a malicious sender can never plant a symlink that
|
||||
later dereferences outside the root. Under --trust-sender that target
|
||||
containment check is relaxed (the receiver trusts the sender and copies the
|
||||
link verbatim, matching rsync -l), but path/leaf confinement is never
|
||||
disabled, so the link still cannot be placed outside the tree. */
|
||||
if (!path || !target || has_path_traversal(path))
|
||||
return false;
|
||||
if (!file_trust_sender && !file_symlink_target_contained(target))
|
||||
return false;
|
||||
char* leaf = NULL;
|
||||
int parent_fd = file_open_secure_parent(path, &leaf, true);
|
||||
|
||||
@@ -54,6 +54,15 @@ bool file_symlink_at_secure(const char* path, const char* target);
|
||||
void file_set_keep_dirlinks(bool enable);
|
||||
bool file_get_keep_dirlinks(void);
|
||||
|
||||
/* --trust-sender receiver process-wide policy (Phase 5). When set, the
|
||||
* receiver trusts that the sender already produced a clean file list and skips
|
||||
* its own redundant up-front re-validation of incoming paths (the empty/".."
|
||||
* rejection and the escaping-symlink-target containment). The low-level
|
||||
* fd-relative confinement primitives below are deliberately NOT disabled by
|
||||
* this flag, so a hostile sender still cannot escape the authorized root. */
|
||||
void file_set_trust_sender(bool enable);
|
||||
bool file_get_trust_sender(void);
|
||||
|
||||
/* A configured fd without a canonical identity deliberately rejects paths. */
|
||||
bool file_set_authorized_root(int fd, const char* canonical_path);
|
||||
|
||||
|
||||
+22
-13
@@ -329,8 +329,12 @@ bool file_special_rdev_valid(int32_t major, int32_t minor, mode_t mode) {
|
||||
*/
|
||||
static FileSaveResult file_save_special_to_disk(const char* root_directory, const File* file,
|
||||
const Config* config) {
|
||||
/* The empty-path and structural checks stay unconditional; the redundant
|
||||
".." list-path re-check is skipped under --trust-sender exactly like the
|
||||
receive layer (confinement is deferred to the secure parent walk below,
|
||||
which is never disabled). */
|
||||
if (!root_directory || !file || !file->path || file->path[0] == '\0' ||
|
||||
has_path_traversal(file->path) || !file->metadata)
|
||||
(!file_get_trust_sender() && has_path_traversal(file->path)) || !file->metadata)
|
||||
return FILE_SAVE_ERROR;
|
||||
|
||||
mode_t mode = file->metadata->mode;
|
||||
@@ -461,7 +465,7 @@ static FileSaveResult file_save_special_to_disk(const char* root_directory, cons
|
||||
* entry is skipped), never aborts. */
|
||||
static FileSaveResult file_save_write_device(const char* root_directory, const File* file) {
|
||||
if (!root_directory || !file || !file->path || file->path[0] == '\0' ||
|
||||
has_path_traversal(file->path))
|
||||
(!file_get_trust_sender() && has_path_traversal(file->path)))
|
||||
return FILE_SAVE_ERROR;
|
||||
if (!file->data)
|
||||
return FILE_SAVE_ERROR;
|
||||
@@ -538,7 +542,7 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi
|
||||
char *backup_path = NULL, *parent_copy = NULL;
|
||||
|
||||
if (!file || !file->path || !file->data || (file->data->size != 0 && !file->data->data) ||
|
||||
has_path_traversal(file->path) ||
|
||||
(!file_get_trust_sender() && has_path_traversal(file->path)) ||
|
||||
(backup_enabled &&
|
||||
(!backup_suffix || backup_suffix[0] == '\0' || strchr(backup_suffix, '/') != NULL ||
|
||||
strcmp(backup_suffix, ".") == 0 || strcmp(backup_suffix, "..") == 0))) {
|
||||
@@ -560,7 +564,7 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi
|
||||
immediately (they are never staged by --delay-updates, matching rsync,
|
||||
where directory creation is not delayed). */
|
||||
if (file->is_dir) {
|
||||
if (file->path[0] == '\0' || has_path_traversal(file->path)) {
|
||||
if (file->path[0] == '\0' || (!file_get_trust_sender() && has_path_traversal(file->path))) {
|
||||
log_message(LOG_LEVEL_ERROR, "Invalid directory path received");
|
||||
return FILE_SAVE_ERROR;
|
||||
}
|
||||
@@ -577,7 +581,8 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi
|
||||
threads start, so it is stable throughout this walk.) */
|
||||
|
||||
if (file->is_symlink) {
|
||||
if (!file->symlink_target || file->path[0] == '\0' || has_path_traversal(file->path)) {
|
||||
if (!file->symlink_target || file->path[0] == '\0' ||
|
||||
(!file_get_trust_sender() && has_path_traversal(file->path))) {
|
||||
log_message(LOG_LEVEL_ERROR, "Invalid symlink entry received");
|
||||
return FILE_SAVE_ERROR;
|
||||
}
|
||||
@@ -595,8 +600,12 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi
|
||||
could escape the receive root (absolute, or relative-with-"..") is never
|
||||
materialized. It is contained (the entry is skipped) rather than failing
|
||||
the whole transfer, so a hostile sender can inject a broken symlink but
|
||||
can never redirect it outside the root. */
|
||||
if (ok && !file_symlink_target_contained(target))
|
||||
can never redirect it outside the root. --trust-sender deliberately
|
||||
relaxes this receiver-side re-validation: a trusted sender's escaping
|
||||
symlink target is copied verbatim (rsync -l parity). The low-level
|
||||
leaf/destination confinement in file_symlink_at_secure still ensures the
|
||||
link itself is placed inside the authorized root. */
|
||||
if (ok && !file_get_trust_sender() && !file_symlink_target_contained(target))
|
||||
ok = false;
|
||||
if (!ok) {
|
||||
/* Skip the escaping/empty target (contained) rather than abort. */
|
||||
@@ -2076,7 +2085,7 @@ File* file_receive(const Config* config, int file_descriptor) {
|
||||
char* path = receive_str(file_descriptor);
|
||||
if (path == NULL)
|
||||
return NULL;
|
||||
if (path[0] == '\0' || has_path_traversal(path)) {
|
||||
if (path[0] == '\0' || (!file_get_trust_sender() && has_path_traversal(path))) {
|
||||
char* escaped_path = output_escape(path, log_get_8_bit_output());
|
||||
log_message(LOG_LEVEL_ERROR, "Invalid received file path: %s",
|
||||
escaped_path ? escaped_path : "<allocation failed>");
|
||||
@@ -2136,7 +2145,7 @@ File* file_receive_directory(int file_descriptor) {
|
||||
char* path = receive_str(file_descriptor);
|
||||
if (path == NULL)
|
||||
return NULL;
|
||||
if (path[0] == '\0' || has_path_traversal(path)) {
|
||||
if (path[0] == '\0' || (!file_get_trust_sender() && has_path_traversal(path))) {
|
||||
char* escaped_path = output_escape(path, log_get_8_bit_output());
|
||||
log_message(LOG_LEVEL_ERROR, "Invalid received directory path: %s",
|
||||
escaped_path ? escaped_path : "<allocation failed>");
|
||||
@@ -2163,7 +2172,7 @@ File* file_receive_hardlink(int file_descriptor) {
|
||||
char* path = receive_str(file_descriptor);
|
||||
if (path == NULL)
|
||||
return NULL;
|
||||
if (path[0] == '\0' || has_path_traversal(path)) {
|
||||
if (path[0] == '\0' || (!file_get_trust_sender() && has_path_traversal(path))) {
|
||||
char* escaped_path = output_escape(path, log_get_8_bit_output());
|
||||
log_message(LOG_LEVEL_ERROR, "Invalid received hard-link path: %s",
|
||||
escaped_path ? escaped_path : "<allocation failed>");
|
||||
@@ -2182,7 +2191,7 @@ File* file_receive_hardlink(int file_descriptor) {
|
||||
free(path);
|
||||
return NULL;
|
||||
}
|
||||
if (target[0] == '\0' || has_path_traversal(target)) {
|
||||
if (target[0] == '\0' || (!file_get_trust_sender() && has_path_traversal(target))) {
|
||||
char* escaped = output_escape(target, log_get_8_bit_output());
|
||||
log_message(LOG_LEVEL_ERROR, "Invalid hard-link target path: %s",
|
||||
escaped ? escaped : "<allocation failed>");
|
||||
@@ -2213,7 +2222,7 @@ File* file_receive_symlink(int file_descriptor, const Config* config) {
|
||||
char* path = receive_str(file_descriptor);
|
||||
if (path == NULL)
|
||||
return NULL;
|
||||
if (path[0] == '\0' || has_path_traversal(path)) {
|
||||
if (path[0] == '\0' || (!file_get_trust_sender() && has_path_traversal(path))) {
|
||||
char* escaped_path = output_escape(path, log_get_8_bit_output());
|
||||
log_message(LOG_LEVEL_ERROR, "Invalid received symlink path: %s",
|
||||
escaped_path ? escaped_path : "<allocation failed>");
|
||||
@@ -2268,7 +2277,7 @@ File* file_receive_special(int file_descriptor) {
|
||||
char* path = receive_str(file_descriptor);
|
||||
if (path == NULL)
|
||||
return NULL;
|
||||
if (path[0] == '\0' || has_path_traversal(path)) {
|
||||
if (path[0] == '\0' || (!file_get_trust_sender() && has_path_traversal(path))) {
|
||||
char* escaped_path = output_escape(path, log_get_8_bit_output());
|
||||
log_message(LOG_LEVEL_ERROR, "Invalid received special path: %s",
|
||||
escaped_path ? escaped_path : "<allocation failed>");
|
||||
|
||||
+85
-27
@@ -78,47 +78,103 @@ static int parse_remote_dest(const char* dest, RemoteDest* r) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
char* ssh_build_remote_command(const char* server_path, bool old_args) {
|
||||
char* ssh_build_remote_command(const char* server_path, bool old_args, char* const* remote_options,
|
||||
int remote_option_count) {
|
||||
const char* path = server_path ? server_path : "fastsync-server";
|
||||
const char* suffix = " --stdio";
|
||||
|
||||
/* Each --remote-option=OPT is appended after " --stdio" as one shell word,
|
||||
escaped with the SAME single-quote boundary used for the server path. This
|
||||
stays safe even in --old-args mode (which leaves the server path unquoted):
|
||||
remote options are always single-quoted individually, so a value containing
|
||||
shell metacharacters (; & | ` $ ()) can never break out of the quoting to
|
||||
inject an unrelated remote command. Values are already validated at CLI
|
||||
parse time (non-empty, no control characters); this layer only adds the
|
||||
escaping boundary. */
|
||||
size_t path_len = strlen(path);
|
||||
size_t suffix_len = strlen(suffix);
|
||||
|
||||
/* The base command (server path, quoted unless --old-args, then " --stdio"). */
|
||||
size_t command_len;
|
||||
if (old_args) {
|
||||
if (path_len > SIZE_MAX - suffix_len - 1)
|
||||
return NULL;
|
||||
char* command = malloc(path_len + suffix_len + 1);
|
||||
if (!command)
|
||||
command_len = path_len + suffix_len + 1;
|
||||
} else {
|
||||
size_t quote_count = 0;
|
||||
for (const char* p = path; *p; p++)
|
||||
if (*p == '\'')
|
||||
quote_count++;
|
||||
if (path_len > SIZE_MAX - suffix_len - 4 ||
|
||||
quote_count > (SIZE_MAX - path_len - suffix_len - 4) / 4)
|
||||
return NULL;
|
||||
memcpy(command, path, path_len);
|
||||
memcpy(command + path_len, suffix, suffix_len + 1);
|
||||
return command;
|
||||
command_len = path_len + quote_count * 4 + suffix_len + 4;
|
||||
}
|
||||
|
||||
/* Quote the executable as one remote-shell word. This is the default safety boundary. */
|
||||
size_t quote_count = 0;
|
||||
for (const char* p = path; *p; p++)
|
||||
if (*p == '\'')
|
||||
quote_count++;
|
||||
if (path_len > SIZE_MAX - suffix_len - 4 ||
|
||||
quote_count > (SIZE_MAX - path_len - suffix_len - 4) / 4)
|
||||
return NULL;
|
||||
size_t command_len = path_len + quote_count * 4 + suffix_len + 4;
|
||||
char* command = malloc(command_len + 1);
|
||||
/* Add each remote option, escaped as one single-quoted word:
|
||||
" '<body>'", i.e. 1 leading space + 1 open quote + body (len + 3 per
|
||||
embedded single quote) + 1 close quote = len + q*3 + 3 bytes.
|
||||
Defense-in-depth against a non-conforming caller: never forward an empty
|
||||
or control-character value, independent of the CLI validation. */
|
||||
for (int i = 0; i < remote_option_count; i++) {
|
||||
const char* opt = remote_options[i];
|
||||
if (!opt || opt[0] == '\0')
|
||||
return NULL;
|
||||
size_t len = 0, q = 0;
|
||||
for (const char* p = opt; *p; p++) {
|
||||
/* Defense-in-depth: never forward a control character (newline/CR/etc.)
|
||||
that could break the single-quoted shell word regardless of the remote
|
||||
shell, independent of the CLI validation. */
|
||||
if ((unsigned char)*p < 0x20 || (unsigned char)*p == 0x7f)
|
||||
return NULL;
|
||||
if (*p == '\'')
|
||||
q++;
|
||||
len++;
|
||||
}
|
||||
if (len > SIZE_MAX - q * 3 || len + q * 3 + 3 > SIZE_MAX - command_len)
|
||||
return NULL;
|
||||
command_len += len + q * 3 + 3;
|
||||
}
|
||||
command_len += 1; /* NUL */
|
||||
|
||||
char* command = malloc(command_len);
|
||||
if (!command)
|
||||
return NULL;
|
||||
char* out = command;
|
||||
*out++ = '\'';
|
||||
for (const char* p = path; *p; p++) {
|
||||
if (*p == '\'') {
|
||||
memcpy(out, "'\\''", 4);
|
||||
out += 4;
|
||||
} else {
|
||||
*out++ = *p;
|
||||
if (old_args) {
|
||||
memcpy(out, path, path_len);
|
||||
out += path_len;
|
||||
memcpy(out, suffix, suffix_len + 1);
|
||||
out += suffix_len;
|
||||
} else {
|
||||
*out++ = '\'';
|
||||
for (const char* p = path; *p; p++) {
|
||||
if (*p == '\'') {
|
||||
memcpy(out, "'\\''", 4);
|
||||
out += 4;
|
||||
} else {
|
||||
*out++ = *p;
|
||||
}
|
||||
}
|
||||
*out++ = '\'';
|
||||
memcpy(out, suffix, suffix_len + 1);
|
||||
out += suffix_len;
|
||||
}
|
||||
*out++ = '\'';
|
||||
memcpy(out, suffix, suffix_len + 1);
|
||||
for (int i = 0; i < remote_option_count; i++) {
|
||||
const char* opt = remote_options[i];
|
||||
*out++ = ' ';
|
||||
*out++ = '\'';
|
||||
for (const char* p = opt; *p; p++) {
|
||||
if (*p == '\'') {
|
||||
memcpy(out, "'\\''", 4);
|
||||
out += 4;
|
||||
} else {
|
||||
*out++ = *p;
|
||||
}
|
||||
}
|
||||
*out++ = '\'';
|
||||
}
|
||||
*out = '\0';
|
||||
return command;
|
||||
}
|
||||
|
||||
@@ -234,7 +290,8 @@ void ssh_free_client_argv(char** argv) {
|
||||
}
|
||||
|
||||
Client* client_connect_ssh(const char* destination, int port, const char* server_path,
|
||||
bool old_args, const char* rsh_command, bool blocking_io) {
|
||||
bool old_args, const char* rsh_command, bool blocking_io,
|
||||
char* const* remote_options, int remote_option_count) {
|
||||
RemoteDest r;
|
||||
if (parse_remote_dest(destination, &r) != 0) {
|
||||
char* escaped = output_escape(destination, false);
|
||||
@@ -312,7 +369,8 @@ Client* client_connect_ssh(const char* destination, int port, const char* server
|
||||
else
|
||||
snprintf(ssh_user, ssh_user_len, "%s", r.host);
|
||||
|
||||
char* remote_command = ssh_build_remote_command(server_path, old_args);
|
||||
char* remote_command =
|
||||
ssh_build_remote_command(server_path, old_args, remote_options, remote_option_count);
|
||||
if (!remote_command)
|
||||
ssh_child_setup_failed(exec_pipe[1]);
|
||||
char** ssh_argv = ssh_build_client_argv(rsh_command, port, ssh_user, remote_command);
|
||||
|
||||
@@ -4,8 +4,15 @@
|
||||
#include "transport_tcp.h"
|
||||
|
||||
Client* client_connect_ssh(const char* destination, int port, const char* server_path,
|
||||
bool old_args, const char* rsh_command, bool blocking_io);
|
||||
char* ssh_build_remote_command(const char* server_path, bool old_args);
|
||||
bool old_args, const char* rsh_command, bool blocking_io,
|
||||
char* const* remote_options, int remote_option_count);
|
||||
/* Build the escaped remote-shell command string (the server program path quoted
|
||||
* as one remote-shell word unless --old-args, followed by ` --stdio` and each
|
||||
* --remote-option value appended as an individually single-quoted shell word).
|
||||
* Every --remote-option value is individually escaped with the '\'' sequence and
|
||||
* values with empty/control characters are rejected at the CLI parse layer. */
|
||||
char* ssh_build_remote_command(const char* server_path, bool old_args, char* const* remote_options,
|
||||
int remote_option_count);
|
||||
/* Build the NULL-terminated child argv for the remote-shell client (argv[0] is
|
||||
* the exec/execvp program). rsh_command is whitespace-split into leading argv
|
||||
* words (NULL or "" selects the default "ssh"); the standard -o family, the
|
||||
|
||||
Reference in New Issue
Block a user