Re: sysupgrade: Allow to use another directory for the sets

2019-11-22 Thread Renaud Allard



On 22/11/2019 16:38, Craig Skinner wrote:

On Tue, 19 Nov 2019 10:35:56 + Stuart Henderson wrote:

We are short on partitions, there is a hard limit (14+swap), disklabel auto
defaults already use 9, and there need to be some free for typical user use
(ports, dest for "make release", people often want a separate /var/www and/or
/var/log).


Oh, I wasn't thinking of single disk desktops, but multi-drive servers
(which have plenty partition letters available to slice disks with).

Here's another idea Stuart:-

The special directory /tmp/vi.recover/ is exempt from boot & daily purging.

Could a similar /tmp/sysupgrade/ default directory suit most situations?

/tmp/ is normally mounted separately from / /home/ /var/
And probably not encrypted, nor over NFS.



Stop trying to discuss what the default should be, this has been 
discussed long enough before sysupgrade was in base. The default in 
/home/_sysupgrade is perfectly fine.


Just like sthen, I also have /tmp on MFS on multiple servers.



smime.p7s
Description: S/MIME Cryptographic Signature


Re: sysupgrade: Allow to use another directory for the sets

2019-11-22 Thread Stuart Henderson
On 2019/11/22 15:38, Craig Skinner wrote:
> Could a similar /tmp/sysupgrade/ default directory suit most situations?

My systems with MFS /tmp say no.



Re: sysupgrade: Allow to use another directory for the sets

2019-11-22 Thread Andrew Grillet
I vote for this.


On Fri, 22 Nov 2019 at 16:12, Craig Skinner  wrote:

> On Tue, 19 Nov 2019 10:35:56 + Stuart Henderson wrote:
> > We are short on partitions, there is a hard limit (14+swap), disklabel
> auto
> > defaults already use 9, and there need to be some free for typical user
> use
> > (ports, dest for "make release", people often want a separate /var/www
> and/or
> > /var/log).
>
> Oh, I wasn't thinking of single disk desktops, but multi-drive servers
> (which have plenty partition letters available to slice disks with).
>
> Here's another idea Stuart:-
>
> The special directory /tmp/vi.recover/ is exempt from boot & daily purging.
>
> Could a similar /tmp/sysupgrade/ default directory suit most situations?
>
> /tmp/ is normally mounted separately from / /home/ /var/
> And probably not encrypted, nor over NFS.
>
> Cheers,
> --
> Craig Skinner | http://linkd.in/yGqkv7
>
>


Re: sysupgrade: Allow to use another directory for the sets

2019-11-22 Thread Craig Skinner
On Tue, 19 Nov 2019 10:35:56 + Stuart Henderson wrote:
> We are short on partitions, there is a hard limit (14+swap), disklabel auto
> defaults already use 9, and there need to be some free for typical user use
> (ports, dest for "make release", people often want a separate /var/www and/or
> /var/log).

Oh, I wasn't thinking of single disk desktops, but multi-drive servers
(which have plenty partition letters available to slice disks with).

Here's another idea Stuart:-

The special directory /tmp/vi.recover/ is exempt from boot & daily purging.

Could a similar /tmp/sysupgrade/ default directory suit most situations?

/tmp/ is normally mounted separately from / /home/ /var/
And probably not encrypted, nor over NFS.

Cheers,
-- 
Craig Skinner | http://linkd.in/yGqkv7



Re: sysupgrade: Allow to use another directory for the sets

2019-11-21 Thread Raimo Niskanen
On Wed, Nov 20, 2019 at 12:06:59PM +0100, Solene Rapenne wrote:
> On Tue, Nov 12, 2019 at 07:02:56PM +0100, 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?
> 
> > 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.83 Oct 2019 12:43:58 -   1.10
> > +++ sysupgrade.812 Nov 2019 18:01:04 -
> > @@ -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,13 @@ 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 .
> >  .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 -  1.32
> > +++ sysupgrade.sh   12 Nov 2019 18:01:04 -
> > @@ -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,16 @@ 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;;
> > f)  FORCE=true;;
> > k)  KEEP=true;;
> > n)  REBOOT=false;;
> > @@ -195,7 +196,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 +204,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
> >  
> 
> I see no objection to this diff. Changes are minimal and it allows using
> another destination safely (_sysupgrade gets appended to the chosen base
> directory)
> 
> ok solene@

I am for this feature, but think the original proposal to have the -d
argument define the directory and not the prefix was better,
if it could be made safe enough.

The recognized bad case as I understand it is to specify -d / which will
erase 3 essential kernels from the upgraded installation.

I suggest to use readlink -f to normalize the argument and check that it is
not / and the question is if that is a sufficient safety measure.
If so, here is Renaud Allard's original proposal with that added
argument check instead.

I also moved the creation of the download directory (mkdir -p ${SETSDIR})
to before checking the directory properties, since it feels safer since
if the target directory does not exist there is no check on the created
directory.  If the parent directory has got the wrong owner, group,
or permissions, the created target directory might not pass the checks
for an existing traget directory.

Index: sysupgrade.8
===
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v
retrieving revision 1.10
diff -u -u -r1.10 sysupgrade.8
--- sysupgrade.83 Oct 2019 12:43:58 -   1.10
+++ sysupgrade.821 Nov 2019 09:07:33 -
@@ -24,6 +24,7 @@
 .Nm
 .Op Fl fkn
 .Op Fl r | s
