Re: [RFC v4 06/31] richacl: In-memory representation and helper functions

["Stefan (metze) Metzmacher" <metze@samba.org>]

[-- Attachment #1: Type: text/plain, Size: 2706 bytes --]

Hi Andreas,

>> I'm wondering if the size of an ace should be dynamic,
>> which might make it possible to support other ace types
>> in future. E.g. supporting other identities like 128-bit values
>> to make it easier to map Windows SIDS.
> 
> I'm working on additionally supporting unmapped user@domain and
> group@domain identifier strings; we have to deal with that case in the
> nfs client; that may be useful for Samba as well.

Can this be any string? So would
"S-1-5-21-4052121579-2079768045-1474639452-1001" also work?

How would the current thread/process get a "token" that would match such
an ace?

>> Even without 128-bit ids, it would be very useful to mark an
>> ace so that it applies to a uid or gid at the same time.
>> This would reduce the size of the ace list when Samba uses
>> IDMAP_TYPE_BOTH, which means a SID is mapped to a unix id, which
>> is user (uid) and group (gid) at the same time. This feature is required
>> in order to support SID-Histories on accounts.
>> Currently Samba needs to add two aces (one uid and one gid)
>> in order to represent one Windows ace.
> 
> It's not clear to me if supporting this would be a good idea right now.
> The kernel would have to treat each such entry like two separate entries
> internally. How would we map a combined user-space "uid + gid"
> number to a kernel uid and gid since it could map to two distinct
> numbers there?

No, the numeric value is the same.

I think richacl_permission() is the only place that requires any action.

	richacl_for_each_entry(ace, acl) {
		unsigned int ace_mask = ace->e_mask;

		if (richace_is_inherit_only(ace))
			continue;
		if (richace_is_owner(ace)) {
			if (!uid_eq(current_fsuid(), inode->i_uid))
				continue;
		} else if (richace_is_group(ace)) {
			if (!in_owning_group)
				continue;
+		} else if (richace_is_unix_both(ace)) {
+			kuid_t uid = current_fsuid();
+
+			if (!uid_eq(uid, ace->e_id.xid) && !in_group_p(ace->e_id.xid))
+				continue;
		} else if (richace_is_unix_user(ace)) {
			kuid_t uid = current_fsuid();

			if (!uid_eq(uid, ace->e_id.uid))
				continue;
		} else if (richace_is_unix_group(ace)) {
			if (!in_group_p(ace->e_id.gid))
				continue;
		} else
			goto entry_matches_everyone;

In general shouldn't kuid_t uid = current_fsuid(); be at the top of the
function just once?

>> I haven't looked at the claims based acls on Windows, but it would be
>> good if the new infrastructure is dynamic enough to support something
>> like that in a future version.
> 
> I don't know, I have yet to see a use case that isn't totally crazy.

Ok, I found the a_version in struct richacl_xattr.

metze


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]