Re: Installer support to fetch/verify bsd.rd for release upgrade

2018-06-03 Thread Robert Peichaer
On Sun, Oct 08, 2017 at 09:56:15AM +, Robert Peichaer wrote:
> Up to now, the upgrade procedure from one to the next release meant
> that you had to manually download and verify the new ramdisk kernel.
> 
> What about if you just needed to boot into the existing bsd.rd and
> it would support downloading and verifying the bsd.rd of the next
> release?
> 
> This diff changes the installer script to support such a scenario.
> 
> 1) Boot the existing bsd.rd and choose (U)pgrade
> 2) Enter the "Server directory" of the new release
>The installer then offers just the bsd.rd
>The on-disk signify key of the new release is used for verify it
> 3) Reboot into the new bsd.rd and do the upgrade
> 
> 
> An important assumption for this to work properly is:
> 
>Upgrades are only supported from one release to the release
>immediately following it. [1]
> 
> 
> It would look like this for the 6.2 to 6.3 upgrade situation.
> (The version numbers are obviously faked)
> 
>   Let's upgrade the sets!
>   Location of sets? (cd0 disk http or 'done') [http]
>   HTTP proxy URL? (e.g. 'http://proxy:8080', or 'none') [none]
>   HTTP Server? (hostname, list#, 'done' or '?') [ftp.hostserver.de]
>   Server directory? [pub/OpenBSD/6.2/amd64] pub/OpenBSD/6.3/amd64
>   Unable to get a verified list of distribution sets.
>   
>   Select sets by entering a set name, a file name pattern or 'all'. De-select
>   sets by prepending a '-', e.g.: '-game*'. Selected sets are labelled '[X]'.
>   [X] bsd.rd
>   Set name(s)? (or 'abort' or 'done') [done]
>   Get/Verify SHA256.sig   100% |**|  2152   00:00
>   Signature Verified
>   Get/Verify bsd.rd   100% |**|  9565 KB00:14
>   Installing bsd.rd   100% |**|  9565 KB00:00
>   Location of sets? (cd0 disk http or 'done') [done]
>   Making all device nodes...done.
>   
>   CONGRATULATIONS! Your OpenBSD upgrade has been successfully completed!
>   To boot the new system, enter 'reboot' at the command prompt.


In October 2017 I've added this "feature" to the installer that allows
to boot with an existing bsd.rd and to download/install the bsd.rd of
the next release which can then be used to do the actual system upgrade.

It depends on differing release versions of the booted bsd.rd and sets
found in the specified HTTP "Server directory". To be more precise it
assumes the sets release version is one version ahead of the bsd.rd.

The logic is meant to be simple and to support this scenario reliably.

This is a best-effort, convenience feature intended for users that do
upgrades from release to release. Users will be able to use it when
they upgrade from 6.3 to 6.4 and so forth.

I'm aware of reports that this does not work as expected, but as far
as I was able to test and verify, the problem most probably was that 
in these cases the 6.2-current bsd.rd was too old to have the feature
or was already on some version of 6.3 which means no release version
difference and so this feature was not triggerd.

Cheers
Robert



Re: Allow disks to be specifid by duid in install.sub

2018-06-02 Thread Robert Peichaer
On Fri, May 18, 2018 at 12:14:36PM +0200, Theo Buehler wrote:
> On Thu, May 17, 2018 at 06:42:15PM -0600, Aaron Bieber wrote:
> > On Thu, May 17, 2018 at 06:37:56PM -0600, Aaron Bieber wrote:
> > > On Fri, Mar 02, 2018 at 07:32:04AM -0700, Aaron Bieber wrote:
> > > > Hi,
> > > > 
> > > > Currently disks can only be entered in the [sw]d[0-9][0-9] format at the
> > > > "Which disk is the root disk?" prompt. This is great for humans, but
> > > > things get tricky when doing an autoinstall upgrade on systems where
> > > > connected disks change frequently.
> > > > 
> > > > This diff lets you put the DUID in the response file.
> > > > 
> > > > If anyone has a better way to determine the disk from the duid, I am all
> > > > ears :D
> > > > 
> > > > Cheers,
> > > > Aaron
> > > > 
> > > 
> > > Thanks to tb@, kn@, Philipp Buehler and phy1729 on #metabug! I tested a
> > > bsd.rd with an auto-upgrade response file. Once with a duid and once
> > > with a disk name. Both worked.
> > > 
> > > I have an OK from tb@ unless anyone objects.
> > > 
> > > The diff has been distilled down to this:
> > > 
> > 
> > Paste fail, here is the latest diff:
> 
> Sleeping over it once more, I must say I'm still not terribly fond of
> this hack and would like to retract my ok for these two reasons:
> 
> * we clobber 'resp' which may then leave a confusing error message.
>   It's only in the $AUTO case, but still.

Theo is right here. The orginal input should be preserved and used
in the error messages and should be written to the autoinstall logfile.

> * In the is_duid case we walk the hw.disknames output twice even though
>   we know that we already have a good device. This is a bit stupid.

The get_rootinfo() function is not very "efficient" anyway because
it is meant to kind of dynamically detect just connected disks.
That's why get_dkdev() is already called multiple times and this
even on every iteration of the while-loop. Which also means walking
through the hw.disknames output multiple times.

It's more about reusing an existing function (get_dkdev_name) that
uses hw.disknames to extract the corresponding disk device name.

> I do think this is going in the right direction, but I'd really like to
> see these two points resolved before it goes in. It's probably not too
> difficult, but I've got too many things on my hand, so I won't be able
> to do it myself anytime soon.

My suggestion is to not special case autoinstall, but to always
use get_dkdev_name() to translate a possible supplied DUID to a
disk device name. If the input $resp was already a disk device
name, get_dkdev_name() just returns this disk device name. So in
both cases, $_dkdev holds the disk device name.

$_dkdev can then be used where a disk device name is required.
$resp can still be used for the error messages and the ai.log.

This would be the resulting diff, which looks more complex but
the change is actually simpler then the previous diff.

I've tested it sucessfully with DUIDs and disk dev names, both
interactivly and with autoinstall.


Index: install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1068
diff -u -p -p -u -r1.1068 install.sub
--- install.sub 29 May 2018 20:37:22 -  1.1068
+++ install.sub 2 Jun 2018 19:27:03 -
@@ -,7 +,7 @@ is_rootdisk() {
 
 # Get global root information. ie. ROOTDISK, ROOTDEV and SWAPDEV.
 get_rootinfo() {
-   local _default=$(get_dkdevs)
+   local _default=$(get_dkdevs) _dkdev
local _q="Which disk is the root disk? ('?' for details)"
 
while :; do
@@ -2231,11 +2231,14 @@ get_rootinfo() {
case $resp in
"?")diskinfo $(get_dkdevs);;
'') ;;
-   *)  if isin "$resp" $(get_dkdevs); then
+   *)  # Translate $resp to disk dev name in case it is a DUID.
+   # get_dkdev_name bounces back the disk dev name if not.
+   _dkdev=$(get_dkdev_name "$resp")
+   if isin "$_dkdev" $(get_dkdevs); then
[[ $MODE == install ]] && break
-   is_rootdisk "$resp" && break
+   is_rootdisk "$_dkdev" && break
echo "$resp is not a valid root disk."
-   _default="$(rmel "$resp" $_default) $resp"
+   _default="$(rmel "$_dkdev" $_default) $_dkdev"
else
echo "no such disk"
fi
@@ -2245,9 +2248,9 @@ get_rootinfo() {
done
log_answers "$_q" "$resp"
 
-   make_dev $resp || exit
+   make_dev $_dkdev || exit
 
-   ROOTDISK=$resp
+   ROOTDISK=$_dkdev
ROOTDEV=${ROOTDISK}a
SWAPDEV=${ROOTDISK}b
 }
===
Stats: --- 6 lines 191 

Re: ksh.kshrc: Fix quoting in {add,pre,del}_path() to work with spaces

2018-02-18 Thread Robert Peichaer
On Sun, Feb 18, 2018 at 12:36:43PM +0100, Klemens Nanni wrote:
> On Tue, Nov 21, 2017 at 08:30:25PM +0100, Klemens Nanni wrote:
> > On Sun, Nov 12, 2017 at 10:43:46PM +0100, Klemens Nanni wrote:
> > > On Sun, Nov 12, 2017 at 09:04:22PM +0000, Robert Peichaer wrote:
> > > > Hmm. I see.
> > > > 
> > > > The {add,del,no,pre}_path functions are in ksh.kshrc since when it was
> > > > imported 21 years ago. If they would be used in ksh.kshrc or anywhere
> > > > else, I'd say it might be worth "fixing" these functions. But they are
> > > > not used anywhere in the tree and I would rather remove them 
> > > > alltogether.
> > > > 
> > > > In case someone uses them, ~/.kshrc seems to be a more appropriate 
> > > > place.
> > > I personally prefer keeping them but won't object if the broader consent
> > > is to remove them.
> > > 
> > > This makes me wonder who else actually uses those (on a regular basis).
> > Bumping this a week later.
> > 
> > Do we fix this (using my earlier diff) or abandon them all together?
> > Here's the latter one as convenient diff.
> > 
> > diff --git a/etc/ksh.kshrc b/etc/ksh.kshrc
> > index 5b5bd040f79..923fdc37541 100644
> > --- a/etc/ksh.kshrc
> > +++ b/etc/ksh.kshrc
> > @@ -119,26 +119,3 @@ case "$-" in
> >  *) # non-interactive
> >  ;;
> >  esac
> > -# commands for both interactive and non-interactive shells
> > -
> > -# is $1 missing from $2 (or PATH) ?
> > -function no_path {
> > -   eval _v="\$${2:-PATH}"
> > -   case :$_v: in
> > -   *:$1:*) return 1;;  # no we have it
> > -   esac
> > -   return 0
> > -}
> > -# if $1 exists and is not in path, append it
> > -function add_path {
> > -   [[ -d ${1:-.} ]] && no_path $* && eval ${2:-PATH}="\$${2:-PATH}:$1"
> > -}
> > -# if $1 exists and is not in path, prepend it
> > -function pre_path {
> > -   [[ -d ${1:-.} ]] && no_path $* && eval ${2:-PATH}="$1:\$${2:-PATH}"
> > -}
> > -# if $1 is in path, remove it
> > -function del_path {
> > -   no_path $* || eval ${2:-PATH}=$(eval echo :'$'${2:-PATH}: |
> > -   sed -e "s;:$1:;:;g" -e "s;^:;;" -e "s;:\$;;")
> > -}
> > 
> We still have the broken _path() functions around and I prefer to simply
> remove them by now.
> 
> OK?

IF nobody else speaks up, I'm for removal as stated in a previous email.



Re: [patch] upon install of new operating system version, do not set root password to empty string

