Re: Various tests give illusory results

[Bernhard Voelker]

On 08/14/2012 02:47 PM, Ondrej Oprala wrote:
> 
> On 08/10/2012 08:53 AM, Bernhard Voelker wrote:
>> On 08/09/2012 05:43 PM, Ondrej Oprala wrote:
>>> Hi, I think I got a fix for this bugzilla
>>> https://bugzilla.redhat.com/show_bug.cgi?id=556358.
>>> I added a bit of permission checking to require_root_ so no tests have
>>> to be rewriten.
>>> Have a nice day :) ,
>>>    Ondrej
>>>
>> Hi Ondrej,
>>
>>> +setuidgid_has_perm_()
>>> +{
>>> +
>>> +  cat << \EOF > cmds.tmp
>>> +  IFS=:
>>> +  for DIR in $PATH; do
>>> +    test -x $DIR || exit 1
>>> +  done
>>> +  exit 0
>>> +EOF
>>> +
>>> +  su -s /bin/sh $NON_ROOT_USERNAME < cmds.tmp
>>> +
>>> +  RET=$?
>>> +  return $RET
>>> +}
>>> +
>> just a thought: if setuidgid is part of the test failure,
>> then why not using setuidgid in here?
>>
>> Furthermore: the problem is finding the correct binary, right?
>> E.g. in tests/rm/fail-2eperm there is already such a test:
>>
>>    # Try to ensure that $NON_ROOT_USERNAME can access
>>    # the required version of rm.
>>    rm_version=$(
>>      setuidgid $NON_ROOT_USERNAME env PATH="$PATH" rm --version |
>>      sed -n '1s/.* //p'
>>    )
>>    case $rm_version in
>>      $PACKAGE_VERSION) ;;
>>      *) skip_ "cannot access just-built rm as user $NON_ROOT_USERNAME";;
>>    esac
>>
>>
>> Have a nice day,
>> Berny
> 
> Well, I wanted to satisfy the requirements from comment #6 as much as 
> possible
> and didn't want to change anything in the existing tests (such as 
> passing args to require_root_ )
> to make it more automatic. If the check were to act on setuidgid output 
> like in the rm test, it'd either
> have to be changed in all tests or require_root_ would need additional 
> arguments (which are again
> changes in the tests). But if it needs to be changed that way, I will, 
> of course, redo it.

Hi Ondrej,

I think grep-ing for "setuidgid" in $0 is the right thing
in order to avoid changing all root-tests.

I just thought that setuidgid_has_perm_() should use the same
same mechanism via setuidgid as the actual test. This is just
to avoid potential problems if there are any discrepancies
between "the su way" and "the setuidgid way" - today or in
future.

And I think you wouldn't have to pass an extra arg - the CU
program? - to require_root_: it doesn't matter which program
is used to check the version. If the right version of e.g.
rm is found, then also the right version of e.g. touch would
be used.

The question I'm still asking myself is: is the only problem
to use the correct binary in the test, or do you also want
to ensure that the current directory is accessible.

To check both, I thought of something like this:

  setuidgid_has_perm_()
  {
    # Try to ensure that $NON_ROOT_USERNAME can access
    # the required version of CU binaries.
    local rm_version=$(
      setuidgid $NON_ROOT_USERNAME env PATH="$PATH" rm --version |
      sed -n '1s/.* //p'
    )
    case "_${rm_version}_" in
      _$PACKAGE_VERSION}_) ;;
      *) return 1;;
    esac

    # Try to ensure that $NON_ROOT_USERNAME can access
    # the temporary test directory.
    setuidgid $NON_ROOT_USERNAME env PATH="$PATH" touch foo
    [ -f foo ] || return 1

    rm -f foo
    return 0
  }

Have a nice day,
Berny