[OpenAFS-devel] Re: Initial implementation of RestrictedQuery, please comment

[Gergely Risko]

On Tue, 18 Mar 2014 09:29:13 -0400, Jeffrey Altman <jaltman@your-file-system.com> writes:

> Please do not add new files.   Add a new command line option to the
> server binaries.

Okay, I can do that.

> The protection database is not required for use by an AFS cache manager
> although I have a mode for the Windows cache manager that does query
> the protection database in order to translate AFS IDs to Windows domain
> SIDs for Win32 APIs.

I see, I still didn't have time to boot up my windows, do you say that
things will have long timeouts with my cell where the ptserver is
firewalled?  Or is this just a mode that is not on by default?

>>     - for fs listacl and fs setacl you don't need ptserver access even
>>       if you're using names,
>
> The Windows shell extensions that permit ACL editing require protection
> database access.

Good to know.  BTW, I'm not advocating firewalling the protection
database, I just wanted to say that for now it seems so that this VL +
Vol work is meaningful in itself at least for me.  Because for small
non-windows sites it seems to be possible to disable PRDB altogether.
But I'm not saying that after this patch we shouldn't have a look on the
ptserver and look around for RPCs to protect.

>>     - of course you can't access membership listing and other ptserver stuff.
>
> Fyi, self service group membership management is one of the hallmarks of
> AFS.  Restricting access to system:authuser + foreign is necessary if
> you wish to permit users to define and manage personal groups.

Okay, let's consider the ptserver in a separate thread.

>> I also have some new questions, maybe a bit controversial:
>> 
>>   - would anything break if we wouldn't return the volume name when the
>>     GetVolumeByID is used if you're unauthenticated?
>
> Yes.  Cache managers that lookup volume group data by ID would not be
> able to hash the data by the name.   This would result in multiple
> volume group entries for the same volume group in the Windows cache
> manager.  There would be negative impacts on volume callback processing.

So my knowledge that I gathered from the source code so far is the
following:

  - GetVolumeById and GetVolumeByName is the old name or something and
    actually the RPCs are called GetEntryById and GetEntryByName in
    vlprocs.c,

  - GetEntryByName can be used as GetEntryById by giving it a number as
    a string, but this case is handled together in vlprocs.c as
    GetEntryByID, so we can do the RestrictedQuery check there and
    rebuff queries which are essentially GetEntryByID queries,

  - in src/afs GetEntryByID is only used in afs_analyze.c (in the
    function VLDB_Same, where GetEntryByName is called with a number): 
  
    - this function seems to be only used in a corner case situation and
      rejecting it's query would cause it to return DUNNO which only
      results in some structure being cleared in line 820,

  - on the normal cache manager workflow GetEntryByName is used with a
    name not an id.

So, would having a (separate) option to reject GetEntryById calls (or
GetEntryByName calls if they contain an id) when they are anonymous
acceptable?  Of course this option would default to false.  I'm not
really afraid of authenticated users bruteforcing the 2^32 space, but
anonymous users might.

I feel that this is the most controversial point in this, but I'd really
love to have it.  Otherwise I will have to wrap my vlserver and
preauthorize the udp requests going to it with a separate program which
is really ugly.  It'd be much nicer to flip a flag in the BosConfig.

I'll not include this for now and start a separate Gerrit request about
it later, but let's brainstorm about this in the meantime.  I'm open to
suggestions.

>>   - is the option handling like this (etc/RestrictedQuery) OK?  Or
>>     should we do something more complicated?  I'd rather not.
>
> The option should be controlled by command line options on the
> individual service processes.
>
> Updates to the man pages are required.

Sure.

Gergely