2017-12-03 Thread Robert Peichaer
On Wed, Nov 29, 2017 at 08:06:39AM +0100, Otto Moerbeek wrote:
> On Tue, Nov 28, 2017 at 06:59:06PM -0500, Ian Sutton wrote:
> 
> > This is a highly theoretical and experimental mitigation which stops the
> > root password on newly upgraded/installed systems from being an empty
> > string. The thinking is that by not shipping an operating system with a
> > known root password, certain classes of attacks involving logging into
> > the root account might be avoided. I would like some feedback from the
> > cryptography team as well as NIST finalists in order to better ascertain
> > the implications of this behaviour.
> 
> Hmm, but afaiks, this is already done on install. What does you diff change?
> 
>   -Otto
> 
> > 
> > Index: src/distrib/miniroot/install.sub
> > ===
> > RCS file: /cvs/src/distrib/miniroot/install.sub,v
> > retrieving revision 1.1032
> > diff -u -p -r1.1032 install.sub
> > --- src/distrib/miniroot/install.sub8 Aug 2017 07:14:05 -   
> > 1.1032
> > +++ src/distrib/miniroot/install.sub28 Nov 2017 23:43:56 -
> > @@ -2732,12 +2732,6 @@ do_install() {
> >  
> > echo
> >  
> > +   while :; do
> > +   ask_password "Password for root account?"
> > +   _rootpass="$_password"
> > +   [[ -n "$_password" ]] && break
> > +   echo "The root password must be set."
> > +   done
> >  
> > # Ask for the root user public ssh key during autoinstall.
> > _rootkey=

This is the exact code, that is already in install.sub.
So I don't understand this proposal.

-- 
-=[rpe]=-



Re: ksh.kshrc: Fix quoting in {add,pre,del}_path() to work with spaces

2017-11-12 Thread Robert Peichaer
On Sun, Nov 12, 2017 at 12:22:27AM +0100, Klemens Nanni wrote:
> On Sat, Nov 11, 2017 at 08:03:36PM +0000, Robert Peichaer wrote:
> > On Sat, Nov 11, 2017 at 08:11:25PM +0100, Klemens Nanni wrote:
> > > pre_path()ing directories with spaces is broken due to bad quoting.
> > > 
> > > This diff takes care of that by properly passing double quotes through
> > > eval and quoting the arguments for no_path() individually.
> > > 
> > > Feedback?
> > 
> > What is actually broken?
> > Can you give examples?
> Sure, pardon me.
> 
>   $ typeset -f add_path
>   function add_path {
>   [[ -d ${1:-.} ]] && no_path $* && eval ${2:-PATH}="\$${2:-PATH}:$1"
>   }
>   $ echo $PATH
>   /usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin
>   $ mkdir some\ bin
>   $ add_path some\ bin
>   add_path: bin: not found
>   $ echo $PATH
>   /usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin
>   $ PATH=$PATH:some\ bin
>   $ del_path some\ bin
>   $ echo $?
>   0
>   $ echo $PATH
>   /usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:some bin/
> 
> Since the double quotes are not escaped and thus quote the string to be
> evaluated itself, `PATH=$PATH:some bin' is eventually being executed as
> seen above.
> 
> pre_path() behaves the same trying to execute `PATH=some bin:$PATH'.
> 
> del_path() silently fails to remove the directory.
> 
> Here's another example showing how to exploit this:
> 
>   $ ls foo
>   ls: foo: No such file or directory
>   $ mkdir '. touch foo'
>   $ add_path '. touch foo'
>   $ ls foo
>   foo
> 
> With my patch behaviour will be as expected.

Hmm. I see.

The {add,del,no,pre}_path functions are in ksh.kshrc since when it was
imported 21 years ago. If they would be used in ksh.kshrc or anywhere
else, I'd say it might be worth "fixing" these functions. But they are
not used anywhere in the tree and I would rather remove them alltogether.

In case someone uses them, ~/.kshrc seems to be a more appropriate place.

-- 
-=[rpe]=-



Re: ksh.kshrc: Fix quoting in {add,pre,del}_path() to work with spaces

2017-11-11 Thread Robert Peichaer
On Sat, Nov 11, 2017 at 08:11:25PM +0100, Klemens Nanni wrote:
> pre_path()ing directories with spaces is broken due to bad quoting.
> 
> This diff takes care of that by properly passing double quotes through
> eval and quoting the arguments for no_path() individually.
> 
> Feedback?

What is actually broken?
Can you give examples?
 
> diff --git a/etc/ksh.kshrc b/etc/ksh.kshrc
> index 5b5bd040f79..66736da5e11 100644
> --- a/etc/ksh.kshrc
> +++ b/etc/ksh.kshrc
> @@ -131,14 +131,14 @@ function no_path {
>  }
>  # if $1 exists and is not in path, append it
>  function add_path {
> - [[ -d ${1:-.} ]] && no_path $* && eval ${2:-PATH}="\$${2:-PATH}:$1"
> + [[ -d "${1:-.}" ]] && no_path "$@" && eval 
> ${2:-PATH}=\"\$${2:-PATH}:$1\"
>  }
>  # if $1 exists and is not in path, prepend it
>  function pre_path {
> - [[ -d ${1:-.} ]] && no_path $* && eval ${2:-PATH}="$1:\$${2:-PATH}"
> + [[ -d "${1:-.}" ]] && no_path "$@" && eval 
> ${2:-PATH}=\"$1:\$${2:-PATH}\"
>  }
>  # if $1 is in path, remove it
>  function del_path {
> - no_path $* || eval ${2:-PATH}=$(eval echo :'$'${2:-PATH}: |
> - sed -e "s;:$1:;:;g" -e "s;^:;;" -e "s;:\$;;")
> + no_path "$1" || eval ${2:-PATH}=\"$(eval echo :'$'${2:-PATH}: |
> + sed -e "s;:$1:;:;g" -e "s;^:;;" -e "s;:\$;;")\"
>  }
> 

-- 
-=[rpe]=-



Re: armv7: newfs efi-partition in when choosing manual-fdisk mbr

2017-11-10 Thread Robert Peichaer
On Fri, Nov 10, 2017 at 10:27:36AM +0200, Artturi Alm wrote:
> Hi,
> 
> currently, just editing the mbr to give more room for u-boot env growth,
> will result in unbootable system, as the installer will fail to mount it,
> and naturally things won't work beyond u-boot after reboot either.
> 
> i'm not really sure about this diff, but it does seem like it might do,
> what i think it should? might take a while, before i can test myself, so
> i thought i'd mail and ask, if there's a reason for how it is atm.? :)
> 
> -Artturi

I do not know enough about the needed/supported armv7 filesystem layout.
The install notes talk about GPT and MBR partitioning, but the install.md
script only offers MBR. Obviously EFI boot is supported reading install.md
but from MBR. I thought that's only possible from a GPT partition.

Anyways. I will happily help to get this right scripting wise.
But right now I don't know if this is the correct way to fix this.

-- 
-=[rpe]=-



Re: /etc/netstart diff

2017-11-09 Thread Robert Peichaer
On Wed, Nov 08, 2017 at 10:47:43PM +0100, Holger Mikolon wrote:
> The veriable $HN_DIR is set in /etc/netstart on line 166 but used only
> once (line 78). The diff below makes use of $HN_DIR in the other cases
> where netstart cares of ip address configuration.
> 
> With below change I can maintain different sets (think "profiles") of
> hostname.if(5) files in separate directories and use them e.g. like this:
> "env HN_DIR=/etc/myprofile sh /etc/netstart"
> 
> Even without such use case it's at least a consistency fix.
> 
> Regards
> Holger
> ;-se

Hallo Holger

This "feature" was introduced for a specific purpose some time ago to
assist the transition to the new hostname.if(5) parser code in the
netstart script. People were able to run netstart with the -n option
and this HN_DIR environment variable to verify that the new parser
works with their possibly non-trivial hostname.if(5) configurations.
We considered this to be a rather delicate transition because we did
not want to break peoples setups.

Now that the parser code proved to not break anything, I rather want
to remove this funcionality, than to extend it.

-- 
-=[rpe]=-



Re: Installer support to fetch/verify bsd.rd for release upgrade

