fix(client): route explicit remote dry-run targets, fail closed on stray status
--dry-run --server-host=H (or TLS / source-bind --address) silently ran the client-side manifest even though a real run contacts the server. Add a client-only, never-serialized server_host_set bit (alongside the existing server_port_set) and extend dry_run_targets_server so every explicit remote target contacts the receiver. Also make incremental_check return the dry-run code (4) only when the session actually requested dry-run; a stray STATUS_DRY_RUN_TRANSFER from a hostile/buggy peer is now a logged protocol error (STATUS_ERROR) instead of falling through to send file data and desync. Both normal send_single_file callers handle rc == 4 explicitly as an abort.
This commit is contained in:
@@ -1106,6 +1106,11 @@ static bool cli_handle_table_option(CliParseCtx* ctx) {
|
|||||||
}
|
}
|
||||||
config->use_metadata = true;
|
config->use_metadata = true;
|
||||||
}
|
}
|
||||||
|
/* Remember that --server-host was explicitly given (the field itself
|
||||||
|
defaults to 127.0.0.1, so a value check cannot distinguish it). Used
|
||||||
|
by --dry-run to route an explicit remote target to the server. */
|
||||||
|
if (entry->offset == offsetof(Config, server_host))
|
||||||
|
config->server_host_set = true;
|
||||||
}
|
}
|
||||||
} else if (apply_table_option(config, entry, NULL) != 0) {
|
} else if (apply_table_option(config, entry, NULL) != 0) {
|
||||||
ctx->exit_code = -1;
|
ctx->exit_code = -1;
|
||||||
|
|||||||
@@ -482,10 +482,11 @@ static void disconnect_transfer_client(Client* client) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/* True when --dry-run should contact a receiver rather than running the
|
/* True when --dry-run should contact a receiver rather than running the
|
||||||
* client-side local manifest. A remote (SSH host:path), daemon
|
* client-side local manifest. Any target a real run would reach over the wire
|
||||||
* (host::module/path), or an explicit --server-port/--port selects the
|
* selects the server-contacting path: a remote (SSH host:path), a daemon
|
||||||
* server-contacting path; a plain local destination keeps the original
|
* (host::module/path), an explicit --server-host, --server-port/--port, TLS, or
|
||||||
* client-side behavior (which never dials the default 127.0.0.1:8080). */
|
* a source-bind --address. A plain local destination (none of these) keeps the
|
||||||
|
* original client-side behavior, which never dials the default 127.0.0.1:8080. */
|
||||||
static bool dry_run_targets_server(const Config* config) {
|
static bool dry_run_targets_server(const Config* config) {
|
||||||
if (!config)
|
if (!config)
|
||||||
return false;
|
return false;
|
||||||
@@ -493,7 +494,13 @@ static bool dry_run_targets_server(const Config* config) {
|
|||||||
return true;
|
return true;
|
||||||
if (config->module && config->module[0] != '\0')
|
if (config->module && config->module[0] != '\0')
|
||||||
return true;
|
return true;
|
||||||
return config->server_port_set;
|
if (config->server_host_set || config->server_port_set)
|
||||||
|
return true;
|
||||||
|
if (config->use_tls)
|
||||||
|
return true;
|
||||||
|
if (config->address != NULL)
|
||||||
|
return true;
|
||||||
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
static bool add_chunk_to_manifest(ArrayList* manifest, const Chunk* chunk) {
|
static bool add_chunk_to_manifest(ArrayList* manifest, const Chunk* chunk) {
|
||||||
@@ -1040,11 +1047,18 @@ static int incremental_check(Client* client, File* file, const Config* config,
|
|||||||
return 3;
|
return 3;
|
||||||
}
|
}
|
||||||
/* Server-contacting --dry-run: the receiver decided the file is not up to
|
/* Server-contacting --dry-run: the receiver decided the file is not up to
|
||||||
date and answered "would transfer" WITHOUT expecting any data. The caller
|
date and answered "would transfer" WITHOUT expecting any data. Treat it as
|
||||||
only uses this in the dry-run path; a non-dry-run sender never receives it
|
the dry-run code ONLY when this session actually requested dry-run. A
|
||||||
because the receiver only emits it when the wire config sets dry_run. */
|
hostile/buggy peer that emits it outside dry-run is a protocol error: fail
|
||||||
if (s == STATUS_DRY_RUN_TRANSFER)
|
closed (and send STATUS_ERROR) rather than fall through to the normal path,
|
||||||
|
which would transmit file data the receiver is not reading and desync. */
|
||||||
|
if (s == STATUS_DRY_RUN_TRANSFER) {
|
||||||
|
if (config->dry_run)
|
||||||
return 4;
|
return 4;
|
||||||
|
log_message(LOG_LEVEL_ERROR, "Unexpected DRY_RUN_TRANSFER status outside a --dry-run session");
|
||||||
|
send_status(client->file_descriptor, STATUS_ERROR);
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
if (s != STATUS_NEXT) {
|
if (s != STATUS_NEXT) {
|
||||||
log_message(LOG_LEVEL_ERROR, "Unexpected server status");
|
log_message(LOG_LEVEL_ERROR, "Unexpected server status");
|
||||||
send_status(client->file_descriptor, STATUS_ERROR);
|
send_status(client->file_descriptor, STATUS_ERROR);
|
||||||
@@ -1478,6 +1492,16 @@ static int send_single_file(Client* client, File* file, Config* config, bool use
|
|||||||
}
|
}
|
||||||
return arc == 0 ? 0 : -1;
|
return arc == 0 ? 0 : -1;
|
||||||
}
|
}
|
||||||
|
// rc == 4: the receiver answered DRY_RUN_TRANSFER, which is only valid in
|
||||||
|
// incremental_check's dedicated dry-run consumer. send_single_file never
|
||||||
|
// runs a dry-run session, so this is a protocol error: abort instead of
|
||||||
|
// falling through and sending data the receiver is not reading.
|
||||||
|
if (rc == 4) {
|
||||||
|
log_message(LOG_LEVEL_ERROR, "Receiver answered DRY_RUN_TRANSFER in a non-dry-run transfer");
|
||||||
|
delta_signature_destroy(sig);
|
||||||
|
send_status(client->file_descriptor, STATUS_ERROR);
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
// rc == 0: unchanged file, skip
|
// rc == 0: unchanged file, skip
|
||||||
// rc == 2: server sent delta signature but sendfile doesn't support delta
|
// rc == 2: server sent delta signature but sendfile doesn't support delta
|
||||||
delta_signature_destroy(sig);
|
delta_signature_destroy(sig);
|
||||||
@@ -1519,6 +1543,14 @@ static int send_single_file(Client* client, File* file, Config* config, bool use
|
|||||||
}
|
}
|
||||||
return arc == 0 ? 0 : -1;
|
return arc == 0 ? 0 : -1;
|
||||||
}
|
}
|
||||||
|
if (rc == 4) {
|
||||||
|
/* See the sendfile branch above: DRY_RUN_TRANSFER is only valid in the
|
||||||
|
dedicated dry-run consumer, never in the normal per-file send path. */
|
||||||
|
log_message(LOG_LEVEL_ERROR, "Receiver answered DRY_RUN_TRANSFER in a non-dry-run transfer");
|
||||||
|
delta_signature_destroy(sig);
|
||||||
|
send_status(client->file_descriptor, STATUS_ERROR);
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
if (rc == 2 && config->use_delta && !config->whole_file) {
|
if (rc == 2 && config->use_delta && !config->whole_file) {
|
||||||
int drc = send_delta(client, file, sig, config);
|
int drc = send_delta(client, file, sig, config);
|
||||||
delta_signature_destroy(sig);
|
delta_signature_destroy(sig);
|
||||||
|
|||||||
@@ -42,6 +42,7 @@ static void config_set_defaults(Config* config) {
|
|||||||
config->server_host = str_dup("127.0.0.1");
|
config->server_host = str_dup("127.0.0.1");
|
||||||
config->server_port = 8080;
|
config->server_port = 8080;
|
||||||
config->server_port_set = false;
|
config->server_port_set = false;
|
||||||
|
config->server_host_set = false;
|
||||||
/* 0 means "--timeout not given": the transport keeps its own built-in 30 s
|
/* 0 means "--timeout not given": the transport keeps its own built-in 30 s
|
||||||
* socket timeout (tcp_set_timeouts ignores non-positive values) and the
|
* socket timeout (tcp_set_timeouts ignores non-positive values) and the
|
||||||
* protocol layer keeps its built-in 60 s per-message deadline. A positive
|
* protocol layer keeps its built-in 60 s per-message deadline. A positive
|
||||||
|
|||||||
@@ -292,6 +292,12 @@ typedef struct Config {
|
|||||||
* existing client-side dry-run behavior instead of dialing the default
|
* existing client-side dry-run behavior instead of dialing the default
|
||||||
* 127.0.0.1:8080. */
|
* 127.0.0.1:8080. */
|
||||||
bool server_port_set;
|
bool server_port_set;
|
||||||
|
/* True when --server-host was explicitly given. CLIENT-ONLY (never
|
||||||
|
* serialized), and distinct from the "127.0.0.1" default: --dry-run uses it
|
||||||
|
* to route an explicit remote target to the server so it reports receiver
|
||||||
|
* state exactly like a real run, instead of silently running the client-side
|
||||||
|
* manifest. */
|
||||||
|
bool server_host_set;
|
||||||
char* tls_cert;
|
char* tls_cert;
|
||||||
char* tls_key;
|
char* tls_key;
|
||||||
char* tls_ca;
|
char* tls_ca;
|
||||||
|
|||||||
Reference in New Issue
Block a user