On Thu, Nov 14, 2019 at 10:32:43AM +0100, Renaud Allard wrote:
> 
> 
> On 11/13/19 11:51 PM, Renaud Allard wrote:
> > 
> > 
> > On 12/11/2019 19:02, Renaud Allard wrote:
> >>
> >>
> >> On 12/11/2019 08:29, Theo de Raadt wrote:
> >>>
> >>> Renaud, please test it for me like this:
> >>>
> >>>       sysupgrade -d /
> >>>
> >>> This interface is dangerously incorrect.
> >>>
> >>
> >> What about this one?
> > 
> > ping.
> > 
> > I haven't seen any reply on the prefix based patch, but what about also 
> > making -k (Keep the files in /home/_sysupgrade) implicit in case -d is 
> > used?
> > 
> 
> Here is a patch which disables the rm (enables -k) when -d is used
> 
> This will require a little bit more work from the admin side, but at 
> least there is no real danger of removal of wrong files.
> 
> Any comment?

I too have a need for this kind of feature, but in my use case it is our
lab that has NFS auto mounted /home/* directories, so /home is a mount
point for the automounter.

We could start with a minimalistic patch to clean up that /home/_sysupgrade
is mentioned in 3 places in the sysupgrade script.  After fixing that the
sysadmin need only change the line SETSDIR=/home/_sysupgrade to change the
sysupgrade directory, which could be a non-documented way to do it.

To implement a more supported feature I think that the sysupgrade
directory is a property of the installation; a specific installation
will always use the same sysupgrade directory.  Hence this should be
a configuration parameter instead of a command line argument.

A configuration parameter could be accomplished through an optional symlink
/etc/_sysupgrade that the sysadmin can create to point at the installation's
sysupgrade directory.  The sysupgrade script would test -s /etc/_sysupgrade
and if there is a symlink readlink -f /etc/_sysupgrade to get SETSDIR.

Does that sound all right?

Anyway, here is my minimalistic patch for script cleanup; the changes for
mkdir is there to detect if e.g an unexpected umask, or incorrect owner of
the parent directory creates a directory with wrong properties:

Index: usr.sbin/sysupgrade/sysupgrade.sh
===================================================================
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
retrieving revision 1.25
diff -u -u -r1.25 sysupgrade.sh
--- usr.sbin/sysupgrade/sysupgrade.sh   28 Sep 2019 17:30:07 -0000      1.25
+++ usr.sbin/sysupgrade/sysupgrade.sh   14 Nov 2019 13:27:34 -0000
@@ -119,6 +119,7 @@
        URL=${MIRROR}/${NEXT_VERSION}/${ARCH}/
 fi
 
+[[ -e ${SETSDIR} ]] || mkdir -p ${SETSDIR}
 if [[ -e ${SETSDIR} ]]; then
        eval $(stat -s ${SETSDIR})
        [[ $st_uid -eq 0 ]] ||
@@ -127,8 +128,6 @@
                 ug_err "${SETSDIR} needs to be owned by root:wheel"
        [[ $st_mode -eq 040755 ]] || 
                ug_err "${SETSDIR} is not a directory with permissions 0755"
-else
-       mkdir -p ${SETSDIR}
 fi
 
 cd ${SETSDIR}
@@ -185,7 +184,7 @@
 
 cat <<__EOT >/auto_upgrade.conf
 Location of sets = disk
-Pathname to the sets = /home/_sysupgrade/
+Pathname to the sets = ${SETSDIR}/
 Set name(s) = done
 Directory does not contain SHA256.sig. Continue without verification = yes
 __EOT
@@ -193,7 +192,7 @@
 if ! ${KEEP}; then
        CLEAN=$(echo SHA256 ${SETS} | sed -e 's/ /,/g')
        cat <<__EOT > /etc/rc.firsttime
-rm -f /home/_sysupgrade/{${CLEAN}}
+rm -f ${SETSDIR}/{${CLEAN}}
 __EOT
 fi


 

> Index: sysupgrade.8
> ===================================================================
> RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v
> retrieving revision 1.10
> diff -u -p -r1.10 sysupgrade.8
> --- sysupgrade.8      3 Oct 2019 12:43:58 -0000       1.10
> +++ sysupgrade.8      14 Nov 2019 09:29:15 -0000
> @@ -24,6 +24,7 @@
>  .Nm
>  .Op Fl fkn
>  .Op Fl r | s
> +.Op Fl d Ar directory
>  .Op Ar installurl
>  .Sh DESCRIPTION
>  .Nm
> @@ -48,6 +49,14 @@ triggering a one-shot upgrade using the 
>  .Pp
>  The options are as follows:
>  .Bl -tag -width Ds
> +.It Fl d Ar directory
> +Choose the prefix of the
> +.Ar directory
> +in which the sets will be downloaded.
> +_sysupgrade will be appended to that name.
> +Default is
> +.Pa /home .
> +This will also implicitely force -k flag.
>  .It Fl f
>  Force an already applied upgrade.
>  The default is to upgrade to latest snapshot only if available.
> Index: sysupgrade.sh
> ===================================================================
> RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
> retrieving revision 1.32
> diff -u -p -r1.32 sysupgrade.sh
> --- sysupgrade.sh     11 Nov 2019 18:26:52 -0000      1.32
> +++ sysupgrade.sh     14 Nov 2019 09:29:15 -0000
> @@ -25,7 +25,6 @@ umask 0022
>  export PATH=/usr/bin:/bin:/usr/sbin:/sbin
>  
>  ARCH=$(uname -m)
> -SETSDIR=/home/_sysupgrade
>  
>  ug_err()
>  {
> @@ -34,7 +33,7 @@ ug_err()
>  
>  usage()
>  {
> -     ug_err "usage: ${0##*/} [-fkn] [-r | -s] [installurl]"
> +     ug_err "usage: ${0##*/} [-fkn] [-r | -s] [-d directory] [installurl]"
>  }
>  
>  unpriv()
> @@ -73,14 +72,18 @@ rmel() {
>       echo -n "$_c"
>  }
>  
> +SETSDIR=/home/_sysupgrade
>  RELEASE=false
>  SNAP=false
>  FORCE=false
>  KEEP=false
>  REBOOT=true
>  
> -while getopts fknrs arg; do
> +while getopts d:fknrs arg; do
>       case ${arg} in
> +     d)      SETSDIR=${OPTARG}/_sysupgrade
> +             KEEP=true
> +             ;;
>       f)      FORCE=true;;
>       k)      KEEP=true;;
>       n)      REBOOT=false;;
> @@ -195,7 +198,7 @@ ${KEEP} && > keep
>  
>  cat <<__EOT >/auto_upgrade.conf
>  Location of sets = disk
> -Pathname to the sets = /home/_sysupgrade/
> +Pathname to the sets = ${SETSDIR}
>  Set name(s) = done
>  Directory does not contain SHA256.sig. Continue without verification = yes
>  __EOT
> @@ -203,7 +206,7 @@ __EOT
>  if ! ${KEEP}; then
>       CLEAN=$(echo SHA256 ${SETS} | sed -e 's/ /,/g')
>       cat <<__EOT > /etc/rc.firsttime
> -rm -f /home/_sysupgrade/{${CLEAN}}
> +rm -f ${SETSDIR}/{${CLEAN}}
>  __EOT
>  fi
>  


-- 

/ Raimo Niskanen, Erlang/OTP, Ericsson AB

Reply via email to