+.Op Fl d Ar directory
 .Op Ar installurl
 .Sh DESCRIPTION
 .Nm
@@ -32,9 +33,7 @@
 to the next release or a new snapshot if available.
 .Pp
 .Nm
-downloads the necessary files to
-.Pa /home/_sysupgrade ,
-verifies them with
+downloads the necessary files, verifies them with
 .Xr signify 1 ,
 and copies bsd.rd to
 .Pa /bsd.upgrade .
@@ -43,8 +42,7 @@
 by default then reboots the system.
 The bootloader will automatically choose
 .Pa /bsd.upgrade ,
-triggering a one-shot upgrade using the files in
-.Pa /home/_sysupgrade .

Re: sysupgrade: Allow to use another directory for the sets

2019-11-20 Thread Renaud Allard



On 11/20/19 12:06 PM, Solene Rapenne wrote:

On Tue, Nov 12, 2019 at 07:02:56PM +0100, 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?



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.83 Oct 2019 12:43:58 -   1.10
+++ sysupgrade.812 Nov 2019 18:01:04 -
@@ -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,13 @@ 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 .
  .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 -  1.32
+++ sysupgrade.sh   12 Nov 2019 18:01:04 -
@@ -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,16 @@ 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;;
f)  FORCE=true;;
k)  KEEP=true;;
n)  REBOOT=false;;
@@ -195,7 +196,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 +204,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
  


I see no objection to this diff. Changes are minimal and it allows using
another destination safely (_sysupgrade gets appended to the chosen base
directory)

ok solene@



Thank you solene@

Anyone else?



smime.p7s
Description: S/MIME Cryptographic Signature


Re: sysupgrade: Allow to use another directory for the sets

2019-11-20 Thread Solene Rapenne
On Tue, Nov 12, 2019 at 07:02:56PM +0100, 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?

> 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 -   1.10
> +++ sysupgrade.8  12 Nov 2019 18:01:04 -
> @@ -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,13 @@ 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 .
>  .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 -  1.32
> +++ sysupgrade.sh 12 Nov 2019 18:01:04 -
> @@ -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,16 @@ 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;;
>   f)  FORCE=true;;
>   k)  KEEP=true;;
>   n)  REBOOT=false;;
> @@ -195,7 +196,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 +204,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
>  

I see no objection to this diff. Changes are minimal and it allows using
another destination safely (_sysupgrade gets appended to the chosen base
directory)

ok solene@



Re: sysupgrade: Allow to use another directory for the sets

2019-11-20 Thread Renaud Allard




On 11/20/19 10:59 AM, Solene Rapenne wrote:


no, this script makes /home/_sysupgrade a symbolic link to the real
location where you want to store the sets. So /home doesn't require more
space than the requirement for an additional symlink.



sysupgrade has a check to ensure the directory is not a link, it will 
refuse to work in that case.




Re: sysupgrade: Allow to use another directory for the sets

2019-11-20 Thread Solene Rapenne
On Wed, Nov 20, 2019 at 10:55:28AM +0100, Renaud Allard wrote:
> Hi Solene,
> 
> On 11/20/19 10:46 AM, Solene Rapenne wrote:
> > wouldn't be easier for people using non standard locations to write a
> > small wrapper script like this (not tested, written in the mail)
> > 
> > #!/bin/sh
> > 
> > MYDEST=/var/sysupgrade
> > 
> > install -d -o root -g wheel $MYDEST
> > rm -fr /home/_sysupgrade
> > ln -s $MYDEST /home/_sysupgrade
> > 
> > sysupgrade -n
> > if [ $? -eq 0 ]
> > then
> > sed -i'' "s,/home/_sysupgrade,$MYDEST" /auto_upgrade.conf
> > reboot
> > fi
> > 
> 
> It is indeed a solution, but that still means you have to download the sets
> in /home and then move them. This can be problematic if space on /home is
> constrained or in the case of slow flash disks for example.
> 


no, this script makes /home/_sysupgrade a symbolic link to the real
location where you want to store the sets. So /home doesn't require more
space than the requirement for an additional symlink.



Re: sysupgrade: Allow to use another directory for the sets

2019-11-20 Thread Renaud Allard

Hi Solene,

On 11/20/19 10:46 AM, Solene Rapenne wrote:

wouldn't be easier for people using non standard locations to write a
small wrapper script like this (not tested, written in the mail)

#!/bin/sh

MYDEST=/var/sysupgrade

install -d -o root -g wheel $MYDEST
rm -fr /home/_sysupgrade
ln -s $MYDEST /home/_sysupgrade

sysupgrade -n
if [ $? -eq 0 ]
then
sed -i'' "s,/home/_sysupgrade,$MYDEST" /auto_upgrade.conf
reboot
fi



It is indeed a solution, but that still means you have to download the 
sets in /home and then move them. This can be problematic if space on 
/home is constrained or in the case of slow flash disks for example.




smime.p7s
Description: S/MIME Cryptographic Signature


Re: sysupgrade: Allow to use another directory for the sets

