diff --git a/README.md b/README.md index d6b7a42..c1253ed 100644 --- a/README.md +++ b/README.md @@ -140,6 +140,7 @@ Config negotiation is sender-driven: the client serializes transfer options and | `--cert ` | TLS certificate file (PEM) | | `--key ` | TLS private key file (PEM) | | `--ca ` | TLS CA certificate file for verification (PEM) | +| `--client-cn ` | Required TLS client certificate common name | ### Server @@ -151,6 +152,9 @@ Config negotiation is sender-driven: the client serializes transfer options and | `--cert ` | TLS certificate file (PEM) | | `--key ` | TLS private key file (PEM) | | `--ca ` | TLS CA certificate file for verification (PEM) | +| `--destination-root ` | Authorized destination root (default: `.`) | +| `--allow-delete` | Permit manifest deletion | +| `--allow-unauthenticated` | Permit plaintext TCP clients | | `-v, --verbose` | Enable debug logging | | `--help` | Show help | @@ -188,7 +192,7 @@ Config negotiation is sender-driven: the client serializes transfer options and 7. **Metadata restoration** — `chmod()`, `chown()`, `utimensat()` on the receiving side 8. **`--delete`** — sender tracks all sent paths; receiver walks destination tree and removes unlisted files/directories 9. **SSH transport** — `socketpair()` + `fork()` + `execvp("ssh", ...)` with `ControlMaster` and port support -10. **TLS transport** — OpenSSL `SSL_CTX` with TLS 1.2 minimum, optional CA verification, transparent `SSL_read`/`SSL_write` via `io_set_ssl()` +10. **TLS transport** — OpenSSL `SSL_CTX` with TLS 1.2 minimum, mutual CA verification, transparent `SSL_read`/`SSL_write` via `io_set_ssl()` 11. **Path traversal protection** — `has_path_traversal()` rejects any file path containing `..` components, preventing directory escape attacks 12. **Connection limiting** — server tracks active connections and rejects new ones beyond `max_connections` (default 100) 13. **Keep-alive** — idle connections receive periodic `STATUS_KEEPALIVE` to detect half-open TCP connections @@ -202,7 +206,7 @@ Config negotiation is sender-driven: the client serializes transfer options and All received file paths are validated by `has_path_traversal()` before any disk operation. Any path containing `..` components is rejected with `STATUS_ERROR`, preventing directory escape attacks. ### TLS Certificate Verification -When `--ca` is provided, the server performs mutual TLS verification (`SSL_VERIFY_PEER` with depth 4). Without `--ca`, TLS is still encrypted but peer certificates are not verified. +TLS requires `--ca` and performs mutual TLS verification (`SSL_VERIFY_PEER` with depth 4). Connections without certificate verification are rejected. ### Connection Limits The server enforces a maximum of 100 concurrent connections (configurable via `max_connections` in `Server`). When the limit is reached, new connections are immediately rejected and closed. @@ -244,12 +248,12 @@ cmake -B build -S . && cmake --build build -j$(nproc) ### Server (TCP mode) ```bash -./build/server +./build/server --allow-unauthenticated ``` ### Server with TLS ```bash -./build/server --tls --cert server.pem --key server-key.pem +./build/server --tls --cert server.pem --key server-key.pem --ca ca.pem --client-cn fastsync-client ``` ### Server via SSH @@ -265,6 +269,9 @@ Place the `fastsync-server` binary in the remote `$PATH`. The client runs `ssh u ./build/client --source-dir /path/to/send --dest-dir /path/to/receive --save-to-disk ``` +Plain TCP requires the explicit `--allow-unauthenticated` server option. Use TLS for +authenticated network connections. + ### Client — TCP with TLS ```bash ./build/client --tls --cert client.pem --key client-key.pem --ca ca.pem \ diff --git a/src/client/client_cli.c b/src/client/client_cli.c index c00a9f2..0a345a7 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -400,8 +400,8 @@ static bool validate_config(const Config* config) { return false; } if (config->use_tls) { - if (!config->tls_cert || !config->tls_key) { - fprintf(stderr, "Error: --tls requires --cert and --key\n"); + if (!config->tls_cert || !config->tls_key || !config->tls_ca) { + fprintf(stderr, "Error: --tls requires --cert, --key, and --ca\n"); return false; } } diff --git a/src/server/server.c b/src/server/server.c index ee34957..44e389e 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -19,10 +19,28 @@ #include #include #include +#include static char* authorized_root; static int authorized_root_fd = -1; static bool allow_delete; +static bool allow_unauthenticated; +static const char* required_client_cn; + +static bool tls_client_identity_allowed(SSL* ssl) { + if (!ssl || !required_client_cn) + return false; + X509* certificate = SSL_get1_peer_certificate(ssl); + if (!certificate) + return false; + char common_name[256]; + int length = X509_NAME_get_text_by_NID(X509_get_subject_name(certificate), NID_commonName, + common_name, sizeof(common_name)); + bool allowed = length >= 0 && (size_t)length < sizeof(common_name) && + strcmp(common_name, required_client_cn) == 0; + X509_free(certificate); + return allowed; +} static bool path_is_within(const char* root, const char* path) { size_t n = strlen(root); @@ -48,7 +66,7 @@ static bool __attribute__((unused)) configure_authorization(const char* root) { return false; } file_set_authorized_root(authorized_root_fd, authorized_root); - utils_set_authorized_root_fd(authorized_root_fd); + utils_set_authorized_root(authorized_root_fd, authorized_root); return true; } @@ -118,9 +136,9 @@ int receive_files(Config* config, int fd) { send_status(fd, STATUS_ERROR); return -1; } - char* full_path = path_cat(config->receive_root_directory, check_path); struct stat st; - bool has_old = full_path && lstat(full_path, &st) == 0; + char* full_path = path_cat(config->receive_root_directory, check_path); + bool has_old = full_path && file_stat_secure(full_path, &st); bool match = has_old && (unsigned long long)st.st_size == check_size && (long long)st.st_mtime == check_mtime; bool sent = send_status(fd, match ? STATUS_OK : STATUS_NEXT); @@ -179,6 +197,18 @@ void handler(int file_descriptor) { close(file_descriptor); return; } + if (!allow_unauthenticated && ssl == NULL) { + log_message(LOG_LEVEL_ERROR, "Rejected unauthenticated plaintext connection"); + config_delete(config); + close(file_descriptor); + return; + } + if (ssl && required_client_cn && !tls_client_identity_allowed(ssl)) { + log_message(LOG_LEVEL_ERROR, "Rejected TLS client with unauthorized identity"); + config_delete(config); + close(file_descriptor); + return; + } char resolved_destination[PATH_MAX]; char* canonical_destination = realpath(config->receive_root_directory, NULL); const char* destination = @@ -281,8 +311,10 @@ static void print_server_usage(void) { printf(" --cert TLS certificate file (PEM)\n"); printf(" --key TLS private key file (PEM)\n"); printf(" --ca TLS CA certificate file (PEM)\n"); + printf(" --client-cn Required TLS client certificate CN\n"); printf(" --destination-root Authorized destination root (default: .)\n"); printf(" --allow-delete Permit manifest deletion\n"); + printf(" --allow-unauthenticated Allow plaintext/anonymous network clients\n"); printf(" -v, --verbose Enable debug logging\n"); printf(" --help Show this help\n"); } @@ -313,10 +345,14 @@ int main(int argc, char* argv[]) { tls_key = argv[++i]; } else if (strcmp(argv[i], "--ca") == 0 && i + 1 < argc) { tls_ca = argv[++i]; + } else if (strcmp(argv[i], "--client-cn") == 0 && i + 1 < argc) { + required_client_cn = argv[++i]; } else if (strcmp(argv[i], "--destination-root") == 0 && i + 1 < argc) { destination_root = argv[++i]; } else if (strcmp(argv[i], "--allow-delete") == 0) { allow_delete = true; + } else if (strcmp(argv[i], "--allow-unauthenticated") == 0) { + allow_unauthenticated = true; } else if (strcmp(argv[i], "-p") == 0 && i + 1 < argc) { char* end; long p = strtol(argv[++i], &end, 10); @@ -343,10 +379,12 @@ int main(int argc, char* argv[]) { return 1; } if (stdio_mode) { + /* SSH authenticates the stdio transport outside of FastSync. */ + allow_unauthenticated = true; io_set_fds(STDIN_FILENO, STDOUT_FILENO); handler(STDIN_FILENO); file_set_authorized_root(-1, NULL); - utils_set_authorized_root_fd(-1); + utils_set_authorized_root(-1, NULL); close(authorized_root_fd); free(authorized_root); return 0; @@ -357,8 +395,8 @@ int main(int argc, char* argv[]) { return 1; } if (use_tls) { - if (!tls_cert || !tls_key) { - fprintf(stderr, "Error: --tls requires --cert and --key\n"); + if (!tls_cert || !tls_key || !tls_ca || !required_client_cn) { + fprintf(stderr, "Error: --tls requires --cert, --key, --ca, and --client-cn\n"); server_delete(&g_server); return 1; } diff --git a/src/shared/chunk.c b/src/shared/chunk.c index 4ccf0d5..bea1a6a 100644 --- a/src/shared/chunk.c +++ b/src/shared/chunk.c @@ -1,4 +1,6 @@ #include +#include +#include #include #include #include @@ -14,18 +16,29 @@ /* Maximum individual file data size within a chunk (64 MB) */ #define MAX_FILE_DATA_SIZE (64ULL * 1024 * 1024) +#define MAX_FILES_PER_CHUNK 65536U Chunk* chunk_create(File** items, int element_count) { + if (element_count < 0 || (element_count > 0 && items == NULL)) + return NULL; Chunk* chunk = (Chunk*)malloc(sizeof(Chunk)); if (chunk == NULL) { perror("ERROR: Could not allocate memory for chunk structure"); return NULL; } - chunk->items = (File**)malloc(element_count * sizeof(File*)); - if (chunk->items == NULL) { - free(chunk); - return NULL; + if (element_count == 0) { + chunk->items = NULL; + } else { + if ((size_t)element_count > SIZE_MAX / sizeof(File*)) { + free(chunk); + return NULL; + } + chunk->items = (File**)malloc((size_t)element_count * sizeof(File*)); + if (chunk->items == NULL) { + free(chunk); + return NULL; + } } for (int i = 0; i < element_count; i++) { @@ -50,15 +63,31 @@ void chunk_destroy(void* item) { } static unsigned long long per_file_serialize_size(File* file, bool use_metadata) { - return sizeof(size_t) + strlen(file->path) + - (use_metadata ? sizeof(int) + (file->metadata ? FILE_METADATA_WIRE_SIZE : 0) : 0) + - sizeof(size_t) + file->data->size; + unsigned long long size = sizeof(size_t); + size_t path_len = strlen(file->path); + unsigned long long metadata_size = + use_metadata ? sizeof(int) + (file->metadata ? FILE_METADATA_WIRE_SIZE : 0) : 0; + if ((unsigned long long)path_len > ULLONG_MAX - size) + return 0; + size += path_len; + if (metadata_size > ULLONG_MAX - size) + return 0; + size += metadata_size; + if (sizeof(size_t) > ULLONG_MAX - size) + return 0; + size += sizeof(size_t); + if ((unsigned long long)file->data->size > ULLONG_MAX - size) + return 0; + return size + file->data->size; } Data* chunk_serialize(Chunk* chunk, bool use_metadata) { unsigned long long data_size = 0; for (int i = 0; i < chunk->element_count; i++) { - data_size += per_file_serialize_size(chunk->items[i], use_metadata); + unsigned long long file_size = per_file_serialize_size(chunk->items[i], use_metadata); + if (file_size == 0 || file_size > ULLONG_MAX - data_size || data_size + file_size > SIZE_MAX) + return NULL; + data_size += file_size; } Data* data = data_create_empty(data_size); if (data == NULL) { @@ -88,10 +117,17 @@ Data* chunk_serialize(Chunk* chunk, bool use_metadata) { Chunk* chunk_deserialize(Data* data, bool use_metadata) { ArrayList* files = array_list_create(file_destroy); + if (files == NULL) + return NULL; char* data_pointer = data->data; size_t remaining_size = data->size; while (remaining_size > 0) { + if ((unsigned int)files->size >= MAX_FILES_PER_CHUNK) { + log_message(LOG_LEVEL_ERROR, "Chunk contains too many files"); + array_list_delete(files); + return NULL; + } if (remaining_size < sizeof(size_t)) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for path length"); array_list_delete(files); @@ -109,6 +145,10 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { return NULL; } + if (path_len == SIZE_MAX) { + array_list_delete(files); + return NULL; + } char* path = malloc(path_len + 1); if (path == NULL) { perror("Could not allocate memory for file path"); @@ -117,34 +157,53 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { } memcpy(path, data_pointer, path_len); path[path_len] = '\0'; + if (memchr(path, '\0', path_len) != NULL) { + free(path); + array_list_delete(files); + return NULL; + } data_pointer += path_len; remaining_size -= path_len; File* file = file_create(path); free(path); + if (file == NULL) { + array_list_delete(files); + return NULL; + } if (use_metadata) { if (remaining_size < sizeof(int)) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for metadata"); + file_destroy(file); array_list_delete(files); return NULL; } // Peek at present flag to determine total size needed before reading int present_flag; memcpy(&present_flag, data_pointer, sizeof(int)); - if (present_flag && remaining_size < sizeof(int) + FILE_METADATA_WIRE_SIZE) { + if ((present_flag != 0 && present_flag != 1) || + (present_flag == 1 && remaining_size < sizeof(int) + FILE_METADATA_WIRE_SIZE)) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for metadata body"); + file_destroy(file); array_list_delete(files); return NULL; } file->metadata = metadata_from_buf(&data_pointer); remaining_size -= sizeof(int); - if (file->metadata) + if (present_flag == 1) { + if (file->metadata == NULL) { + file_destroy(file); + array_list_delete(files); + return NULL; + } remaining_size -= FILE_METADATA_WIRE_SIZE; + } } if (remaining_size < sizeof(size_t)) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for data size"); + file_destroy(file); array_list_delete(files); return NULL; } @@ -156,6 +215,7 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { if (remaining_size < file_data_size) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for file content"); + file_destroy(file); array_list_delete(files); return NULL; } @@ -164,30 +224,48 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { if (file_data_size > MAX_FILE_DATA_SIZE) { log_message(LOG_LEVEL_ERROR, "File data size %zu exceeds maximum %llu", file_data_size, (unsigned long long)MAX_FILE_DATA_SIZE); + file_destroy(file); array_list_delete(files); return NULL; } - void* file_data = malloc(file_data_size); + size_t allocation_size = file_data_size > 0 ? file_data_size : 1; + void* file_data = malloc(allocation_size); if (file_data == NULL) { perror("Could not allocate memory for file data"); + file_destroy(file); array_list_delete(files); return NULL; } memcpy(file_data, data_pointer, file_data_size); + Data* replacement = data_create(file_data, file_data_size); + if (replacement == NULL) { + file_destroy(file); + array_list_delete(files); + return NULL; + } data_destroy(file->data); - file->data = data_create(file_data, file_data_size); + file->data = replacement; data_pointer += file_data_size; remaining_size -= file_data_size; - array_list_add(files, file); + if (!array_list_add(files, file)) { + file_destroy(file); + array_list_delete(files); + return NULL; + } } File** file_array = (File**)array_list_to_array(files); + if (files->size > 0 && file_array == NULL) { + array_list_delete(files); + return NULL; + } Chunk* chunk = chunk_create(file_array, files->size); free(file_array); - files->item_destroyer = NULL; + if (chunk != NULL) + files->item_destroyer = NULL; array_list_delete(files); return chunk; diff --git a/src/shared/config.c b/src/shared/config.c index 4c1a3a0..b99303e 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -100,6 +100,35 @@ static void config_set_defaults(Config* config) { config->compress_choice = NULL; } +static bool valid_wire_bool(int value) { + return value == 0 || value == 1; +} + +static bool validate_received_config(const Config* config) { + return valid_wire_bool(config->save_to_disk) && valid_wire_bool(config->use_multithreading) && + valid_wire_bool(config->use_chunk_serialization) && + valid_wire_bool(config->use_compression) && valid_wire_bool(config->use_metadata) && + valid_wire_bool(config->use_sendfile) && valid_wire_bool(config->use_delete) && + valid_wire_bool(config->use_incremental) && valid_wire_bool(config->use_delta) && + valid_wire_bool(config->backup) && valid_wire_bool(config->follow_symlinks) && + valid_wire_bool(config->copy_links) && valid_wire_bool(config->safe_links) && + valid_wire_bool(config->copy_unsafe_links) && + valid_wire_bool(config->preserve_hard_links) && valid_wire_bool(config->preserve_acls) && + valid_wire_bool(config->preserve_xattrs) && valid_wire_bool(config->preserve_devices) && + valid_wire_bool(config->preserve_sparse) && valid_wire_bool(config->update) && + valid_wire_bool(config->inplace) && valid_wire_bool(config->append) && + valid_wire_bool(config->append_verify) && valid_wire_bool(config->delete_excluded) && + valid_wire_bool(config->delete_after) && valid_wire_bool(config->relative) && + valid_wire_bool(config->prune_empty_dirs) && valid_wire_bool(config->partial) && + valid_wire_bool(config->delete_before) && valid_wire_bool(config->checksum) && + (!config->use_compression || + (config->compression_level >= 1 && config->compression_level <= 22)) && + config->chunk_size > 0 && config->chunk_size <= MAX_CHUNK_SIZE && + config->delta_block_size >= DELTA_BLOCK_SIZE_MIN && + config->delta_block_size <= DELTA_BLOCK_SIZE_MAX && + config->delta_max_file_size <= DELTA_MAX_FILE_SIZE && config->max_delete >= 0; +} + Config* config_create(void) { Config* config = malloc(sizeof(Config)); if (!config) @@ -320,18 +349,26 @@ Config* config_receive(int file_descriptor) { int tmp; if (!receive_int(file_descriptor, &tmp)) goto error; + if (!valid_wire_bool(tmp)) + goto error; config->save_to_disk = tmp; if (!receive_int(file_descriptor, &tmp)) goto error; + if (!valid_wire_bool(tmp)) + goto error; config->use_multithreading = tmp; if (!receive_int(file_descriptor, &tmp)) goto error; config->use_chunk_serialization = tmp; if (!receive_int(file_descriptor, &tmp)) goto error; + if (!valid_wire_bool(tmp)) + goto error; config->use_compression = tmp; if (!receive_int(file_descriptor, &tmp)) goto error; + if (!valid_wire_bool(tmp)) + goto error; config->use_metadata = tmp; if (!receive_int(file_descriptor, &tmp)) goto error; @@ -340,15 +377,23 @@ Config* config_receive(int file_descriptor) { goto error; if (!receive_int(file_descriptor, &tmp)) goto error; + if (!valid_wire_bool(tmp)) + goto error; config->use_sendfile = tmp; if (!receive_int(file_descriptor, &tmp)) goto error; + if (!valid_wire_bool(tmp)) + goto error; config->use_delete = tmp; if (!receive_int(file_descriptor, &tmp)) goto error; + if (!valid_wire_bool(tmp)) + goto error; config->use_incremental = tmp; if (!receive_int(file_descriptor, &tmp)) goto error; + if (!valid_wire_bool(tmp)) + goto error; config->use_delta = tmp; if (!receive_n_data(file_descriptor, &config->delta_block_size, sizeof(config->delta_block_size))) goto error; @@ -440,6 +485,11 @@ Config* config_receive(int file_descriptor) { send_status(file_descriptor, STATUS_ERROR); goto error; } + if (!validate_received_config(config)) { + fprintf(stderr, "Invalid configuration received from client\n"); + send_status(file_descriptor, STATUS_ERROR); + goto error; + } if (!send_status(file_descriptor, STATUS_OK)) goto error; return config; diff --git a/src/shared/delta.c b/src/shared/delta.c index 26685db..172b526 100644 --- a/src/shared/delta.c +++ b/src/shared/delta.c @@ -36,6 +36,9 @@ DeltaSignature* delta_signature_create(const void* old_file_data, uint64_t old_f if (old_file_data == NULL || old_file_size == 0 || block_size == 0) return NULL; + if (old_file_size > DELTA_MAX_FILE_SIZE || old_file_size > UINT32_MAX * (uint64_t)block_size) + return NULL; + uint32_t block_count = (uint32_t)((old_file_size + block_size - 1) / block_size); DeltaSignature* sig = malloc(sizeof(DeltaSignature)); @@ -118,6 +121,13 @@ DeltaSignature* delta_signature_deserialize(const Data* data) { return NULL; } + if (sig->block_size == 0 || sig->block_size > DELTA_BLOCK_SIZE_MAX || + sig->file_size > DELTA_MAX_FILE_SIZE || sig->file_size == 0 || + (sig->file_size + sig->block_size - 1) / sig->block_size != sig->block_count) { + free(sig); + return NULL; + } + uint64_t expected = sizeof(uint64_t) + sizeof(uint32_t) + sizeof(uint32_t) + (uint64_t)sig->block_count * (sizeof(uint32_t) + sizeof(uint32_t)); if (data->size < expected) { @@ -463,7 +473,8 @@ Delta* delta_deserialize(const Data* data) { void* delta_apply(const void* old_data, uint64_t old_size, const Delta* delta, uint32_t block_size) { if (!old_data || !delta || (delta->new_file_size > 0 && delta->instructions == NULL) || - (delta->instruction_count > 0 && block_size == 0)) + (delta->instruction_count > 0 && block_size == 0) || + delta->new_file_size > DELTA_MAX_FILE_SIZE || delta->new_file_size > SIZE_MAX) return NULL; void* output = malloc((size_t)delta->new_file_size); diff --git a/src/shared/file.c b/src/shared/file.c index b333ce1..0575655 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -23,6 +23,9 @@ #include "protocol.h" #include "utils.h" +#define MAX_SERVER_DELETE_COUNT 100000U +#define MAX_FILE_DATA_SIZE (64ULL * 1024 * 1024) + bool file_checksum(File* file, uint64_t* checksum) { if (!file || !checksum || !file->data) return false; @@ -155,7 +158,28 @@ bool file_send_single_calls(File* file, int file_descriptor, bool use_metadata, static bool to_disk_secure(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse, const FileMetadata* metadata); -static int open_secure_parent(const char* path, char** leaf_out); +static int open_secure_parent(const char* path, char** leaf_out, bool create_dirs); + +bool file_path_exists_secure(const char* path) { + struct stat st; + return file_stat_secure(path, &st); +} + +bool file_stat_secure(const char* path, struct stat* st) { + if (!path || !st) + return false; + char* leaf = NULL; + int parent_fd = open_secure_parent(path, &leaf, false); + if (parent_fd < 0) + return false; + int fd = openat(parent_fd, leaf, O_RDONLY | O_NONBLOCK | O_CLOEXEC | O_NOFOLLOW); + bool exists = fd >= 0 && fstat(fd, st) == 0 && S_ISREG(st->st_mode); + if (fd >= 0) + close(fd); + close(parent_fd); + free(leaf); + return exists; +} static bool rename_secure(const char* old_path, const char* new_path); static int authorized_root_fd = -1; static char* authorized_root_path; @@ -440,8 +464,16 @@ static File* receive_delta_file(int fd, const Config* config, const char* check_ } } + Data* replacement = data_create(new_data, (size_t)new_size); + if (replacement == NULL) { + file_destroy(file); + free(old_data); + delta_signature_destroy(sig); + send_status(fd, STATUS_ERROR); + return NULL; + } data_destroy(file->data); - file->data = data_create(new_data, (size_t)new_size); + file->data = replacement; free(old_data); delta_signature_destroy(sig); @@ -483,6 +515,12 @@ static File* receive_delta_file(int fd, const Config* config, const char* check_ send_status(fd, STATUS_ERROR); return NULL; } + if (uncompressed->size > MAX_FILE_DATA_SIZE) { + data_destroy(uncompressed); + file_destroy(file); + send_status(fd, STATUS_ERROR); + return NULL; + } file_data = uncompressed; } @@ -493,6 +531,7 @@ static File* receive_delta_file(int fd, const Config* config, const char* check_ delta_signature_destroy(sig); free(old_data); + send_status(fd, STATUS_ERROR); return NULL; } @@ -532,7 +571,7 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { int old_fd = -1; if (full_path) { char* leaf = NULL; - int parent_fd = open_secure_parent(full_path, &leaf); + int parent_fd = open_secure_parent(full_path, &leaf, false); if (parent_fd >= 0) { old_fd = openat(parent_fd, leaf, O_RDONLY | O_CLOEXEC | O_NOFOLLOW); free(leaf); @@ -542,7 +581,7 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { } unsigned long long old_size = has_old_file ? (unsigned long long)st.st_size : 0; void* old_data = NULL; - if (has_old_file && old_size > 0) { + if (has_old_file && old_size > 0 && old_size <= DELTA_MAX_FILE_SIZE && old_size <= SIZE_MAX) { old_data = malloc((size_t)old_size); if (old_data) { size_t got = 0; @@ -643,6 +682,12 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { send_status(fd, STATUS_ERROR); return NULL; } + if (uncompressed->size > MAX_FILE_DATA_SIZE) { + data_destroy(uncompressed); + file_destroy(file); + send_status(fd, STATUS_ERROR); + return NULL; + } file_data = uncompressed; } @@ -651,7 +696,7 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { return file; } -static int open_secure_parent(const char* path, char** leaf_out) { +static int open_secure_parent(const char* path, char** leaf_out, bool create_dirs) { char* copy = str_dup(path); if (!copy) return -1; @@ -691,7 +736,7 @@ static int open_secure_parent(const char* path, char** leaf_out) { while (component) { if (strcmp(component, ".") != 0 && strcmp(component, "..") != 0) { int next = openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); - if (next < 0 && errno == ENOENT && mkdirat(fd, component, 0755) == 0) + if (create_dirs && next < 0 && errno == ENOENT && mkdirat(fd, component, 0755) == 0) next = openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); if (next < 0) { close(fd); @@ -711,8 +756,8 @@ static int open_secure_parent(const char* path, char** leaf_out) { static bool rename_secure(const char* old_path, const char* new_path) { char *old_leaf = NULL, *new_leaf = NULL; - int old_parent = open_secure_parent(old_path, &old_leaf); - int new_parent = open_secure_parent(new_path, &new_leaf); + int old_parent = open_secure_parent(old_path, &old_leaf, true); + int new_parent = open_secure_parent(new_path, &new_leaf, true); bool ok = old_parent >= 0 && new_parent >= 0 && renameat(old_parent, old_leaf, new_parent, new_leaf) == 0; if (old_parent >= 0) @@ -741,7 +786,7 @@ static bool write_all(int fd, const void* data, unsigned long long size) { static bool to_disk_secure(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse, const FileMetadata* metadata) { char* leaf = NULL; - int dirfd = open_secure_parent(path, &leaf); + int dirfd = open_secure_parent(path, &leaf, true); if (dirfd < 0) return false; int fd = -1; @@ -1011,6 +1056,11 @@ File* file_receive(const Config* config, int file_descriptor) { file_destroy(file); return NULL; } + if (file_data_uncompressed->size > MAX_FILE_DATA_SIZE) { + data_destroy(file_data_uncompressed); + file_destroy(file); + return NULL; + } file_data = file_data_uncompressed; } data_destroy(file->data); @@ -1068,7 +1118,8 @@ int receive_manifest(int fd, const Config* config, int* next_status) { return *status_out == STATUS_FINISHED ? 0 : -1; } fprintf(stderr, "Deleting files not in manifest...\n"); - bool deletion_ok = delete_extras(config->receive_root_directory, manifest); + bool deletion_ok = + delete_extras_limited(config->receive_root_directory, manifest, MAX_SERVER_DELETE_COUNT); array_list_delete(manifest); return deletion_ok ? 0 : -1; } diff --git a/src/shared/file.h b/src/shared/file.h index e763f91..b103bfe 100644 --- a/src/shared/file.h +++ b/src/shared/file.h @@ -40,6 +40,8 @@ bool to_disk(const char* path, const void* data, unsigned long long data_size, b bool file_save_to_disk(const char* root_directory, const File* file, const Config* config); void file_set_authorized_root(int fd, const char* canonical_path); File* receive_incremental_check(int fd, const Config* config, bool* skipped); +bool file_path_exists_secure(const char* path); +bool file_stat_secure(const char* path, struct stat* st); int receive_manifest(int fd, const Config* config, int* next_status); #endif diff --git a/src/shared/metadata.c b/src/shared/metadata.c index 49052ca..2dbb7af 100644 --- a/src/shared/metadata.c +++ b/src/shared/metadata.c @@ -76,6 +76,11 @@ FileMetadata* metadata_from_buf(char** buf) { memcpy(&mtime_nsec, *buf, sizeof(mtime_nsec)); *buf += sizeof(mtime_nsec); m->mtime_nsec = (long)mtime_nsec; + if (present != 1 || mtime_nsec < 0 || mtime_nsec >= 1000000000LL || mode < 0 || uid < 0 || + gid < 0) { + free(m); + return NULL; + } return m; } @@ -175,7 +180,8 @@ FileMetadata* metadata_receive(int file_descriptor, int* ok) { void file_restore_metadata(const char* path, const FileMetadata* metadata) { if (metadata == NULL) return; - if (chmod(path, metadata->mode & 07777 & ~(S_ISUID | S_ISGID)) != 0) + mode_t safe_mode = metadata->mode & 0777 & ~(S_IWGRP | S_IWOTH); + if (chmod(path, safe_mode) != 0) log_message(LOG_LEVEL_WARNING, "Failed to chmod %s: %s", path, strerror(errno)); /* Never apply client-supplied ownership. The descriptor API below is the receiver write path; retain this legacy API only for compatibility. */ @@ -192,7 +198,8 @@ bool file_restore_metadata_fd(int fd, const FileMetadata* metadata) { if (fd < 0 || metadata == NULL) return metadata == NULL; bool ok = true; - if (fchmod(fd, metadata->mode & 07777 & ~(S_ISUID | S_ISGID)) != 0) + mode_t safe_mode = metadata->mode & 0777 & ~(S_IWGRP | S_IWOTH); + if (fchmod(fd, safe_mode) != 0) ok = false; /* Client uid/gid values are deliberately not authoritative. */ struct timespec times[2] = {{.tv_sec = 0, .tv_nsec = UTIME_OMIT}, diff --git a/src/shared/multiprocessing.c b/src/shared/multiprocessing.c index 5edbec2..09f38e5 100644 --- a/src/shared/multiprocessing.c +++ b/src/shared/multiprocessing.c @@ -234,9 +234,9 @@ int receive_thread(void* pipeline_context) { } char* full_path = path_cat(config->receive_root_directory, check_path); struct stat st; - bool has_old = full_path && lstat(full_path, &st) == 0; + bool has_old = full_path && file_stat_secure(full_path, &st); bool match = has_old && (unsigned long long)st.st_size == check_size && - (long long)st.st_mtime == check_mtime; + (long long)st.st_mtime == check_mtime && S_ISREG(st.st_mode); if (!send_status(file_descriptor, match ? STATUS_OK : STATUS_NEXT)) RECEIVE_THREAD_FAIL(); free(full_path); diff --git a/src/shared/protocol.c b/src/shared/protocol.c index a3a3aed..8e90001 100644 --- a/src/shared/protocol.c +++ b/src/shared/protocol.c @@ -13,7 +13,7 @@ #define RECEIVE_TIMEOUT_SEC 60 /* 60 second per-message timeout */ #define SEND_TIMEOUT_SEC 60 -#define MAX_CONNECTION_MEMORY (1024ULL * 1024 * 1024) /* 1 GB total per connection */ +#define MAX_CONNECTION_MEMORY (256ULL * 1024 * 1024) /* bounded cumulative receive budget */ static __thread int io_read_fd = -1; static __thread int io_write_fd = -1; @@ -25,7 +25,7 @@ static struct timespec bw_last_refill = {0, 0}; static mtx_t bw_mutex; static once_flag bw_mutex_once = ONCE_FLAG_INIT; -static __thread unsigned long long total_allocated_bytes = 0; +static __thread unsigned long long total_allocated_bytes; void io_set_fds(int read_fd, int write_fd) { io_read_fd = read_fd; @@ -260,6 +260,11 @@ char* receive_str(int file_descriptor) { free(data); return NULL; } + if (memchr(data, '\0', size) != NULL) { + free(data); + log_message(LOG_LEVEL_ERROR, "Received string contains an embedded NUL"); + return NULL; + } data[size] = '\0'; total_allocated_bytes += size + 1; log_message(LOG_LEVEL_DEBUG, "Received String: %s", data); @@ -287,9 +292,7 @@ Data* receive_data(int file_descriptor) { } size_t allocation_size = size == 0 ? 1 : (size_t)size; if (allocation_size > MAX_CONNECTION_MEMORY - total_allocated_bytes) { - log_message(LOG_LEVEL_ERROR, "Per-connection memory limit exceeded (%llu + %llu > %llu)", - (unsigned long long)total_allocated_bytes, size, - (unsigned long long)MAX_CONNECTION_MEMORY); + log_message(LOG_LEVEL_ERROR, "Per-connection memory limit exceeded"); return NULL; } void* data = malloc(allocation_size); diff --git a/src/shared/transport_tls.c b/src/shared/transport_tls.c index 49fc554..15d5727 100644 --- a/src/shared/transport_tls.c +++ b/src/shared/transport_tls.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -34,6 +35,10 @@ static void log_ssl_errors(void) { static SSL_CTX* create_ssl_ctx(bool is_server, const char* cert, const char* key, const char* ca_path) { + if (!is_server && !ca_path) { + log_message(LOG_LEVEL_ERROR, "TLS clients require a CA certificate path"); + return NULL; + } const SSL_METHOD* method = is_server ? TLS_server_method() : TLS_client_method(); SSL_CTX* ctx = SSL_CTX_new(method); if (!ctx) { @@ -42,9 +47,23 @@ static SSL_CTX* create_ssl_ctx(bool is_server, const char* cert, const char* key return NULL; } - SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION); + if (SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION) != 1) { + SSL_CTX_free(ctx); + return NULL; + } + if (SSL_CTX_set_cipher_list(ctx, "HIGH:!aNULL:!eNULL:!MD5:!RC4:!3DES") != 1) { + SSL_CTX_free(ctx); + return NULL; + } if (cert && key) { + struct stat key_stat; + if (stat(key, &key_stat) != 0 || !S_ISREG(key_stat.st_mode) || key_stat.st_uid != geteuid() || + (key_stat.st_mode & (S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH))) { + log_message(LOG_LEVEL_ERROR, "TLS private key must be owned by the current user and private"); + SSL_CTX_free(ctx); + return NULL; + } if (SSL_CTX_use_certificate_file(ctx, cert, SSL_FILETYPE_PEM) <= 0) { log_message(LOG_LEVEL_ERROR, "Failed to load certificate: %s", cert); log_ssl_errors(); @@ -91,7 +110,10 @@ static SSL* wrap_fd_with_ssl(int fd, SSL_CTX* ctx, bool is_server, const char* h // Enable hostname verification for client connections when a hostname is provided. // Must be done before SSL_connect to take effect during the handshake. if (!is_server && hostname) { - SSL_set1_host(ssl, hostname); + if (SSL_set1_host(ssl, hostname) != 1) { + SSL_free(ssl); + return NULL; + } } // Retry SSL_accept/SSL_connect on WANT_READ/WANT_WRITE (non-blocking handshake) diff --git a/src/shared/utils.c b/src/shared/utils.c index 9e11988..89c4761 100644 --- a/src/shared/utils.c +++ b/src/shared/utils.c @@ -7,13 +7,67 @@ #include #include #include +#include #include #include static int authorized_root_fd = -1; +static char* authorized_root_path; + +void utils_set_authorized_root(int fd, const char* canonical_path) { + authorized_root_fd = fd; + free(authorized_root_path); + authorized_root_path = canonical_path ? str_dup(canonical_path) : NULL; +} void utils_set_authorized_root_fd(int fd) { - authorized_root_fd = fd; + utils_set_authorized_root(fd, NULL); +} + +static bool path_is_within_root(const char* root, const char* path) { + size_t root_len = strlen(root); + return strncmp(root, path, root_len) == 0 && (path[root_len] == '\0' || path[root_len] == '/'); +} + +static int open_authorized_destination(const char* dest_root) { + if (authorized_root_fd < 0 || !authorized_root_path || !dest_root || + !path_is_within_root(authorized_root_path, dest_root)) + return -1; + + int dirfd = dup(authorized_root_fd); + if (dirfd < 0) + return -1; + + const char* relative_path = dest_root + strlen(authorized_root_path); + while (*relative_path == '/') + relative_path++; + char* relative = str_dup(*relative_path ? relative_path : "."); + if (!relative) { + close(dirfd); + return -1; + } + + char* saveptr = NULL; + char* component = strtok_r(relative, "/", &saveptr); + while (component) { + if (strcmp(component, ".") == 0 || strcmp(component, "..") == 0) { + free(relative); + close(dirfd); + return -1; + } + int next = openat(dirfd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + if (next < 0) { + free(relative); + close(dirfd); + return -1; + } + close(dirfd); + dirfd = next; + component = strtok_r(NULL, "/", &saveptr); + } + + free(relative); + return dirfd; } bool mkdir_r(const char* path) { @@ -143,7 +197,8 @@ static bool is_dir_in_manifest(const char* rel_path, ArrayList* manifest) { return false; } -static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifest) { +static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifest, + size_t max_delete, size_t* deleted_count) { int scanfd = dup(dirfd); if (scanfd < 0) return false; @@ -173,12 +228,17 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes int childfd = openat(dirfd, entry->d_name, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); bool child_removed = false; if (childfd >= 0) { - child_removed = delete_extras_fd(childfd, child_rel, manifest); + child_removed = delete_extras_fd(childfd, child_rel, manifest, max_delete, deleted_count); close(childfd); } - if (child_removed && !is_dir_in_manifest(child_rel, manifest) && - unlinkat(dirfd, entry->d_name, AT_REMOVEDIR) != 0 && errno != ENOENT) { - operation_ok = false; + if (child_removed && !is_dir_in_manifest(child_rel, manifest)) { + if (*deleted_count >= max_delete) { + operation_ok = false; + } else if (unlinkat(dirfd, entry->d_name, AT_REMOVEDIR) != 0 && errno != ENOENT) { + operation_ok = false; + } else { + (*deleted_count)++; + } } else if (!child_removed) { all_removed = false; } @@ -192,8 +252,15 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes } } if (!found) { + if (*deleted_count >= max_delete) { + operation_ok = false; + free(child_rel); + continue; + } if (unlinkat(dirfd, entry->d_name, 0) != 0 && errno != ENOENT) operation_ok = false; + else + (*deleted_count)++; fprintf(stderr, " Deleted: %s\n", child_rel); } else { all_removed = false; @@ -206,18 +273,27 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes return operation_ok; } -bool delete_extras(const char* dest_root, ArrayList* manifest) { - int rootfd = authorized_root_fd >= 0 - ? dup(authorized_root_fd) - : open(dest_root, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); +bool delete_extras_limited(const char* dest_root, ArrayList* manifest, size_t max_delete) { + int rootfd; + if (authorized_root_fd >= 0) { + rootfd = + authorized_root_path ? open_authorized_destination(dest_root) : dup(authorized_root_fd); + } else { + rootfd = open(dest_root, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + } if (rootfd < 0) return false; - bool ok = delete_extras_fd(rootfd, "", manifest); + size_t deleted_count = 0; + bool ok = delete_extras_fd(rootfd, "", manifest, max_delete, &deleted_count); if (close(rootfd) != 0) ok = false; return ok; } +bool delete_extras(const char* dest_root, ArrayList* manifest) { + return delete_extras_limited(dest_root, manifest, SIZE_MAX); +} + bool has_path_traversal(const char* path) { if (!path) return false; diff --git a/src/shared/utils.h b/src/shared/utils.h index d04daf5..6d3e8d2 100644 --- a/src/shared/utils.h +++ b/src/shared/utils.h @@ -2,6 +2,7 @@ #define UTILS_H #include "array_list.h" +#include #include bool mkdir_r(const char* path); @@ -9,6 +10,8 @@ char* str_dup(const char* string); char* path_cat(const char* path1, const char* path2); bool glob_match(const char* pattern, const char* str); bool delete_extras(const char* dest_root, ArrayList* manifest); +bool delete_extras_limited(const char* dest_root, ArrayList* manifest, size_t max_delete); +void utils_set_authorized_root(int fd, const char* canonical_path); void utils_set_authorized_root_fd(int fd); bool has_path_traversal(const char* path); diff --git a/tests/integration/common.py b/tests/integration/common.py index 08cc9c9..2e9653a 100644 --- a/tests/integration/common.py +++ b/tests/integration/common.py @@ -25,7 +25,9 @@ class ServerManager: def start(self, extra_args=None): self.stop() self._port = _find_free_port() - cmd = SERVER_CMD + ["-p", str(self._port)] + # Plain TCP is intentionally explicit in the server; integration tests + # exercise that opt-in mode rather than relying on the secure default. + cmd = SERVER_CMD + ["-p", str(self._port), "--allow-unauthenticated"] if extra_args: cmd += extra_args self._proc = subprocess.Popen(cmd, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) diff --git a/tests/integration/test_tls.py b/tests/integration/test_tls.py index 06e8caa..29074b2 100644 --- a/tests/integration/test_tls.py +++ b/tests/integration/test_tls.py @@ -102,6 +102,7 @@ class TestTLSBasic: with ServerManager() as server: server.start(extra_args=[ "--tls", "--cert", certs["server_cert"], "--key", certs["server_key"], + "--ca", certs["ca"], "--client-cn", "fastsync-client", ]) result, dur = run_client( SOURCE_DIR, DEST_DIR, @@ -125,6 +126,7 @@ class TestTLSBasic: with ServerManager() as server: server.start(extra_args=[ "--tls", "--cert", certs["server_cert"], "--key", certs["server_key"], + "--ca", certs["ca"], "--client-cn", "fastsync-client", ]) result, dur = run_client( SOURCE_DIR, DEST_DIR, @@ -149,6 +151,7 @@ class TestTLSBasic: with ServerManager() as server: server.start(extra_args=[ "--tls", "--cert", certs["server_cert"], "--key", certs["server_key"], + "--ca", certs["ca"], "--client-cn", "fastsync-client", ]) result, dur = run_client( SOURCE_DIR, DEST_DIR,