strip setuid/setgid bits on backup (was Re: small security-related rsync extension)

[Martin Pool]

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 at 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 at 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
>