2019-11-20 Thread Solene Rapenne
On Wed, Nov 20, 2019 at 10:37:02AM +0100, Raimo Niskanen wrote:
> On Tue, Nov 19, 2019 at 09:00:33AM +, Stuart Henderson wrote:
> > On 2019/11/18 14:45, Raimo Niskanen wrote:
> > > On Mon, Nov 18, 2019 at 01:23:27PM +0100, Renaud Allard wrote:
> > > > 
> > > > 
> > > > On 11/18/19 10:08 AM, Raimo Niskanen wrote:
> > > > 
> > > > > 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.
> > > > > 
> > > > 
> > > > As it was said earlier, this doesn't solve the removal issue. With your 
> > > > patch, please try "ln -s /home/_sysupgrade / ; sysupgrade".
> > > > 
> > > 
> > > This is actually not yet in my patch.  I just want to, as a first step
> > > towards either of our solutions, patch to have the /home/_sysupgrade 
> > > literal
> > > in only one place in the sysupgrade script, which would facilitate 
> > > patching
> > > the sysupgrade script before calling it, as a poor man's solution.
> > > Plus, the script gets cleaner.
> > > 
> > > The symlink suggestion is a future patch.
> > > 
> > > But I think your suggestion to instead specify the parent directory of the
> > > _syspatch directory should be sufficient to remedy the removal issue both
> > > for your command line suggestion, as for this future patch symlink
> > > suggestion.
> > > 
> > > It is a pity nobody else has responded to that prefix suggestion of yours!
> > 
> > It does avoid the removal issue but it seems an awkward user interface to
> > pass the parent directory. Perhaps better to enforce that the path matches
> > */_sysupgrade, though this also doesn't feel quite right. Perhaps add
> > /_sysupgrade if it wasn't already present...
> > 
> > > I think the feature itself is more important than if it is implemented
> > > with a command line argument or a symlink.
> > > 
> > > Other than that.  What do you or anyone else think about my argument that
> > > the location of the system upgrade download directory is an installation
> > > configuration that will not change between upgrades and hence it would be
> > > nice to not have to remember the command line argument for every future
> > > sysupgrade.
> > 
> > Generally agree, but I'm not keen on a proliferation of tiny pseudo-config
> > files, and this feels maybe part of something bigger. A similar argument
> > could be made for the choice of release/stable vs snaps (-s / -r here,
> > -Dsnap in pkg_add) which could be in some kind of "system config" alongside
> > source URLs (base, packages, firmware), a list of sets, maybe some pkg_add
> > related things (whether to install debug packages?), ...
> 
> In that case, in the current state of the matter, since there is no such
> common config file yet, and all other tols use command line arguments for
> similar features, we should use a command line argument here too, like
> Renaud Allard proposes.  Until there is a suitable config file.
> 
> Since the prefix solution is regarded as awkward, and we want go guard
> against clumsy admins using / as sysupgrade directory which would remove
> kernels from the updated installation, why not normalize the -d argument
> with readlink -f and simply check that it is not / .  There are of
> course othere directories that are inappropriate to use, but the person
> doing the upgrade has to be trusted to some degree...
> 
> And I see no reason to not at least apply my minimalistic patch suggestion
> that simply defines the upgrade directory in only one place in sysupgrade.
> -- 
> 
> / Raimo Niskanen, Erlang/OTP, Ericsson AB
> 

wouldn't be easier for people using non standard locations to write a
small wrapper script like this (not tested, written in the mail)

#!/bin/sh

MYDEST=/var/sysupgrade

install -d -o root -g wheel $MYDEST
rm -fr /home/_sysupgrade
ln -s $MYDEST /home/_sysupgrade

sysupgrade -n
if [ $? -eq 0 ]
then
sed -i'' "s,/home/_sysupgrade,$MYDEST" /auto_upgrade.conf
reboot
fi



Re: sysupgrade: Allow to use another directory for the sets

2019-11-20 Thread Raimo Niskanen
On Tue, Nov 19, 2019 at 09:00:33AM +, Stuart Henderson wrote:
> On 2019/11/18 14:45, Raimo Niskanen wrote:
> > On Mon, Nov 18, 2019 at 01:23:27PM +0100, Renaud Allard wrote:
> > > 
> > > 
> > > On 11/18/19 10:08 AM, Raimo Niskanen wrote:
> > > 
> > > > 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.
> > > > 
> > > 
> > > As it was said earlier, this doesn't solve the removal issue. With your 
> > > patch, please try "ln -s /home/_sysupgrade / ; sysupgrade".
> > > 
> > 
> > This is actually not yet in my patch.  I just want to, as a first step
> > towards either of our solutions, patch to have the /home/_sysupgrade literal
> > in only one place in the sysupgrade script, which would facilitate patching
> > the sysupgrade script before calling it, as a poor man's solution.
> > Plus, the script gets cleaner.
> > 
> > The symlink suggestion is a future patch.
> > 
> > But I think your suggestion to instead specify the parent directory of the
> > _syspatch directory should be sufficient to remedy the removal issue both
> > for your command line suggestion, as for this future patch symlink
> > suggestion.
> > 
> > It is a pity nobody else has responded to that prefix suggestion of yours!
> 
> It does avoid the removal issue but it seems an awkward user interface to
> pass the parent directory. Perhaps better to enforce that the path matches
> */_sysupgrade, though this also doesn't feel quite right. Perhaps add
> /_sysupgrade if it wasn't already present...
> 
> > I think the feature itself is more important than if it is implemented
> > with a command line argument or a symlink.
> > 
> > Other than that.  What do you or anyone else think about my argument that
> > the location of the system upgrade download directory is an installation
> > configuration that will not change between upgrades and hence it would be
> > nice to not have to remember the command line argument for every future
> > sysupgrade.
> 
> Generally agree, but I'm not keen on a proliferation of tiny pseudo-config
> files, and this feels maybe part of something bigger. A similar argument
> could be made for the choice of release/stable vs snaps (-s / -r here,
> -Dsnap in pkg_add) which could be in some kind of "system config" alongside
> source URLs (base, packages, firmware), a list of sets, maybe some pkg_add
> related things (whether to install debug packages?), ...

