access(2) using effective uid instead of real one?

[Nicolas Thery]

Hello,

According to man and posix, access(2) uses the real user UID instead
of the effective one.  However it looks like the functions doing the
checking (ufs_access, vop_helper_access) use the effective uid  and I
can't find anywhere where the effective uid passed to these functions
would be changed for the real one (as fbsd does).

I also wrote a small program that confirms this.

Assuming I'm not missing something, Is it intentional or a bug?

Cheers,
Nicolas

[Simon 'corecode' Schubert]

Nicolas Thery wrote:

> Hello,
>
> According to man and posix, access(2) uses the real user UID instead
> of the effective one.  However it looks like the functions doing the
> checking (ufs_access, vop_helper_access) use the effective uid  and I
> can't find anywhere where the effective uid passed to these functions
> would be changed for the real one (as fbsd does).
>
> I also wrote a small program that confirms this.
>
> Assuming I'm not missing something, Is it intentional or a bug?

I'd say it is a bug.

cheers
   simon

--
   <3 the future  +++  RENT this banner advert  +++   ASCII Ribbon   /"\
   rock the past  +++  space for low CHF NOW!1  +++     Campaign     \ /
Party Enjoy Relax   |   http://dragonflybsd.org      Against  HTML   \
Dude 2c 2 the max   !   http://golden-apple.biz       Mail + News   / \

[Alex Hornung]

As far as I can see it should be trivial to change it touse the real
uid in vop_helper_access. Just change the references to cr_uid and
cr_gid to cr_ruid and cr_rgid.
If this is how it should be or shouldn't... I don't know.

2009/8/10 Nicolas Thery <nthery@...>:

> Hello,
>
> According to man and posix, access(2) uses the real user UID instead
> of the effective one.  However it looks like the functions doing the
> checking (ufs_access, vop_helper_access) use the effective uid  and I
> can't find anywhere where the effective uid passed to these functions
> would be changed for the real one (as fbsd does).
>
> I also wrote a small program that confirms this.
>
> Assuming I'm not missing something, Is it intentional or a bug?
>
> Cheers,
> Nicolas
>

[Nicolas Thery]

2009/8/11 Alex <ahornung@...>:
> As far as I can see it should be trivial to change it touse the real
> uid in vop_helper_access. Just change the references to cr_uid and
> cr_gid to cr_ruid and cr_rgid.
> If this is how it should be or shouldn't... I don't know.

I reckon that's one possibility.

Alternatively, the credentials passed to VOP_ACCESS() can be changed
as shown in the patch below.  Doing it this way simplifies the
incoming implementation of faccessat(2) which can check either the
effective or real uid/gid.

diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index 0c723e4..12d3b53 100644
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -2318,8 +2318,16 @@ int
 kern_access(struct nlookupdata *nd, int aflags)
 {
  struct vnode *vp;
+ struct ucred *cr;
  int error, flags;

+ /*
+ * Perform check with real uid/gid
+ */
+ cr = cratom(&nd->nl_cred);
+ cr->cr_uid = cr->cr_ruid;
+ cr->cr_groups[0] = cr->cr_rgid;
+
  if ((error = nlookup(nd)) != 0)
  return (error);
 retry:

[Matthew Dillon]

    I would rather not change the creds.  It seems simple enough to
    adjust the access helper to use the real ids.

                                        -Matt

[Nicolas Thery]

2009/8/11 Matthew Dillon <dillon@...>:
>    I would rather not change the creds.  It seems simple enough to
>    adjust the access helper to use the real ids.

It is admittedly easier and more explicit to do it as Alex and you suggest.

To implement faccessat, which can check either real of effective ids
based on a flag argument, do you prefer:

1/ Pass this "effective vs real" flag to VOP_ACCESS (either in a_mode
or as a new arg).

2/ Adding a new VOP_FACCESSAT operation.

The more general question is: can we break the VOP API (assuming we
change all in-tree clients)?

Cheers,
Nicolas

[Simon 'corecode' Schubert]

Nicolas Thery wrote:
> 1/ Pass this "effective vs real" flag to VOP_ACCESS (either in a_mode
> or as a new arg).

+1

> 2/ Adding a new VOP_FACCESSAT operation.

-1

> The more general question is: can we break the VOP API (assuming we
> change all in-tree clients)?

We can change the internal kernel APIs.

cheers
   simon

--
   <3 the future  +++  RENT this banner advert  +++   ASCII Ribbon   /"\
   rock the past  +++  space for low CHF NOW!1  +++     Campaign     \ /
Party Enjoy Relax   |   http://dragonflybsd.org      Against  HTML   \
Dude 2c 2 the max   !   http://golden-apple.biz       Mail + News   / \

[Matthew Dillon]

:It is admittedly easier and more explicit to do it as Alex and you suggest.
:
:To implement faccessat, which can check either real of effective ids
:based on a flag argument, do you prefer:
:
:1/ Pass this "effective vs real" flag to VOP_ACCESS (either in a_mode
:or as a new arg).
:
:2/ Adding a new VOP_FACCESSAT operation.
:
:The more general question is: can we break the VOP API (assuming we
:change all in-tree clients)?
:
:Cheers,
:Nicolas

    Definitely (1).  I'd say just pass the new flags straight through
    as a new argument to the base vop_access structure, but change the
    VOP_ACCESS macro in sys/vfsops.h:

#define VOP_ACCESS(vp, mode, cred) \
        vop_access(*(vp)->v_ops, vp, mode, 0, cred)
#define VOP_ACCESS_FLAGS(vp, mode, flags, cred) \
        vop_access(*(vp)->v_ops, vp, mode, flags, cred)

     Add a new flags element to struct vop_access_args and pass and
     initialize the new element in the vop_access() function in
     vfs_vopops.c

                                        -Matt
                                        Matthew Dillon
                                        <dillon@...>

[Nicolas Thery]

2009/8/11 Matthew Dillon <dillon@...>:

> :It is admittedly easier and more explicit to do it as Alex and you suggest.
> :
> :To implement faccessat, which can check either real of effective ids
> :based on a flag argument, do you prefer:
> :
> :1/ Pass this "effective vs real" flag to VOP_ACCESS (either in a_mode
> :or as a new arg).
> :
> :2/ Adding a new VOP_FACCESSAT operation.
> :
> :The more general question is: can we break the VOP API (assuming we
> :change all in-tree clients)?
> :
> :Cheers,
> :Nicolas
>
>    Definitely (1).  I'd say just pass the new flags straight through
>    as a new argument to the base vop_access structure, but change the
>    VOP_ACCESS macro in sys/vfsops.h:
>
> #define VOP_ACCESS(vp, mode, cred)                      \
>        vop_access(*(vp)->v_ops, vp, mode, 0, cred)
> #define VOP_ACCESS_FLAGS(vp, mode, flags, cred)         \
>        vop_access(*(vp)->v_ops, vp, mode, flags, cred)
>
>     Add a new flags element to struct vop_access_args and pass and
>     initialize the new element in the vop_access() function in
>     vfs_vopops.c

Thanks Matt and Simon for the feedback.

I'll try first to refactor the various access implementations to call
vop_helper_access() if possible.