Add a new daemon security option: "munge symlinks".

diff --git a/clientserver.c b/clientserver.c
index 9207b1a..cb17438 100644 (file)
--- 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;
 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
 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;
        }
 
                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
        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 0b4c4e2..72dded8 100644 (file)
--- 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 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;
 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);
                }
                        }
                        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
        }
        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");
                }
                                linkname_len - 1);
                        overflow_exit("recv_file_entry");
                }

+               if (munge_symlinks)
+                       linkname_len += SYMLINK_PREFIX_LEN;

        }
        else
 #endif
        }
        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);
                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);
                } else
                        read_sbuf(f, bp, linkname_len - 1);

-               if (sanitize_paths)
-                       sanitize_path(bp, bp, "", lastdir_depth);

        }
 #endif
 
        }
 #endif
 

diff --git a/loadparm.c b/loadparm.c
index 982fda7..0915000 100644 (file)
--- a/loadparm.c
+++ b/loadparm.c
@@ -157,6 +157,7 @@ typedef struct
        BOOL ignore_errors;
        BOOL ignore_nonreadable;
        BOOL list;
        BOOL ignore_errors;
        BOOL ignore_nonreadable;
        BOOL list;

+       BOOL munge_symlinks;

        BOOL read_only;
        BOOL strict_modes;
        BOOL transfer_logging;
        BOOL read_only;
        BOOL strict_modes;
        BOOL transfer_logging;


@@ -205,6 +206,7 @@ static service sDefault =
  /* ignore_errors; */          False,
  /* ignore_nonreadable; */     False,
  /* list; */                   True,
  /* ignore_errors; */          False,
  /* ignore_nonreadable; */     False,
  /* list; */                   True,

+ /* munge_symlinks; */         (BOOL)-1,

  /* read_only; */              True,
  /* strict_modes; */           True,
  /* transfer_logging; */       False,
  /* 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},
  {"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},
  {"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_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)
 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 39aa491..8c9673d 100644 (file)
--- a/rsync.h
+++ b/rsync.h
@@ -32,6 +32,9 @@
 #define DEFAULT_LOCK_FILE "/var/run/rsyncd.lock"
 #define URL_PREFIX "rsync://"
 
 #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
 #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 d442a27..052ccc7 100644 (file)
--- 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
 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.
 
 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).
 
 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
 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 (executable)
index 0000000..2aa27f3
--- /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 (<PIPE>) {
+    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 <<EOT;
+Usage: munge-symlinks --munge|--unmunge [--all] [DIR|SYMLINK...]
+
+--munge      Add the $SYMLINK_PREFIX prefix to symlinks if not already
+             present, or always when combined with --all.
+--unmunge    Remove one $SYMLINK_PREFIX prefix from symlinks or all
+             such prefixes with --all.
+
+See the "munge symlinks" option in the rsyncd.conf manpage for more details.
+EOT
+}

diff --git a/testsuite/rsync.fns b/testsuite/rsync.fns
index e5ae719..926a42e 100644 (file)
--- a/testsuite/rsync.fns
+++ b/testsuite/rsync.fns
@@ -234,6 +234,7 @@ build_rsyncd_conf() {
 
 pid file = $pidfile
 use chroot = no
 
 pid file = $pidfile
 use chroot = no

+munge symlinks = no

 hosts allow = localhost 127.0.0.0/24 192.168.0.0/16 10.0.0.0/8 $hostname
 log file = $logfile
 log format = %i %h [%a] %m (%u) %l %f%L
 hosts allow = localhost 127.0.0.0/24 192.168.0.0/16 10.0.0.0/8 $hostname
 log file = $logfile
 log format = %i %h [%a] %m (%u) %l %f%L