Re: [Netatalk-devel] Permission semantics: inherited privs with UNIX privs and strict POSIX privs

[didier <dga...@ma...>]

Hi,
Le mercredi 13 août 2008 à 12:19 +0200, Frank Lahm a écrit : 
> Hello netatalk-devel,
> 
> 
I know that it's not always easy but can you try to keep your patch as
small as possible with one issue/patch? 
Or maybe you didn't work on the CVS tree and only made a diff against
CVS? If it's the case I'd like to know the source used because it's
buggy. 

For example in your patch:
1) fix for a compilation error if DEBUG1 is defined (I will commit this
one). 

2) remove some gcc warning, well rather hiding them than fixing them:
- convert_string_allocate() add a (char *) cast, I'm not sure it's the
right thing to do, maybe passing a usc2_t is really bad, I'd rather keep
the warning. 

- replace mktemp with mkstemp. This one is wrong because in our case
mkstemp is 100% as insecure as mktemp and the gcc warning is badly
written: you can't replace mktemp with mkstemp, they don't have the same
semantic! mkstemp *creates* the file and now afpd leaks a file
descriptor each time afp_exchangefiles is called.
 
3) Remove some {} for one line if block. I don't mind them, it's just
that it makes the patch harder to read.

Now about the meat.
In my understanding you're doing three unrelated things:
1) don't apply afpd -m option (umask) in chmod,  personally I would like
to keep a way to veto client requests, we need to fix the manual with
the actual behavior though.
  
2) extend umask:0xxx volume parameter use. it makes a lot of sens
currently umask is rather useless.

3) I'm not sure :) 
Before we had or at least it was the idea:
upriv --> unix semantic 
!upriv --> inherited privileges

and a test (I shortcut AFPVOL_...): 
vol_unix_priv(vol) (afp_version >= 30 && ((vol)->v_flags & PRIV))

ie for pre AFP3 it's always inherited privileges.

Now
posix --> old upriv ? 
noupriv --> old !upriv ?

And something between (the default) !POSIX but PRIV which:
- doesn't return an error in set[xxxx]params if called with set unix
priv but doesn't change privileges.

- don't follow inherited privileges but unix ones each time
vol_unix_priv() is used. 

Is it what you're trying to do?  

Regards
Didier

> 
> 
> proposal:
> netatalk should activate AFP 3.1 UNIX privileges by default as all
> other available AFP implementations do.
> 
> 
> Taking Apples afpd as a reference:
> Apple's afpd supports two permission modes: POSIX mode and inherited
> privileges. Both supporting UNIX privileges. Inherited privileges
> are /somehow/ implemented at the server side.
> 
> 
> Attached is a patch for netatalk head that tries to mimic that.
> 
> 
> In order to preserve current inherited semantics, on top of UNIX
> privileges a inherited permissions model is implemented by 
> - inheriting privileges (as already implemented in netatalk) and
> - ignoring any mode changing calls from clients
> unless a new volume preference "posix" enables them giving POSIX
> semantics.
> 
> 
> 
> UNIX privileges can be deactivated on a per volume basis with another
> new preference "noupriv", "upriv" is set by default.
> 
> 
> 
> Patch overview (best viewed monospaced):
> 
> 
> FILE                        FUNCTION            DESCRIPTION
> ==================================================================
> etc/afpd/main.c             main                set umask to 0! later
> we must apply requested umask
>                                                 manually in any
> effected codepath
> etc/afpd/directory.c        afp_createdir       new "vol" arg to
> netatalk_mkdir call
>                             netatalk_mkdir      changed prototype:
> must include volume;
>                                                 call ad_mkdir with
> applied saved umask and volume umask
>                             copydir             add vol arg to
> netatalk_mkdir call
>                             setdirparams        in case DIRPBIT_UNIXPR
> test if request is for a POSIX
>                                                 volumes (new volume
> option), only proceed if it is
> etc/afpd/file.c             afp_createfile      apply saved umask and
> volume umask
>                             setfilparams        in case FILPBIT_UNIXPR
> test if request is for a POSIX
>                                                 volumes (new volume
> option), only proceed if it is
> etc/afpd/unix.c             stickydirmode       do not apply umask
>                             setfilunixmode      ignore if not a POSIX
> volume even if we have further callers
>                                                 like FPExchangeFiles
> that want to apply modes. We rely
>                                                 on the fact that files
> and dirs have to be created with
>                                                 correct mode in the
> first place!
>                             setfilmode          do not apply umask
>                              setdirunixmode     if !POSIX vol:
> unchanged
>                                                 if POSIX: no sticky
> stuff, chmod directly, preserve S_ISGID
> etc/afpd/volume.c           vol_opt_names[]     new: {AFPVOL_POSIX,
> "POSIX"}
>                             volset              test volopt
> "noupriv" and "posix"
>                             creatvol            if AFPVOL_POSIX then
> volume->v_ad_options |= ADVOL_POSIX
>                             readvolfile         set default option
> AFPVOL_UNIX_PRIV
> etc/afpd/volume.h           -                   #define AFPVOL_POSIX
> (1 << 23)
> include/atalk/adouble.h     -                   #define ADVOL_POSIX (1
> << 4)
> libatalk/adouble/ad_open.c  NEW: ad_chgrp       change group to
> parameter stat->st_gid
>                                                 Do I still need this?
>                             ad_open             change test for
> ad->ad_options & ADVOL_UNIXPRIV to
>                                                 ad->ad_options &
> ADVOL_POSIX 


>