Included below is a shar archive containing two patches that together: 1) make backup files get their setuid and setgid bits stripped by default 2) add a "-s" option that allows backup files to continue to have these privileges This means that if you update a collection of binaries with rsync, and one or more of them has a local-root security problem, the backup file(s) created when you fix the problem in your source archive won't remain exploitable. The patch is relative to 2.5.4. The backup-dir support is attempted but untested. We're using the default backup behavior (with ~) in production. I'd be pleased if someone who uses backup-dir were to try it out and let me know how it goes. I'd also be pleased if this were to find its way into the main distribution in some form. Thank you. #!/bin/sh # This is a shell archive (shar 3.32) # made 06/21/2002 20:17 UTC by dcslib@tokyo.acs.uci.edu # Source directory /dcslibsrc/network/rsync/exportable-patches # # existing files WILL be overwritten # # This shar contains: # length mode name # ------ ---------- ------------------------------------------ # 1798 -rw-r--r-- backup-priv-backups # 1339 -rw-r--r-- options-priv-backups # if touch 2>&1 | fgrep 'amc' > /dev/null then TOUCH=touch else TOUCH=true fi # ============= backup-priv-backups =============echo "x - extracting backup-priv-backups (Text)" sed 's/^X//' << 'SHAR_EOF' > backup-priv-backups && X*** backup.c.t Sun May 6 23:59:37 2001 X--- backup.c Fri Jun 21 13:15:51 2002 X*************** X*** 29,34 **** X--- 29,56 ---- X extern int preserve_devices; X extern int preserve_links; X extern int preserve_hard_links; X+ extern int priv_backups; X+ X+ #ifdef HAVE_CHMOD X+ static int strip_perm(char *fname) X+ { X+ struct stat buf; X+ if (link_stat(fname,&buf) != 0) { X+ rprintf(FERROR,"stat failed\n"); X+ return 0; X+ } X+ X+ if (S_ISREG(buf.st_mode) && (buf.st_mode & (S_ISUID | S_ISGID))) { X+ mode_t new_mode; X+ new_mode = buf.st_mode & 01777; X+ if (do_chmod(fname,new_mode) != 0) { X+ rprintf(FERROR,"chmod failed\n"); X+ return 0; X+ } X+ } X+ return 1; X+ } X+ #endif X X /* simple backup creates a backup with a suffix in the same directory */ X static int make_simple_backup(char *fname) X*************** X*** 46,54 **** X rsyserr(FERROR, errno, "rename %s to backup %s", fname, fnamebak); X return 0; X } X! } else if (verbose > 1) { X! rprintf(FINFO,"backed up %s to %s\n",fname,fnamebak); X } X return 1; X } X X--- 68,86 ---- X rsyserr(FERROR, errno, "rename %s to backup %s", fname, fnamebak); X return 0; X } X! } else { X! if (verbose > 1) { X! rprintf(FINFO,"backed up %s to %s\n",fname,fnamebak); X! } X! #ifdef HAVE_CHMOD X! if (!priv_backups && strip_perm(fnamebak) == 0) { X! return 0; X! } else if (verbose > 1) { X! rprintf(FINFO,"Stripped setuid and/or setgid from %s\n",fnamebak); X! } X! #endif X } X+ X return 1; X } X X*************** X*** 271,276 **** X--- 303,314 ---- X fname, keep_name, strerror(errno)); X }; X set_perms (keep_name, file, NULL, 0); X+ /* may mean an extra stat */ X+ #ifdef HAVE_CHMOD X+ if (!priv_backups && strip_perm(keep_name) == 0) { X+ return 0; X+ } X+ #endif X free_file (file); X free (file); X SHAR_EOF $TOUCH -am 06211315102 backup-priv-backups && chmod 0644 backup-priv-backups || echo "restore of backup-priv-backups failed" set `wc -c backup-priv-backups`;Wc_c=$1 if test "$Wc_c" != "1798"; then echo original size 1798, current size $Wc_c fi # ============= options-priv-backups =============echo "x - extracting options-priv-backups (Text)" sed 's/^X//' << 'SHAR_EOF' > options-priv-backups && X--- options.c.t Fri Jun 21 08:56:31 2002 X+++ options.c Fri Jun 21 09:41:41 2002 X@@ -21,6 +21,9 @@ X #include "rsync.h" X #include "popt.h" X X+#ifdef HAVE_CHMOD X+int priv_backups = 0; X+#endif X int make_backups = 0; X int whole_file = -1; X int copy_links = 0; X@@ -188,6 +191,7 @@ X rprintf(F," -b, --backup make backups (default %s suffix)\n",BACKUP_SUFFIX); X rprintf(F," --backup-dir make backups into this directory\n"); X rprintf(F," --suffix=SUFFIX override backup suffix\n"); X+ rprintf(F," --priv-backups preserve set[ug]id on backups\n"); X rprintf(F," -u, --update update only (don't overwrite newer files)\n"); X rprintf(F," -l, --links copy symlinks as symlinks\n"); X rprintf(F," -L, --copy-links copy the referent of symlinks\n"); X@@ -291,6 +295,9 @@ X {"include-from", 0, POPT_ARG_STRING, 0, OPT_INCLUDE_FROM}, X {"safe-links", 0, POPT_ARG_NONE, &safe_symlinks}, X {"help", 'h', POPT_ARG_NONE, 0, 'h'}, X+#ifdef HAVE_CHMOD X+ {"priv-backups", 's', POPT_ARG_NONE, &priv_backups}, X+#endif X {"backup", 'b', POPT_ARG_NONE, &make_backups}, X {"dry-run", 'n', POPT_ARG_NONE, &dry_run}, X {"sparse", 'S', POPT_ARG_NONE, &sparse_files}, SHAR_EOF $TOUCH -am 06210941102 options-priv-backups && chmod 0644 options-priv-backups || echo "restore of options-priv-backups failed" set `wc -c options-priv-backups`;Wc_c=$1 if test "$Wc_c" != "1339"; then echo original size 1339, current size $Wc_c fi exit 0 -- Dan Stromberg UCI/NACS/DCS -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 232 bytes Desc: not available Url : http://lists.samba.org/archive/rsync/attachments/20020621/307b74d4/attachment.bin
Martin Pool
2002-Jul-08 01:01 UTC
strip setuid/setgid bits on backup (was Re: small security-related rsync extension)
Any thoughts on whether this should go in? I can see arguments either way. It seems like we ought to think about whether it would be better to do it as part of a generalized --chmod or --chmod-backup facility. -- Martin On 21 Jun 2002, Dan Stromberg <strombrg@nis.acs.uci.edu> wrote:> Included below is a shar archive containing two patches that together: > > 1) make backup files get their setuid and setgid bits stripped by > default > > 2) add a "-s" option that allows backup files to continue to have > these privileges > > This means that if you update a collection of binaries with rsync, and > one or more of them has a local-root security problem, the backup > file(s) created when you fix the problem in your source archive won't > remain exploitable. > > The patch is relative to 2.5.4. > > The backup-dir support is attempted but untested. We're using the > default backup behavior (with ~) in production. I'd be pleased if > someone who uses backup-dir were to try it out and let me know how it > goes. > > I'd also be pleased if this were to find its way into the main > distribution in some form. > > Thank you. > > #!/bin/sh > # This is a shell archive (shar 3.32) > # made 06/21/2002 20:17 UTC by dcslib@tokyo.acs.uci.edu > # Source directory /dcslibsrc/network/rsync/exportable-patches > # > # existing files WILL be overwritten > # > # This shar contains: > # length mode name > # ------ ---------- ------------------------------------------ > # 1798 -rw-r--r-- backup-priv-backups > # 1339 -rw-r--r-- options-priv-backups > # > if touch 2>&1 | fgrep 'amc' > /dev/null > then TOUCH=touch > else TOUCH=true > fi > # ============= backup-priv-backups =============> echo "x - extracting backup-priv-backups (Text)" > sed 's/^X//' << 'SHAR_EOF' > backup-priv-backups && > X*** backup.c.t Sun May 6 23:59:37 2001 > X--- backup.c Fri Jun 21 13:15:51 2002 > X*************** > X*** 29,34 **** > X--- 29,56 ---- > X extern int preserve_devices; > X extern int preserve_links; > X extern int preserve_hard_links; > X+ extern int priv_backups; > X+ > X+ #ifdef HAVE_CHMOD > X+ static int strip_perm(char *fname) > X+ { > X+ struct stat buf; > X+ if (link_stat(fname,&buf) != 0) { > X+ rprintf(FERROR,"stat failed\n"); > X+ return 0; > X+ } > X+ > X+ if (S_ISREG(buf.st_mode) && (buf.st_mode & (S_ISUID | S_ISGID))) { > X+ mode_t new_mode; > X+ new_mode = buf.st_mode & 01777; > X+ if (do_chmod(fname,new_mode) != 0) { > X+ rprintf(FERROR,"chmod failed\n"); > X+ return 0; > X+ } > X+ } > X+ return 1; > X+ } > X+ #endif > X > X /* simple backup creates a backup with a suffix in the same directory */ > X static int make_simple_backup(char *fname) > X*************** > X*** 46,54 **** > X rsyserr(FERROR, errno, "rename %s to backup %s", fname, fnamebak); > X return 0; > X } > X! } else if (verbose > 1) { > X! rprintf(FINFO,"backed up %s to %s\n",fname,fnamebak); > X } > X return 1; > X } > X > X--- 68,86 ---- > X rsyserr(FERROR, errno, "rename %s to backup %s", fname, fnamebak); > X return 0; > X } > X! } else { > X! if (verbose > 1) { > X! rprintf(FINFO,"backed up %s to %s\n",fname,fnamebak); > X! } > X! #ifdef HAVE_CHMOD > X! if (!priv_backups && strip_perm(fnamebak) == 0) { > X! return 0; > X! } else if (verbose > 1) { > X! rprintf(FINFO,"Stripped setuid and/or setgid from %s\n",fnamebak); > X! } > X! #endif > X } > X+ > X return 1; > X } > X > X*************** > X*** 271,276 **** > X--- 303,314 ---- > X fname, keep_name, strerror(errno)); > X }; > X set_perms (keep_name, file, NULL, 0); > X+ /* may mean an extra stat */ > X+ #ifdef HAVE_CHMOD > X+ if (!priv_backups && strip_perm(keep_name) == 0) { > X+ return 0; > X+ } > X+ #endif > X free_file (file); > X free (file); > X > SHAR_EOF > $TOUCH -am 06211315102 backup-priv-backups && > chmod 0644 backup-priv-backups || > echo "restore of backup-priv-backups failed" > set `wc -c backup-priv-backups`;Wc_c=$1 > if test "$Wc_c" != "1798"; then > echo original size 1798, current size $Wc_c > fi > # ============= options-priv-backups =============> echo "x - extracting options-priv-backups (Text)" > sed 's/^X//' << 'SHAR_EOF' > options-priv-backups && > X--- options.c.t Fri Jun 21 08:56:31 2002 > X+++ options.c Fri Jun 21 09:41:41 2002 > X@@ -21,6 +21,9 @@ > X #include "rsync.h" > X #include "popt.h" > X > X+#ifdef HAVE_CHMOD > X+int priv_backups = 0; > X+#endif > X int make_backups = 0; > X int whole_file = -1; > X int copy_links = 0; > X@@ -188,6 +191,7 @@ > X rprintf(F," -b, --backup make backups (default %s suffix)\n",BACKUP_SUFFIX); > X rprintf(F," --backup-dir make backups into this directory\n"); > X rprintf(F," --suffix=SUFFIX override backup suffix\n"); > X+ rprintf(F," --priv-backups preserve set[ug]id on backups\n"); > X rprintf(F," -u, --update update only (don't overwrite newer files)\n"); > X rprintf(F," -l, --links copy symlinks as symlinks\n"); > X rprintf(F," -L, --copy-links copy the referent of symlinks\n"); > X@@ -291,6 +295,9 @@ > X {"include-from", 0, POPT_ARG_STRING, 0, OPT_INCLUDE_FROM}, > X {"safe-links", 0, POPT_ARG_NONE, &safe_symlinks}, > X {"help", 'h', POPT_ARG_NONE, 0, 'h'}, > X+#ifdef HAVE_CHMOD > X+ {"priv-backups", 's', POPT_ARG_NONE, &priv_backups}, > X+#endif > X {"backup", 'b', POPT_ARG_NONE, &make_backups}, > X {"dry-run", 'n', POPT_ARG_NONE, &dry_run}, > X {"sparse", 'S', POPT_ARG_NONE, &sparse_files}, > SHAR_EOF > $TOUCH -am 06210941102 options-priv-backups && > chmod 0644 options-priv-backups || > echo "restore of options-priv-backups failed" > set `wc -c options-priv-backups`;Wc_c=$1 > if test "$Wc_c" != "1339"; then > echo original size 1339, current size $Wc_c > fi > exit 0 >