In that case, in the current state of the matter, since there is no such
common config file yet, and all other tols use command line arguments for
similar features, we should use a command line argument here too, like
Renaud Allard proposes.  Until there is a suitable config file.

Since the prefix solution is regarded as awkward, and we want go guard
against clumsy admins using / as sysupgrade directory which would remove
kernels from the updated installation, why not normalize the -d argument
with readlink -f and simply check that it is not / .  There are of
course othere directories that are inappropriate to use, but the person
doing the upgrade has to be trusted to some degree...

And I see no reason to not at least apply my minimalistic patch suggestion
that simply defines the upgrade directory in only one place in sysupgrade.
-- 

/ Raimo Niskanen, Erlang/OTP, Ericsson AB



Re: sysupgrade: Allow to use another directory for the sets

2019-11-19 Thread Theo de Raadt
Craig Skinner  wrote:

> On Thu, 7 Nov 2019 14:42:32 + Stuart Henderson wrote:
> > On 2019/11/07 11:15, Craig Skinner wrote:
> > > On Wed, 6 Nov 2019 13:41:07 +0100 Renaud Allard wrote:
> > > > Given the amount of people which encrypt /home directory on their 
> > > > servers, it might be useful to be able to define another directory for 
> > > > the sets in sysupgrade as /home_sysupgrade will not be available in 
> > > > that 
> > > > case.
> > > 
> > > How about /var/cache/sysupgrade/ as the default?
> > > 
> > > i.e: sysupgrade caches files which are variable over time.
> > > 
> > > 
> > 
> > The merits of different filesystems were already discussed when choosing the
> > current default - disklabel auto layout usually leaves a fair amount of 
> > space
> > in /home, reduces risk of running out of space in a fairly import system fs,
> > and avoids consuming space on an fs where files are unpacked during the
> > upgrade install.
> > 
> 
> If the installer created a 750Mb /var/cache/ partition, and sysupgrade's cache
> directory is hard coded as /var/cache/sysupgrade/, would that simply solve
> the various problems people are having & scripting difficulties?
> 
> Other tools which cache files in /home/ or /var/db/ could also use /var/cache/

crazy.  not going to happen.



Re: sysupgrade: Allow to use another directory for the sets

2019-11-19 Thread Raimo Niskanen
On Tue, Nov 19, 2019 at 09:00:33AM +, Stuart Henderson wrote:
> On 2019/11/18 14:45, Raimo Niskanen wrote:
> > On Mon, Nov 18, 2019 at 01:23:27PM +0100, Renaud Allard wrote:
> > > 
> > > 
> > > On 11/18/19 10:08 AM, Raimo Niskanen wrote:
> > > 
> > > > 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.
> > > > 
> > > 
> > > As it was said earlier, this doesn't solve the removal issue. With your 
> > > patch, please try "ln -s /home/_sysupgrade / ; sysupgrade".
> > > 
> > 
> > This is actually not yet in my patch.  I just want to, as a first step
> > towards either of our solutions, patch to have the /home/_sysupgrade literal
> > in only one place in the sysupgrade script, which would facilitate patching
> > the sysupgrade script before calling it, as a poor man's solution.
> > Plus, the script gets cleaner.
> > 
> > The symlink suggestion is a future patch.
> > 
> > But I think your suggestion to instead specify the parent directory of the
> > _syspatch directory should be sufficient to remedy the removal issue both
> > for your command line suggestion, as for this future patch symlink
> > suggestion.
> > 
> > It is a pity nobody else has responded to that prefix suggestion of yours!
> 
> It does avoid the removal issue but it seems an awkward user interface to
> pass the parent directory. Perhaps better to enforce that the path matches

Yes.

> */_sysupgrade, though this also doesn't feel quite right. Perhaps add
> /_sysupgrade if it wasn't already present...

Or, since readlink -f normalizes the path one could simply check that
the result is not / which should fix the important case.

> 
> > I think the feature itself is more important than if it is implemented
> > with a command line argument or a symlink.
> > 
> > Other than that.  What do you or anyone else think about my argument that
> > the location of the system upgrade download directory is an installation
> > configuration that will not change between upgrades and hence it would be
> > nice to not have to remember the command line argument for every future
> > sysupgrade.
> 
> Generally agree, but I'm not keen on a proliferation of tiny pseudo-config
> files, and this feels maybe part of something bigger. A similar argument
> could be made for the choice of release/stable vs snaps (-s / -r here,
> -Dsnap in pkg_add) which could be in some kind of "system config" alongside
> source URLs (base, packages, firmware), a list of sets, maybe some pkg_add
> related things (whether to install debug packages?), ...

