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.8 3 Oct 2019 12:43:58 -0000 1.10
> > +++ sysupgrade.8 12 Nov 2019 18:01:04 -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,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 -0000 1.32
> > +++ sysupgrade.sh 12 Nov 2019 18:01:04 -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,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.8 3 Oct 2019 12:43:58 -0000 1.10
+++ sysupgrade.8 21 Nov 2019 09:07:33 -0000
@@ -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 .
+triggering a one-shot upgrade using the downloaded files.
.Pp
The options are as follows:
.Bl -tag -width Ds
@@ -53,8 +51,7 @@
The default is to upgrade to latest snapshot only if available.
This option has no effect on releases.
.It Fl k
-Keep the files in
-.Pa /home/_sysupgrade .
+Keep the downloaded files.
By default they will be deleted after the upgrade.
.It Fl n
Fetch and verify the files and create
@@ -66,6 +63,12 @@
.It Fl s
Upgrade to a snapshot.
This is the default if the system is currently running a snapshot.
+.It Fl d Ar directory
+Choose the
+.Ar directory
+in which the sets will be downloaded.
+Default is
+.Pa /home/_sysupgrade .
.El
.Sh FILES
.Bl -tag -width "/auto_upgrade.conf" -compact
@@ -77,7 +80,7 @@
.Ox
mirror top-level URL for fetching an upgrade.
.It Pa /home/_sysupgrade
-Directory the upgrade is downloaded to.
+Default directory the upgrade is downloaded to.
.El
.Sh SEE ALSO
.Xr signify 1 ,
Index: sysupgrade.sh
===================================================================
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
retrieving revision 1.25
diff -u -u -r1.25 sysupgrade.sh
--- sysupgrade.sh 28 Sep 2019 17:30:07 -0000 1.25
+++ sysupgrade.sh 21 Nov 2019 09:07:44 -0000
@@ -25,7 +25,6 @@
export PATH=/usr/bin:/bin:/usr/sbin:/sbin
ARCH=$(uname -m)
-SETSDIR=/home/_sysupgrade
ug_err()
{
@@ -34,7 +33,7 @@
usage()
{
- ug_err "usage: ${0##*/} [-fkn] [-r | -s] [installurl]"
+ ug_err "usage: ${0##*/} [-fkn] [-r | -s] [-d directory] [installurl]"
}
unpriv()
@@ -70,19 +69,24 @@
echo -n "$_c"
}
+SETSDIR=/home/_sysupgrade
RELEASE=false
SNAP=false
FORCE=false
KEEP=false
REBOOT=true
-while getopts fknrs arg; do
+while getopts fknrsd: arg; do
case ${arg} in
f) FORCE=true;;
k) KEEP=true;;
n) REBOOT=false;;
r) RELEASE=true;;
s) SNAP=true;;
+ d) SETSDIR=$(readlink -f ${OPTARG})
+ if [[ $? -ne 0 ]] || [[ ${SETSDIR} = / ]]; then
+ ug_err "${OPTARG}: bad parameter value for -d"
+ fi;;
*) usage;;
esac
done
@@ -119,6 +123,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 +132,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 +188,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 +196,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
--
/ Raimo Niskanen, Erlang/OTP, Ericsson AB
