Couldn't resist, eh :)?

Well, now that you've started on it, I guess we have to discuss it more...

This looks good to me.  Some minor comments are all I have.

If you're going to do TCP addresses, the normal thing to do is address/netmask pairs. tcp://192.168.1.0/8 would be more or less how nmap would do it. tcp://192.168.1.0/255.255.255.0 would be one way the NFS exports file would accept it (the other is valid for older NFS versions too, I think).

If we don't do something along these lines, we're making it hard for people with subnets that don't fall on a 8 byte boundary.

We could do the same thing in the security section to get the network and mask. Note that I don't particularly *like* either format above, so if there is something better that someone can think of, great. I specifically don't like it because we use that trailing '/' as a break between an address and a path in other places, so i feel like the '/' is being misused here. A ':' would also be inappropriate, because we use that to indicate trailing port numbers.

This could be a lot of string comparisons at the start of every function. That's not so great, but I don't think that we should worry about it for now.

One other thing I'd like to see is a function or two for pulling out these lists of addresses in the config parsing code, to keep us from having N copies of the same stuff.

Last thing: I'm not convinced that we should do anything different for the Security stuff, other than possibly moving it into the same ExportOptions section as everything else. We can try to make that more generic some other time.

Thanks!

Rob

Murali Vilayannur wrote:
Hi Rob, Sam, Phil
What do you think of the attached patch? Couldn't resist taking a stab at
this :) (I added a new BMI interface and method-specific callback
to query if an address falls within a wildcard range)
Attached patch implements root squashing or all squashing or
read only exports to a restricted set of clients that is specified in the
config file like so
    <ExportOptions>
                AllSquash tcp://127.*
                RootSquash tcp://10.0.0.* tcp://1.5.*
                ReadOnly   tcp://110.2.*
                AnonUID    65534
                AnonGID    65534
     </ExportOptions>
Hopefully, the semantics of the config file are fairly intuitive..
Right now, the patch allows only IP address-based wildcards (no
hostnames) and it is supported only for the bmi-tcp module and nothing
else..
There are still *some minor differences* in end-result between what we
have and what nfs implements. I need to dig a bit more to find out why we don't 
get
the same behavior (I was mistaken that it was a client-side problem and
can be fixed easily. This looks like some permission non-conformance on
the server-side for getattr?)

Does it even make sense to allow a nobody user to chgrp/chown a file?
Do you need to fix those values, or just let the setattr fail?

Hmm.. Well after fixing those values, setattr will fail if the operation
is not allowed on the object :). I was under the impression that the whole
point of RootSquashed exports was to prevent local root's from being root
on the server machine as well, which means that if a root-squashed user is
doing a chgrp/chown on someone else's file it should fail.
This patch should prevent the squashed root user from touching someone
else's file or doing a chmod, chgrp, chown.

Likewise, applying these other uid/gid manipulations in the prelude as
you do in the patch makes equally good sense.  Maybe you could do the
setattr-related changes at the same time in prelude (if needed at all),
to keep all that code in one place?  Just an idea.

Attached patch does what you mentioned above. All the code is now
contained in prelude.sm!

It does get more complicated if we want to be able to apply root squash
only for a specific set of clients, for example.  I don't want to try to
figure that one out right now though; we got more performance to chase
after first.

Attached patch does what you stated above :)
Next step would be to somehow remove the SecurityOptions section and move
it all to the ExportOptions section. This would need a way for the prelude
code to somehow close a socket/connection forcibly if it does not
originate from a secure port, perhaps BMI_FORCEFUL_CANCEL mode might do the 
trick?
Comments welcome,
Thanks
Murali
_______________________________________________
PVFS2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers

Reply via email to