Bug 164048 - /etc/rc.d/hostid is not symlink aware
Summary: /etc/rc.d/hostid is not symlink aware
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-12 08:20 UTC by dirkx
Modified: 2017-12-31 22:32 UTC (History)
0 users

See Also:


Attachments
file.diff (560 bytes, patch)
2012-01-12 08:20 UTC, dirkx
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description dirkx 2012-01-12 08:20:11 UTC
/sbin/dhclient-script, /etc/rc.d/hostid and /etc/rc.d/sshd are the only three scripts that write in /etc. In some embedded environments, heavily-audited/secure, PXE boots, NFS root's and in, say, virtualbox or Xen, it can be very nice to have a fully-read only / and/or /etc.

dhclient-script and /etc/rc.d/sshd are both aware of the fact that the destination may be a symlink. /etc/rc.d/hostid is not (yet).

Below is a suggestion. Note that this is an alternative to setting the hostid_file to some place.

How-To-Repeat: Create a read only /etc; put a symlink for all the writable files:

   ln -s /var/run/resolv.conf /etc
   ln -s /var/run/hostid /etc
   ln -s /var/db/
   ln -s /var/db/ssh_host_dsa_key /etc/ssh
   ln -s /var/db/ssh_host_key /etc/ssh
   ln -s /var/db/ssh_host_rsa_key /etc/ssh
   ln -s /var/db/ssh_host_dsa_key.pub /etc/ssh
   ln -s /var/db/ssh_host_key.pub /etc/ssh
   ln -s /var/db/ssh_host_rsa_key.pub /etc/ssh
 
reboot - and observe that all works - but that

   cat /etc/hostid

shows it to be empty . Apply below - reboot and see that /var/run/hostid now contains the hostid - visible also as /etc/hostid.
Comment 1 Matthew Story 2012-01-12 16:48:48 UTC
On Thu, Jan 12, 2012 at 3:15 AM, Dirk-Willem van Gulik <dirkx@webweaving.org
> wrote:
[...snip]

>       # If ${hostid_file} already exists, we take UUID from there.
> -       if [ -r ${hostid_file} ]; then
> +       # If ${hostid_file} already exists, we take UUID from there. We use
> +       # a -f rather than a -r check as the histid_file may in fact be
> +       # a symbolic link.
>