Agreed. /etc/installurl is now a tiny one item config file but was once
a different file containing more items.

Perhaps something to go back to?
-- 

/ Raimo Niskanen, Erlang/OTP, Ericsson AB



Re: sysupgrade: Allow to use another directory for the sets

2019-11-19 Thread Renaud Allard




On 11/19/19 11:11 AM, Craig Skinner wrote:


If the installer created a 750Mb /var/cache/ partition, and sysupgrade's cache
directory is hard coded as /var/cache/sysupgrade/, would that simply solve
the various problems people are having & scripting difficulties?

Other tools which cache files in /home/ or /var/db/ could also use /var/cache/

If other tools store data in /var/cache, how are you sure 750Mb will be 
enough for everyone? /home is probably the safest _default_, but there 
are clearly reasons for an override in some cases.




Re: sysupgrade: Allow to use another directory for the sets

2019-11-19 Thread Stuart Henderson
On 2019/11/19 10:11, Craig Skinner wrote:
> On Thu, 7 Nov 2019 14:42:32 + Stuart Henderson wrote:
> > On 2019/11/07 11:15, Craig Skinner wrote:
> > > On Wed, 6 Nov 2019 13:41:07 +0100 Renaud Allard wrote:
> > > > Given the amount of people which encrypt /home directory on their 
> > > > servers, it might be useful to be able to define another directory for 
> > > > the sets in sysupgrade as /home_sysupgrade will not be available in 
> > > > that 
> > > > case.
> > > 
> > > How about /var/cache/sysupgrade/ as the default?
> > > 
> > > i.e: sysupgrade caches files which are variable over time.
> > > 
> > > 
> > 
> > The merits of different filesystems were already discussed when choosing the
> > current default - disklabel auto layout usually leaves a fair amount of 
> > space
> > in /home, reduces risk of running out of space in a fairly import system fs,
> > and avoids consuming space on an fs where files are unpacked during the
> > upgrade install.
> > 
> 
> If the installer created a 750Mb /var/cache/ partition, and sysupgrade's cache
> directory is hard coded as /var/cache/sysupgrade/, would that simply solve
> the various problems people are having & scripting difficulties?
> 
> Other tools which cache files in /home/ or /var/db/ could also use /var/cache/
> 
> 
> Cheers,
> -- 
> Craig Skinner | http://linkd.in/yGqkv7
> 

We are short on partitions, there is a hard limit (14+swap), disklabel auto
defaults already use 9, and there need to be some free for typical user use
(ports, dest for "make release", people often want a separate /var/www and/or
/var/log).



Re: sysupgrade: Allow to use another directory for the sets

2019-11-19 Thread Renaud Allard



On 11/19/19 10:00 AM, Stuart Henderson wrote:

It does avoid the removal issue but it seems an awkward user interface to
pass the parent directory. Perhaps better to enforce that the path matches
*/_sysupgrade, though this also doesn't feel quite right. Perhaps add
/_sysupgrade if it wasn't already present...



That's what I proposed in my latest patch, including the forcing of non 
removal if -d is used. Did you check my latest patch?




smime.p7s
Description: S/MIME Cryptographic Signature


Re: sysupgrade: Allow to use another directory for the sets

2019-11-19 Thread Craig Skinner
On Thu, 7 Nov 2019 14:42:32 + Stuart Henderson wrote:
> On 2019/11/07 11:15, Craig Skinner wrote:
> > On Wed, 6 Nov 2019 13:41:07 +0100 Renaud Allard wrote:
> > > Given the amount of people which encrypt /home directory on their 
> > > servers, it might be useful to be able to define another directory for 
> > > the sets in sysupgrade as /home_sysupgrade will not be available in that 
> > > case.
> > 
> > How about /var/cache/sysupgrade/ as the default?
> > 
> > i.e: sysupgrade caches files which are variable over time.
> > 
> > 
> 
> The merits of different filesystems were already discussed when choosing the
> current default - disklabel auto layout usually leaves a fair amount of space
> in /home, reduces risk of running out of space in a fairly import system fs,
> and avoids consuming space on an fs where files are unpacked during the
> upgrade install.
> 

If the installer created a 750Mb /var/cache/ partition, and sysupgrade's cache
directory is hard coded as /var/cache/sysupgrade/, would that simply solve
the various problems people are having & scripting difficulties?

Other tools which cache files in /home/ or /var/db/ could also use /var/cache/


Cheers,
-- 
Craig Skinner | http://linkd.in/yGqkv7



Re: sysupgrade: Allow to use another directory for the sets

2019-11-19 Thread Stuart Henderson
On 2019/11/18 14:45, Raimo Niskanen wrote:
> On Mon, Nov 18, 2019 at 01:23:27PM +0100, Renaud Allard wrote:
> > 
> > 
> > On 11/18/19 10:08 AM, Raimo Niskanen wrote:
> > 
> > > 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.
> > > 
> > 
> > As it was said earlier, this doesn't solve the removal issue. With your 
> > patch, please try "ln -s /home/_sysupgrade / ; sysupgrade".
> > 
> 
> This is actually not yet in my patch.  I just want to, as a first step
> towards either of our solutions, patch to have the /home/_sysupgrade literal
> in only one place in the sysupgrade script, which would facilitate patching
> the sysupgrade script before calling it, as a poor man's solution.
> Plus, the script gets cleaner.
> 
> The symlink suggestion is a future patch.
> 
> But I think your suggestion to instead specify the parent directory of the
> _syspatch directory should be sufficient to remedy the removal issue both
> for your command line suggestion, as for this future patch symlink
> suggestion.
> 
> It is a pity nobody else has responded to that prefix suggestion of yours!

