fix(p8-security): close review gaps in the ownership gate and copy-as failure propagation
- H3: a daemon module without 'client owner = yes' now also has super-user device activity forced off (char/block mknod, --write-devices), so a root daemon can no longer be made to create/write raw devices under AUTO. The entries are skipped, preserving ordinary -a pushes. - H1/H2: propagate a failed required --copy-as chown from symlink metadata restore and implicitly-created parent directories, so the entry (and run) reports failure instead of a wrong-owner success. - Docs/help/headers updated for A2/A3 and the device clamp; startup warning spells out the client-owner risk. - Tests: daemon device clamp (skipped without opt-in, created with opt-in), updated --super/--fake-super expectations.
This commit is contained in:
+5
-5
@@ -169,11 +169,11 @@ void print_usage(void) {
|
||||
printf(" re-apply it (fd-relative) on a privileged run; the\n");
|
||||
printf(" recording format diverges from rsync's user.rsync.%%stat%%\n");
|
||||
printf(" --super Permit the receiver to attempt super-user activities\n");
|
||||
printf(" (ownership application, char/block device-node\n");
|
||||
printf(" creation) within the confined receive root. Never\n");
|
||||
printf(" elevates privileges and never bypasses confinement;\n");
|
||||
printf(" with no explicit identity policy, ownership follows\n");
|
||||
printf(" raw numeric ids (as if --numeric-ids)\n");
|
||||
printf(" (char/block device-node creation, --write-devices)\n");
|
||||
printf(" within the confined receive root. Never elevates\n");
|
||||
printf(" privileges and never bypasses confinement; ownership\n");
|
||||
printf(" is still applied only with an explicit identity flag\n");
|
||||
printf(" (--numeric-ids/--chown/--usermap/--groupmap)\n");
|
||||
printf(" --no-super Forbid those super-user activities even when the\n");
|
||||
printf(" receiver is running as root\n");
|
||||
printf(" --chmod <changes> Modify transferred permissions (rsync syntax)\n");
|
||||
|
||||
+23
-9
@@ -246,12 +246,25 @@ static const char* server_module_gate(const Config* config, void* context) {
|
||||
client could force arbitrary ownership inside the module root. The
|
||||
standalone/SSH server has a single operator-authorized root and keeps
|
||||
honoring these. */
|
||||
if (!module->client_owner && identity_ownership_requested(effective)) {
|
||||
log_message(LOG_LEVEL_ERROR,
|
||||
"daemon module '%s' refuses client-chosen ownership/super-user activities "
|
||||
"(no `client owner = yes` opt-in); refusing",
|
||||
config->module);
|
||||
return "client-chosen ownership is not permitted by this daemon module";
|
||||
if (!module->client_owner) {
|
||||
/* Ownership: refuse the whole transfer up front (a clear failure). Uses the
|
||||
original config so an explicit --super is caught even though super_mode is
|
||||
clamped to OFF below. */
|
||||
if (identity_ownership_requested(config)) {
|
||||
log_message(LOG_LEVEL_ERROR,
|
||||
"daemon module '%s' refuses client-chosen ownership/super-user activities "
|
||||
"(no `client owner = yes` opt-in); refusing",
|
||||
config->module);
|
||||
return "client-chosen ownership is not permitted by this daemon module";
|
||||
}
|
||||
/* Super-user DEVICE activities (char/block mknod and --write-devices) are
|
||||
permitted under the default AUTO mode, so without this clamp a root daemon
|
||||
would still let a non-opted module create arbitrary device nodes and write
|
||||
raw devices. Force them off for this connection: those entries are
|
||||
skipped (never mknod'ed) while an ordinary `-a` push still succeeds
|
||||
without device nodes, matching the operator's least-privilege choice.
|
||||
The operator-level --no-super veto is already folded into this. */
|
||||
effective->super_mode = SUPER_MODE_OFF;
|
||||
}
|
||||
if (module->auth_user_count > 0) {
|
||||
/* Auth-required module (Wave B): verify the presented credentials against
|
||||
@@ -769,9 +782,10 @@ int main(int argc, char* argv[]) {
|
||||
for (int i = 0; i < g_daemon_conf->module_count; i++) {
|
||||
if (g_daemon_conf->modules[i].client_owner)
|
||||
log_message(LOG_LEVEL_WARNING,
|
||||
"daemon module '%s' allows client-chosen ownership "
|
||||
"(`client owner = yes`); clients may request arbitrary owner ids within "
|
||||
"that module root",
|
||||
"daemon module '%s' allows client-chosen ownership and super-user device "
|
||||
"activities (`client owner = yes`); clients may request arbitrary owner ids "
|
||||
"and device nodes within that module root -- pair it with `auth users` "
|
||||
"unless the module is intentionally open to the network",
|
||||
g_daemon_conf->modules[i].name);
|
||||
}
|
||||
/* Daemon credential store (Wave B). --password-file and --early-input
|
||||
|
||||
+10
-2
@@ -582,8 +582,16 @@ int file_open_secure_parent(const char* path, char** leaf_out, bool create_dirs)
|
||||
(a pre-existing destination directory is left alone, matching
|
||||
rsync's transferred-entry scope); the helper is a no-op unless an
|
||||
identity policy is active. */
|
||||
if (created && identity_copy_as_active())
|
||||
identity_apply_ownership_link(fd, component, 0, 0);
|
||||
if (created && identity_copy_as_active() &&
|
||||
!identity_apply_ownership_link(fd, component, 0, 0)) {
|
||||
/* A REQUIRED --copy-as ownership that cannot be applied to a
|
||||
directory this walk just created must fail the entry rather than
|
||||
leave that implicit parent owned by the receiver. */
|
||||
close(fd);
|
||||
free(copy);
|
||||
free(leaf);
|
||||
return -1;
|
||||
}
|
||||
next = openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -689,7 +689,7 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi
|
||||
suppresses the timestamps; ownership stays gated by the identity policy.
|
||||
A symlink has no children, so this can be applied immediately. */
|
||||
if (ok && config && config->use_metadata)
|
||||
file_restore_symlink_metadata(link_path, file->metadata, config->omit_link_times);
|
||||
ok = file_restore_symlink_metadata(link_path, file->metadata, config->omit_link_times);
|
||||
free(link_path);
|
||||
return ok ? FILE_SAVE_WRITTEN : FILE_SAVE_ERROR;
|
||||
}
|
||||
|
||||
@@ -679,8 +679,8 @@ static void identity_log_chown_failure(const char* what, uid_t uid, gid_t gid) {
|
||||
* restricted root, root-squash, or a read-only mount) the run would be
|
||||
* silently producing the WRONG ownership, so surface it at ERROR. The
|
||||
* caller (identity_apply_ownership*) then reports the ENTRY as failed rather
|
||||
* than as written; the receiver never claims a --copy-as success it did not
|
||||
* achieve, but a single entry failure does not abort the whole run. */
|
||||
* than as written, which becomes a FILE_SAVE_ERROR and fails the transfer
|
||||
* (fail-fast) instead of reporting overall success with the wrong owner. */
|
||||
if (errno == EPERM || errno == EACCES) {
|
||||
if (identity_copy_as_active())
|
||||
log_message(LOG_LEVEL_ERROR,
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
#include <sys/types.h>
|
||||
|
||||
/*
|
||||
* Identity mapping: --numeric-ids / --usermap / --groupmap / --chown.
|
||||
* Identity mapping: --numeric-ids / --usermap / --groupmap / --chown / --copy-as.
|
||||
*
|
||||
* FastSync transmits uid/gid numerically (int32 on the wire) and, by design,
|
||||
* NEVER applies client-supplied ownership unless a user explicitly opts in with
|
||||
@@ -87,9 +87,10 @@ bool identity_ownership_requested(const Config* config);
|
||||
|
||||
/* Apply the negotiated ownership to an already-written file descriptor.
|
||||
* source_uid/source_gid are the transmitted numeric ids. Resolution order:
|
||||
* a matching usermap/groupmap rule, then --chown, then --numeric-ids (raw),
|
||||
* then a best-effort name lookup on the receiver's own databases (skipped when
|
||||
* the transmitted id has no name on this system). Only calls fchown() when the
|
||||
* --copy-as (highest priority, forces both ids), then a matching
|
||||
* usermap/groupmap rule, then --chown, then --numeric-ids (raw), then a
|
||||
* best-effort name lookup on the receiver's own databases (skipped when the
|
||||
* transmitted id has no name on this system). Only calls fchown() when the
|
||||
* result differs from the current value.
|
||||
*
|
||||
* Returns false ONLY when an active --copy-as ownership application failed: its
|
||||
|
||||
@@ -357,17 +357,20 @@ void file_restore_metadata(const char* path, const FileMetadata* metadata,
|
||||
}
|
||||
}
|
||||
|
||||
void file_restore_symlink_metadata(const char* path, const FileMetadata* metadata,
|
||||
bool file_restore_symlink_metadata(const char* path, const FileMetadata* metadata,
|
||||
bool omit_link_times) {
|
||||
if (path == NULL || metadata == NULL)
|
||||
return;
|
||||
return true;
|
||||
char* leaf = NULL;
|
||||
int parent_fd = file_open_secure_parent(path, &leaf, false);
|
||||
if (parent_fd < 0)
|
||||
return;
|
||||
return !identity_copy_as_active();
|
||||
/* Ownership (only when the identity policy is active) via lchown semantics:
|
||||
fchownat with AT_SYMLINK_NOFOLLOW never dereferences the link. */
|
||||
identity_apply_ownership_link(parent_fd, leaf, (int32_t)metadata->uid, (int32_t)metadata->gid);
|
||||
fchownat with AT_SYMLINK_NOFOLLOW never dereferences the link. A failed
|
||||
REQUIRED --copy-as ownership marks the entry failed; every other policy is
|
||||
best-effort. */
|
||||
bool owned = identity_apply_ownership_link(parent_fd, leaf, (int32_t)metadata->uid,
|
||||
(int32_t)metadata->gid);
|
||||
/* Symlink mode: not settable on Linux (fchmodat AT_SYMLINK_NOFOLLOW returns
|
||||
EOPNOTSUPP/ENOTSUP); attempt it for platforms that support it and quietly
|
||||
ignore the unsupported case so the transfer never fails over it. */
|
||||
@@ -392,6 +395,7 @@ void file_restore_symlink_metadata(const char* path, const FileMetadata* metadat
|
||||
}
|
||||
close(parent_fd);
|
||||
free(leaf);
|
||||
return owned;
|
||||
}
|
||||
|
||||
bool file_restore_metadata_fd(int fd, const FileMetadata* metadata, bool preserve_executability) {
|
||||
|
||||
@@ -44,8 +44,10 @@ bool file_restore_metadata_fd(int fd, const FileMetadata* metadata, bool preserv
|
||||
* under the authorized root. `omit_link_times` (-J/--omit-link-times)
|
||||
* suppresses the timestamps; the link's mode/ownership are still attempted
|
||||
* (ownership stays gated by the identity policy and by default is not applied).
|
||||
* A null metadata or an unfollowable parent is a harmless no-op. */
|
||||
void file_restore_symlink_metadata(const char* path, const FileMetadata* metadata,
|
||||
* A null metadata or an unfollowable parent is a harmless no-op. Returns false
|
||||
* only when a REQUIRED --copy-as ownership application failed, so the caller can
|
||||
* report the entry as failed instead of claiming a wrong-owner success. */
|
||||
bool file_restore_symlink_metadata(const char* path, const FileMetadata* metadata,
|
||||
bool omit_link_times);
|
||||
|
||||
/* Compare timestamps using rsync's whole-second modification window. */
|
||||
|
||||
+7
-5
@@ -89,11 +89,13 @@ void fake_super_store_fd(int fd, uint32_t uid, uint32_t gid, uint32_t mode, int6
|
||||
/* --fake-super replay: parse the FAKESUPER_XATTR record previously written on
|
||||
* `fd` by fake_super_store_fd and re-apply uid/gid/mode/mtime fd-relative.
|
||||
* Best-effort: absence of the xattr or a malformed record is a silent no-op
|
||||
* that never fails the transfer; fchown is applied only when permitted (a
|
||||
* non-root EPERM/EACCES is skipped silently, matching FastSync's identity
|
||||
* philosophy), and the mode is sanitized exactly like the normal metadata path
|
||||
* (group/other write bits never granted). Returns true when the xattr was
|
||||
* present and parsed. */
|
||||
* that never fails the transfer. The OWNER leg is applied only when an explicit
|
||||
* ownership identity policy is active (numeric-ids/chown/usermap/groupmap/
|
||||
* copy-as), when super-user activities are permitted, and when --copy-as is not
|
||||
* authoritative; a non-root EPERM/EACCES is skipped silently, matching
|
||||
* FastSync's identity philosophy. The mode is sanitized exactly like the normal
|
||||
* metadata path (group/other write bits never granted). Returns true when the
|
||||
* xattr was present and parsed. */
|
||||
bool fake_super_restore_fd(int fd);
|
||||
|
||||
#endif
|
||||
Reference in New Issue
Block a user