per the test man-page, `-r' tests for readability, regardless of type, and
`-f' tests for the existence of a regular file.  `-r' does include an
implicit test for existence, so `-r' will in fact work for symlinks, and
fail reliably if the symlink source_file does not exist (relevant bits from
the test man-page at the bottom of this message):

$ # setup target/src dirs for demonstration of test
$ mkdir target src
$ # setup source_files for sym-linking
$ jot - 1 10 | xargs -I {} touch src/{}
$ # symlink in files
$ find "$PWD/src" -type f -depth 1 -maxdepth 1 -print0 | xargs -0 -n1 sh -c
'ln -s "$*" target/' worker
$ # make target read-only, note that mode 0400 on target will result in -r
to fail
$ chmod 500 target
$ # demonstrate that -r works with symlinks
$ jot - 1 10 | while read trg; do [ -r "target/$trg" ] && echo "I can read
target/$trg"; done
I can read target/1
I can read target/2
I can read target/3
I can read target/4
I can read target/5
I can read target/6
I can read target/7
I can read target/8
I can read target/9
I can read target/10
$ # demonstrate that -f also works with symlinks
$  jot - 1 10 | while read trg; do [ -f "target/$trg" ] && echo
"target/$trg is a regular file"; done
target/1 is a regular file
target/2 is a regular file
target/3 is a regular file
target/4 is a regular file
target/5 is a regular file
target/6 is a regular file
target/7 is a regular file
target/8 is a regular file
target/9 is a regular file
target/10 is a regular file



> +       #
> +       if [ -f ${hostid_file} ]; then
>                hostid_set `cat ${hostid_file}`
>

with this patch, if ${hostid_file} exists, and is non-readable, cat
${hostid_file} will fail, and yield no $1 to hostid_set (effectively
identical to a hostid_file that is empty).  this is not the desired
behavior:

$ # using our above setup, make target/1 unreadable
$ chmod 000 target/1
$ # demonstrate failure of the new test with an unreadable, but existing
file
$ [ -f target/1 ] && cat target/1
cat: target/1: Permission denied


>        else
>                # No hostid file, generate UUID.
> -               hostid_generate
> +               hostid_reset
>

This line is actually why you are seeing a hostid_file on restart.  The
hostid_file does not exist on your system, and per the comment, and
implementation, if a hostid_file does not exist, one is generated and set
via sysctl (via the hostid_set function).

Your suggested change changes the functionality of this program to both
generate a hostid, and then store it in a hostid file.  This seems to me to
be a change in functionality, and not a bug.


>  [...snip]
>

There is a small race condition in this file (unless rc.d is doing some
locking on hostid_file in the caller)

if [ -r ${hostid_file} ]; then
    hostid_set `cat ${hostid_file}`
else
    ...

Insofar as it's possible (however unlikely) that the file mode (or file
mode of the parents) could change between the test and the read.  This
could probably be resolved using lockf, but it's definitely not a big deal.

---------------snippits from man 1 test-----------------

     -r file       True if file exists and is readable.
     [...snip]
     -f file       True if file exists and is a regular file.



-- 
regards,
matt
Comment 2 dirkx 2012-01-12 19:47:18 UTC
On 12 jan. 2012, at 17:48, Matthew Story wrote:

> On Thu, Jan 12, 2012 at 3:15 AM, Dirk-Willem van Gulik =
<dirkx@webweaving.org> wrote:=20
> [...snip]=20
>       # If ${hostid_file} already exists, we take UUID from there.
> -       if [ -r ${hostid_file} ]; then
> +       # If ${hostid_file} already exists, we take UUID from there. =
We use
> +       # a -f rather than a -r check as the histid_file may in fact =
be
> +       # a symbolic link.
>=20
> per the test man-page, `-r' tests for readability, regardless of type, =
and `-f' tests for the existence of a regular file.  `-r' does include =
an implicit test for existence, so `-r' will in fact work for symlinks, =
and fail reliably if the symlink source_file does not exist (relevant =
bits from the test man-page at the bottom of this message):
=85.
> with this patch, if ${hostid_file} exists, and is non-readable, cat =
${hostid_file} will fail, and yield no $1 to hostid_set (effectively =
identical to a hostid_file that is empty).  this is not the desired =
behavior:

Totally understood - but wanted to stay close to the behavior of =
dhclient-script as I understand it.  And this happens to also make the =
behavior of /etc/rc.d/sshd on first run the same. Keep in mind that one =
can always set the rc variable.
...
> This line is actually why you are seeing a hostid_file on restart.  =
The hostid_file does not exist on your system, and per the comment, and =
implementation, if a hostid_file does not exist, one is generated and =
set via sysctl (via the hostid_set function).

Agreed - as _set is better.

> There is a small race condition in this file (unless rc.d is doing =
some locking on hostid_file in the caller)
=85.

Right - which in this case is one we should not worry about - this is =
during boot - as the rc.d files are ran one by one; and generally not =
twice. However - the lock issue does affect  /sbin/dhclient-script - and =
I've seen this behaviour there in the wild.

Dw=
Comment 3 Matthew Story 2012-01-12 20:51:48 UTC
On Thu, Jan 12, 2012 at 2:47 PM, Dirk-Willem van Gulik <dirkx@webweaving.org
> wrote:

>
> On 12 jan. 2012, at 17:48, Matthew Story wrote:
>
> > On Thu, Jan 12, 2012 at 3:15 AM, Dirk-Willem van Gulik <
> dirkx@webweaving.org> wrote:
>
[...snip]

> Totally understood - but wanted to stay close to the behavior of
> dhclient-script as I understand it.  And this happens to also make the
> behavior of /etc/rc.d/sshd on first run the same. Keep in mind that one can
> always set the rc variable.
>

it makes sense to test for existence (and not readability) for rc.d/sshd,
as it goes on to create files if they do not exist:

    if [ -f /etc/ssh/ssh_host_key ]; then
        echo "You already have an RSA host key" \
            "in /etc/ssh/ssh_host_key"
        echo "Skipping protocol version 1 RSA Key Generation"
    else
        /usr/bin/ssh-keygen -t rsa1 -b 1024 \
            -f /etc/ssh/ssh_host_key -N ''
    fi

in the existing implementation of rc.d/hostid, it does not create the file
on ``start'' if it does not exist, so detection of readability is more
correct (although in the typical use-case e.g. running as root, existence
and readability are ostensibly synonymous).


> [...snip]
>
> Agreed - as _set is better.
> [...snip]
>

So the question is not about respecting symlinks, but wether or not a
``host_id_file'' should be created if one does not exist, for the ``start''
command.  I'm not sure if this behavior is desirable, considering that the
de facto behavior is to respect hardware derived ``smbios.system.uuid'',
and writing that value to disk would potentially require an additional
reset on hardware change.

As you can easily generate a ``host_id_file'' if one does not exist by
invoking the ``reset'' command, and the sysctl is set at start properly,
either from ``host_id_file'', hardware or via the ``uuidgen'' program, this
seems superfluous to me ... but I defer to the maintainer.

-- 
regards,
matt
Comment 4 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 08:00:34 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped