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..6ab4e4f 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -23,6 +23,7 @@ static char* authorized_root; static int authorized_root_fd = -1; static bool allow_delete; +static bool allow_unauthenticated; static bool path_is_within(const char* root, const char* path) { size_t n = strlen(root); @@ -34,6 +35,15 @@ static bool valid_batch_path(const char* path) { strchr(path, '\0') == path + strlen(path); } +static bool batch_path_exists_secure(const char* path, const char* root) { + char* full_path = path_cat(root, path); + if (!full_path) + return false; + bool exists = file_path_exists_secure(full_path); + free(full_path); + return exists; +} + static bool __attribute__((unused)) configure_authorization(const char* root) { char resolved[PATH_MAX]; if (!root || !realpath(root, resolved)) @@ -121,7 +131,8 @@ int receive_files(Config* config, int fd) { 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 match = has_old && (unsigned long long)st.st_size == check_size && + bool secure_exists = batch_path_exists_secure(check_path, config->receive_root_directory); + bool match = has_old && secure_exists && (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); free(full_path); @@ -179,6 +190,12 @@ 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; + } char resolved_destination[PATH_MAX]; char* canonical_destination = realpath(config->receive_root_directory, NULL); const char* destination = @@ -283,6 +300,7 @@ static void print_server_usage(void) { printf(" --ca TLS CA certificate file (PEM)\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"); } @@ -317,6 +335,8 @@ int main(int argc, char* argv[]) { 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); @@ -357,8 +377,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) { + fprintf(stderr, "Error: --tls requires --cert, --key, and --ca\n"); server_delete(&g_server); return 1; } diff --git a/src/shared/chunk.c b/src/shared/chunk.c index 4ccf0d5..480342e 100644 --- a/src/shared/chunk.c +++ b/src/shared/chunk.c @@ -14,6 +14,7 @@ /* 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) { Chunk* chunk = (Chunk*)malloc(sizeof(Chunk)); @@ -88,10 +89,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 +117,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,6 +129,11 @@ 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; @@ -180,7 +197,11 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { 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); diff --git a/src/shared/config.c b/src/shared/config.c index 4c1a3a0..529512b 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -100,6 +100,33 @@ 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) && + 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 +347,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 +375,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 +483,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..343c0be 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)); @@ -463,7 +466,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..3df95bb 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -155,7 +155,24 @@ 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) { + if (!path) + return false; + char* leaf = NULL; + int parent_fd = open_secure_parent(path, &leaf, false); + if (parent_fd < 0) + return false; + struct stat st; + int fd = openat(parent_fd, leaf, O_RDONLY | O_CLOEXEC | O_NOFOLLOW); + bool exists = fd >= 0 && fstat(fd, &st) == 0; + 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; @@ -532,7 +549,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 +559,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; @@ -651,7 +668,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 +708,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 +728,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 +758,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; diff --git a/src/shared/file.h b/src/shared/file.h index e763f91..33fbd52 100644 --- a/src/shared/file.h +++ b/src/shared/file.h @@ -40,6 +40,7 @@ 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); int receive_manifest(int fd, const Config* config, int* next_status); #endif diff --git a/src/shared/protocol.c b/src/shared/protocol.c index a3a3aed..56fb852 100644 --- a/src/shared/protocol.c +++ b/src/shared/protocol.c @@ -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); diff --git a/src/shared/transport_tls.c b/src/shared/transport_tls.c index 49fc554..2b373ef 100644 --- a/src/shared/transport_tls.c +++ b/src/shared/transport_tls.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -43,8 +44,19 @@ static SSL_CTX* create_ssl_ctx(bool is_server, const char* cert, const char* key } SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION); + 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(); 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..d74ef15 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"], ]) 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"], ]) 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"], ]) result, dur = run_client( SOURCE_DIR, DEST_DIR,