2017-10-09 Thread Robert Peichaer
On Sun, Oct 08, 2017 at 09:56:15AM +, Robert Peichaer wrote:
> Up to now, the upgrade procedure from one to the next release meant
> that you had to manually download and verify the new ramdisk kernel.
> 
> What about if you just needed to boot into the existing bsd.rd and
> it would support downloading and verifying the bsd.rd of the next
> release?
> 
> This diff changes the installer script to support such a scenario.
> 
> 1) Boot the existing bsd.rd and choose (U)pgrade
> 2) Enter the "Server directory" of the new release
>The installer then offers just the bsd.rd
>The on-disk signify key of the new release is used for verify it
> 3) Reboot into the new bsd.rd and do the upgrade
> 
> 
> An important assumption for this to work properly is:
> 
>Upgrades are only supported from one release to the release
>immediately following it. [1]
> 
> 
> It would look like this for the 6.2 to 6.3 upgrade situation.
> (The version numbers are obviously faked)
> 
>   Let's upgrade the sets!
>   Location of sets? (cd0 disk http or 'done') [http]
>   HTTP proxy URL? (e.g. 'http://proxy:8080', or 'none') [none]
>   HTTP Server? (hostname, list#, 'done' or '?') [ftp.hostserver.de]
>   Server directory? [pub/OpenBSD/6.2/amd64] pub/OpenBSD/6.3/amd64
>   Unable to get a verified list of distribution sets.
>   
>   Select sets by entering a set name, a file name pattern or 'all'. De-select
>   sets by prepending a '-', e.g.: '-game*'. Selected sets are labelled '[X]'.
>   [X] bsd.rd
>   Set name(s)? (or 'abort' or 'done') [done]
>   Get/Verify SHA256.sig   100% |**|  2152   00:00
>   Signature Verified
>   Get/Verify bsd.rd   100% |**|  9565 KB00:14
>   Installing bsd.rd   100% |**|  9565 KB00:00
>   Location of sets? (cd0 disk http or 'done') [done]
>   Making all device nodes...done.
>   
>   CONGRATULATIONS! Your OpenBSD upgrade has been successfully completed!
>   To boot the new system, enter 'reboot' at the command prompt.
> 
> 
> Here's the diff and below is a more detailed description.

Well, here's a diff, that actually works.


Index: install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1036
diff -u -p -p -u -r1.1036 install.sub
--- install.sub 4 Oct 2017 18:19:33 -   1.1036
+++ install.sub 9 Oct 2017 17:41:59 -
@@ -1330,6 +1330,15 @@ sane_install() {
 #
 select_sets() {
local _avail=$1 _selected=$2 _f _action _col=$COLUMNS
+   local _bsd_rd _no_sets=true
+
+   if [[ $MODE == upgrade ]]; then
+   for _f in $_avail; do
+   [[ $_f != bsd* ]] && _no_sets=false
+   [[ $_f == bsd.rd* ]] && _bsd_rd=$_f
+   done
+   $_no_sets && UPGRADE_BSDRD=true _avail=$_bsd_rd 
_selected=$_bsd_rd
+   fi
 
# account for 4 spaces added to the sets list
let COLUMNS=_col-8
@@ -1516,6 +1525,9 @@ install_files() {
! $_unpriv ftp -D "$_t" -Vmo - "$_src/SHA256.sig" 
>"$_cfile.sig" &&
_issue="Cannot fetch SHA256.sig" && break
 
+   $UPGRADE_BSDRD &&
+   PUB_KEY=/mnt/etc/signify/openbsd-$((VERSION + 
1))-base.pub
+
# Verify signature file with public keys.
! unpriv -f "$_cfile" \
signify -Vep $PUB_KEY -x "$_cfile.sig" -m "$_cfile" &&
@@ -1576,7 +1588,9 @@ install_files() {
tar -zxphf - -C /mnt
fi
;;
-   *)  $_unpriv ftp -D Installing -Vmo - "$_fsrc" >"/mnt/$_f"
+   *)  $UPGRADE_BSDRD && [[ $_f == bsd.rd* ]] &&
+   cp /mnt/$_f /mnt/$_f.old.$VERSION
+   $_unpriv ftp -D Installing -Vmo - "$_fsrc" >"/mnt/$_f"
;;
esac
if (($?)); then
@@ -1587,6 +1601,7 @@ install_files() {
fi
else
DEFAULTSETS=$(rmel $_f $DEFAULTSETS)
+   $UPGRADE_BSDRD && DEFAULTSETS=
fi
[[ -d $_tmpsrc ]] && rm -f "$_tmpsrc/$_f"
done
@@ -3139,6 +3154,7 @@ PUB_KEY=/etc/signify/openbsd-${VERSION}-
 ROOTDEV=
 ROOTDISK=
 SETDIR="$VNAME/$ARCH"
+UPGRADE_BSDRD=false
 V4_DHCPCONF=false
 V6_AUTOCONF=false
 WLANLIST=/tmp/i/wlanlist
===
Stats: --- 1 lines 60 chars
Stats: +++ 17 lines 525 chars
Stats: 16 lines
Stats: 465 chars

-- 
-=[rpe]=-



Installer support to fetch/verify bsd.rd for release upgrade

2017-10-08 Thread Robert Peichaer
Up to now, the upgrade procedure from one to the next release meant
that you had to manually download and verify the new ramdisk kernel.

What about if you just needed to boot into the existing bsd.rd and
it would support downloading and verifying the bsd.rd of the next
release?

This diff changes the installer script to support such a scenario.

1) Boot the existing bsd.rd and choose (U)pgrade
2) Enter the "Server directory" of the new release
   The installer then offers just the bsd.rd
   The on-disk signify key of the new release is used for verify it
3) Reboot into the new bsd.rd and do the upgrade


An important assumption for this to work properly is:

   Upgrades are only supported from one release to the release
   immediately following it. [1]


It would look like this for the 6.2 to 6.3 upgrade situation.
(The version numbers are obviously faked)

  Let's upgrade the sets!
  Location of sets? (cd0 disk http or 'done') [http]
  HTTP proxy URL? (e.g. 'http://proxy:8080', or 'none') [none]
  HTTP Server? (hostname, list#, 'done' or '?') [ftp.hostserver.de]
  Server directory? [pub/OpenBSD/6.2/amd64] pub/OpenBSD/6.3/amd64
  Unable to get a verified list of distribution sets.
  
  Select sets by entering a set name, a file name pattern or 'all'. De-select
  sets by prepending a '-', e.g.: '-game*'. Selected sets are labelled '[X]'.
  [X] bsd.rd
  Set name(s)? (or 'abort' or 'done') [done]
  Get/Verify SHA256.sig   100% |**|  2152   00:00
  Signature Verified
  Get/Verify bsd.rd   100% |**|  9565 KB00:14
  Installing bsd.rd   100% |**|  9565 KB00:00
  Location of sets? (cd0 disk http or 'done') [done]
  Making all device nodes...done.
  
  CONGRATULATIONS! Your OpenBSD upgrade has been successfully completed!
  To boot the new system, enter 'reboot' at the command prompt.


Here's the diff and below is a more detailed description.


Index: install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1036
diff -u -p -p -u -r1.1036 install.sub
--- install.sub 4 Oct 2017 18:19:33 -   1.1036
+++ install.sub 7 Oct 2017 14:02:19 -
@@ -1330,6 +1330,13 @@ sane_install() {
 #
 select_sets() {
local _avail=$1 _selected=$2 _f _action _col=$COLUMNS
+   local _bsd_rd _no_sets=true
+
+   [[ $MODE == upgrade ]] && for _f in $_avail; do
+   [[ $_f != bsd* ]] && _no_sets=false
+   [[ $_f == bsd.rd* ]] && _bsd_rd=$_f
+   done
+   $_no_sets && UPGRADE_BSDRD=true _avail=$_bsd_rd _selected=$_bsd_rd
 
# account for 4 spaces added to the sets list
let COLUMNS=_col-8
@@ -1517,6 +1524,8 @@ install_files() {
_issue="Cannot fetch SHA256.sig" && break
 
# Verify signature file with public keys.
+   $UPGRADE_BSDRD &&
+   PUB_KEY=/mnt/etc/signify/openbsd-$((VERSION + 
1))-base.pub
! unpriv -f "$_cfile" \
signify -Vep $PUB_KEY -x "$_cfile.sig" -m "$_cfile" &&
_issue="Signature check of SHA256.sig failed" && break
@@ -1576,7 +1585,9 @@ install_files() {
tar -zxphf - -C /mnt
fi
;;
-   *)  $_unpriv ftp -D Installing -Vmo - "$_fsrc" >"/mnt/$_f"
+   *)  $UPGRADE_BSDRD && [[ $_f == bsd.rd* ]] &&
+   cp /mnt/$_f /mnt/$_f.old.$VERSION
+   $_unpriv ftp -D Installing -Vmo - "$_fsrc" >"/mnt/$_f"
;;
esac
if (($?)); then
@@ -1587,6 +1598,7 @@ install_files() {
fi
else
DEFAULTSETS=$(rmel $_f $DEFAULTSETS)
+   $UPGRADE_BSDRD && DEFAULTSETS=
fi
[[ -d $_tmpsrc ]] && rm -f "$_tmpsrc/$_f"
done
@@ -3139,6 +3151,7 @@ PUB_KEY=/etc/signify/openbsd-${VERSION}-
 ROOTDEV=
 ROOTDISK=
 SETDIR="$VNAME/$ARCH"
+UPGRADE_BSDRD=false
 V4_DHCPCONF=false
 V6_AUTOCONF=false
 WLANLIST=/tmp/i/wlanlist
===
Stats: --- 1 lines 60 chars
Stats: +++ 14 lines 508 chars
Stats: 13 lines
Stats: 448 chars


The installer downloads the new SHA256.sig from the location of the
new release and extracts the list of files. It then prepares the
list for the selection step. At this point all the set files
containing the new release number are skipped, because they don't
match the version of the current (old) bsd.rd leaving only the
kernels.

Right before the set selection step, the installer looks at the
list of files and if there are only kernels, it assumes to be in
this "upgrade only the bsd.rd" scenario. It then sets the list to
the bsd.rd kernel and sets the global UPGRADE_BSDRD variable to

Re: [PATCH] run security(8) on first boot

2017-07-30 Thread Robert Peichaer
On Sat, Jul 29, 2017 at 05:25:51PM -0400, Joe Gidi wrote:
> I did a couple of fresh installs the other day, which reminded me of a
> minor irritation and prompted me to think about a possible solution.
> 
> The first run of security(8) on a fresh install is not terribly helpful.
> It produces a huge email report since it diffs all the /etc/changelist
> files against /dev/null. If you're already familiar with OpenBSD and
> understand this behavior, you probably disregard this email and drive on.
> 
> If you're a new user, this is probably surprising and somewhat misleading.
> After all, you've just installed an operating system that takes
> justifiable pride in sane, secure defaults, and the next morning you
> receive a multi-thousand-line insecurity report that calls out every
> important configuration file on the system.
> 
> I think the simplest way to prevent this would be for install.sub to add a
> line to /etc/rc.firsttime that runs security(8) and discards the output,
> or perhaps logs it to a file, rather than emailing it. This would "prime
> the pump" by populating /var/backups with as-installed copies of the
> changelist files, and then the first nightly run of security(8) would only
> show files that have actually been changed post-install.
> 
> Of course, this also means you have virgin copies of your config files
> stashed away immediately, in case you need one before the nightly
> security(8) run can back them up for you.
> 
> This will make the first boot take longer, perhaps by several minutes on
> slower platforms. Of course, the first boot is already slower due to key
> generation, etc.
> 
> Diff below was tested in an amd64 bsd.rd and seems to behave as expected.
> I have *not* built a full release or tested every possible use case; I
> know there are sometimes issues with space on some install media, and
> hopefully this small addition would not cause an overflow.
> 
> Does anyone see value in this? If not, I suppose it might end up living in
> my install.site.

The "prime the pumb" aspect to have an initial state in /var/backups
is the only thing I would see as a pro argument for such a change.

But I guess there is bikeshedding potential as to what is considered
the "initial state". Is it the state on first boot or how the system
is configured afterwards? Think of solutions like cfengine, ansible,
salt, etc that might run after system installation to apply changes.

You can always execute security(8) at any point you consider the
initial state if they care about this aspect.

-- 
-=[rpe]=-



Move {install,upgrade}.site script to the end of installs/upgrades (again)

2017-07-18 Thread Robert Peichaer
Originally, the installer executed the {install,upgrade}.site script
at the end of installs and upgrades. Over time, code was after this
step and now a list of things happen AFTER this script is executed.

- make underlying device nodes for softraid devices
- install the boot-block on disk
- switch to MP kernel on multi-processor systems
- update kernel.SHA256
- relink kernel
- prepare execution of sysmerge in batch mode on reboot 
- prepare execution of fw_update on reboot
- prepare mail with response file to root/admin user

This diff moves the execution of the {install,upgrade}.site script
to the end of the install/upgrade process again.


NOTE: If you use the {install,upgrade}.site functionality, make sure
your script still works as expected.



Index: install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1024
diff -u -p -p -u -r1.1024 install.sub
--- install.sub 17 Jul 2017 18:02:31 -  1.1024
+++ install.sub 17 Jul 2017 18:18:17 -
@@ -2639,8 +2639,6 @@ finish_up() {
)
fi
 
-   [[ -x /mnt/$MODE.site ]] && chroot /mnt /$MODE.site
-
# In case this is a softraid device, make sure all underlying
# device nodes exist before installing boot-blocks on disk.
make_dev $(bioctl $ROOTDISK 2>/dev/null | sed -n 's/.*<\(.*\)>$/\1/p')
@@ -2680,6 +2678,8 @@ finish_up() {
 
# Email installer questions and their answers to root on next boot.
prep_root_mail /tmp/i/$MODE.resp "$(hostname) $MODE response file"
+
+   [[ -x /mnt/$MODE.site ]] && chroot /mnt /$MODE.site
 
# Store entropy for the next boot.
store_random
-- 
-=[rpe]=-



Re: rc: Use IFS when looking for carp interfaces

2017-07-17 Thread Robert Peichaer
On Mon, Jul 17, 2017 at 03:39:29PM +0200, Klemens Nanni wrote:
> The Internal Field Seperator is meant for this so use it instead of
> reading and stripping ':' again.
> 
> Feedback? Comments?
> 
> Index: rc
> ===
> RCS file: /cvs/src/etc/rc,v
> retrieving revision 1.508
> diff -u -p -r1.508 rc
> --- rc17 Jul 2017 12:02:53 -  1.508
> +++ rc17 Jul 2017 13:33:54 -
> @@ -351,8 +350,8 @@ if [[ $1 == shutdown ]]; then
>   [[ -f /etc/rc.shutdown ]] && sh /etc/rc.shutdown
>   fi
>  
> - ifconfig | while read _if _junk; do
> - [[ $_if == carp+([0-9]): ]] && ifconfig ${_if%:} down
> + ifconfig | while IFS=: read _if _junk; do
> + [[ $_if == carp+([0-9]) ]] && ifconfig $_if down
>   done
>  
>   exit 0

Unnecessary micro optimization.

-- 
-=[rpe]=-



Re: rc: reorder_libs: [1/2] Drop unused _l, exit early on failure

2017-07-17 Thread Robert Peichaer
On Mon, Jul 17, 2017 at 03:00:34PM +0200, Klemens Nanni wrote:
> On Sun, Jul 16, 2017 at 09:09:44AM +0000, Robert Peichaer wrote:
> > The rationale to picking the library versions before remounting was
> > to keep the time window having rw /usr as small as possible.
> > If that's deemed ok, I'm too OK with switching the steps.
> Considering the fact that the now simplified version picking routine
> takes virtually no time, I'd like finish this up.
> 
> Here's the updated diff checking r/w status beforehand.
> 
> Index: rc
> ===
> RCS file: /cvs/src/etc/rc,v
> retrieving revision 1.508
> diff -u -p -r1.508 rc
> --- rc17 Jul 2017 12:02:53 -  1.508
> +++ rc17 Jul 2017 12:56:07 -
> @@ -170,12 +170,6 @@ reorder_libs() {
>  
>   echo -n 'reordering libraries:'
>  
> - # Only choose the latest version of the libraries.
> - for _liba in /usr/lib/lib{c,crypto}; do
> - _libas="$_libas $(ls $_liba.so.+([0-9.]).a | sort -V | tail -1)"
> - done
> - _libas=${_libas# }
> -
>   # Remount read-write, if /usr/lib is on a read-only ffs filesystem.
>   if [[ $_mp == *' type ffs '*'read-only'* ]]; then
>   if mount -u -w $_dkdev; then
> @@ -185,6 +179,12 @@ reorder_libs() {
>   return
>   fi
>   fi
> +
> + # Only choose the latest version of the libraries.
> + for _liba in /usr/lib/lib{c,crypto}; do
> + _libas="$_libas $(ls $_liba.so.+([0-9.]).a | sort -V | tail -1)"
> + done
> + _libas=${_libas# }
>  
>   for _liba in $_libas; do
>   _tmpdir=$(mktemp -dq /tmp/_librebuild.) && (
 
OK

-- 
-=[rpe]=-



Re: rc: Use here document for temporary pf rule set

2017-07-16 Thread Robert Peichaer
On Sun, Jul 16, 2017 at 03:24:19PM +0200, Klemens Nanni wrote:
> On Sun, Jul 16, 2017 at 12:41:09PM +0000, Robert Peichaer wrote:
> > On Sun, Jul 16, 2017 at 02:28:59PM +0200, Klemens Nanni wrote:
> > > On Sun, Jul 16, 2017 at 12:11:55PM +0000, Robert Peichaer wrote:
> > > > On Sun, Jul 16, 2017 at 01:37:56PM +0200, Klemens Nanni wrote:
> > > > > This removes on level of indent, avoids the ugly RULES="$RULES ..."
> > > > > repitition and spares a print.
> > > > > 
> > > > > We could do a 'pfctl -ef -' right away but I kept changing and 
> > > > > enabling
> > > > > clearly seperated. Regarding the leading newlines and tabs of the 
> > > > > inner
> > > > > echo: pf perfectly munges those, no need to clear them.
> > > > > 
> > > > > The "don't" -> "do not" is neccessary since otherwise the shell would
> > > > > choke on it as opening quote.
> > > > > 
> > > > > 
> > > > > Feedback? Comments?
> > > > 
> > > > Nice idea. The only maby irrelevant concern I have is, that using the
> > > > here-document approach uses a temporary file and if that for some reason
> > > > fails, we end up without this or mangled rules.
> > > sh reads the temporary file in 512 bytes chunks, the here document is
> > > about 2.0K in size.
> > > 
> > > I didn't bother intercepting sh with gdb and simulating a scenario where
> > > the temporary file cannot be written but in case the user has no disk
> > > space left I'd expect it to not be created at all since.
> > > 
> > > In general I'd say that if /tmp doesn't have 2.0K left users probably
> > > have more serious problems anyway.
> > 
> > Have you thought about diskless(8) setups?
> If /tmp is served via NFS and the here document's IO fails, 'pfctl -f -'
> won't see any rule as it's not being executed.
> 
> I still think running out of space would cause more problems than just
> this one but I shall be glad to be corrected.
> 
> Here's another approach still using $RULES but with saner quoting and
> without the potentially dangerous here document.
> 
> 
> Index: rc
> ===
> RCS file: /cvs/src/etc/rc,v
> retrieving revision 1.507
> diff -u -p -r1.507 rc
> --- rc4 Jul 2017 19:02:11 -   1.507
> +++ rc16 Jul 2017 13:23:02 -
> @@ -402,31 +399,35 @@ wsconsctl_conf
>  
>  # Set initial temporary pf rule set.
>  if [[ $pf != NO ]]; then
> - RULES="block all"
> - RULES="$RULES\npass on lo0"
> - RULES="$RULES\npass in proto tcp from any to any port ssh keep state"
> - RULES="$RULES\npass out proto { tcp, udp } from any to any port domain 
> keep state"
> - RULES="$RULES\npass out inet proto icmp all icmp-type echoreq keep 
> state"
> - RULES="$RULES\npass out inet proto udp from any port bootpc to any port 
> bootps"
> - RULES="$RULES\npass in inet proto udp from any port bootps to any port 
> bootpc"
> - if ifconfig lo0 inet6 >/dev/null 2>&1; then
> - RULES="$RULES\npass out inet6 proto icmp6 all icmp6-type 
> neighbrsol"
> - RULES="$RULES\npass in inet6 proto icmp6 all icmp6-type 
> neighbradv"
> - RULES="$RULES\npass out inet6 proto icmp6 all icmp6-type 
> routersol"
> - RULES="$RULES\npass in inet6 proto icmp6 all icmp6-type 
> routeradv"
> - RULES="$RULES\npass out inet6 proto udp from any port 
> dhcpv6-client to any port dhcpv6-server"
> - RULES="$RULES\npass in inet6 proto udp from any port 
> dhcpv6-server to any port dhcpv6-client"
> - fi
> - RULES="$RULES\npass in proto carp keep state (no-sync)"
> - RULES="$RULES\npass out proto carp !received-on any keep state 
> (no-sync)"
> - if [[ $(sysctl vfs.mounts.nfs 2>/dev/null) == *[1-9]* ]]; then
> - # Don't kill NFS.
> - RULES="set reassemble yes no-df\n$RULES"
> - RULES="$RULES\npass in proto { tcp, udp } from any port { 
> sunrpc, nfsd } to any"
> - RULES="$RULES\npass out proto { tcp, udp } from any to any port 
> { sunrpc, nfsd } !received-on any"
> - fi
> - print -- "$RULES" | pfctl -f -
> - pfctl -e
> + RULES='
> + block all
> + pass on lo0
> + pass in p

Re: rc: Use here document for temporary pf rule set

2017-07-16 Thread Robert Peichaer
On Sun, Jul 16, 2017 at 02:28:59PM +0200, Klemens Nanni wrote:
> On Sun, Jul 16, 2017 at 12:11:55PM +0000, Robert Peichaer wrote:
> > On Sun, Jul 16, 2017 at 01:37:56PM +0200, Klemens Nanni wrote:
> > > This removes on level of indent, avoids the ugly RULES="$RULES ..."
> > > repitition and spares a print.
> > > 
> > > We could do a 'pfctl -ef -' right away but I kept changing and enabling
> > > clearly seperated. Regarding the leading newlines and tabs of the inner
> > > echo: pf perfectly munges those, no need to clear them.
> > > 
> > > The "don't" -> "do not" is neccessary since otherwise the shell would
> > > choke on it as opening quote.
> > > 
> > > 
> > > Feedback? Comments?
> > 
> > Nice idea. The only maby irrelevant concern I have is, that using the
> > here-document approach uses a temporary file and if that for some reason
> > fails, we end up without this or mangled rules.
> sh reads the temporary file in 512 bytes chunks, the here document is
> about 2.0K in size.
> 
> I didn't bother intercepting sh with gdb and simulating a scenario where
> the temporary file cannot be written but in case the user has no disk
> space left I'd expect it to not be created at all since.
> 
> In general I'd say that if /tmp doesn't have 2.0K left users probably
> have more serious problems anyway.

Have you thought about diskless(8) setups?

-- 
-=[rpe]=-



Re: rc: Use here document for temporary pf rule set

2017-07-16 Thread Robert Peichaer
On Sun, Jul 16, 2017 at 01:37:56PM +0200, Klemens Nanni wrote:
> This removes on level of indent, avoids the ugly RULES="$RULES ..."
> repitition and spares a print.
> 
> We could do a 'pfctl -ef -' right away but I kept changing and enabling
> clearly seperated. Regarding the leading newlines and tabs of the inner
> echo: pf perfectly munges those, no need to clear them.
> 
> The "don't" -> "do not" is neccessary since otherwise the shell would
> choke on it as opening quote.
> 
> 
> Feedback? Comments?

Nice idea. The only maby irrelevant concern I have is, that using the
here-document approach uses a temporary file and if that for some reason
fails, we end up without this or mangled rules.



Re: rc: reorder_libs: [2/2] Pick archive versions more efficiently

2017-07-16 Thread Robert Peichaer
On Sun, Jul 16, 2017 at 01:55:02PM +0200, Klemens Nanni wrote:
> On Sun, Jul 16, 2017 at 10:26:25AM +0000, Robert Peichaer wrote:
> > But I'd like to stay strict matching the filenames.
> > 
> > +   for _liba in /usr/lib/lib{c,crypto}; do
> > +   _libas="$_libas $(ls $_liba.so.+([0-9.]).a | sort -V | tail -1)"
> > +   done
> > 
> > > + _libas=${_libas# }
> Agreed, I had a similiar approach first but then tried to reduce the
> differences to the essentials.
> 
> Here's an updated diff taking this into while also dropping $_l together
> with this hunk instead of the other one.
> 
> Feedback?
> 
> Index: rc
> ===
> RCS file: /cvs/src/etc/rc,v
> retrieving revision 1.507
> diff -u -p -r1.507 rc
> --- rc4 Jul 2017 19:02:11 -   1.507
> +++ rc16 Jul 2017 11:54:36 -
> @@ -158,7 +158,7 @@ make_keys() {
>  
>  # Re-link libraries, placing the objects in a random order.
>  reorder_libs() {
> - local _dkdev _l _liba _libas _mp _tmpdir _remount=false _error=false
> + local _dkdev _liba _libas _mp _tmpdir _remount=false _error=false
>  
>   [[ $library_aslr == NO ]] && return
>  
> @@ -171,13 +171,10 @@ reorder_libs() {
>   echo -n 'reordering libraries:'
>  
>   # Only choose the latest version of the libraries.
> - for _liba in /usr/lib/libc.so.*.a /usr/lib/libcrypto.so.*.a; do
> - _liba=$(ls ${_liba%%.[0-9]*}*.a | sort -V | tail -1)
> - for _l in $_libas; do
> - [[ $_l == $_liba ]] && continue 2
> - done
> - _libas="$_libas $_liba"
> + for _liba in /usr/lib/lib{c,crypto}; do
> + _libas="$_libas $(ls $_liba.so.+([0-9.]).a | sort -V | tail -1)"
>   done
> + _libas=${_libas# }
>  
>   # Remount read-write, if /usr/lib is on a read-only ffs filesystem.
>   if [[ $_mp == *' type ffs '*'read-only'* ]]; then

Provided you tested this throughly, OK.

-- 
-=[rpe]=-



Re: rc: reorder_libs: [2/2] Pick archive versions more efficiently

2017-07-16 Thread Robert Peichaer
On Sun, Jul 16, 2017 at 01:23:00PM +0200, Klemens Nanni wrote:
> On Sun, Jul 16, 2017 at 10:26:25AM +0000, Robert Peichaer wrote:
> > On Sun, Jul 16, 2017 at 03:37:15AM +0200, Klemens Nanni wrote:
> > > Why looping over all existing archives, picking the latest version of
> > > the current archive, skipping it in case it's already in our list of
> > > selected latest versions or adding it otherwise?
> > > 
> > > The current code runs ls|sort|tail about n * (v - 1) times for n
> > > different libraries and v versions respectively since the globbed list
> > > is almost always sorted already, effectively adding the latest versions
> > > after skipping all others.
> > 
> > Yeah well, until the globbing isn't sorting as expected.
> > 
> > $ ls -1 libc.so.*.a
> > libc.so.89.10.a
> > libc.so.89.6.a
> > libc.so.89.7.a
> > libc.so.89.8.a
> > libc.so.89.9.a
> Exactly, that's why sort(1) is used.
> > 
> > But that's not the point anyway in your otherwise perfectly valid change.
> >  
> > > This diff makes it much clearer and simpler by sorting and picking
> > > only as many versions as there are libraries to reorder (two). Globbing
> > > is done within the loop so future libraries with different naming
> > > schemes comes at no cost.
> > > 
> > > Applies cleanly to both the current revision as well as my previous diff
> > > but the previous one will fail on top of this one.
> > > 
> > > Feedback? Comments?
> > > 
> > > Index: rc
> > > ===
> > > RCS file: /cvs/src/etc/rc,v
> > > retrieving revision 1.507
> > > diff -u -p -r1.507 rc
> > > --- rc4 Jul 2017 19:02:11 -   1.507
> > > +++ rc16 Jul 2017 01:15:43 -
> > > @@ -171,13 +171,10 @@ reorder_libs() {
> > >   echo -n 'reordering libraries:'
> > >  
> > >   # Only choose the latest version of the libraries.
> > > - for _liba in /usr/lib/libc.so.*.a /usr/lib/libcrypto.so.*.a; do
> > > - _liba=$(ls ${_liba%%.[0-9]*}*.a | sort -V | tail -1)
> > > - for _l in $_libas; do
> > > - [[ $_l == $_liba ]] && continue 2
> > > - done
> > > - _libas="$_libas $_liba"
> > > + for _liba in 'libc.so.*.a' 'libcrypto.so.*.a'; do
> > > + _libas="$_libas $(ls /usr/lib/$_liba | sort -V | tail -1)"
> > >   done
> > 
> > That's definitely a much better approach.
> > 
> > But I'd like to stay strict matching the filenames.
> > 
> > +   for _liba in /usr/lib/lib{c,crypto}; do
> > +   _libas="$_libas $(ls $_liba.so.+([0-9.]).a | sort -V | tail -1)"
> > +   done
> > 
> > > + _libas=${_libas# }
> > 
> > This would be another way of suppressing the blank right away.
> > But it's not really necessary anyway, because the leading blank is
> > removed in the list of the other for-loop.
> > 
> > _libas="${_libas:+$_libas }$(ls $_liba.so.+([0-9.]).a | sort -V | tail 
> > -1)"
> I explicitly avoided this since stripping the leading space in the end
> seemed clearer about what's going on. Also, this check is done for each
> library whereas only the first one is true.
> 
> I also thought about leaving it in but stripping it makes clear I know
> what I'm doing and future changes where $_libas might get quoted will
> be working as expected.

To the contrary what my previous answer might indicate, I don't care
that much either. If you want to explicitely remove the blank, go for
it.

-- 
-=[rpe]=-



Re: rc: reorder_libs: [2/2] Pick archive versions more efficiently

2017-07-16 Thread Robert Peichaer
On Sun, Jul 16, 2017 at 03:37:15AM +0200, Klemens Nanni wrote:
> Why looping over all existing archives, picking the latest version of
> the current archive, skipping it in case it's already in our list of
> selected latest versions or adding it otherwise?
> 
> The current code runs ls|sort|tail about n * (v - 1) times for n
> different libraries and v versions respectively since the globbed list
> is almost always sorted already, effectively adding the latest versions
> after skipping all others.

Yeah well, until the globbing isn't sorting as expected.

$ ls -1 libc.so.*.a
libc.so.89.10.a
libc.so.89.6.a
libc.so.89.7.a
libc.so.89.8.a
libc.so.89.9.a

But that's not the point anyway in your otherwise perfectly valid change.
 
> This diff makes it much clearer and simpler by sorting and picking
> only as many versions as there are libraries to reorder (two). Globbing
> is done within the loop so future libraries with different naming
> schemes comes at no cost.
> 
> Applies cleanly to both the current revision as well as my previous diff
> but the previous one will fail on top of this one.
> 
> Feedback? Comments?
> 
> Index: rc
> ===
> RCS file: /cvs/src/etc/rc,v
> retrieving revision 1.507
> diff -u -p -r1.507 rc
> --- rc4 Jul 2017 19:02:11 -   1.507
> +++ rc16 Jul 2017 01:15:43 -
> @@ -171,13 +171,10 @@ reorder_libs() {
>   echo -n 'reordering libraries:'
>  
>   # Only choose the latest version of the libraries.
> - for _liba in /usr/lib/libc.so.*.a /usr/lib/libcrypto.so.*.a; do
> - _liba=$(ls ${_liba%%.[0-9]*}*.a | sort -V | tail -1)
> - for _l in $_libas; do
> - [[ $_l == $_liba ]] && continue 2
> - done
> - _libas="$_libas $_liba"
> + for _liba in 'libc.so.*.a' 'libcrypto.so.*.a'; do
> + _libas="$_libas $(ls /usr/lib/$_liba | sort -V | tail -1)"
>   done

That's definitely a much better approach.

But I'd like to stay strict matching the filenames.

+   for _liba in /usr/lib/lib{c,crypto}; do
+   _libas="$_libas $(ls $_liba.so.+([0-9.]).a | sort -V | tail -1)"
+   done

> + _libas=${_libas# }

This would be another way of suppressing the blank right away.
But it's not really necessary anyway, because the leading blank is
removed in the list of the other for-loop.

_libas="${_libas:+$_libas }$(ls $_liba.so.+([0-9.]).a | sort -V | tail 
-1)"
  
>   # Remount read-write, if /usr/lib is on a read-only ffs filesystem.
>   if [[ $_mp == *' type ffs '*'read-only'* ]]; then
> 

-- 
-=[rpe]=-



Re: rc: reorder_libs: [1/2] Drop unused _l, exit early on failure

2017-07-16 Thread Robert Peichaer
On Sun, Jul 16, 2017 at 07:18:00AM +0200, Theo Buehler wrote:
> On Sun, Jul 16, 2017 at 03:34:07AM +0200, Klemens Nanni wrote:
> > $_l is not used and picking the latest archive versions is of no use
> > if /usr/lib cannot be written to.
> > 
> > This patch applies cleanly before my next one but not vice versa.
> > 
> > Feedback? OK?
> 
> _l is only unused after your second patch :)
> 
> hoisting the remount over picking the library version makes sense,
> but you should keep it after the "echo -n 'reordering libraries:'"

The rationale to picking the library versions before remounting was
to keep the time window having rw /usr as small as possible.
If that's deemed ok, I'm too OK with switching the steps.



Re: RFC 7217: /etc/{rc,netstart} [4/8]

2017-07-15 Thread Robert Peichaer
On Sat, Jul 15, 2017 at 05:09:43PM +, Florian Obser wrote:
> netstart & simplification suggested by naddy
> 
> OK?
> 
> diff --git etc/netstart etc/netstart
> index 71890bc7a5b..047eef1ab83 100644
> --- etc/netstart
> +++ etc/netstart
> @@ -190,6 +190,8 @@ if $PRINT_ONLY && (($# == 0)); then
>   exit 1
>  fi
>  
> +$PRINT_ONLY || sysctl -q "net.inet6.ip6.soiikey=$( +
>  # If we were invoked with a list of interface names, just reconfigure these
>  # interfaces (or bridges), add default routes and return.
>  if (($# > 0)); then
> diff --git etc/rc etc/rc
> index 48e5671335f..a2e23b163a1 100644
> --- etc/rc
> +++ etc/rc
> @@ -154,6 +154,12 @@ make_keys() {
>   fi
>  
>   ssh-keygen -A
> +
> + if [[ ! -f /etc/soii.key ]]; then
> + openssl rand -hex 16 > /etc/soii.key && \
> + chmod 600 /etc/soii.key && sysctl -q \
> + "net.inet6.ip6.soiikey=$( + fi
>  }
>  
>  # Re-link libraries, placing the objects in a random order.

OK

-- 
-=[rpe]=-



Re: RFC 7217: installer support [8/8]

2017-07-15 Thread Robert Peichaer
On Sat, Jul 15, 2017 at 05:16:04PM +, Florian Obser wrote:
> OK?
> 
> diff --git distrib/miniroot/install.sub distrib/miniroot/install.sub
> index 26cecd81cbc..52360686b38 100644
> --- distrib/miniroot/install.sub
> +++ distrib/miniroot/install.sub
> @@ -2988,6 +2988,9 @@ do_upgrade() {
>   hostname $(stripcom /tmp/i/myname)
>   THESETS="$THESETS site$VERSION-$(hostname -s).tgz"
>  
> + _f=/mnt/etc/soii.key
> + [[ -f $_f ]] && sysctl net.inet6.ip6.soiikey=$(<$_f)
> +
>   # Configure the network.
>   enable_network

Still OK for the install.sub part ;-)



Re: RFC 7217 installer bits

2017-07-14 Thread Robert Peichaer
On Fri, Jul 14, 2017 at 03:43:52PM +, Florian Obser wrote:
> diff --git distrib/miniroot/install.sub distrib/miniroot/install.sub
> index 26cecd81cbc..bf6c562c882 100644
> --- distrib/miniroot/install.sub
> +++ distrib/miniroot/install.sub
> @@ -2988,6 +2988,9 @@ do_upgrade() {
>   hostname $(stripcom /tmp/i/myname)
>   THESETS="$THESETS site$VERSION-$(hostname -s).tgz"
>  
> + _f=/mnt/etc/soii.key
> + [[ -f $_f ]] && sysctl net.inet6.ip6.soiikey=$(stripcom $_f)
> +
>   # Configure the network.
>   enable_network
>  

OK for the install.sub part.



Re: RFC 7217: random but stable addresses (take 3)

2017-07-14 Thread Robert Peichaer
On Fri, Jul 14, 2017 at 11:56:02AM +, Florian Obser wrote:
> next try
> - sha512
> - fixed key size
> - /etc/soii.key
> - man page tweaks from sthen & jmc and rewording by me to get rid of 48 bits
> - link local address is updated when soii flag is toggled
> 
> If this is the final version I can cut up the diff and send parts if
> people prefer. But I also take OKs for the big one :)
> 
> diff --git etc/rc etc/rc
> index 48e5671335f..47dc78362c2 100644
> --- etc/rc
> +++ etc/rc
> @@ -47,6 +47,14 @@ update_limit() {
>   done
>  }
>  
> +# Apply soii.key settings.
> +soii_key() {
> + stripcom /etc/soii.key |
> + while read _line; do
> + sysctl -q "net.inet6.ip6.soiikey=$_line"
> + done
> +}
> +
>  # Apply sysctl.conf(5) settings.
>  sysctl_conf() {
>   stripcom /etc/sysctl.conf |
> @@ -60,6 +68,7 @@ sysctl_conf() {
>   update_limit -n openfiles;;
>   esac
>   done
> + soii_key
>  }
>  
>  # Apply mixerctl.conf(5) settings.
> @@ -154,6 +163,11 @@ make_keys() {
>   fi
>  
>   ssh-keygen -A
> +
> + if [[ ! -f /etc/soii.key ]]; then
> + openssl rand -hex 16 > /etc/soii.key && \
> + chmod 600 /etc/soii.key && soii_key
> + fi
>  }
>  
>  # Re-link libraries, placing the objects in a random order.

OK for the rc parts.



Re: install.sub: Fix scrambled address list in v6_defroute()

2017-07-10 Thread Robert Peichaer
On Sun, Jul 09, 2017 at 09:42:32AM +0200, Klemens Nanni wrote:
> On Wed, Jun 14, 2017 at 03:00:11AM +0200, Klemens Nanni wrote:
> > Installing -current the other day showed a broken list when picking
> > the IPv6 default route just like reported on bugs@ five days ago[1].
> 1:http://marc.info/?l=openbsd-bugs=149704197715791
> > 
> > This behaviour can be reproduced manually running the code from
> > v6_defroute():
> > 
> > $ # source/define bsort()
> > $ _if=trunk0
> > $ bsort $(ping6 -n -c 2 ff02::2%$_if 2>/dev/null |
> > sed -n '/bytes from/{s/^.*from //;s/,.*$//;p;}' |
> > sed -n 'G;s/\n/&&/;/^\(.*\n\).*\n\1/d;h;P')
> > fe80:::__ff:fe__:___%trunk0: hlim=64 icmp_seq=0 icmp_seq=1 ms 
> > time=0.431 time=0.802
> > 
> > The first sed filters for those lines containing the router addresses
> > and does two things:
> > 1. strip leading text up to the address
> > 2. strip trailing text after the first ','
> > 
> > However, as far as i can see 'ping6 -nc2' will never output lines that
> > would match the second criteria thus making this part of the command
> > obsolete.
> > 
> > The second sed removes duplicate addresses (not neccessarily consecutive
> > ones). This works fine for itself, but as seen above its input contains
> > more than just addresses. Besides that, sorting through sed just before
> > having bsort() cleaning the list is duplicate effort here.
> > 
> > With this patch bsort() gets passed possibly duplicate IPv6 addresses
> > one per line so _routers will eventually contain a sorted list of unique
> > router addresses; this fixes the list shown during the installer.
> > 
> > 
> > While debugging this, I noticed that stopping after two echo replies
> > would sometimes show only one router; increasing this limit to three
> > made both of my network's routers reply/show up reliably.
> I'm somewhat sursprised noone else has fixed or complained about this
> so far. This still annoys me when setting up boxes in (IPv6-only)
> networks with more than one router.
> 
> Feedback? Comments?
> 
> Marginally updated diff dropping ^ and $ from the sed command.

I'd be OK with your previous version of this diff.



Re: [PATCH] etc/ksh.kshrc - unify command substitution

2017-07-10 Thread Robert Peichaer
On Fri, Jul 07, 2017 at 05:47:46AM +0100, Raf Czlonka wrote:
> Hi all,
> 
> I've noticed that etc/ksh.kshrc uses both types of command substitution
> `command` and $(command). The below diff unifies it and uses
> $(command) notation consistently.
> 
> While there:
> 
> - remove ':' (null utility) from the very first line of the file -
>   I *do* understand what it does but it doesn't seem like it's needed
>   at all, unless I'm missing something (as is the case with some idioms)
> - remove basename(1) invocation and use parameter expansion instead
> - simplify one if conditional by replacing it with && and grouping
>   commands

Having sent a lot of similar diffs myself, I recommend to only change
one thing in a diff and not to mix stuff. Better send two diffs with one
type of change each than one diff with multiple.

> Regards,
> 
> Raf
> 
> Index: etc/ksh.kshrc
> ===
> RCS file: /cvs/src/etc/ksh.kshrc,v
> retrieving revision 1.27
> diff -u -p -u -r1.27 ksh.kshrc
> --- etc/ksh.kshrc 14 Sep 2016 18:34:51 -  1.27
> +++ etc/ksh.kshrc 7 Jul 2017 04:38:58 -
> @@ -1,4 +1,3 @@
> -:
>  #$OpenBSD: ksh.kshrc,v 1.27 2016/09/14 18:34:51 rpe Exp $
>  #
>  # NAME:
> @@ -39,7 +38,7 @@ case "$-" in
>   0) PS1S='# ';;
>   esac
>   PS1S=${PS1S:-'$ '}
> - HOSTNAME=${HOSTNAME:-`uname -n`}
> + HOSTNAME=${HOSTNAME:-$(uname -n)}

OK

>   HOST=${HOSTNAME%%.*}
>  
>   PROMPT="$USER:!$PS1S"
> @@ -49,8 +48,8 @@ case "$-" in
>   PS1=$PPROMPT
>   # $TTY is the tty we logged in on,
>   # $tty is that which we are in now (might by pty)
> - tty=`tty`
> - tty=`basename $tty`
> + tty=$(tty)
> + tty=${tty##*/}

OK

>   TTY=${TTY:-$tty}
>   # $console is the system console device
>   console=$(sysctl kern.consdev)
> @@ -74,9 +73,8 @@ case "$-" in
>   xterm*)
>   ILS='\033]1;'; ILE='\007'
>   WLS='\033]2;'; WLE='\007'
> - if ps -p $PPID -o command | grep -q telnet; then
> + { ps -p $PPID -o command | grep -q telnet; } &&
>   export TERM=xterms
> - fi

If at all this would be

ps -p $PPID -o command | grep -q telnet &&
export TERM=xterms

But I doubt this is any improvement compared to if-then-else.
In contrast to the installer script, there is no need for a
terse shell scripting style.

>   ;;
>   *)  ;;
>   esac
> @@ -117,7 +115,7 @@ case "$-" in
>   alias o='fg %-'
>   alias df='df -k'
>   alias du='du -k'
> - alias rsize='eval `resize`'
> + alias rsize='eval $(resize)'

OK

>  ;;
>  *)   # non-interactive
>  ;;
> @@ -142,6 +140,6 @@ function pre_path {
>  }
>  # if $1 is in path, remove it
>  function del_path {
> - no_path $* || eval ${2:-PATH}=`eval echo :'$'${2:-PATH}: |
> - sed -e "s;:$1:;:;g" -e "s;^:;;" -e "s;:\$;;"`
> + no_path $* || eval ${2:-PATH}=$(eval echo :'$'${2:-PATH}: |
> + sed -e "s;:$1:;:;g" -e "s;^:;;" -e "s;:\$;;")

OK

>  }
> 

-- 
-=[rpe]=-



Re: install.sub: Typo/whitespace nit

2017-07-07 Thread Robert Peichaer
On Tue, Jul 04, 2017 at 02:14:58AM +0200, Klemens Nanni wrote:
> Remove duplicate full stop and add space after function name.
> 
> Feedback/OK?
> 
> Index: install.sub
> ===
> RCS file: /cvs/src/distrib/miniroot/install.sub,v
> retrieving revision 1.1019
> diff -u -p -r1.1019 install.sub
> --- install.sub   2 Jul 2017 12:45:43 -   1.1019
> +++ install.sub   4 Jul 2017 00:11:59 -
> @@ -275,7 +275,7 @@ diskinfo() {
>   _i=${_i##+([[:space:],])}
>   _i=${_i%%+([[:space:],])}
> 
> - # Extract Network Address Authority information from dmesg..
> + # Extract Network Address Authority information from dmesg.
>   _n=$(dmesg | sed -En '/^'$_d' at /h;${g;s/^.* 
> ([a-z0-9]+\.[a-zA-Z0-9_]+)$/\1/p;}')
> 
>   # Extract disk size from disklabel output.
> @@ -2968,7 +2970,7 @@ do_install() {
>   finish_up
> }
> 
> -do_upgrade(){
> +do_upgrade() {
>   # Get $ROOTDISK and $ROOTDEV
>   get_rootinfo
 
OK



Re: install.sub: Clean v[46]_info() ouput

2017-07-03 Thread Robert Peichaer
On Wed, Jun 14, 2017 at 05:37:07PM +0200, Klemens Nanni wrote:
> With this patch, v[46]_info() both output exactly what their description
> says.
> 
> As of now, these functions are only used through
>   set -- $(v4_info $_if)
> which gracefully handles any constellation of whitespaces in the output
> just fine. However future usage might change (or not) and being precise
> about a function's output doesn't hurt.
> 
> The sed command itself is a tad clearer/smarter that way as well, imho.
> 
> Here's what I mean (whitespaces visualised):
> 
>   $ v4_info trunk0
>   UP
>   ??? ??192.168.1.20xff00??broadcast??192.168.1.255
>   $ v4_info_clean trunk0
>   UP
>   192.168.1.2??0xff00??broadcast??192.168.1.255
> 
> 
> Feeback/OK?
> 
> Index: install.sub
> ===
> RCS file: /cvs/src/distrib/miniroot/install.sub,v
> retrieving revision 1.1014
> diff -u -p -r1.1014 install.sub
> --- install.sub   3 Jun 2017 22:27:41 -   1.1014
> +++ install.sub   14 Jun 2017 15:13:05 -
> @@ -896,13 +896,14 @@ __EOT
> }
> 
> # Obtain and output the inet information related to interface $1.
> -# Should output '   '.
> +# Outputs '\n  '.
> v4_info() {
>   ifconfig $1 inet | sed -n '
> - 1s/.* - 1s/.*<.*/DOWN/p
> - /inet/s/netmask//
> - /inet/s///p'
> + 1{  s/.* + s/.*<.*/DOWN/p; }
> + /inet/{ s///
> + s/^[[:space:]]*//
> + s/netmask //p; }'
> }
> 
> # Convert a netmask in hex format ($1) to dotted decimal format.
> @@ -981,14 +982,15 @@ v4_config() {
> }
> 
> # Obtain and output the inet6 information related to interface $1.
> -# Should output ''.
> +# Outputs '\n  '.
> v6_info() {
>   ifconfig $1 inet6 | sed -n '
> - 1s/.* - 1s/.*<.*/DOWN/p
> - /scopeid/d
> - /inet6/s/prefixlen//
> - /inet6/s///p'
> + 1{  s/.* + s/.*<.*/DOWN/p; }
> + /inet6/{s///
> + /scopeid/d
> + s/^[[:space:]]*//
> + s/prefixlen //p; }'
> }
> 
> # Set up IPv6 default route on interface $1.

Dokument explicitely possible outputs and tweak the sed expressions
to remove the superfluous whitespaces. I guess that does the trick.

Index: install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1019
diff -u -p -p -u -r1.1019 install.sub
--- install.sub 2 Jul 2017 12:45:43 -   1.1019
+++ install.sub 3 Jul 2017 22:34:04 -
@@ -896,13 +896,16 @@ __EOT
 }
 
 # Obtain and output the inet information related to interface $1.
-# Should output '   '.
+# Outputs one of:
+# 
+# 
+# \n  [\n]
 v4_info() {
ifconfig $1 inet | sed -n '
1s/.*

Re: install.sub: ieee80211_{scan,config}: Allow quoted SSIDs

2017-07-02 Thread Robert Peichaer
On Thu, Jun 15, 2017 at 12:09:20AM +0200, Klemens Nanni wrote:
> Instead of ignoring SSIDs containing whitespaces, slightly adjust the
> commands to take everything in between 'nwid ' and ' chan' as SSID; if
> it has double quotes at start *and* end, simply remove those.
> 
> This enables users to select networks such as "Unitymedia WifiSpot"
> "FRITZ!Box 7490" for example which are common among the quoted ones at
> least here in germany.
> 
> The only SSIDs known to break this are those containing ' chan ' as this
> substring is used as delimiter. Picking "some chan 4 me" would therefore
> result in _nwid being assigned '"some' (literal double quote), but than
> reasonably acceptable (compared to the current behaviour).
> 
> 
> Since cat's -n flag already takes care of the space between numbers and
> input lines, remove the leading tab to avoid excessive widths for lines
> with long SSIDs.
> 
> Feedback/OK?

I agree, that supporting SSIDs containing whitespaces would be an improvement.

> 
> Index: install.sub
> ===
> RCS file: /cvs/src/distrib/miniroot/install.sub,v
> retrieving revision 1.1014
> diff -u -p -r1.1014 install.sub
> --- install.sub   3 Jun 2017 22:27:41 -   1.1014
> +++ install.sub   14 Jun 2017 22:06:47 -
> @@ -1060,10 +1060,9 @@ v6_config() {
>  # Perform an 802.11 network scan on interface $1.
>  # The result is cached in $WLANLIST.
>  ieee80211_scan() {
> - # N.B. Skipping quoted nwid's for now.
>   [[ -f $WLANLIST ]] ||
>   ifconfig $1 scan |
> - sed -n 's/^ nwid \([^"]\)/\1/p' >$WLANLIST
> + sed -n 's/^[[:space:]]*nwid //p' >$WLANLIST
>   cat $WLANLIST
>  }
>  
> @@ -1078,15 +1077,16 @@ ieee80211_config() {
>   # Empty scan cache.
>   rm -f $WLANLIST
>  
> - while [[ -z $_nwid ]]; do
> + while [[ -z "$_nwid" ]]; do
>   ask_until "Access point? (ESSID, 'any', list# or '?')" "any"
>   case "$resp" in
>   +([0-9]))
> - _nwid=$(ieee80211_scan $_if | sed -n "${resp}s/ .*//p")
> + _nwid=$(ieee80211_scan $_if | sed -n ${resp}'{s/ chan 
> .*//p;q;}')
>   [[ -z $_nwid ]] && echo "There is no line $resp."
> + [[ $_nwid = \"*\" ]] && _nwid=${_nwid#\"} 
> _nwid=${_nwid%\"}
>   ;;
>   \?) ieee80211_scan $_if |
> - sed -n 's/^\([^ ]*\) chan .* bssid \([^ ]*\) 
> .*$/   \1 (\2)/p' |
> + sed -n 's/^\(.*\) chan .* bssid \([^ ]*\) 
> .*$/\1 (\2)/p' |
>   cat -n | more -c
>   ;;
>   *)  _nwid=$resp

Here's a slightly different but completely untested approach ...

ieee80211_scan()
  - Extract the needed information (nwid, bssid) using a very specific
sed expression. Any line, not matching this expr is ignored.

  - Remove leading and trailing double-quotes in case of nwids with
spaces.

  - Write nwid and bssid into WLANLIST as '()'.

ieee80211_config()
  - just print WLANLIST using ieee80211_scan() if the user chooses
'?' which has the right format already

  - in case the user selects an entry from WLANLIST using a number,
remove the '()' part from the line, resulting in
the nwid (without double-quotes)

  - using the quote() function with the ifconfig command ensures,
that the nwid is quoted properly with single-quotes in case it
contains spaces

  - using the quote() function when writing the nwid to the hostname.if
files ensures that the nwid is quoted properly with single-quotes
in case it contains spaces

The parse_hn_line() function in netstart does handle quoted nwids
properly when processing the hostname.if config lines as far as I
can see.


Index: install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1019
diff -u -p -p -u -r1.1019 install.sub
--- install.sub 2 Jul 2017 12:45:43 -   1.1019
+++ install.sub 2 Jul 2017 16:21:42 -
@@ -1060,10 +1060,10 @@ v6_config() {
 # Perform an 802.11 network scan on interface $1.
 # The result is cached in $WLANLIST.
 ieee80211_scan() {
-   # N.B. Skipping quoted nwid's for now.
[[ -f $WLANLIST ]] ||
ifconfig $1 scan |
-   sed -n 's/^ nwid \([^"]\)/\1/p' >$WLANLIST
+   sed -En 's/^[[:space:]]+nwid (.*) chan [0-9]+ bssid 
([[:xdigit:]:]+) .*$/\1 (\2)/p' |
+   sed 's/"\(.*\)"/\1/' >$WLANLIST
cat $WLANLIST
 }
 
@@ -1082,12 +1082,11 @@ ieee80211_config() {
ask_until "Access point? (ESSID, 'any', list# or '?')" "any"
case "$resp" in
+([0-9]))
-   _nwid=$(ieee80211_scan $_if | sed -n "${resp}s/ .*//p")
+   

Re: kernel relinking at install/upgrade time

2017-06-30 Thread Robert Peichaer
On Mon, Jun 26, 2017 at 02:35:55PM -0600, Theo de Raadt wrote:
> There is a diff in snapshots which does kernel relinking during
> install or upgrade.
> 
> Really amazing...

This is now committed to the tree.

-- 
-=[rpe]=-



Re: Update list of invalid users in install.sub

2017-05-06 Thread Robert Peichaer
On Fri, May 05, 2017 at 07:04:55PM +, Callum R. Davies wrote:
> Hi tech@, was looking through the tree for the providence of the
> amusing "No really..." message in the installer and saw that the list
> of invalid users needed updating.  Names are in the order found in
> passwd, with the exception of ftp.
> 
> Index: distrib/miniroot/install.sub
> ===
> RCS file: /cvs/src/distrib/miniroot/install.sub,v
> retrieving revision 1.1000
> diff -u -p -r1.1000 install.sub
> --- distrib/miniroot/install.sub  1 May 2017 14:29:39 -   1.1000
> +++ distrib/miniroot/install.sub  5 May 2017 18:58:54 -
> @@ -1945,7 +1945,7 @@ user_setup() {
>   y|yes)  _q="No really, what is the lower-case loginname, or 
> 'no'?"
>   continue
>   ;;
> - root|daemon|operator|bin|sshd|www|nobody|ftp)
> + root|daemon|operator|bin|sshd|uucp|www|nobody|build|ftp)
>   ;;
>   [a-z]*([a-z0-9_]))
>   ((${#resp} <= 31)) && break
> 

You're right about build, just committed that one.
uucp is not part of the system anymore.

-- 
-=[rpe]=-



Re: Xorg stipple

2017-02-26 Thread Robert Peichaer
On Sun, Feb 26, 2017 at 10:43:50AM +0100, Landry Breuil wrote:
> On Wed, Mar 09, 2016 at 05:09:13PM -0600, joshua stein wrote:
> > Is anyone seriously finding video/Xorg bugs through the default X
> > stipple pattern anymore?  Xorg changed the default to draw a black
> > background a while ago (with stipple enabled using the -retro flag),
> > but we have this local change that reverted it while adding a silly
> > -retard flag in order to show the black background.
> > 
> > I think we can finally stop partying like it's 1989 (vax is dead,
> > after all) and have X show a solid black background by default.
> 
> Reviving this thread because everyone likes to party like it's
> 1989^W^W^W^Wthe smell of a sunday morning bikeshed From what i
> understand, we default to the -retro mode, that option is #ifndef'ed
> out, but Xserver(1) still mentions it. We instead provide (since xserver
> 1.6.4, 8 years ago) an undocumented -retard option which is supposed to
> have the default 'black background' behaviour. And then, there's -br
> flag.
> 
> So.. should we fix the code (from what i understood in the thread,
> there was opposition) or the manpage ?
> Or document (where?) that one can use xsetroot -solid in
> /etc/X11/xdm/Xsetup_0 to paint the default xdm/X background to its
> own bikeshed color ?
> 

xmd(1) already points to Xsetup_0:

   DisplayManager.DISPLAY.setup
  This specifies a program which is run (as root) before offering
  the Login window.  This may be used to change the appearance of
  the screen around the Login window or to put up other windows
  (e.g., you may want to run xconsole here).  By default, no
  program is run.  The conventional name for a file used here is
  Xsetup.  See the section Setup Program.

-- 
-=[rpe]=-



Re: add empty /root/.ssh/authorized_keys to mtree/sets ?

2017-02-05 Thread Robert Peichaer
On Sun, Feb 05, 2017 at 10:46:41AM +0100, Landry Breuil wrote:
> Hi,
> 
> when installing 'throwaway' VMs (manually, not always using autoinstall for
> $REASONS) i've often found myself having to do right after the install:
> install -d -m 700 /root/.ssh
> install -m 600 /dev/null /root/.ssh/authorized_keys
> (or touch /root/.ssh/authorized_keys && chmod 600
> /root/.ssh/authorized_keys, ymmv)
> 
> those are present in /etc/skel for "real" users, so why not creating
> them for the root account ? install.sub also creates /mnt/root/.ssh when
> using autoinstall and giving an ssh pubkey, so that'll be one less step
> to do there.
> 
> We advise ppl to set prohibit-password for PermitRootLogin, so why not make it
> easier to use it ? This ways, the correct modes are set.. i often fat-fingered
> this, to see sshd complaining (rightly!) about bad modes on 
> .ssh/authorized_keys.

Conceptually I'd like this going in.

> Conceptual (untested) diff below for discussion, i'll build a release with it
> depending on the feedback/opinions..
> 
> Landry
> 
> Index: Makefile
> ===
> RCS file: /cvs/src/etc/Makefile,v
> retrieving revision 1.449
> diff -u -r1.449 Makefile
> --- Makefile  2 Feb 2017 21:35:05 -   1.449
> +++ Makefile  5 Feb 2017 09:34:58 -
> @@ -110,6 +110,8 @@
>   ${DESTDIR}/root/.Xdefaults; \
>   ${INSTALL} -c -o root -g wheel -m 644 dot.cvsrc \
>   ${DESTDIR}/root/.cvsrc; \
> + ${INSTALL} -c -o root -g wheel -m 600 /dev/null \
> + ${DESTDIR}/root/.ssh/authorized_keys
>   rm -f ${DESTDIR}/.cshrc ${DESTDIR}/.profile; \
>   ${INSTALL} -c -o root -g wheel -m 644 dot.cshrc \
>   ${DESTDIR}/.cshrc; \
> Index: mtree/4.4BSD.dist
> ===
> RCS file: /cvs/src/etc/mtree/4.4BSD.dist,v
> retrieving revision 1.293
> diff -u -r1.293 4.4BSD.dist
> --- mtree/4.4BSD.dist 27 Dec 2016 09:17:52 -  1.293
> +++ mtree/4.4BSD.dist 5 Feb 2017 09:34:58 -
> @@ -118,6 +118,8 @@
>  mnt
>  ..
>  root mode=0700
> +.ssh uname=root mode=0700
> +..
>  ..
>  sbin
>  ..
> 



Re: clean up and modernize test calls in bsd.obj.mk

2017-01-23 Thread Robert Peichaer
On Tue, Jan 24, 2017 at 01:01:57PM +1000, Theo Buehler wrote:
> We're currently using several idioms for conditionally executing code in
> bsd.obj.mk. I'd like to unify them for the sake of readability and
> consistency.  This was done joint with rpe.

OK rpe@
 
> Index: share/mk/bsd.obj.mk
> ===
> RCS file: /cvs/src/share/mk/bsd.obj.mk,v
> retrieving revision 1.18
> diff -u -p -r1.18 bsd.obj.mk
> --- share/mk/bsd.obj.mk   24 Jan 2017 02:56:50 -  1.18
> +++ share/mk/bsd.obj.mk   24 Jan 2017 02:58:50 -
> @@ -33,20 +33,19 @@ obj! _SUBDIRUSE
>   SETOWNER=:; \
>   fi; \
>   [[ -z $$MKDIRS ]] && MKDIRS="mkdir -p"; \
> - if test $$here != $$subdir ; then \
> + if [[ $$here != $$subdir ]]; then \
>   dest=${BSDOBJDIR}/$$subdir ; \
>   echo "$$here/${__objdir} -> $$dest"; \
> - if test ! -L ${__objdir} -o \
> - X`readlink ${__objdir}` != X$$dest; \
> + if [[ ! -L ${__objdir} || `readlink ${__objdir}` != $$dest ]]; \
>   then \
> - if test -e ${__objdir}; then rm -rf ${__objdir}; fi; \
> + [[ -e ${__objdir} ]] && rm -rf ${__objdir}; \
>   ln -sf $$dest ${__objdir}; \
>   $$SETOWNER ${__objdir}; \
>   fi; \
> - if test -d ${BSDOBJDIR}; then \
> - test -d $$dest || $$MKDIRS $$dest; \
> + if [[ -d ${BSDOBJDIR} ]]; then \
> + [[ -d $$dest ]] || $$MKDIRS $$dest; \
>   else \
> - if test -e ${BSDOBJDIR}; then \
> + if [[ -e ${BSDOBJDIR} ]]; then \
>   echo "${BSDOBJDIR} is not a directory"; \
>   else \
>   echo "${BSDOBJDIR} does not exist"; \
> @@ -54,7 +53,7 @@ obj! _SUBDIRUSE
>   fi; \
>   else \
>   dest=$$here/${__objdir} ; \
> - if test ! -d ${__objdir} ; then \
> + if [[ ! -d ${__objdir} ]]; then \
>   echo "making $$dest" ; \
>   $$MKDIRS $$dest; \
>   $$SETOWNER $$dest; \



Re: Installer error

2017-01-11 Thread Robert Peichaer
On Wed, Jan 11, 2017 at 01:21:30PM +0100, Theo Buehler wrote:
> On Wed, Jan 11, 2017 at 01:10:12PM +0100, Theo Buehler wrote:
> > On Wed, Jan 11, 2017 at 11:52:02AM +, Pedro Caetano wrote:
> > > Hi tech@
> > > 
> > > I was running an headless installation via serial using today's snapshot
> > > (10th January), and noticed something odd in the end of the installation
> > > proccess.
> > > 
> > > Transcription below:
> > > What timezone are you in? ('?' for list) [Canada/Mountain] Europe/Lisbon
> > > Saving configuration files...sed: /tmp/i/hosts: No such file or directory
> > > done.
> > > Making all device nodes...done.
> > > Multiprocessor machine; using bsd.mp instead of bsd.
> > 
> > The hosts file need not exist if no interfaces were configured (as is
> > noted in a comment a few lines after that sed command), so simply check
> > whether it exists before running sed:
> 
> This way we don't check twice whether the file exists:

OK rpe

> 
> Index: install.sub
> ===
> RCS file: /var/cvs/src/distrib/miniroot/install.sub,v
> retrieving revision 1.945
> diff -u -p -r1.945 install.sub
> --- install.sub   10 Jan 2017 17:50:58 -  1.945
> +++ install.sub   11 Jan 2017 12:19:58 -
> @@ -2714,15 +2714,14 @@ do_install(){
>   # domain information or aliases. These are lines the user added/changed
>   # manually.
>  
> - # Remove entry for ftp.openbsd.org before final hosts file is created.
> - sed -i '/129.128.5.191/d' /tmp/i/hosts
> -
>   # Add common entries.
>   echo "127.0.0.1\tlocalhost" >/mnt/etc/hosts
>   echo "::1\t\tlocalhost" >>/mnt/etc/hosts
>  
>   # Note we may have no hosts file if no interfaces were configured.
>   if [[ -f /tmp/i/hosts ]]; then
> + # Remove the entry for ftp.openbsd.org
> + sed -i '/129.128.5.191/d' /tmp/i/hosts
>   _dn=$(get_fqdn)
>   while read _addr _hn _aliases; do
>   if [[ -n $_aliases || $_hn != ${_hn%%.*} || -z $_dn ]]; 
> then
> 

-- 
-=[rpe]=-



Re: Improve error message in rcctl(8)

2016-09-06 Thread Robert Peichaer
> Hi tech@,
> 
> Daemon names historically match Antoine's alphanumeric proposal, and I
> think underscore is a bit too much, if it's present use minus instead.
> The logic behind this?  Match this to word termination symbols in ksh.
> 
> Kind regards,
> Anton

$ find /usr/ports -name '*_*.rc' -type f | wc -l
  85

-- 
-=[rpe]=-



Re: anti-ROP mechanism in libc

2016-04-25 Thread Robert Peichaer
On Mon, Apr 25, 2016 at 10:57:37AM -0400, Ted Unangst wrote:
> Theo de Raadt wrote:
> > +   cp -p /usr/lib/$_lib /usr/lib/$_tmplib
> > +   install -o root -g bin -m 0444 $_lib /usr/lib/$_lib &&
> > +   rm -f /usr/lib/$_tmplib ||
> > +   mv /usr/lib/$_tmplib /usr/lib/$_lib
> 
> I'm a little confused by what's going on here. If the install fails, do we
> still want to overwrite the lib?
 

If the install fails, the original library file is restored.

The "install .. && rm .. || mv ..." is identical to if-then-else and could
be written like this too.

if install -o root -g bin -m 0444 $_lib /usr/lib/$_lib; then
rm -f /usr/lib/$_tmplib
else
mv /usr/lib/$_tmplib /usr/lib/$_lib
fi



Re: [PATCH] make 'set +o' useful and POSIX compatible

2016-03-06 Thread Robert Peichaer
On Sun, Mar 06, 2016 at 10:56:45AM +0100, Martin Natano wrote:
> On Sun, Mar 06, 2016 at 05:32:16AM +0100, Martijn Dekker wrote:
> > The command 'set -o' shows the current shell options in an unspecified
> > format. Less well-known is the variant 'set +o', which should output the
> > current shell options "in a format that is suitable for reinput to the
> > shell as commands that achieve the same options settings".[*]
> > 
> > That means it should be possible to do something like
> > 
> > save_options=$(set +o)
> > 
> > then change some options, then later restore the shell options with
> > 
> > eval "$save_options"
> > 
> > On all pdksh variants (as well as zsh), 'set +o' is currently inadequate
> > for that purpose because it only outputs the currently active shell
> > options, and not the inactive ones.
> > 
> > Even the old ksh88, which pdksh is a clone of (and on which most of
> > POSIX is based), acts correctly in this regard.
> > 
> > This simple patch makes 'set +o' compatible with the POSIX spec.
> 
> Makes sense to me. However, I'm somewhat concerned this might break
> existing shell scripts. Anyone here have a script that would break with
> this change? Or a script that relies on POSIX behaviour and gets fixed
> with this diff?
> 
> natano

As far as I can tell, at least in our tree I found no scripts that use that
functionality.



Re: netstart: only call ifautoconf is rtsolif is populated

2015-09-13 Thread Robert Peichaer
On Sun, Sep 13, 2015 at 01:35:02PM +0100, Stuart Henderson wrote:
> On 2015/09/13 13:19, Stuart Henderson wrote:
> > Avoid printing "IPv6 autoconf:" if you have no v6 rtsol interfaces.
> > OK?
> 
> As suggested by rpe, just do the check once in ifautoconf.
> (I was trying to avoid the indent, but it's not too horrible)
> 
> Index: netstart
> ===
> RCS file: /cvs/src/etc/netstart,v
> retrieving revision 1.154
> diff -u -p -r1.154 netstart
> --- netstart  11 Sep 2015 12:21:52 -  1.154
> +++ netstart  13 Sep 2015 12:33:38 -
> @@ -154,16 +154,18 @@ ifmstart() {
>  # IPv6 autoconf the interfaces in the list at $rtsolif
>  # Usage: ifautoconf
>  ifautoconf() {
> - printf 'IPv6 autoconf:'
> - # $ip6kernel will not have been set if we were invoked with a
> - # list of interface names
> - if ifconfig lo0 inet6 >/dev/null 2>&1; then
> - for curif in $rtsolif; do
> - printf ' %s' $curif
> - ifconfig $curif inet6 autoconf
> - done
> + if [[ -n $rtsolif ]]; then
> + printf 'IPv6 autoconf:'
> + # $ip6kernel will not have been set if we were invoked with a
> + # list of interface names
> + if ifconfig lo0 inet6 >/dev/null 2>&1; then
> + for curif in $rtsolif; do
> + printf ' %s' $curif
> + ifconfig $curif inet6 autoconf
> + done
> + fi
> + echo
>   fi
> - echo
>  }
>  
>  # Get network related vars from rc.conf using the parsing routine from 
> rc.subr.
> 

fwiw. OK rpe@

-- 
-=[rpe]=-



Re: Update afterboot(8) for new PermitRootLogin default

2015-08-01 Thread Robert Peichaer
On Sat, Aug 01, 2015 at 08:25:06AM +0100, Jason McIntyre wrote:
 On Fri, Jul 31, 2015 at 07:20:36PM -0400, Michael Reed wrote:
  Hi all,
  
  I noticed that the default for the sshd_config option PermitRootLogin
  changed from yes to no [1], but afterboot(8) still refers to it as
  if yes is the default.
  
  Perhaps the sub-section could be reworded a bit to clarify the new
  default, but I'll leave that to the developers as I'm unsure what they want.
  
  Regards,
  Michael
  
  [1]: http://www.openbsd.org/faq/current.html#20150428
  
  
  
  Index: src/share/man/man8/afterboot.8
  ===
  RCS file: /cvs/src/share/man/man8/afterboot.8,v
  retrieving revision 1.147
  diff -u -p -r1.147 afterboot.8
  --- src/share/man/man8/afterboot.8  30 Jul 2015 08:03:49 -  1.147
  +++ src/share/man/man8/afterboot.8  31 Jul 2015 23:02:11 -
  @@ -90,12 +90,12 @@ If that option was not used, see the par
   .Sx Add new users
   below.
   .Pp
  -To deny root logins over the network, edit the
  +To permit root logins over the network, edit the
   .Pa /etc/ssh/sshd_config
   file and set
   .Cm PermitRootLogin
   to
  -.Dq no
  +.Dq yes
   (see
   .Xr sshd_config 5 ) .
   .Ss Root password
  
 
 if we do it this way, it almost sounds like we're recommending people do
 this. if the default is now root logins are denied, i'd say telling
 people how to permit them has no relevant place in afterboot(8). so i
 propose zapping it altogether.
 
 ok?
 jmc

I think that would make sense.
fwiw, OK rpe@

-- 
-=[rpe]=-



Re: sed -i

2015-07-17 Thread Robert Peichaer
On Fri, Jul 17, 2015 at 06:10:46PM +0200, Jasper Lievisse Adriaanse wrote:
 Hi,
 
 Here's a diff to add the '-i' flag to sed to do inplace edits. It's mostly
 from FreeBSD with some adjustments to prevent a race with unlink() and fopen()
 during the tempfile creation.
 
 It's been tested in a full ports bulk (thanks aja), and went through a build
 of base and xenocara.
 Regress tests will also be added for this.
 
 This diff is already OK millert@. Any more OKs?

I can not comment on the implementation itself, but I would love to have
this feature in our sed.

So fwiw, OK rpe

-- 
-=[rpe]=-



Re: autoinstall(8) tweaks

2015-04-07 Thread Robert Peichaer
On Mon, Apr 06, 2015 at 09:01:51PM +0100, Robert Peichaer wrote:
 On Mon, Apr 06, 2015 at 09:48:58PM +0800, Nathanael Rensen wrote:
  A couple of autoinstall(8) tweaks that I find useful.
  
  I find it convenient to be able to specify a path to the response file.
  I also prefer to use the DHCP supplied hostname rather than the MAC
  address.
 
 Actually, these ideas are quite interesting...
 
 Being able to use the hostname seems to be a good addition, easy enough
 to implement and doesn't interfere with existing functionality.
 
 Using the filename DHCP option to carry over the path to the response
 file on the webserver is possible, but that needs some extra setup.
 
 If the responsefile is in documentroot/path/to, like:
 
 /var/www/htdocs/path/to/install.conf
 
 The tftp setup would need to be setup like this:
 
 /tftpboot/etc/boot.conf
 /tftpboot/pxeboot
 /tftpboot/path/to/auto_install - ../../pxeboot
 /tftpboot/path/to/auto_upgrade - ../../pxeboot
 
  It's handy for the log to remain on the host as a reference. Sometimes the
  email gets lost (e.g. caught in a spam trap).
 
 Not sure about this yet.
 
 
 I've attached a slighly modified diff for the hostname and path addition
 leaving out the manpage parts for now.

Here's the manpage diff for that...


Index: autoinstall.8
===
RCS file: /home/cvs/src/share/man/man8/autoinstall.8,v
retrieving revision 1.11
diff -p -u -r1.11 autoinstall.8
--- autoinstall.8   23 Oct 2014 21:33:21 -  1.11
+++ autoinstall.8   7 Apr 2015 09:38:02 -
@@ -49,12 +49,16 @@ always fetches the response file via the
 .Nm
 uses HTTP to fetch one of the files
 .Pa install.conf
-or
+,
 .Ar MAC_address Ns - Ns Pa install.conf
+or
+.Ar hostname Ns - Ns Pa install.conf
 for install answers, or one of
 .Pa upgrade.conf
-or
+,
 .Ar MAC_address Ns - Ns Pa upgrade.conf
+or
+.Ar hostname Ns - Ns Pa upgrade.conf
 for upgrade answers.
 The URL used to fetch the file is constructed from information provided in
 the
@@ -71,6 +75,7 @@ then the URLs tried are, in order:
 .Sm off
 .Bd -unfilled -offset indent
 .No http:// Ar next-server No / Ar MAC_address No -install.conf
+.No http:// Ar next-server No / Ar hostname No -install.conf
 .No http:// Ar next-server No /install.conf
 .Ed
 .Sm on
@@ -79,7 +84,10 @@ where
 .Ar MAC_address
 is a string of six hex octets separated by colons
 representing the MAC
-address of the interface being used to fetch the files.
+address of the interface being used to fetch the files,
+and
+.Ar hostname
+is the hostname assigned to the system by DHCP.
 .Pp
 If the
 .Ar filename
@@ -89,6 +97,7 @@ the URLs tried are, in order:
 .Sm off
 .Bd -unfilled -offset indent
 .No http:// Ar next-server No / Ar MAC_address No -upgrade.conf
+.No http:// Ar next-server No / Ar hostname No -upgrade.conf
 .No http:// Ar next-server No /upgrade.conf
 .Ed
 .Sm on
@@ -109,6 +118,10 @@ file to be
 .Cm auto_install
 or
 .Cm auto_upgrade .
+.Pp
+To use a subdirectory as response file location on the HTTP server, the same
+directory structure has to exist in the tftproot directory containing the
+auto_install or auto_upgrade symbolic links.
 .Pp
 Note that in these cases, the HTTP server and TFTP server must
 be on the same machine.



Re: autoinstall(8) tweaks

2015-04-06 Thread Robert Peichaer
On Mon, Apr 06, 2015 at 09:48:58PM +0800, Nathanael Rensen wrote:
 A couple of autoinstall(8) tweaks that I find useful.
 
 I find it convenient to be able to specify a path to the response file.
 I also prefer to use the DHCP supplied hostname rather than the MAC
 address.

Actually, these ideas are quite interesting...

Being able to use the hostname seems to be a good addition, easy enough
to implement and doesn't interfere with existing functionality.

Using the filename DHCP option to carry over the path to the response
file on the webserver is possible, but that needs some extra setup.

If the responsefile is in documentroot/path/to, like:

/var/www/htdocs/path/to/install.conf

The tftp setup would need to be setup like this:

/tftpboot/etc/boot.conf
/tftpboot/pxeboot
/tftpboot/path/to/auto_install - ../../pxeboot
/tftpboot/path/to/auto_upgrade - ../../pxeboot

 It's handy for the log to remain on the host as a reference. Sometimes the
 email gets lost (e.g. caught in a spam trap).

Not sure about this yet.


I've attached a slighly modified diff for the hostname and path addition
leaving out the manpage parts for now.

Lightly tested, but seems to work.


Index: install.sub
===
RCS file: /home/cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.830
diff -p -u -r1.830 install.sub
--- install.sub 6 Apr 2015 13:34:23 -   1.830
+++ install.sub 6 Apr 2015 16:46:54 -
@@ -304,7 +304,7 @@ retrap() {
 
 # Fetch response file for autoinstall.
 get_responsefile() {
-   local _rf _ifdev _mac _mode _server _lf
+   local _rf _ifdev _mac _mode _server _lf _hn _path
action=
 
[[ -f /auto_upgrade.conf ]]  _rf=/auto_upgrade.conf _mode=upgrade
@@ -328,16 +328,19 @@ get_responsefile() {
[[ -n $_ifdev ]]  dhclient $_ifdev || break
_lf=/var/db/dhclient.leases.$_ifdev
_server=$(sed /^ *next-server /!d;s///;s/;$//;q $_lf)
-   _mode=$(sed -E '/^ *filename 
auto_(install|upgrade);$/!d;s//\1/;q' $_lf)
-   hostname $(sed -E '/^ *option host-name (.*);$/!d;s//\1/;q' 
$_lf)
+   _mode=$(sed -E '/^ *filename 
(.*\/)?auto_(install|upgrade);$/!d;s//\2/;q' $_lf)
+   _path=$(sed -E '/^ *filename (.*\/)[^/]+;$/!d;s//\1/;q' $_lf)
+   _hn=$(sed -E '/^ *option host-name (.*);$/!d;s//\1/;q' $_lf)
+   hostname $_hn
done
 
# Fetch response file if server and mode are known, otherwise tell which
-   # one was missing. First try to fetch mac-mode.conf, then mode.conf.
+   # one was missing. First try to fetch mac-mode.conf, then
+   # hostname-mode.conf, and finally mode.conf.
if [[ -n $_server  -n $_mode ]]; then
_mac=$(ifconfig $_ifdev | sed 's/.*lladdr \(.*\)/\1/p;d')
-   for _rf in {$_mac-,}$_mode; do
-   _url=http://$_server/$_rf.conf
+   for _rf in {$_mac-,${_hn:+$_hn-,}}$_mode; do
+   _url=http://$_server/$_path$_rf.conf
echo Fetching $_url
if ftp -Vo /ai.$_mode.conf $_url 2/dev/null; then
action=$_mode

-- 
-=[rpe]=-



Re: Small ifconfig output tweak for inet6?

2015-03-27 Thread Robert Peichaer
On Thu, Mar 26, 2015 at 05:46:12PM +0100, Henning Brauer wrote:
 * Mike Belopuhov m...@belopuhov.com [2015-03-26 14:36]:
  On 26 March 2015 at 14:27, Stuart Henderson st...@openbsd.org wrote:
   seems reasonable. (I'd quite like that for v4 too, though it wouldn't
   cope with non-contiguous netmask ;)
  non-contiguous netmasks for IPv4 addresses configured on an interface?
  is that possible?  what's the use case?
  perhaps you're confusing this with  non-contiguous netmasks in the radix
  tree that are entered by the ipsec flows containing port numbers?
 
 I don't think we need to worry about non-contiguous netmasks here.
 
  however I agree that if we do this for ipv6 we should do it for ipv4 as well
  but then do we care about tons of stuff out there parsing ifconfig output?
 
 that's the prime question. I would love to move to CIDR notation - are
 we breaking people's scripts with that? The inet side has been the same
 for, what, decades?

The v6_info() function in the installer would need a change, but that's
an easy fix.

-- 
-=[rpe]=-



Re: Do you need/prefer the non-DUID option in the installer?

2015-03-15 Thread Robert Peichaer
On Sun, Mar 15, 2015 at 09:03:45PM +0300, Vadim Zhukov wrote:
 15 ?? 2015 ??. 20:50  Theo de Raadt 
 dera...@cvs.openbsd.org
 ??:
 
   On Sun, Mar 15, 2015 at 11:24:32AM -0400, Kenneth Westerback wrote:
Using DUIDs in the installed /etc/fstab has been the default for some
 time now.
   
We'd like to eliminate the question in the installer and just use
DUIDs unconditionally.
   
But first we need to know you are aware of any circumstances where
people need or prefer to use the non-DUID option when installing?
  
   I prefer not using DUIDs.
 
  OK, I think Ken made a mistake mentioning preferences.  The real
  question is if anyone has a use-case where DUIDs do not work.
 
  Preference has nothing to do with it.  If DUIDs have no downsides,
  and only the upsides that they were designed to support, then it is
  time to remove the installation question.
 
  The non-DUID access patterns continue to work, of course.  That is
  also not part of the question.
 
 Virtualization appliances: after cloning you could get a different drive
 ID, right? - and thus get a non-bootable system. That's the only real issue
 I know. Hope to be wrong. :)
 
 --
 Vadim Zhukov

At least on VMware, the DUID does not change after cloning.

-- 
-=[rpe]=-



Re: fuck you pkg.conf

2014-11-29 Thread Robert Peichaer
On Sat, Nov 29, 2014 at 04:32:20PM +0100, Mark Kettenis wrote:
  Date: Sat, 29 Nov 2014 09:27:51 -0500
  From: Ted Unangst t...@tedunangst.com
  
  On Sat, Nov 29, 2014 at 14:02, Antoine Jacoutot wrote:
   But that said, why does your pkg.conf keep returning? I don't have one on
   my laptop at all, I probably removed it once after installing, but it
   
   It returns each time I upgrade using bsd.rd.
   
  
  This seems like a mistake. I like that the install script remembers
  things like timezone and mirror for installation, but it always
  prompts me. It's never an invisible setting. And two, it's not
  actually something configured on the installed system.
 
 It would be rather annoying if pkg_add would ask me where I'd want to
 fetch my packages from every time I run it.
 
 Changing the install script to only create /etc/pkg.conf when doing a
 new install would make more sense.

This sound like a reasonable compromise to me.

Index: install.sub
===
RCS file: /home/cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.799
diff -p -u -r1.799 install.sub
--- install.sub 18 Nov 2014 19:00:16 -  1.799
+++ install.sub 29 Nov 2014 19:35:51 -
@@ -1952,12 +1952,9 @@ finish_up() {
done /mnt/etc/fstab
fi
 
-   # Create or update pkg.conf with the new package path, if any
-   if [[ -n $PACKAGE_PATH ]]; then
-   grep -v '^[ ]*installpath[  ]*=' /mnt/etc/pkg.conf 
2/dev/null /tmp/pkgconf
-   print -r -- installpath = $PACKAGE_PATH /tmp/pkgconf
-   cp /tmp/pkgconf /mnt/etc/pkg.conf
-   fi
+   # Create an initial pkg.conf with the package path during install.
+   [[ -n $PACKAGE_PATH  $MODE == install ]] 
+   print -r -- installpath = $PACKAGE_PATH /mnt/etc/pkg.conf
 
echo -n Making all device nodes...
(cd /mnt/dev; sh MAKEDEV all
-- 
-=[rpe]=-



Re: new rc.conf(8) manual

2014-08-19 Thread Robert Peichaer
On Tue, Aug 19, 2014 at 10:44:54PM +0200, Ingo Schwarze wrote:
 Hi,
 
 while working on rcctl(8), i noticed that the rc.conf(8) manual
 is of...  err, how can i express it politely...  somewhat doubtful
 quality.  Here is a stab at it.
 
 As a first step, i moved all the examples from the DESCRIPTION to
 the EXAMPLES esction.  After that, the DESCRIPTION section was
 basically empty, so i had clean earth to till.
 
 Do not attempt to read the diff.  Just apply it and read the result.
 
 OK?
   Ingo

Hi Ingo

In the Base system daemon configuration variables section, in the
last paragraph this is incorrect.

... including a string containing only a single blank character, ...

Looking at the _rc_parse_conf() code in rc.subr says, that any leading
and trailing blanks on the value side of key=value get stripped away.

And maybe it's worth a note that in case of multiple lines with the same
key, only the last is used.

Besides that OK rpe@



Re: autoinstall(8): remove System hostname from example

2014-04-26 Thread Robert Peichaer
On Sat, Apr 26, 2014 at 07:54:44AM -0400, Kenneth Westerback wrote:
 On 26 April 2014 07:45, Patrik Lundin patrik.lundin@gmail.com wrote:
  On Sat, Apr 26, 2014 at 07:21:28AM -0400, Kenneth Westerback wrote:
 
  Assuming you mean dhclient.conf and not dhcpd.conf, the hostname of
  the system is not currently set by dhclient even if it is supplied.
  The install process does cause dhclient to send the configured
  hostname to the dhcpd server for it's own record keeping purposes.
 
 
  I was thinking about the 'option host-name foo;' at the end of
  autoinstall(8). From what I can tell this is picked up by
  get_responsefile() in /usr/src/distrib/miniroot/install.sub like so:
 
  2118 # Prime hostname with host-name option
  2119 hostname $(sed -E '/^ *option host-name (.*);$/!d;s//\1/;q' 
  $_lf)
 
  I have installed a box with a recent snapshot and the resulting machine
  will be named after what is set in dhcpd.conf.
 
  Regards,
  Patrik Lundin
 
 AH, sorry. I thought you were saying we could rely on dhclient setting
 the hostname after install. Too little caffeine makes for deficient
 answers. :-)
 
  Ken
 

Hmm. Currently I'd say that's redundant information in the manpage, but ...
I've a diff for install.sub pending, that will allow autoinstall without
dhcp and/or network and in this case the System hostname in the responsefile
makes sense. Anyway, I see no immediate reason to change the manpage but will
not object if someone wants to do that.



Re: typo in distrib/miniroot/install.sub

2014-02-02 Thread Robert Peichaer
On Sun, Feb 02, 2014 at 04:08:09PM +0100, Markus Lude wrote:
 Hello,
 
 I noticed a typo in distrib/miniroot/install.sub. Fix is attached below.
 
 Regards,
 Markus
 

 Index: install.sub
 ===
 RCS file: /cvs/src/distrib/miniroot/install.sub,v
 retrieving revision 1.738
 diff -u -p -r1.738 install.sub
 --- install.sub   1 Feb 2014 23:16:16 -   1.738
 +++ install.sub   2 Feb 2014 15:01:45 -
 @@ -1290,7 +1290,7 @@ install_files() {
  
   [[ -n $_unver ]]  : ${_issue:=Unverified sets: ${_unver% }}
   if [[ -n $_issue ]] 
 - ! ask_yn $_issue. Continue without verifcation?; then
 + ! ask_yn $_issue. Continue without verification?; then
   [[ -d $_tmpsrc ]]  rm -rf $_tmpsrc
   $auto  exit 1
   return

fixed, thanks

-- 
-=[rpe]=-