TapTap/FastSync
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 2 Wiki Activity

security(shared): fix -K TOCTOU, ssh old-args quoting, TLS opts, secret-file perms, sparse dedup

Browse Source 
- file: open -K dirlink referents via a race-safe relative O_NOFOLLOW walk
  from the authorized-root fd instead of re-opening an absolute realpath()
  result (removes the intermediate-symlink swap TOCTOU).
- transport_ssh: always single-quote the server path, including --old-args,
  so no mode can inject shell metacharacters.
- transport_tls: set SSL_OP_NO_COMPRESSION and (guarded) SSL_OP_NO_RENEGOTIATION.
- credentials: reject --password-file/--early-input with any group/other
  permission bit; chmod 0600 the affected test fixtures.
- file_store: export file_store_write_sparse() and remove the verbatim
  file.c duplicate.
This commit is contained in:
TapTap
2026-09-12 15:01:48 +02:00
parent b8db810ee5
commit abad1664ba
11 changed files with 314 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/shared/credentials.c
+25
View File
@@ -8,6 +8,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
/* One store entry: a username and its password's SHA-256 hex digest. The
* plaintext password never appears here (and never on the daemon host). */
@@ -35,6 +36,23 @@ static bool is_comment_char(char c) {
return c == '#' || c == ';';
}
/* A --password-file / --early-input carries plaintext or credential material
* and must not be accessible to group or other, mirroring the TLS private-key
* check in transport_tls.c. Reject any group/other permission bit (including
* execute) with a clear error. A stat failure is left for the caller's fopen
* to report, so a missing file keeps its existing "cannot open" message. */
static bool secret_file_is_private(const char* path, char* err, size_t err_size) {
struct stat st;
if (stat(path, &st) != 0)
return true;
if (!S_ISREG(st.st_mode) || (st.st_mode & (S_IRWXG | S_IRWXO)) != 0) {
set_error(err, err_size,
"refusing to read secret file '%s': permissions must be owner-only (0600)", path);
return false;
}
return true;
}
/* Trim leading/trailing ASCII space and tab in place; returns the new start. */
static char* trim_space(char* s) {
while (*s == ' ' || *s == '\t')
@@ -124,6 +142,11 @@ static CredentialStore* load_store_file(const char* path, char* err, size_t err_
if (!path)
return store;
if (!secret_file_is_private(path, err, err_size)) {
credentials_free(store);
return NULL;
}
FILE* fp = fopen(path, "r");
if (!fp) {
set_error(err, err_size, "cannot open credential file '%s': %s", path, strerror(errno));
@@ -305,6 +328,8 @@ int credentials_read_secret_file(const char* path, char** user_out, char** passw
set_error(err, err_size, "no --password-file path");
return -1;
}
if (!secret_file_is_private(path, err, err_size))
return -1;
FILE* fp = fopen(path, "r");
if (!fp) {
set_error(err, err_size, "cannot open password file '%s': %s", path, strerror(errno));
src/shared/file.c
+56 -51
View File
@@ -16,6 +16,7 @@
#include "data.h"
#include "delta.h"
#include "file.h"
#include "file_store.h"
#include "identity.h"
#include "log.h"
#include "metadata.h"
@@ -37,44 +38,6 @@ static bool write_all(int fd, const void* data, unsigned long long size) {
return true;
}
/* A run of NUL bytes at least this long is emitted as a hole (lseek) rather
* than written, so the resulting file is genuinely sparse on the filesystem. */
#define SPARSE_HOLE_MIN 4096U
/* Sparse-aware writer (--sparse/-S). Walks `data`; any all-zero run of at
* least SPARSE_HOLE_MIN bytes is skipped with lseek(SEEK_CUR) so the block is
* never allocated (a real hole on the destination); every other byte is written
* normally. The file is pre-sized with ftruncate by the callers before this
* runs, so holes are guaranteed and the offset bookkeeping stays correct
* (each lseek advances the fd offset exactly as a write of that many bytes
* would). After the final run, ftruncate(size) guarantees the logical size is
* exactly `size` even when the tail was a hole. The full file image is in
* memory, so no wire change is needed. Returns false on I/O error. */
static bool write_all_sparse(int fd, const unsigned char* data, unsigned long long size) {
unsigned long long i = 0;
while (i < size) {
if (data[i] == 0) {
unsigned long long run_start = i;
while (i < size && data[i] == 0)
i++;
unsigned long long run_len = i - run_start;
if (run_len >= SPARSE_HOLE_MIN) {
if (lseek(fd, (off_t)run_len, SEEK_CUR) < 0)
return false;
} else if (!write_all(fd, data + run_start, run_len)) {
return false;
}
} else {
unsigned long long run_start = i;
while (i < size && data[i] != 0)
i++;
if (!write_all(fd, data + run_start, i - run_start))
return false;
}
}
return ftruncate(fd, (off_t)size) == 0;
}
/* Preallocate `size` bytes on `fd` before any data is written (--preallocate).
* posix_fallocate reserves real disk blocks, so an out-of-space condition
* (ENOSPC/EDQUOT) surfaces up front instead of partway through a transfer;
@@ -515,6 +478,50 @@ out:
return ok;
}
/* Open the directory named by canonical absolute `resolved`, which the caller
* has already verified lies beneath `root` (the canonical authorized root).
* Each component is opened relative to the authorized-root fd with O_NOFOLLOW,
* so a directory swapped for a symlink after the realpath() check cannot
* redirect the open outside the root -- the walk simply fails. This replaces
* re-opening the absolute resolved path (TOCTOU). Returns an O_DIRECTORY fd,
* or -1 (the root itself and any error are refused). */
static int open_dir_beneath_root(const char* resolved, const char* root) {
size_t root_len = strlen(root);
const char* rel = resolved + root_len;
while (*rel == '/')
rel++;
if (*rel == '\0')
return -1;
int fd = dup(authorized_root_fd);
if (fd < 0)
return -1;
char* copy = str_dup(rel);
if (!copy) {
close(fd);
return -1;
}
char* save = NULL;
for (char* component = strtok_r(copy, "/", &save); component;
component = strtok_r(NULL, "/", &save)) {
if (strcmp(component, ".") == 0)
continue;
/* A canonical realpath() output never contains "." or ".."; refuse ".."
defensively rather than let it climb toward the root. */
int next = strcmp(component, "..") == 0
? -1
: openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
if (next < 0) {
close(fd);
free(copy);
return -1;
}
close(fd);
fd = next;
}
free(copy);
return fd;
}
int file_open_secure_parent(const char* path, char** leaf_out, bool create_dirs) {
char* copy = str_dup(path);
if (!copy)
@@ -619,16 +626,13 @@ int file_open_secure_parent(const char* path, char** leaf_out, bool create_dirs)
(resolved[strlen(root)] == '/' || resolved[strlen(root)] == '\0')) {
struct stat rst;
if (stat(resolved, &rst) == 0 && S_ISDIR(rst.st_mode)) {
/* Re-open the resolved directory WITHOUT following a symlink and
re-verify it is still a directory inode, so a symlink swapped
in between realpath() and open() (TOCTOU) cannot redirect this
fd outside the root. */
next = open(resolved, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
struct stat ofst;
if (next >= 0 && (fstat(next, &ofst) != 0 || !S_ISDIR(ofst.st_mode))) {
close(next);
next = -1;
}
/* Open the resolved directory through a relative no-follow walk
from the authorized-root fd instead of re-opening the
absolute `resolved` path: swapping an intermediate directory
for a symlink between realpath() and open() (TOCTOU) then
merely fails the walk rather than redirecting the fd outside
the root. */
next = open_dir_beneath_root(resolved, root);
}
}
}
@@ -938,7 +942,7 @@ static bool file_to_disk_secure_impl(const char* path, const void* data,
ok = ftruncate(fd, (off_t)data_size) == 0;
if (ok || !sparse || data_size == 0)
ok = sparse && data_size > 0
? write_all_sparse(fd, (const unsigned char*)data, data_size)
? file_store_write_sparse(fd, (const unsigned char*)data, data_size)
: write_all(fd, data, data_size);
if (ok)
ok = ftruncate(fd, (off_t)data_size) == 0;
@@ -1046,8 +1050,9 @@ static bool file_to_disk_secure_impl(const char* path, const void* data,
failure may leave partial data that --partial retention can rename. */
if (ok || (!sparse || data_size == 0)) {
write_attempted = true;
ok = sparse && data_size > 0 ? write_all_sparse(fd, (const unsigned char*)data, data_size)
: write_all(fd, data, data_size);
ok = sparse && data_size > 0
? file_store_write_sparse(fd, (const unsigned char*)data, data_size)
: write_all(fd, data, data_size);
}
if (ok && metadata)
ok = file_restore_metadata_fd(fd, metadata, preserve_executability);
src/shared/file_store.c
+5 -4
View File
@@ -152,7 +152,7 @@ static bool write_all(int fd, const void* data, unsigned long long size) {
* would). After the final run, ftruncate(size) guarantees the logical size is
* exactly `size` even when the tail was a hole. The full file image is in
* memory, so no wire change is needed. Returns false on I/O error. */
static bool write_all_sparse(int fd, const unsigned char* data, unsigned long long size) {
bool file_store_write_sparse(int fd, const unsigned char* data, unsigned long long size) {
unsigned long long i = 0;
while (i < size) {
if (data[i] == 0) {
@@ -191,7 +191,7 @@ bool file_store_write_secure(const char* path, const void* data, unsigned long l
if (fd >= 0) {
if (sparse && data_size > 0) {
if (ftruncate(fd, (off_t)data_size) == 0)
ok = write_all_sparse(fd, data, data_size);
ok = file_store_write_sparse(fd, data, data_size);
} else {
ok = write_all(fd, data, data_size);
}
@@ -219,8 +219,9 @@ bool file_store_write_secure(const char* path, const void* data, unsigned long l
if (sparse && data_size > 0)
ok = ftruncate(fd, (off_t)data_size) == 0;
if (ok || (!sparse || data_size == 0))
ok = (sparse && data_size > 0) ? write_all_sparse(fd, (const unsigned char*)data, data_size)
: write_all(fd, data, data_size);
ok = (sparse && data_size > 0)
? file_store_write_sparse(fd, (const unsigned char*)data, data_size)
: write_all(fd, data, data_size);
if (ok && metadata)
ok = file_restore_metadata_fd(fd, metadata, preserve_executability);
if (close(fd) != 0)
src/shared/file_store.h
+7
View File
@@ -10,5 +10,12 @@ bool file_store_rename_secure(const char* old_path, const char* new_path);
bool file_store_write_secure(const char* path, const void* data, unsigned long long data_size,
bool inplace, bool sparse, const FileMetadata* metadata,
bool preserve_executability);
/* Sparse-aware write (--sparse/-S): every all-zero run of at least
* SPARSE_HOLE_MIN bytes is skipped with lseek(SEEK_CUR) so it becomes a real
* hole; every other byte is written. The caller pre-sizes the file with
* ftruncate; this function also ftruncate()s to `size` at the end so a trailing
* hole keeps the exact logical length. Shared by the file_store and file write
* paths. Returns false on write/lseek/ftruncate error. */
bool file_store_write_sparse(int fd, const unsigned char* data, unsigned long long size);
#endif
src/shared/transport_ssh.c
+30 -40
View File
@@ -84,32 +84,29 @@ char* ssh_build_remote_command(const char* server_path, bool old_args, char* con
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. */
escaped with the SAME single-quote boundary used for the server path, 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;
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;
command_len = path_len + quote_count * 4 + suffix_len + 4;
}
/* The base command: the server path is ALWAYS quoted as one single-quoted
shell word (remote options below reuse the same escaping), then
" --stdio". Quoting the path is the only injection-safe construction: an
unquoted path would carry shell metacharacters straight into the remote
shell command. --old-args is kept for CLI/ABI compatibility but no longer
disables that protection. */
(void)old_args;
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;
/* Add each remote option, escaped as one single-quoted word:
" '<body>'", i.e. 1 leading space + 1 open quote + body (len + 3 per
@@ -141,25 +138,18 @@ char* ssh_build_remote_command(const char* server_path, bool old_args, char* con
if (!command)
return NULL;
char* out = command;
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++ = '\'';
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);
out += suffix_len;
for (int i = 0; i < remote_option_count; i++) {
const char* opt = remote_options[i];
*out++ = ' ';
src/shared/transport_tls.c
+9
View File
@@ -48,6 +48,15 @@ static SSL_CTX* create_ssl_ctx(bool is_server, const char* cert, const char* key
return NULL;
}
/* Harden the context: never negotiate TLS compression (the CRIME attack
* vector) and never honour a post-handshake renegotiation request.
* SSL_OP_NO_RENEGOTIATION is only available from OpenSSL 1.1.1, so it is
* guarded to keep older headers building. */
SSL_CTX_set_options(ctx, SSL_OP_NO_COMPRESSION);
#ifdef SSL_OP_NO_RENEGOTIATION
SSL_CTX_set_options(ctx, SSL_OP_NO_RENEGOTIATION);
#endif
if (SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION) != 1) {
SSL_CTX_free(ctx);
return NULL;