From 9585b27678ee8b94f2f260e8d8d61ff4381f2fa3 Mon Sep 17 00:00:00 2001 From: Wayne Davison Date: Mon, 26 Nov 2007 21:58:19 -0800 Subject: [PATCH] Add a new daemon security option: "munge symlinks". --- clientserver.c | 13 +++++++++ flist.c | 15 +++++++++-- loadparm.c | 4 +++ rsync.h | 3 +++ rsyncd.conf.yo | 43 +++++++++++++++++++++++++----- support/munge-symlinks | 60 ++++++++++++++++++++++++++++++++++++++++++ testsuite/rsync.fns | 1 + 7 files changed, 131 insertions(+), 8 deletions(-) create mode 100755 support/munge-symlinks diff --git a/clientserver.c b/clientserver.c index 9207b1a7..cb17438b 100644 --- a/clientserver.c +++ b/clientserver.c @@ -58,6 +58,7 @@ extern char curr_dir[]; char *auth_user; int read_only = 0; int module_id = -1; +int munge_symlinks = 0; struct chmod_mode_struct *daemon_chmod_modes; /* module_dirlen is the length of the module_dir string when in daemon @@ -624,6 +625,18 @@ static int rsync_module(int f_in, int f_out, int i, char *addr, char *host) sanitize_paths = 1; } + if ((munge_symlinks = lp_munge_symlinks(i)) < 0) + munge_symlinks = !use_chroot; + if (munge_symlinks) { + STRUCT_STAT st; + if (stat(SYMLINK_PREFIX, &st) == 0 && S_ISDIR(st.st_mode)) { + rprintf(FLOG, "Symlink munging is unsupported when a %s directory exists.\n", + SYMLINK_PREFIX); + io_printf(f_out, "@ERROR: daemon security issue -- contact admin\n", name); + exit_cleanup(RERR_UNSUPPORTED); + } + } + if (am_root) { /* XXXX: You could argue that if the daemon is started * by a non-root user and they explicitly specify a diff --git a/flist.c b/flist.c index 0b4c4e2f..72dded84 100644 --- a/flist.c +++ b/flist.c @@ -63,6 +63,7 @@ extern int copy_links; extern int copy_unsafe_links; extern int protocol_version; extern int sanitize_paths; +extern int munge_symlinks; extern int need_unsorted_flist; extern int unsort_ndx; extern struct stats stats; @@ -200,6 +201,11 @@ static int readlink_stat(const char *path, STRUCT_STAT *stp, char *linkbuf) } return x_stat(path, stp, NULL); } + if (munge_symlinks && am_sender && llen > SYMLINK_PREFIX_LEN + && strncmp(linkbuf, SYMLINK_PREFIX, SYMLINK_PREFIX_LEN) == 0) { + memmove(linkbuf, linkbuf + SYMLINK_PREFIX_LEN, + llen - SYMLINK_PREFIX_LEN + 1); + } } return 0; #else @@ -794,6 +800,8 @@ static struct file_struct *recv_file_entry(struct file_list *flist, linkname_len - 1); overflow_exit("recv_file_entry"); } + if (munge_symlinks) + linkname_len += SYMLINK_PREFIX_LEN; } else #endif @@ -914,10 +922,13 @@ static struct file_struct *recv_file_entry(struct file_list *flist, if (first_hlink_ndx >= flist->ndx_start) { struct file_struct *first = flist->files[first_hlink_ndx - flist->ndx_start]; memcpy(bp, F_SYMLINK(first), linkname_len); + } else if (munge_symlinks) { + strlcpy(bp, SYMLINK_PREFIX, linkname_len); + bp += SYMLINK_PREFIX_LEN; + linkname_len -= SYMLINK_PREFIX_LEN; + read_sbuf(f, bp, linkname_len - 1); } else read_sbuf(f, bp, linkname_len - 1); - if (sanitize_paths) - sanitize_path(bp, bp, "", lastdir_depth); } #endif diff --git a/loadparm.c b/loadparm.c index 982fda76..0915000d 100644 --- a/loadparm.c +++ b/loadparm.c @@ -157,6 +157,7 @@ typedef struct BOOL ignore_errors; BOOL ignore_nonreadable; BOOL list; + BOOL munge_symlinks; BOOL read_only; BOOL strict_modes; BOOL transfer_logging; @@ -205,6 +206,7 @@ static service sDefault = /* ignore_errors; */ False, /* ignore_nonreadable; */ False, /* list; */ True, + /* munge_symlinks; */ (BOOL)-1, /* read_only; */ True, /* strict_modes; */ True, /* transfer_logging; */ False, @@ -319,6 +321,7 @@ static struct parm_struct parm_table[] = {"log format", P_STRING, P_LOCAL, &sDefault.log_format, NULL,0}, {"max connections", P_INTEGER,P_LOCAL, &sDefault.max_connections, NULL,0}, {"max verbosity", P_INTEGER,P_LOCAL, &sDefault.max_verbosity, NULL,0}, + {"munge symlinks", P_BOOL, P_LOCAL, &sDefault.munge_symlinks, NULL,0}, {"name", P_STRING, P_LOCAL, &sDefault.name, NULL,0}, {"outgoing chmod", P_STRING, P_LOCAL, &sDefault.outgoing_chmod, NULL,0}, {"path", P_PATH, P_LOCAL, &sDefault.path, NULL,0}, @@ -422,6 +425,7 @@ FN_LOCAL_BOOL(lp_fake_super, fake_super) FN_LOCAL_BOOL(lp_ignore_errors, ignore_errors) FN_LOCAL_BOOL(lp_ignore_nonreadable, ignore_nonreadable) FN_LOCAL_BOOL(lp_list, list) +FN_LOCAL_BOOL(lp_munge_symlinks, munge_symlinks) FN_LOCAL_BOOL(lp_read_only, read_only) FN_LOCAL_BOOL(lp_strict_modes, strict_modes) FN_LOCAL_BOOL(lp_transfer_logging, transfer_logging) diff --git a/rsync.h b/rsync.h index 39aa4916..8c9673da 100644 --- a/rsync.h +++ b/rsync.h @@ -32,6 +32,9 @@ #define DEFAULT_LOCK_FILE "/var/run/rsyncd.lock" #define URL_PREFIX "rsync://" +#define SYMLINK_PREFIX "/rsyncd-munged/" +#define SYMLINK_PREFIX_LEN ((int)sizeof SYMLINK_PREFIX - 1) + #define BACKUP_SUFFIX "~" /* a non-zero CHAR_OFFSET makes the rolling sum stronger, but is diff --git a/rsyncd.conf.yo b/rsyncd.conf.yo index d442a279..052ccc7e 100644 --- a/rsyncd.conf.yo +++ b/rsyncd.conf.yo @@ -130,12 +130,15 @@ the advantage of extra protection against possible implementation security holes, but it has the disadvantages of requiring super-user privileges, of not being able to follow symbolic links that are either absolute or outside of the new root path, and of complicating the preservation of usernames and groups -(see below). When "use chroot" is false, for security reasons, -symlinks may only be relative paths pointing to other files within the root -path, and leading slashes are removed from most absolute paths (options -such as bf(--backup-dir), bf(--compare-dest), etc. interpret an absolute path as -rooted in the module's "path" dir, just as if chroot was specified). -The default for "use chroot" is true. +(see below). When "use chroot" is false, rsync will: (1) munge symlinks by +default for security reasons (see "munge symlinks" for a way to turn this +off, but only if you trust your users), (2) substitute leading slashes in +absolute paths with the module's path (so that options such as +bf(--backup-dir), bf(--compare-dest), etc. interpret an absolute path as +rooted in the module's "path" dir), and (3) trim ".." path elements from +args if rsync believes they would escape the chroot. +The default for "use chroot" is true, and is the safer choice (especially +if the module is not read-only). In order to preserve usernames and groupnames, rsync needs to be able to use the standard library functions for looking up names and IDs (i.e. @@ -159,6 +162,34 @@ access to some of the excluded files inside the directory (rsync tries to do this automatically, but you might as well specify both to be extra sure). +dit(bf(munge symlinks)) The "munge symlinks" option tells rsync to modify +all incoming symlinks in a way that makes them unusable but recoverable +(see below). This should help protect your files from user trickery when +your daemon module is writable. The default is disabled when "use chroot" +is on and enabled when "use chroot" is off. + +If you disable this option on a daemon that is not read-only, there +are tricks that a user can play with uploaded symlinks to access +daemon-excluded items (if your module has any), and, if "use chroot" +is off, rsync can even be tricked into showing or changing data that +is outside the module's path (as access-permissions allow). + +The way rsync disables the use of symlinks is to prefix each one with +the string "/rsyncd-munged/". This prevents the links from being used +as long as that directory does not exist. When this option is enabled, +rsync will refuse to run if that path is a directory or a symlink to +a directory. When using the "munge symlinks" option in a chroot area, +you should add this path to the exclude setting for the module so that +the user can't try to create it. + +Note: rsync makes no attempt to verify that any pre-existing symlinks in +the hierarchy are as safe as you want them to be. If you setup an rsync +daemon on a new area or locally add symlinks, you can manually protect your +symlinks from being abused by prefixing "/rsyncd-munged/" to the start of +every symlink's value. There is a perl script in the support directory +of the source code named "munge-symlinks" that can be used to add or remove +this prefix from your symlinks. + dit(bf(max connections)) The "max connections" option allows you to specify the maximum number of simultaneous connections you will allow. Any clients connecting when the maximum has been reached will receive a diff --git a/support/munge-symlinks b/support/munge-symlinks new file mode 100755 index 00000000..2aa27f36 --- /dev/null +++ b/support/munge-symlinks @@ -0,0 +1,60 @@ +#!/usr/bin/perl +# This script will either prefix all symlink values with the string +# "/rsyncd-munged/" or remove that prefix. + +use strict; +use Getopt::Long; + +my $SYMLINK_PREFIX = '/rsyncd-munged/'; + +my $munge_opt; + +&GetOptions( + 'munge' => sub { $munge_opt = 1 }, + 'unmunge' => sub { $munge_opt = 0 }, + 'all' => \( my $all_opt ), + 'help|h' => \( my $help_opt ), +) or &usage; + +&usage if $help_opt || !defined $munge_opt; + +my $munged_re = $all_opt ? qr/^($SYMLINK_PREFIX)+(?=.)/ : qr/^$SYMLINK_PREFIX(?=.)/; + +push(@ARGV, '.') unless @ARGV; + +open(PIPE, '-|', 'find', @ARGV, '-type', 'l') or die $!; + +while () { + chomp; + my $lnk = readlink($_) or next; + if ($munge_opt) { + next if !$all_opt && $lnk =~ /$munged_re/; + $lnk =~ s/^/$SYMLINK_PREFIX/; + } else { + next unless $lnk =~ s/$munged_re//; + } + if (!unlink($_)) { + warn "Unable to unlink symlink: $_ ($!)\n"; + } elsif (!symlink($lnk, $_)) { + warn "Unable to recreate symlink: $_ -> $lnk ($!)\n"; + } else { + print "$_ -> $lnk\n"; + } +} + +close PIPE; +exit; + +sub usage +{ + die <