It does avoid the removal issue but it seems an awkward user interface to
pass the parent directory. Perhaps better to enforce that the path matches
*/_sysupgrade, though this also doesn't feel quite right. Perhaps add
/_sysupgrade if it wasn't already present...

> I think the feature itself is more important than if it is implemented
> with a command line argument or a symlink.
> 
> Other than that.  What do you or anyone else think about my argument that
> the location of the system upgrade download directory is an installation
> configuration that will not change between upgrades and hence it would be
> nice to not have to remember the command line argument for every future
> sysupgrade.

Generally agree, but I'm not keen on a proliferation of tiny pseudo-config
files, and this feels maybe part of something bigger. A similar argument
could be made for the choice of release/stable vs snaps (-s / -r here,
-Dsnap in pkg_add) which could be in some kind of "system config" alongside
source URLs (base, packages, firmware), a list of sets, maybe some pkg_add
related things (whether to install debug packages?), ...



Re: sysupgrade: Allow to use another directory for the sets

2019-11-18 Thread Raimo Niskanen
On Mon, Nov 18, 2019 at 01:23:27PM +0100, Renaud Allard wrote:
> 
> 
> On 11/18/19 10:08 AM, Raimo Niskanen wrote:
> 
> > 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.
> > 
> 
> As it was said earlier, this doesn't solve the removal issue. With your 
> patch, please try "ln -s /home/_sysupgrade / ; sysupgrade".
> 

This is actually not yet in my patch.  I just want to, as a first step
towards either of our solutions, patch to have the /home/_sysupgrade literal
in only one place in the sysupgrade script, which would facilitate patching
the sysupgrade script before calling it, as a poor man's solution.
Plus, the script gets cleaner.

The symlink suggestion is a future patch.

But I think your suggestion to instead specify the parent directory of the
_syspatch directory should be sufficient to remedy the removal issue both
for your command line suggestion, as for this future patch symlink
suggestion.

It is a pity nobody else has responded to that prefix suggestion of yours!

I think the feature itself is more important than if it is implemented
with a command line argument or a symlink.

Other than that.  What do you or anyone else think about my argument that
the location of the system upgrade download directory is an installation
configuration that will not change between upgrades and hence it would be
nice to not have to remember the command line argument for every future
sysupgrade.

Best regards
-- 

/ Raimo Niskanen, Erlang/OTP, Ericsson AB



Re: sysupgrade: Allow to use another directory for the sets

2019-11-18 Thread Renaud Allard



On 11/18/19 10:08 AM, Raimo Niskanen wrote:


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.



As it was said earlier, this doesn't solve the removal issue. With your 
patch, please try "ln -s /home/_sysupgrade / ; sysupgrade".




smime.p7s
Description: S/MIME Cryptographic Signature


Re: sysupgrade: Allow to use another directory for the sets

2019-11-18 Thread Raimo Niskanen
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 -  1.25
+++ usr.sbin/sysupgrade/sysupgrade.sh   14 Nov 2019 13:27:34 -
@@ -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 -   1.10
> +++ sysupgrade.8  14 Nov 2019 09:29:15 -
> @@ -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 -  1.32
> +++ sysupgrade.sh 14 Nov 2019 09:29:15 -
> @@ -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]"
>  }
>  

Re: sysupgrade: Allow to use another directory for the sets

2019-11-14 Thread Renaud Allard



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?
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 -	1.10
+++ sysupgrade.8	14 Nov 2019 09:29:15 -
@@ -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 -	1.32
+++ sysupgrade.sh	14 Nov 2019 09:29:15 -
@@ -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
 


Re: sysupgrade: Allow to use another directory for the sets

2019-11-13 Thread Renaud Allard



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?




smime.p7s
Description: S/MIME Cryptographic Signature


Re: sysupgrade: Allow to use another directory for the sets

2019-11-12 Thread Renaud Allard



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?
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.83 Oct 2019 12:43:58 -   1.10
+++ sysupgrade.812 Nov 2019 18:01:04 -
@@ -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,13 @@ 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 .
 .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 -  1.32
+++ sysupgrade.sh   12 Nov 2019 18:01:04 -
@@ -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,16 @@ 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;;
f)  FORCE=true;;
k)  KEEP=true;;
n)  REBOOT=false;;
@@ -195,7 +196,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 +204,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
 


Re: sysupgrade: Allow to use another directory for the sets

2019-11-12 Thread Renaud Allard




On 11/12/19 8:29 AM, Theo de Raadt wrote:

Renaud Allard  wrote:


+.It Fl d Ar directory
+Choose the
+.Ar directory
+in which the sets will be downloaded.
+Default is
+.Pa /home/_sysupgrade .


...


+   d)  SETSDIR=${OPTARG};;


...


-rm -f /home/_sysupgrade/{${CLEAN}}
+rm -f ${SETSDIR}/{${CLEAN}}


Renaud, please test it for me like this:

  sysupgrade -d /

