From 0b52f94da727c4881b58c1cd6f2cf2a824e02b30 Mon Sep 17 00:00:00 2001 From: Wayne Davison Date: Thu, 14 Feb 2008 13:24:16 -0800 Subject: [PATCH] Some daemon security improvements, including the new parameters "charset" and "numeric ids". --- NEWS | 17 +++++++++-- clientserver.c | 31 ++++++++++++++++--- flist.c | 2 +- loadparm.c | 12 ++++++-- options.c | 20 +++++++++++-- rsync.yo | 18 +++++++++--- rsyncd.conf.yo | 80 +++++++++++++++++++++++++++++++++++--------------- uidlist.c | 12 ++++++-- 8 files changed, 149 insertions(+), 43 deletions(-) diff --git a/NEWS b/NEWS index 3aeebb99..e765eb25 100644 --- a/NEWS +++ b/NEWS @@ -27,7 +27,7 @@ Changes since 2.6.9: symlink-munging behavior designed to make symlinks safer while also allowing absolute symlinks to be stored and retrieved. This also has the effect of making symlinks unusable while they're in the daemon's - hierarchy. See the daemon option "munge symlinks" for full details. + hierarchy. See the daemon's "munge symlinks" parameter for details. BUG FIXES: @@ -36,6 +36,14 @@ Changes since 2.6.9: options: --compare-dest, --link-dest, --copy-dest, --partial-dir, --backup-dir, --temp-dir, and --files-from. + - A daemon can now be told to disable all user- and group-name translation + on a per-module basis. This avoids a potential problem with a writable + daemon module that has "use chroot" enabled -- if precautions weren't + taken, a user could try to add a missing library and get rsync to use + it. This makes rsync safer by default, and more configurable when id- + translation is not desired. See the daemon's "numeric ids" parameter + for full details. + - If a file's data arrived successfully on the receiving side but the rename of the temporary file to the destination file failed AND the --remove-source-files (or the deprecated --remove-sent-files) option @@ -106,7 +114,7 @@ Changes since 2.6.9: - Using --only-write-batch to a daemon receiver now work properly (older versions would update some files while writing the batch). - - Avoid outputing a "file has vanished" message when the file is a broken + - Avoid outputting a "file has vanished" message when the file is a broken symlink and --copy-unsafe-links or --copy-dirlinks are used (the code already handled this for --copy-links). @@ -156,7 +164,7 @@ Changes since 2.6.9: - Added the --fake-super option that allows a non-super user to preserve all attributes of a file by using a special extended-attribute idiom. It even supports the storing of foreign ACL data on your backup server. - There is also an analogous "fake super" option for an rsync daemon. + There is also an analogous "fake super" parameter for an rsync daemon. - Added the --iconv option, which allows rsync to convert filenames from one character-set to another during the transfer. The default is to make @@ -168,6 +176,9 @@ Changes since 2.6.9: "--enable-iconv=." is a good choice. See the rsync manpage for an explanation of the --iconv option's settings. + - A new daemon config parameter, "charset", lets you control the character- + set that is used during an --iconv transfer to/from a daemon module. + - Added the --skip-compress=LIST option to override the default list of file suffixes that will not be compressed when using --compress. diff --git a/clientserver.c b/clientserver.c index 71cb9791..eb83867d 100644 --- a/clientserver.c +++ b/clientserver.c @@ -35,6 +35,7 @@ extern int ignore_errors; extern int kluge_around_eof; extern int daemon_over_rsh; extern int sanitize_paths; +extern int numeric_ids; extern int filesfrom_fd; extern int remote_protocol; extern int protocol_version; @@ -54,6 +55,10 @@ extern char *tmpdir; extern struct chmod_mode_struct *chmod_modes; extern struct filter_list_struct server_filter_list; extern char curr_dir[]; +#ifdef ICONV_OPTION +extern char *iconv_opt; +extern iconv_t ic_send, ic_recv; +#endif char *auth_user; int read_only = 0; @@ -385,6 +390,13 @@ static int rsync_module(int f_in, int f_out, int i, char *addr, char *host) pid_t pre_exec_pid = 0; char *request = NULL; +#ifdef ICONV_CONST + iconv_opt = lp_charset(i); + if (*iconv_opt) + setup_iconv(); + iconv_opt = NULL; +#endif + if (!allow_access(addr, host, lp_hosts_allow(i), lp_hosts_deny(i))) { rprintf(FLOG, "rsync denied on module %s from %s (%s)\n", name, host, addr); @@ -703,10 +715,6 @@ static int rsync_module(int f_in, int f_out, int i, char *addr, char *host) if (write_batch < 0) dry_run = 1; -#ifdef ICONV_CONST - setup_iconv(); -#endif - if (lp_fake_super(i)) am_root = -1; else if (am_root < 0) /* Treat --fake-super from client as --super. */ @@ -772,6 +780,21 @@ static int rsync_module(int f_in, int f_out, int i, char *addr, char *host) exit_cleanup(RERR_UNSUPPORTED); } + if (!iconv_opt) { + if (ic_send != (iconv_t)-1) { + iconv_close(ic_send); + ic_send = (iconv_t)-1; + } + if (ic_recv != (iconv_t)-1) { + iconv_close(ic_recv); + ic_recv = (iconv_t)-1; + } + } + + if (!numeric_ids + && (use_chroot ? lp_numeric_ids(i) != False : lp_numeric_ids(i) == True)) + numeric_ids = -1; /* Set --numeric-ids w/o breaking protocol. */ + if (lp_timeout(i) && lp_timeout(i) > io_timeout) set_io_timeout(lp_timeout(i)); diff --git a/flist.c b/flist.c index a0970216..20350459 100644 --- a/flist.c +++ b/flist.c @@ -2037,7 +2037,7 @@ struct file_list *send_file_list(int f, int argc, char *argv[]) flist_sort_and_clean(flist, 0); file_total += flist->used; - if (!numeric_ids && !inc_recurse) + if (numeric_ids <= 0 && !inc_recurse) send_id_list(f); /* send the io_error flag */ diff --git a/loadparm.c b/loadparm.c index 010e089a..c1749b76 100644 --- a/loadparm.c +++ b/loadparm.c @@ -124,6 +124,7 @@ static global Globals; typedef struct { char *auth_users; + char *charset; char *comment; char *dont_compress; char *exclude; @@ -158,6 +159,7 @@ typedef struct BOOL ignore_nonreadable; BOOL list; BOOL munge_symlinks; + BOOL numeric_ids; BOOL read_only; BOOL strict_modes; BOOL transfer_logging; @@ -173,7 +175,8 @@ typedef struct static service sDefault = { /* auth_users; */ NULL, - /* comment; */ NULL, + /* charset; */ NULL, + /* comment; */ NULL, /* dont_compress; */ DEFAULT_DONT_COMPRESS, /* exclude; */ NULL, /* exclude_from; */ NULL, @@ -207,6 +210,7 @@ static service sDefault = /* ignore_nonreadable; */ False, /* list; */ True, /* munge_symlinks; */ (BOOL)-1, + /* numeric_ids; */ (BOOL)-1, /* read_only; */ True, /* strict_modes; */ True, /* transfer_logging; */ False, @@ -301,6 +305,7 @@ static struct parm_struct parm_table[] = {"socket options", P_STRING, P_GLOBAL,&Globals.socket_options, NULL,0}, {"auth users", P_STRING, P_LOCAL, &sDefault.auth_users, NULL,0}, + {"charset", P_STRING, P_LOCAL, &sDefault.charset, NULL,0}, {"comment", P_STRING, P_LOCAL, &sDefault.comment, NULL,0}, {"dont compress", P_STRING, P_LOCAL, &sDefault.dont_compress, NULL,0}, {"exclude from", P_STRING, P_LOCAL, &sDefault.exclude_from, NULL,0}, @@ -322,6 +327,7 @@ static struct parm_struct parm_table[] = {"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}, + {"numeric ids", P_BOOL, P_LOCAL, &sDefault.numeric_ids, 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}, @@ -392,6 +398,7 @@ FN_GLOBAL_STRING(lp_socket_options, &Globals.socket_options) FN_GLOBAL_INTEGER(lp_rsync_port, &Globals.rsync_port) FN_LOCAL_STRING(lp_auth_users, auth_users) +FN_LOCAL_STRING(lp_charset, charset) FN_LOCAL_STRING(lp_comment, comment) FN_LOCAL_STRING(lp_dont_compress, dont_compress) FN_LOCAL_STRING(lp_exclude, exclude) @@ -413,12 +420,12 @@ FN_LOCAL_STRING(lp_postxfer_exec, postxfer_exec) FN_LOCAL_STRING(lp_prexfer_exec, prexfer_exec) FN_LOCAL_STRING(lp_refuse_options, refuse_options) FN_LOCAL_STRING(lp_secrets_file, secrets_file) -FN_LOCAL_INTEGER(lp_syslog_facility, syslog_facility) FN_LOCAL_STRING(lp_temp_dir, temp_dir) FN_LOCAL_STRING(lp_uid, uid) FN_LOCAL_INTEGER(lp_max_connections, max_connections) FN_LOCAL_INTEGER(lp_max_verbosity, max_verbosity) +FN_LOCAL_INTEGER(lp_syslog_facility, syslog_facility) FN_LOCAL_INTEGER(lp_timeout, timeout) FN_LOCAL_BOOL(lp_fake_super, fake_super) @@ -426,6 +433,7 @@ 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_numeric_ids, numeric_ids) 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/options.c b/options.c index 5a24dd2f..d12d28a7 100644 --- a/options.c +++ b/options.c @@ -202,7 +202,7 @@ static int itemize_changes = 0; static int refused_delete, refused_archive_part, refused_compress; static int refused_partial, refused_progress, refused_delete_before; static int refused_delete_during; -static int refused_inplace; +static int refused_inplace, refused_no_iconv; static char *max_size_arg, *min_size_arg; static char tmp_partialdir[] = ".~tmp~"; @@ -441,7 +441,7 @@ enum {OPT_VERSION = 1000, OPT_DAEMON, OPT_SENDER, OPT_EXCLUDE, OPT_EXCLUDE_FROM, OPT_FILTER, OPT_COMPARE_DEST, OPT_COPY_DEST, OPT_LINK_DEST, OPT_HELP, OPT_INCLUDE, OPT_INCLUDE_FROM, OPT_MODIFY_WINDOW, OPT_MIN_SIZE, OPT_CHMOD, OPT_READ_BATCH, OPT_WRITE_BATCH, OPT_ONLY_WRITE_BATCH, OPT_MAX_SIZE, - OPT_NO_D, OPT_APPEND, + OPT_NO_D, OPT_APPEND, OPT_NO_ICONV, OPT_SERVER, OPT_REFUSED_BASE = 9000}; static struct poptOption long_options[] = { @@ -612,6 +612,7 @@ static struct poptOption long_options[] = { {"temp-dir", 'T', POPT_ARG_STRING, &tmpdir, 0, 0, 0 }, #ifdef ICONV_OPTION {"iconv", 0, POPT_ARG_STRING, &iconv_opt, 0, 0, 0 }, + {"no-iconv", 0, POPT_ARG_NONE, 0, OPT_NO_ICONV, 0, 0 }, #endif {"ipv4", '4', POPT_ARG_VAL, &default_af_hint, AF_INET, 0, 0 }, {"ipv6", '6', POPT_ARG_VAL, &default_af_hint, AF_INET6, 0, 0 }, @@ -758,6 +759,8 @@ static void set_refuse_options(char *bp) refused_progress = op->val; else if (wildmatch("inplace", op->longName)) refused_inplace = op->val; + else if (wildmatch("no-iconv", op->longName)) + refused_no_iconv = op->val; break; } if (!is_wild) @@ -878,8 +881,11 @@ int parse_arguments(int *argc_p, const char ***argv_p, int frommain) if (ref && *ref) set_refuse_options(ref); - if (am_daemon) + if (am_daemon) { set_refuse_options("log-file*"); + if (!*lp_charset(module_id)) + set_refuse_options("iconv"); + } #ifdef ICONV_OPTION if (!am_daemon && !protect_args && (arg = getenv("RSYNC_ICONV")) != NULL && *arg) @@ -1127,6 +1133,10 @@ int parse_arguments(int *argc_p, const char ***argv_p, int frommain) read_batch = 1; break; + case OPT_NO_ICONV: + iconv_opt = NULL; + break; + case OPT_MAX_SIZE: if ((max_size = parse_size_arg(&max_size_arg, 'b')) <= 0) { snprintf(err_buf, sizeof err_buf, @@ -1254,6 +1264,10 @@ int parse_arguments(int *argc_p, const char ***argv_p, int frommain) else need_unsorted_flist = 1; } + if (refused_no_iconv && !iconv_opt) { + create_refuse_error(refused_no_iconv); + return 0; + } #endif if (protect_args == 1) { diff --git a/rsync.yo b/rsync.yo index 5879ad4a..99567285 100644 --- a/rsync.yo +++ b/rsync.yo @@ -1422,7 +1422,7 @@ characters are not translated (such as ~, $, ;, &, etc.). Wildcards are expanded on the remote host by rsync (instead of the shell doing it). If you use this option with bf(--iconv), the args will also be translated -from the local to the remote character set. The translation happens before +from the local to the remote character-set. The translation happens before wild-cards are expanded. See also the bf(--files-from) option. dit(bf(-T, --temp-dir=DIR)) This option instructs rsync to use DIR as a @@ -2033,12 +2033,17 @@ dit(bf(--iconv=CONVERT_SPEC)) Rsync can convert filenames between character sets using this option. Using a CONVERT_SPEC of "." tells rsync to look up the default character-set via the locale setting. Alternately, you can fully specify what conversion to do by giving a local and a remote charset -separated by a comma (local first), e.g. bf(--iconv=utf8,iso88591). (Run -"iconv --list" to see a list of the charset names that a machine supports.) -Finally, you can specify a CONVERT_SPEC of "-" to turn off any conversion. +separated by a comma in the order bf(--iconv=LOCAL,REMOTE), e.g. +bf(--iconv=utf8,iso88591). This order ensures that the option +will stay the same whether you're pushing or pulling files. +Finally, you can specify either bf(--no-iconv) or a CONVERT_SPEC of "-" +to turn off any conversion. The default setting of this option is site-specific, and can also be affected via the RSYNC_ICONV environment variable. +For a list of what charset names your local iconv library supports, you can +run "iconv --list". + If you specify the bf(--protect-args) option (bf(-s)), rsync will translate the filenames you specify on the command-line that are being sent to the remote host. See also the bf(--files-from) option. @@ -2049,6 +2054,11 @@ specifying matching rules that can match on both sides of the transfer. For instance, you can specify extra include/exclude rules if there are filename differences on the two sides that need to be accounted for. +When you pass an bf(--iconv) option to an rsync daemon that allows it, the +daemon uses the charset specified in its "charset" configuration parameter +regardless of the remote charset you actually pass. Thus, you may feel free to +specify just the local charset for a daemon transfer (e.g. bf(--iconv=utf8)). + dit(bf(-4, --ipv4) or bf(-6, --ipv6)) Tells rsync to prefer IPv4/IPv6 when creating sockets. This only affects sockets that rsync has direct control over, such as the outgoing socket when directly contacting an diff --git a/rsyncd.conf.yo b/rsyncd.conf.yo index 625566ed..04903f67 100644 --- a/rsyncd.conf.yo +++ b/rsyncd.conf.yo @@ -133,8 +133,9 @@ to the "path" before starting the file transfer with the client. This has 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, rsync will: (1) munge symlinks by +of the new root path, and of complicating the preservation of users and groups +by name (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 @@ -144,27 +145,38 @@ 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 +When this option is enabled, rsync will not attempt to map users and groups +by name (by default), but instead copy IDs as though bf(--numeric-ids) had +been specified. In order to enable name-mapping, rsync needs to be able to use the standard library functions for looking up names and IDs (i.e. -code(getpwuid()), code(getgrgid()), code(getpwname()), and code(getgrnam())). This means a -process in the chroot namespace will need to have access to the resources +code(getpwuid()), code(getgrgid()), code(getpwname()), and code(getgrnam())). +This means the rsync +process in the chroot hierarchy will need to have access to the resources used by these library functions (traditionally /etc/passwd and -/etc/group). If these resources are not available, rsync will only be -able to copy the IDs, just as if the bf(--numeric-ids) option had been -specified. - -Note that you are free to setup user/group information in the chroot area -differently from your normal system. For example, you could abbreviate -the list of users and groups. Also, you can protect this information from -being downloaded/uploaded by adding an exclude rule to the rsyncd.conf file -(e.g. "bf(exclude = /etc/**)"). Note that having the exclusion affect uploads -is a relatively new feature in rsync, so make sure your daemon is -at least 2.6.3 to effect this. Also note that it is safest to exclude a -directory and all its contents combining the rule "/some/dir/" with the -rule "/some/dir/**" just to be sure that rsync will not allow deeper -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). +/etc/group, but perhaps additional dynamic libraries as well). + +If you copy the necessary resources into the module's chroot area, you +should protect them through your OS's normal user/group or ACL settings (to +prevent the rsync module's user from being able to change them), and then +hide them from the user's view via "exclude" (see how in the discussion of +that option). At that point it will be safe to enable the mapping of users +and groups by name using the "numeric ids" daemon option (see below). + +Note also that you are free to setup custom user/group information in the +chroot area that is different from your normal system. For example, you +could abbreviate the list of users and groups. + +dit(bf(numeric ids)) Enabling the "numeric ids" option disables the mapping +of users and groups by name for the current daemon module. This prevents +the daemon from trying to load any user/group-related files or libraries. +Enabling this option makes the transfer behave as if the client had passed +the bf(--numeric-ids) command-line option. By default, this parameter is +enabled for chroot modules and disabled for non-chroot modules. + +A chroot-enabled module should not have this option enabled unless you've +taken steps to ensure that the module has the necessary resources it needs +to translate names, and that it is not possible for a user to change those +resources. dit(bf(munge symlinks)) The "munge symlinks" option tells rsync to modify all incoming symlinks in a way that makes them unusable but recoverable @@ -200,6 +212,19 @@ path elements that rsync believes will allow a symlink to escape the module's hierarchy. There are tricky ways to work around this, though, so you had better trust your users if you choose this combination of options. +dit(bf(charset)) This specifies the name of the character set in which the +module's filenames are stored. If the client uses an bf(--iconv) option, +the daemon will use the value of the "charset" parameter regardless of the +character set the client actually passed. This allows the daemon to +support charset conversion in a chroot module without extra files in the +chroot area, and also ensures that name-translation is done in a consistent +manner. If the "charset" parameter is not set, the bf(--iconv) option is +refused, just as if "iconv" had been specified via "refuse options". + +If you wish to force users to always use bf(--iconv) for a particular +module, add "no-iconv" to the "refuse options" parameter. Keep in mind +that this will restrict access to your module to very new rsync clients. + 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 @@ -297,6 +322,13 @@ from a daemon and files deleted on a daemon when sending to a daemon, but it doesn't exclude files from being deleted on a client when receiving from a daemon. +When you want to exclude a directory and all its contents, it is safest to +use a rule that does both, such as "/some/dir/***" (the three stars tells +rsync to exclude the directory itself and everything inside it). This is +better than just excluding the directory alone with "/some/dir/", as it +helps to guard against attempts to trick rsync into accessing files deeper +in the hierarchy. + dit(bf(exclude from)) The "exclude from" option specifies a filename on the daemon that contains exclude patterns, one per line. This is only superficially equivalent @@ -509,9 +541,9 @@ quote(tt( refuse options = c delete)) The reason the above refuses all delete options is that the options imply bf(--delete), and implied options are refused just like explicit options. As an additional safety feature, the refusal of "delete" also refuses -bf(remove-sent-files) when the daemon is the sender; if you want the latter +bf(remove-source-files) when the daemon is the sender; if you want the latter without the former, instead refuse "delete-*" -- that refuses all the -delete modes without affecting bf(--remove-sent-files). +delete modes without affecting bf(--remove-source-files). When an option is refused, the daemon prints an error message and exits. To prevent all compression when serving files, @@ -533,7 +565,7 @@ of the patterns will not be compressed during transfer. See the bf(--skip-compress) option in the bf(rsync)(1) manpage for the list of file suffixes that are not compressed by default. Specifying a value -for the bf(dont compress) option changes the default when the daemon is +for the "dont compress" option changes the default when the daemon is the sender. dit(bf(pre-xfer exec), bf(post-xfer exec)) You may specify a command to be run diff --git a/uidlist.c b/uidlist.c index 48e0318d..bb7a3013 100644 --- a/uidlist.c +++ b/uidlist.c @@ -316,6 +316,10 @@ uid_t recv_user_name(int f, uid_t uid) if (!name) out_of_memory("recv_user_name"); read_sbuf(f, name, len); + if (numeric_ids < 0) { + free(name); + name = NULL; + } node = recv_add_uid(uid, name); /* node keeps name's memory */ return node->id2; } @@ -328,6 +332,10 @@ gid_t recv_group_name(int f, gid_t gid, uint16 *flags_ptr) if (!name) out_of_memory("recv_group_name"); read_sbuf(f, name, len); + if (numeric_ids < 0) { + free(name); + name = NULL; + } node = recv_add_gid(gid, name); /* node keeps name's memory */ if (flags_ptr && node->flags & FLAG_SKIP_GROUP) *flags_ptr |= FLAG_SKIP_GROUP; @@ -341,13 +349,13 @@ void recv_id_list(int f, struct file_list *flist) id_t id; int i; - if ((preserve_uid || preserve_acls) && !numeric_ids) { + if ((preserve_uid || preserve_acls) && numeric_ids <= 0) { /* read the uid list */ while ((id = read_varint30(f)) != 0) recv_user_name(f, id); } - if ((preserve_gid || preserve_acls) && !numeric_ids) { + if ((preserve_gid || preserve_acls) && numeric_ids <= 0) { /* read the gid list */ while ((id = read_varint30(f)) != 0) recv_group_name(f, id, NULL); -- 2.34.1