coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Various tests give illusory results


From: Ondrej Oprala
Subject: Re: Various tests give illusory results
Date: Tue, 14 Aug 2012 17:55:28 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120717 Thunderbird/14.0

On 08/14/2012 04:53 PM, Bernhard Voelker wrote:
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
Hi,

You're right about the rm test. I originally wanted to make the check by checking
if $NON_ROOT_USERNAME has execute permissions for the src folder, but this
looks cleaner. There's still the very rare case of $NON_ROOT_USERNAME
having execute permissions for rm but not having them for other binaries using
setuidgid, but that is something that would have to be done purposefully and
personally I don't see any reason to do that.

Anyway, the attachment should hopefully have it right.

Cheers,
  Ondrej.

Attachment: DIFF
Description: Text document


reply via email to

[Prev in Thread] Current Thread [Next in Thread]