This interface is dangerously incorrect.



No need to blank the disk :)

What about a prefix approach then? Like if you specify /var, it goes to 
/var/_sysupgrade.




Re: sysupgrade: Allow to use another directory for the sets

2019-11-11 Thread Theo de Raadt
Renaud Allard  wrote:

> +.It Fl d Ar directory
> +Choose the
> +.Ar directory
> +in which the sets will be downloaded.
> +Default is
> +.Pa /home/_sysupgrade .

...

> + d)  SETSDIR=${OPTARG};;

...

> -rm -f /home/_sysupgrade/{${CLEAN}}
> +rm -f ${SETSDIR}/{${CLEAN}}

Renaud, please test it for me like this:

 sysupgrade -d /

This interface is dangerously incorrect.



Re: sysupgrade: Allow to use another directory for the sets

2019-11-11 Thread Jason McIntyre
On Tue, Nov 12, 2019 at 08:06:52AM +0100, Renaud Allard wrote:
> 
> 
> On 09/11/2019 12:52, Klemens Nanni wrote:
> > On Fri, Nov 08, 2019 at 11:59:20AM +, Stuart Henderson wrote:
> >>> Given the amount of people which encrypt /home directory on their servers,
> >>> it might be useful to be able to define another directory for the sets in
> >>> sysupgrade as /home_sysupgrade will not be available in that case.
> >>
> >> This (encrypted /home but not boot-loader-assisted FDE) doesn't seem
> >> like it would be all that common a thing to me, but I can think of
> >> other use cases for moving the directory.
> >>
> >> I know a similar request was rejected in earlier sysupgrade, but now
> >> that sysupgrade uses /auto_upgrade.conf rather than special code in
> >> install.sub it seems a reasonable change to make.
> >>
> >>> +.It Fl d Ar directory
> >>> +Choose the
> >>
> >> Nit: trailing whitespace, otherwise it's OK sthen@
> > I agree with sthen here, but the diff still misses an update to usage();
> > 
> > SETSDIR values containing spaces will also blow up the script.  Not
> > reachable due to earlier errors, but this is worth noting as we install
> > `rm -f ${SETSDIR}{${CLEAN}}` into rc.firsttime(8) which is run as root,
> > so careful with blindly rm(1)'ing user input.
> > 
> 
> Here a reviewed patch for the whitespace and usage() update.
> 
> Maybe we can already commit this while trying to find an elegant 
> solution to the path with whitespace. Any suggestions into solving the 
> whitespace issue?


hi.

in both SYNOPSIS and usage() single letter options go first. so you want
to put -d after fkn. in this particular case it should probably go after
-r | -s too i.e. just before "installurl".

jmc

> 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 -   1.10
> +++ sysupgrade.8  12 Nov 2019 07:04:05 -
> @@ -22,6 +22,7 @@
>  .Nd upgrade system to the next release or a new snapshot
>  .Sh SYNOPSIS
>  .Nm
> +.Op Fl d Ar directory
>  .Op Fl fkn
>  .Op Fl r | s
>  .Op Ar installurl
> @@ -48,6 +49,12 @@ triggering a one-shot upgrade using the 
>  .Pp
>  The options are as follows:
>  .Bl -tag -width Ds
> +.It Fl d Ar directory
> +Choose the
> +.Ar directory
> +in which the sets will be downloaded.
> +Default is
> +.Pa /home/_sysupgrade .
>  .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 -  1.32
> +++ sysupgrade.sh 12 Nov 2019 07:04:05 -
> @@ -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] [-d directory] [-r | -s] [installurl]"
>  }
>  
>  unpriv()
> @@ -73,14 +72,16 @@ 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};;
>   f)  FORCE=true;;
>   k)  KEEP=true;;
>   n)  REBOOT=false;;
> @@ -195,7 +196,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 +204,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
>  



Re: sysupgrade: Allow to use another directory for the sets

2019-11-11 Thread Renaud Allard



On 09/11/2019 12:52, Klemens Nanni wrote:

On Fri, Nov 08, 2019 at 11:59:20AM +, Stuart Henderson wrote:

Given the amount of people which encrypt /home directory on their servers,
it might be useful to be able to define another directory for the sets in
sysupgrade as /home_sysupgrade will not be available in that case.


This (encrypted /home but not boot-loader-assisted FDE) doesn't seem
like it would be all that common a thing to me, but I can think of
other use cases for moving the directory.

I know a similar request was rejected in earlier sysupgrade, but now
that sysupgrade uses /auto_upgrade.conf rather than special code in
install.sub it seems a reasonable change to make.


+.It Fl d Ar directory
+Choose the


Nit: trailing whitespace, otherwise it's OK sthen@

I agree with sthen here, but the diff still misses an update to usage();

SETSDIR values containing spaces will also blow up the script.  Not
reachable due to earlier errors, but this is worth noting as we install
`rm -f ${SETSDIR}{${CLEAN}}` into rc.firsttime(8) which is run as root,
so careful with blindly rm(1)'ing user input.



Here a reviewed patch for the whitespace and usage() update.

Maybe we can already commit this while trying to find an elegant 
solution to the path with whitespace. Any suggestions into solving the 
whitespace issue?
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.83 Oct 2019 12:43:58 -   1.10
+++ sysupgrade.812 Nov 2019 07:04:05 -
@@ -22,6 +22,7 @@
 .Nd upgrade system to the next release or a new snapshot
 .Sh SYNOPSIS
 .Nm
