Paths

Table of Contentst

sh: Add -o verify to use O_VERIFY when sourcing scripts
ClosedPublic
Actions

Authored by stephane.rochoy_stormshield.eu on May 26 2021, 7:28 AM.

Details

Reviewers

mw
sjg
imp
jilles

Group Reviewers

manpages

Commits

rGd2c233176f44: sh: Add -o verify to use O_VERIFY when sourcing scripts

Summary

Add -o verify to sh to make it use O_VERIFY when sourcing scripts and reading profiles.

Useful in conjunction with mac_veriexec to help protect at least some parts of the boot sequence, e.g., /etc/rc*. (If OK I'll submit another patch to let init spawn rc with something like sh -o verify /etc/rc).

I used truss to ensure O_VERIFY is effectively used:

# sh
# set -o
Current option settings
errexit         off
…
verify          off
# set -o verify
# set -o
Current option settings
errexit         off
…
verify          on
# set +o verify
# set -o
Current option settings
errexit         off
…
verify          off
# ^D
# truss /bin/sh -o verify /tmp/hello.sh 2>&1 | grep open
…
openat(AT_FDCWD,"/tmp/hello.sh",O_RDONLY|O_CLOEXEC|O_VERIFY,00) = 3 (0x3)
# ls -li /bin/sh /bin/-sh
3858755 -r-xr-xr-x  2 1000  0  171504 May 25 13:10 /bin/-sh
3858755 -r-xr-xr-x  2 1000  0  171504 May 25 13:10 /bin/sh
# truss -- -sh -o verify /tmp/hello.sh 2>&1 | grep open
…
openat(AT_FDCWD,"/tmp/hello.sh",O_RDONLY|O_CLOEXEC|O_VERIFY,00) = 3 (0x3)
openat(AT_FDCWD,"/etc/profile",O_RDONLY|O_CLOEXEC|O_VERIFY,00) ERR#2 'No such file or directory'
openat(AT_FDCWD,"/root/.profile",O_RDONLY|O_CLOEXEC|O_VERIFY,00) = 3 (0x3)
# cat /tmp/foo.sh 
#!/bin/sh
set -o verify
. /tmp/hello.sh
# truss sh /tmp/foo.sh 2>&1 | grep open
…
openat(AT_FDCWD,"/tmp/foo.sh",O_RDONLY|O_CLOEXEC,00) = 3 (0x3)
openat(AT_FDCWD,"/tmp/hello.sh",O_RDONLY|O_CLOEXEC|O_VERIFY,00) = 3 (0x3)

Any hint on the Good Way to create some tests would be welcome :)

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 40798
Build 37687: arc lint + arc unit

Event Timeline

stephane.rochoy_stormshield.eu created this revision.May 26 2021, 7:28 AM

stephane.rochoy_stormshield.eu held this revision as a draft.

Herald added a reviewer: jilles. · View Herald TranscriptMay 26 2021, 7:28 AM

Herald added a reviewer: manpages. · View Herald Transcript

Harbormaster completed remote builds in B39458: Diff 89886.May 26 2021, 7:28 AM

stephane.rochoy_stormshield.eu published this revision for review.May 26 2021, 7:29 AM

stephane.rochoy_stormshield.eu edited the summary of this revision. (Show Details)

stephane.rochoy_stormshield.eu retitled this revision from sh: Add -o verify to use O_VERIFY when sourceing scripts to sh: Add -o verify to use O_VERIFY when sourcing scripts.May 26 2021, 7:33 AM

kd added a subscriber: kd.May 26 2021, 8:20 AM

OK, for the man page part.

Looks reasonable, assuming veriexec itself is reasonable (in many cases, it seems to me that verifying the root filesystem would be a simpler and more reliable approach).

The verify option affects:

scripts passed on the command line (sh SOMEFILE)
startup files: /etc/profile, .profile, /etc/suid_profile and the ENV file
files sourced via the . special builtin
function autoloading via a DIR%func entry in PATH
execution of edited commands via fc -e (so fc -e will be broken)

The verify option does not affect:

