[vbox-dev] [PATCH] Removed chroot comment in redhat_postinstall.sh

[Timothy Tacker]

Valdis, thank you for your feedback.

As you may know, --nochroot is used on %post sections in Kickstart files to
tell Anaconda to run post install scripts without chroot. The default is to
run them with chroot; but that's a default in Anaconda, not Virtual box.

I used the redhat67_ks.cfg Kickstart file VirtualBox includes as a template
to write a customized Kickstart file for use with CentOS 7, but I didn't
include --nochroot after seeing the comment. The redhat_postinstall.sh
script failed, but it worked with --nochroot. It took me a while to figure
that out though, because I trusted the information the comment provided to
be correct.

I assume the Kickstart files for Red Hat distributions are all using
--nochroot because the redhat_postinstall.sh script included with
VirtualBox doesn't work with chroot, someone else already realized that,
and they included --nochroot accordingly.

I would have changed the comment to state the script expects NOT to be
running chrooted, but I don't know if that's true in every case. I haven't
tested other distributions, so I don't know if there's variation on this
between older distributions, or even CentOS and Red Hat Enterprise
Linux. I only
have reason to believe it's true in some, or at least this one case; and if
that's correct, then the comment provides incorrect information, which has
potential to mislead other people, as it did me.

I agree with you that an analysis of the code would be desirable; and if
anyone has enough familiarity with the code in the redhat_postinstall.sh
script, bandwidth to do an analysis, and would like to volunteer, that
would be appreciated.

Due to changes in Red Hat 8, I'm also writing a Kickstart file for CentOS
8. This may require me looking closer at redhat_postinstall.sh and becoming
more familiar with it. I can't commit to it; but if this becomes the case,
I may not only contribute the new Kickstart file but may also be able to
answer some of your other questions. Depending on schedule, this may take a
while; so in the interim, I thought it best to try to avoid others any
problems that may be caused by the comment. It's not an ultimate, or ideal,
solution; but it seemed a small, quick, and iterative improvement,
nonetheless.

With the above in mind, would you be amenable to suggesting different
language for the comment, which at least bring the issue to the attention
of the reader?

Timothy Tacker

On Sun, Jul 19, 2020, 11:30 PM Valdis Klētnieks <valdis.kletnieks at vt.edu>
wrote:

> On Sun, 19 Jul 2020 02:48:22 -0500, Timothy Tacker said:
>
> > All Kickstart files for Red Hat distributions in the UnattendedTemplates
> > directory run the redhat_postinstall.sh script with --nochroot in the
> %post
> > section.
>
> > Nonetheless, the redhat_postinstall.sh script includes the following
> > comment:
> > # Note! This script expects to be running chrooted (inside new sytem).
> >
> > The patch below removes this comment. I'm licensing this patch under the
> > MIT license. Please review and consider integrating. Feedback is welcome.
> > Thanks!
>
> I'd be leery of a patch that simply discards a comment - it obviously
> *used*
> to be important enough to make the note.  Probably what is *actually*
> needed
> here is an analysis of what the code used to do, what it does now, and
> whether
> it should be refactored to make --nochroot the default rather than every
> single
> user having to specify it.
>
> Homework problem for the student - if the script doesn't actually expect to
> be in a chroot by default, why are all the calls passing --nochroot? What
> change
> in behavior does that cause? And what can go wrong if somebody doesn't know
> it expects to be chrooted?
>
> (Sorry, over the last four decades I've seen entirely too many "let's
> remove
> the comment" patches that ended up sprouting a CVE because somebody didn't
> know something important because it wasn't documented...)
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.virtualbox.org/pipermail/vbox-dev/attachments/20200720/f9b94087/attachment.html>