+.Op Fl d Ar directory
 .Op Fl fkn
 .Op Fl r | s
 .Op Ar installurl
@@ -48,6 +49,12 @@ triggering a one-shot upgrade using the 
 .Pp
 The options are as follows:
 .Bl -tag -width Ds
+.It Fl d Ar directory
+Choose the
+.Ar directory
+in which the sets will be downloaded.
+Default is
+.Pa /home/_sysupgrade .
 .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 -  1.32
+++ sysupgrade.sh   12 Nov 2019 07:04:05 -
@@ -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] [-d directory] [-r | -s] [installurl]"
 }
 
 unpriv()
@@ -73,14 +72,16 @@ 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};;
f)  FORCE=true;;
k)  KEEP=true;;
n)  REBOOT=false;;
@@ -195,7 +196,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 +204,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
 


Re: sysupgrade: Allow to use another directory for the sets

2019-11-09 Thread Klemens Nanni
On Fri, Nov 08, 2019 at 11:59:20AM +, Stuart Henderson wrote:
> > Given the amount of people which encrypt /home directory on their servers,
> > it might be useful to be able to define another directory for the sets in
> > sysupgrade as /home_sysupgrade will not be available in that case.
> 
> This (encrypted /home but not boot-loader-assisted FDE) doesn't seem
> like it would be all that common a thing to me, but I can think of
> other use cases for moving the directory.
> 
> I know a similar request was rejected in earlier sysupgrade, but now
> that sysupgrade uses /auto_upgrade.conf rather than special code in
> install.sub it seems a reasonable change to make.
> 
> > +.It Fl d Ar directory
> > +Choose the 
> 
> Nit: trailing whitespace, otherwise it's OK sthen@
I agree with sthen here, but the diff still misses an update to usage();

SETSDIR values containing spaces will also blow up the script.  Not
reachable due to earlier errors, but this is worth noting as we install
`rm -f ${SETSDIR}{${CLEAN}}` into rc.firsttime(8) which is run as root,
so careful with blindly rm(1)'ing user input.



Re: sysupgrade: Allow to use another directory for the sets

2019-11-08 Thread Stuart Henderson
On 2019/11/06 13:41, Renaud Allard wrote:
> Hello,
> 
> Given the amount of people which encrypt /home directory on their servers,
> it might be useful to be able to define another directory for the sets in
> sysupgrade as /home_sysupgrade will not be available in that case.

This (encrypted /home but not boot-loader-assisted FDE) doesn't seem
like it would be all that common a thing to me, but I can think of
other use cases for moving the directory.

I know a similar request was rejected in earlier sysupgrade, but now
that sysupgrade uses /auto_upgrade.conf rather than special code in
install.sub it seems a reasonable change to make.

> +.It Fl d Ar directory
> +Choose the 

Nit: trailing whitespace, otherwise it's OK sthen@





Re: sysupgrade: Allow to use another directory for the sets

2019-11-07 Thread Craig Skinner
On Wed, 6 Nov 2019 13:41:07 +0100 Renaud Allard wrote:
> Given the amount of people which encrypt /home directory on their 
> servers, it might be useful to be able to define another directory for 
> the sets in sysupgrade as /home_sysupgrade will not be available in that 
> case.

How about /var/cache/sysupgrade/ as the default?

i.e: sysupgrade caches files which are variable over time.


Cheers,
-- 
Craig Skinner | http://linkd.in/yGqkv7



sysupgrade: Allow to use another directory for the sets

2019-11-06 Thread Renaud Allard

Hello,

Given the amount of people which encrypt /home directory on their 
servers, it might be useful to be able to define another directory for 
the sets in sysupgrade as /home_sysupgrade will not be available in that 
case.

Here is a patch for this.

Regards
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 -	1.10
+++ sysupgrade.8	6 Nov 2019 12:36:48 -
@@ -22,6 +22,7 @@
 .Nd upgrade system to the next release or a new snapshot
 .Sh SYNOPSIS
 .Nm
+.Op Fl d Ar directory
 .Op Fl fkn
 .Op Fl r | s
 .Op Ar installurl
@@ -48,6 +49,12 @@ triggering a one-shot upgrade using the 
 .Pp
 The options are as follows:
 .Bl -tag -width Ds
+.It Fl d Ar directory
+Choose the 
+.Ar directory
+in which the sets will be downloaded.
+Default is
+.Pa /home/_sysupgrade .
 .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.30
diff -u -p -r1.30 sysupgrade.sh
--- sysupgrade.sh	3 Nov 2019 18:22:45 -	1.30
+++ sysupgrade.sh	6 Nov 2019 12:36:48 -
@@ -25,7 +25,6 @@ umask 0022
 export PATH=/usr/bin:/bin:/usr/sbin:/sbin
 
 ARCH=$(uname -m)
-SETSDIR=/home/_sysupgrade
 
 ug_err()
 {
@@ -73,14 +72,16 @@ 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};;
 	f)	FORCE=true;;
 	k)	KEEP=true;;
 	n)	REBOOT=false;;
@@ -192,7 +193,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
@@ -200,7 +201,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
 


smime.p7s
Description: S/MIME Cryptographic Signature