any other execution of commands via fc
command substitutions in wordexp() (via the undocumented freebsd_wordexp builtin)
passing data to eval

If interactive shells are expected to have -o verify enabled, it is probably better to disable verification for fc -e.

This revision is now accepted and ready to land.May 29 2021, 4:49 PM

Neat, but not going to be portable.
FWIW I use veriexec -x some/file to test whether the file is verified.
Eg. we modify rc.subr to provide a couple of functions is_verified and vdot which does . only after verify file.
This allows shell scripts to be careful about what they consume, while still being portable (not that big. a deal really ;-)
eg. our rc.subr has:

# _rc_verify file
#       if VERIEXEC is active check that $file is verified
#
veriexec=/sbin/veriexec
if test -s $veriexec && $veriexec -i active > /dev/null 2>&1; then
_rc_verify() { $veriexec -x $1; }
else
_rc_verify() { : ; }
fi

# indicate that we have vdot
_VDOT_SH=:

# convenience function - skip if not verified
vdot()
{
        if test -s $1 && _rc_verify $1 > /dev/null 2>&1; then
                . $1
        fi
}

And in run_rc_script we have

# for Junos we want to log and verify things                            
rc_trace 0 "$_file $_arg"                                               
case "$_file" in                                                        
*local*.d/*) ;;         # allow it                                      
*)      # don't use it if we don't trust it                             
        _rc_verify $_file || return                                     
        ;;                                                              
esac

But fwiw

In D30464#685956, @jilles wrote:

Looks reasonable, assuming veriexec itself is reasonable (in many cases, it seems to me that verifying the root filesystem would be a simpler and more reliable approach).

FWIW we do that; initial rootfs is an iso image which is verified by loader, and the majority of s/w is in iso images which are also verified before mount (mdconfig will fail if not verified),
regardless, if veriexec is being enforced sh cannot run any binaries which. are not verified.

Since shell scripts generally cannot do anything the user cannot, they do not pose quite the same risk, still it is handy esp during /etc/rc to be able to check that. sh is not consuming unverified files - unless explicitly allowed for.

In D30464#686275, @sjg wrote:

Neat, but not going to be portable.
FWIW I use veriexec -x some/file to test whether the file is verified.
Eg. we modify rc.subr to provide a couple of functions is_verified and vdot which does . only after verify file.
This allows shell scripts to be careful about what they consume, while still being portable (not that big. a deal really ;-)

Well I use a similar approach in fact. I also test the state of Veriexec before sourcing (or executing) things. But -o verify is used to prevent a race condition.

In D30464#686275, @sjg wrote:

Neat, but not going to be portable.
FWIW I use veriexec -x some/file to test whether the file is verified.
Eg. we modify rc.subr to provide a couple of functions is_verified and vdot which does . only after verify file.
This allows shell scripts to be careful about what they consume, while still being portable (not that big. a deal really ;-)

AFAIK mac_veriexec does not block the opening of files with O_VERIFY if inactive. (i.e. the new sh verify option blocks nothing if mac_veriexec is inactive / not loaded).
Could you elaborate more on the portability issue?

In D30464#686275, @sjg wrote:

vdot()
{
        if test -s $1 && _rc_verify $1 > /dev/null 2>&1; then
                . $1
        fi
}

Besides I believe there is race condition here. The file $1 can be tampered with after the call to _rc_verify and before the source call (.)

In D30464#686402, @sebastien.bini_stormshield.eu wrote:
In D30464#686275, @sjg wrote:
vdot()
{
        if test -s $1 && _rc_verify $1 > /dev/null 2>&1; then
                . $1
        fi
}
Besides I believe there is race condition here. The file $1 can be tampered with after the call to _rc_verify and before the source call

$1 is a local variable to vdot. Who could possibly change it?

In D30464#686471, @imp wrote:

$1 is a local variable to vdot. Who could possibly change it?

$1 does not change :) But it represents a filename and it may not be resolved to the same inode when passed to _rc_verify and to dot :/ I know it's probably hard to exploit, but it remains a race condition nonetheless.

AFAIK mac_veriexec does not block the opening of files with O_VERIFY if inactive. (i.e. the new sh verify option blocks nothing if mac_veriexec is inactive / not loaded).

Correct; O_VERIFY like most restrictions is only effective when veriexec is being enforced.
But veriexec -x can be used when veriexec is only *active*.
My package system for Junos leverages this (internal dev builds may not always enforce since developers have much the same requirements as script kiddies ;-).

Could you elaborate more on the portability issue?

A script that uses -o verify cannot be portable (to other shells or older versions of same shell) but I don't think that matters very much depending on how it is used.

In D30464#686471, @imp wrote:
In D30464#686402, @sebastien.bini_stormshield.eu wrote:
In D30464#686275, @sjg wrote:
vdot()
{
        if test -s $1 && _rc_verify $1 > /dev/null 2>&1; then
                . $1
        fi
}
Besides I believe there is race condition here. The file $1 can be tampered with after the call to _rc_verify and before the source call
$1 is a local variable to vdot. Who could possibly change it?

He means the file it is refering to, and yes there is a race.

BTW wrt creating test cases, assuming you have a system which is capable of enforcing veriexec and still operate, a package which contains a manifest with various failures is handy.
I have something like that for testing the verifying loader - it cannot be installed using my package system; since it would fail all the pre-install checks, so a manual install.sh
script is provided.

You can also do simple tests by just copying a verified file such. that it gets a new inode.
Eg /bin/sh may be verified and work just fine, cp /bin/sh /tmp; /tmp/sh - Authentication error same trick applies to any file.

In D30464#687944, @sjg wrote:

BTW wrt creating test cases, assuming you have a system which is capable of enforcing veriexec and still operate, a package which contains a manifest with various failures is handy.
I have something like that for testing the verifying loader - it cannot be installed using my package system; since it would fail all the pre-install checks, so a manual install.sh
script is provided.

You can also do simple tests by just copying a verified file such. that it gets a new inode.
Eg /bin/sh may be verified and work just fine, cp /bin/sh /tmp; /tmp/sh - Authentication error same trick applies to any file.

You're right, I could add some tests that would run only if Veriexec is enforced. But I wonder if it would be OK to have tests that would report success (or quietly don't run) when Veriexec isn't enforced for example? Do you know of some test that somewhat disable itself when the build machine is not suitable/properly setup?

In D30464#685956, @jilles wrote:

Looks reasonable, assuming veriexec itself is reasonable (in many cases, it seems to me that verifying the root filesystem would be a simpler and more reliable approach).

The verify option affects:

[…]

function autoloading via a DIR%func entry in PATH

@jilles I didn't know about the function autoloading feature. Is it documented somewhere? I didn't find anything in sh(1).

execution of edited commands via fc -e (so fc -e will be broken)

I don't understand why you think fc -e would be broken.

AFAIK, it's used to execute a command found in the history after editing it so there's two cases depending on what's involved in the edited command:

a sourced script in which case, -o verify will trigger the use of O_VERIFY
an executable, in which case, -o verify is off-topic, i.e., execve will require O_VERIFY whatever the sh setup is

The verify option does not affect:

any other execution of commands via fc

[…]

If interactive shells are expected to have -o verify enabled, it is probably better to disable verification for fc -e.

Do you suggest to distinguish between a script sourced via sh foo.sh or . foo.sh from a script sourced via fc -e (or any other fc call)?

@imp @jilles @sjg @mw Is there anything that still prevent this patch from landing?

In D30464#689064, @stephane.rochoy_stormshield.eu wrote:

In D30464#685956, @jilles wrote:

Looks reasonable, assuming veriexec itself is reasonable (in many cases, it seems to me that verifying the root filesystem would be a simpler and more reliable approach).

The verify option affects:

[…]

function autoloading via a DIR%func entry in PATH

@jilles I didn't know about the function autoloading feature. Is it documented somewhere? I didn't find anything in sh(1).

It is not documented, although example functions exist in /usr/src/bin/sh/funcs/. There is more information about this feature at https://www.in-ulm.de/~mascheck/various/ash/functionpathsearch.html .

execution of edited commands via fc -e (so fc -e will be broken)

I don't understand why you think fc -e would be broken.

AFAIK, it's used to execute a command found in the history after editing it so there's two cases depending on what's involved in the edited command:

a sourced script in which case, -o verify will trigger the use of O_VERIFY

an executable, in which case, -o verify is off-topic, i.e., execve will require O_VERIFY whatever the sh setup is

The verify option does not affect:

any other execution of commands via fc

[…]

If interactive shells are expected to have -o verify enabled, it is probably better to disable verification for fc -e.

Do you suggest to distinguish between a script sourced via sh foo.sh or . foo.sh from a script sourced via fc -e (or any other fc call)?

Invocations of fc without the -l option re-execute commands from the history. With fc -s (together with one or two operands), the commands are executed directly, and -o verify has no effect. Otherwise, the commands are first written to a file, an editor is started on the file, and the edited file is executed, and -o verify affects the edited file. I don't think this distinction makes sense.

On the other hand, in situations like . FILE or function autoloading, verification if -o verify is enabled is what was asked for, whereever the command came from. If someone wants to source a file without verification, they can eval "$(cat -- FILE)" or set +o verify (hence, it does not make sense to disallow +o verify).

In D30464#690655, @jilles wrote:

It is not documented, although example functions exist in /usr/src/bin/sh/funcs/. There is more information about this feature at https://www.in-ulm.de/~mascheck/various/ash/functionpathsearch.html .

That's an interesting feature. And truss confirms O_VERIFY is used in this case too: openat(AT_FDCWD,"…/t/funcs/foo",O_RDONLY|O_CLOEXEC|O_VERIFY,00) = 3 (0x3)

Invocations of fc without the -l option re-execute commands from the history. With fc -s (together with one or two operands), the commands are executed directly, and -o verify has no effect. Otherwise, the commands are first written to a file, an editor is started on the file, and the edited file is executed, and -o verify affects the edited file. I don't think this distinction makes sense.

The fact that -o verify has no effect when executing commands from the history is the expected behavior. If the command is run via execve, O_VERIFY is enforced by rtld, not by sh.

On the other hand, in situations like . FILE or function autoloading, verification if -o verify is enabled is what was asked for, whereever the command came from. If someone wants to source a file without verification, they can eval "$(cat -- FILE)" or set +o verify (hence, it does not make sense to disallow +o verify).

Agreed.

In D30464#686275, @sjg wrote:

veriexec=/sbin/veriexec
if test -s $veriexec && $veriexec -i active > /dev/null 2>&1; then
_rc_verify() { $veriexec -x $1; }
else
_rc_verify() { : ; }
fi

@sjg FYI, a few days ago, @imp committed e5be21d19b41 that fix veriexec -x.

2khramtsov_gmail.com added a subscriber: 2khramtsov_gmail.com.Jul 8 2021, 2:23 PM

@jilles I took some time to think about fc -e (sorry for the lag). The only viable approach seems to just never enforce O_VERIFY with edited history entries. Not sure it's worth trying to ensure the shell is interactive. What do you think?

This revision now requires review to proceed.Aug 2 2021, 9:23 AM

Harbormaster completed remote builds in B40798: Diff 93095.Aug 2 2021, 9:23 AM

Is there any follow-up in this review? I'd like to commit the patch soon

OK. Thanks for the reminder.

This revision is now accepted and ready to land.Sep 22 2021, 8:42 PM

Closed by commit rGd2c233176f44: sh: Add -o verify to use O_VERIFY when sourcing scripts (authored by stephane.rochoy_stormshield.eu, committed by wma). · Explain WhySep 23 2021, 8:43 AM

This revision was automatically updated to reflect the committed changes.

wma added a commit: rGd2c233176f44: sh: Add -o verify to use O_VERIFY when sourcing scripts.

sebastien.bini_stormshield.eu mentioned this in D34622: init: allow to start script executions with sh -o verify.Mar 21 2022, 2:32 PM

wma mentioned this in rGf3dba162bd46: init: allow to start script executions with sh -o verify.Oct 11 2022, 7:52 AM